Skip to content
Browse files

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

  • Loading branch information...
1 parent 360f41d commit 55dd5dbd7270158ad1c4b9da46cf16931ca8d92e @DDDamian committed
View
2 xbmc/cores/AudioEngine/Encoders/AEEncoderFFmpeg.cpp
@@ -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 */
View
44 xbmc/cores/AudioEngine/Engines/SoftAE/SoftAE.cpp
@@ -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;
}
@@ -834,34 +837,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();
- 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();
@gnif
gnif added a note

This is not needed, CSharedLock leaves the lock on distruction

@DDDamian Owner

Was just for practising good form ;) but happy to omit.

@gnif
gnif added a note

Yeah, its fine to leave there, but it would be doubling up on a check inside the CSharedLock class on it's destruction, which is not required. General practice in the XBMC code base I have observed is to allow the class destruction to release the lock unless special handling is required such as that in CSoftAE::Run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
return total;
}
View
8 xbmc/cores/AudioEngine/Engines/SoftAE/SoftAE.h
@@ -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 55dd5db

@gnif

Nice catch here, this issue as you might remember plagued me for quite some time

@DDDamian
Owner

Thx gnif - never hit it as I run HDMI to an AAC-capable receiver, but there were several bug reports on this and didn't want this to hit the masses with Frodo releases and all those optical connections, plus AAC is getting very common.

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