Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

[AE][SoftAE] Fix erroneous buffer duration calculations when transcoding

  • Loading branch information...
commit a3aef739a90682c5f5703401d7d7c574650ce020 1 parent 63b6ee7
Damian Huckle DDDamian authored
2  xbmc/cores/AudioEngine/Encoders/AEEncoderFFmpeg.cpp
View
@@ -243,7 +243,7 @@ int CAEEncoderFFmpeg::Encode(float *data, unsigned int frames)
if (m_BufferSize != m_OutputSize)
{
m_OutputSize = m_BufferSize;
- m_OutputRatio = (float)m_NeededFrames / m_OutputSize;
+ m_OutputRatio = (double)m_NeededFrames / m_OutputSize;
}
/* return the number of frames used */
44 xbmc/cores/AudioEngine/Engines/SoftAE/SoftAE.cpp
View
@@ -232,6 +232,9 @@ void CSoftAE::InternalOpenSink()
{
if (m_masterStream->m_initChannelLayout == AE_CH_LAYOUT_2_0)
m_transcode = false;
+ m_encoderInitFrameSizeMul = 1.0 / (newFormat.m_channelLayout.Count() *
+ (CAEUtil::DataFormatToBits(newFormat.m_dataFormat) >> 3));
+ m_encoderInitSampleRateMul = 1.0 / newFormat.m_sampleRate;
}
}
@@ -338,8 +341,8 @@ void CSoftAE::InternalOpenSink()
CLog::Log(LOGINFO, " Frame Size : %d", newFormat.m_frameSize);
m_sinkFormat = newFormat;
- m_sinkFormatSampleRateMul = 1.0 / (float)newFormat.m_sampleRate;
- m_sinkFormatFrameSizeMul = 1.0 / (float)newFormat.m_frameSize;
+ m_sinkFormatSampleRateMul = 1.0 / (double)newFormat.m_sampleRate;
+ m_sinkFormatFrameSizeMul = 1.0 / (double)newFormat.m_frameSize;
m_sinkBlockSize = newFormat.m_frames * newFormat.m_frameSize;
// check if sink controls volume, if so, init the volume.
m_sinkHandlesVolume = m_sink->HasVolume();
@@ -398,7 +401,7 @@ void CSoftAE::InternalOpenSink()
SetupEncoder(encoderFormat);
m_encoderFormat = encoderFormat;
if (encoderFormat.m_frameSize > 0)
- m_encoderFrameSizeMul = 1.0 / (float)encoderFormat.m_frameSize;
+ m_encoderFrameSizeMul = 1.0 / (double)m_sinkFormat.m_frameSize;
else
m_encoderFrameSizeMul = 1.0;
}
@@ -845,34 +848,55 @@ IAEStream *CSoftAE::FreeStream(IAEStream *stream)
double CSoftAE::GetDelay()
{
- double delay = (double)m_buffer.Used() * m_sinkFormatFrameSizeMul *m_sinkFormatSampleRateMul;
+ double delayBuffer = 0.0, delaySink = 0.0, delayTranscoder = 0.0;
+
CSharedLock sinkLock(m_sinkLock);
if (m_sink)
- delay += m_sink->GetDelay();
+ delaySink = m_sink->GetDelay();
sinkLock.Leave();
if (m_transcode && m_encoder && !m_rawPassthrough)
- delay += m_encoder->GetDelay((double)m_encodedBuffer.Used() * m_encoderFrameSizeMul);
+ {
+ delayBuffer = (double)m_buffer.Used() * m_encoderInitFrameSizeMul * m_encoderInitSampleRateMul;
+ delayTranscoder = m_encoder->GetDelay((double)m_encodedBuffer.Used() * m_encoderFrameSizeMul);
+ }
+ else
+ delayBuffer = (double)m_buffer.Used() * m_sinkFormatFrameSizeMul *m_sinkFormatSampleRateMul;
- return delay;
+ //CLog::Log(LOGNOTICE, "Buffer:%f Sink:%f Transcoder:%f Total:%f", (float)delaybuffer, (float)delaysink, (float)delaytranscoder,
+ //(float)(delaybuffer + delaysink + delaytranscoder));
+
+ return delayBuffer + delaySink + delayTranscoder;
}
double CSoftAE::GetCacheTime()
{
- double time = (double)m_buffer.Used() * m_sinkFormatFrameSizeMul * m_sinkFormatSampleRateMul;
+ double timeBuffer = 0.0, timeSink = 0.0, timeTranscoder = 0.0;
+
CSharedLock sinkLock(m_sinkLock);
if (m_sink)
- time += m_sink->GetCacheTime();
+ timeSink = m_sink->GetCacheTime();
+ sinkLock.Leave();
Geoffrey McRae Collaborator
gnif added a note

sinkLock should be held while querying m_encoder as it could be swapped out from under you.

Geoffrey McRae Collaborator
gnif added a note

I cant patch this right now, but it needs to be fixed as this IS a bug.

Damian Huckle Collaborator
DDDamian added a note

both lock fixes addressed in e980328 as well as calculating proper buffer durations during transcode in GetCacheTotal()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
- return time;
+ if (m_transcode && m_encoder && !m_rawPassthrough)
+ {
+ timeBuffer = (double)m_buffer.Used() * m_encoderInitFrameSizeMul * m_encoderInitSampleRateMul;
+ timeTranscoder = m_encoder->GetDelay((double)m_encodedBuffer.Used() * m_encoderFrameSizeMul);
+ }
+ else
+ timeBuffer = (double)m_buffer.Used() * m_sinkFormatFrameSizeMul *m_sinkFormatSampleRateMul;
+
+ return timeBuffer + timeSink + timeTranscoder;
}
double CSoftAE::GetCacheTotal()
{
double total = (double)m_buffer.Size() * m_sinkFormatFrameSizeMul * m_sinkFormatSampleRateMul;
+
CSharedLock sinkLock(m_sinkLock);
if (m_sink)
total += m_sink->GetCacheTotal();
+ sinkLock.Leave();
Geoffrey McRae Collaborator
gnif added a note

Again, not required, please kill this line

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
return total;
}
8 xbmc/cores/AudioEngine/Engines/SoftAE/SoftAE.h
View
@@ -152,12 +152,14 @@ class CSoftAE : public IThreadedAE
AESinkInfoList m_sinkInfoList;
IAESink *m_sink;
AEAudioFormat m_sinkFormat;
- float m_sinkFormatSampleRateMul;
- float m_sinkFormatFrameSizeMul;
+ double m_sinkFormatSampleRateMul;
+ double m_sinkFormatFrameSizeMul;
unsigned int m_sinkBlockSize;
bool m_sinkHandlesVolume;
AEAudioFormat m_encoderFormat;
- float m_encoderFrameSizeMul;
+ double m_encoderFrameSizeMul;
+ double m_encoderInitSampleRateMul;
+ double m_encoderInitFrameSizeMul;
unsigned int m_bytesPerSample;
CAEConvert::AEConvertFrFn m_convertFn;

2 comments on commit a3aef73

Geoffrey McRae

Again, not required, please kill this line

Geoffrey McRae

sinkLock should be held while querying m_encoder as it could be swapped out from under you.

Memphiz
Owner

Can we be sure that all these denumerators are sane for preventing divbyzero?

Damian Huckle
Collaborator

There are some basic asserts for framerates and/or sample rates being zero (would be some bad audio) but it is worth some better checking and a graceful escape route.

Geoffrey McRae

I cant patch this right now, but it needs to be fixed as this IS a bug.

Damian Huckle

both lock fixes addressed in e980328 as well as calculating proper buffer durations during transcode in GetCacheTotal()

Please sign in to comment.
Something went wrong with that request. Please try again.