From 607d2bdc0fd8cd0f07cb9cd7452845c9a3cceac2 Mon Sep 17 00:00:00 2001 From: Karl Tomlinson Date: Thu, 18 Apr 2024 00:01:14 +0000 Subject: [PATCH] Bug 1891885 don't use mInternalInBuffer to drain resampler r=pehrsons as this sometimes left unconsumed samples in the buffer, which interfered with the timing of the buffer size reset after an underrun. Differential Revision: https://phabricator.services.mozilla.com/D207659 --- dom/media/WavDumper.h | 15 ++++-- dom/media/driftcontrol/DynamicResampler.cpp | 6 +-- dom/media/driftcontrol/DynamicResampler.h | 47 +++++++++---------- .../gtest/TestAudioDriftCorrection.cpp | 31 +++++++++--- 4 files changed, 62 insertions(+), 37 deletions(-) diff --git a/dom/media/WavDumper.h b/dom/media/WavDumper.h index de4195066a7b..971fa8a32ffe 100644 --- a/dom/media/WavDumper.h +++ b/dom/media/WavDumper.h @@ -107,13 +107,23 @@ class WavDumper { if (!mFile) { return; } - WriteDumpFileHelper(aBuffer, aSamples); + if (aBuffer) { + WriteDumpFileHelper(aBuffer, aSamples); + } else { + constexpr size_t blockSize = 128; + T block[blockSize] = {}; + for (size_t remaining = aSamples; remaining;) { + size_t toWrite = std::min(remaining, blockSize); + fwrite(block, sizeof(T), toWrite, mFile); + remaining -= toWrite; + } + } + fflush(mFile); } private: void WriteDumpFileHelper(const int16_t* aInput, size_t aSamples) { mozilla::Unused << fwrite(aInput, sizeof(int16_t), aSamples, mFile); - fflush(mFile); } void WriteDumpFileHelper(const float* aInput, size_t aSamples) { @@ -127,7 +137,6 @@ class WavDumper { MOZ_ASSERT(rv); } mozilla::Unused << fwrite(buf.Elements(), buf.Length(), 1, mFile); - fflush(mFile); } FILE* mFile = nullptr; diff --git a/dom/media/driftcontrol/DynamicResampler.cpp b/dom/media/driftcontrol/DynamicResampler.cpp index e6f230278ed6..a2eddb94fbb8 100644 --- a/dom/media/driftcontrol/DynamicResampler.cpp +++ b/dom/media/driftcontrol/DynamicResampler.cpp @@ -47,7 +47,9 @@ void DynamicResampler::EnsurePreBuffer(media::TimeUnit aDuration) { if (buffered.IsZero()) { // Wait for the first input segment before deciding how much to pre-buffer. // If it is large it indicates high-latency, and the buffer would have to - // handle that. + // handle that. This also means that the pre-buffer is not set up just + // before a large input segment would extend the buffering beyond the + // desired level. return; } @@ -93,7 +95,6 @@ void DynamicResampler::ResampleInternal(const float* aInBuffer, MOZ_ASSERT(mInRate); MOZ_ASSERT(mOutRate); - MOZ_ASSERT(aInBuffer); MOZ_ASSERT(aInFrames); MOZ_ASSERT(*aInFrames > 0); MOZ_ASSERT(aOutBuffer); @@ -125,7 +126,6 @@ void DynamicResampler::ResampleInternal(const int16_t* aInBuffer, MOZ_ASSERT(mInRate); MOZ_ASSERT(mOutRate); - MOZ_ASSERT(aInBuffer); MOZ_ASSERT(aInFrames); MOZ_ASSERT(*aInFrames > 0); MOZ_ASSERT(aOutBuffer); diff --git a/dom/media/driftcontrol/DynamicResampler.h b/dom/media/driftcontrol/DynamicResampler.h index c1b9000aa011..60083569d054 100644 --- a/dom/media/driftcontrol/DynamicResampler.h +++ b/dom/media/driftcontrol/DynamicResampler.h @@ -174,24 +174,24 @@ class DynamicResampler final { } uint32_t totalOutFramesNeeded = aOutFrames; - auto resample = [&] { - mInternalInBuffer[aChannelIndex].ReadNoCopy( - [&](const Span& aInBuffer) -> uint32_t { - if (!totalOutFramesNeeded) { - return 0; - } - uint32_t outFramesResampled = totalOutFramesNeeded; - uint32_t inFrames = aInBuffer.Length(); - ResampleInternal(aInBuffer.data(), &inFrames, aOutBuffer, - &outFramesResampled, aChannelIndex); - aOutBuffer += outFramesResampled; - totalOutFramesNeeded -= outFramesResampled; - mInputTail[aChannelIndex].StoreTail(aInBuffer.To(inFrames)); - return inFrames; - }); + auto resample = [&](const T* aInBuffer, uint32_t aInLength) -> uint32_t { + uint32_t outFramesResampled = totalOutFramesNeeded; + uint32_t inFrames = aInLength; + ResampleInternal(aInBuffer, &inFrames, aOutBuffer, &outFramesResampled, + aChannelIndex); + aOutBuffer += outFramesResampled; + totalOutFramesNeeded -= outFramesResampled; + mInputTail[aChannelIndex].StoreTail(aInBuffer, inFrames); + return inFrames; }; - resample(); + mInternalInBuffer[aChannelIndex].ReadNoCopy( + [&](const Span& aInBuffer) -> uint32_t { + if (!totalOutFramesNeeded) { + return 0; + } + return resample(aInBuffer.Elements(), aInBuffer.Length()); + }); if (totalOutFramesNeeded == 0) { return false; @@ -204,8 +204,7 @@ class DynamicResampler final { ((CheckedUint32(totalOutFramesNeeded) * mInRate + mOutRate - 1) / mOutRate) .value(); - mInternalInBuffer[aChannelIndex].WriteSilence(totalInFramesNeeded); - resample(); + resample(nullptr, totalInFramesNeeded); } mIsPreBufferSet = false; return true; @@ -324,16 +323,16 @@ class DynamicResampler final { } template void StoreTail(const T* aInBuffer, uint32_t aInFrames) { - if (aInFrames >= MAXSIZE) { - PodCopy(Buffer(), aInBuffer + aInFrames - MAXSIZE, MAXSIZE); - mSize = MAXSIZE; + const T* inBuffer = aInBuffer; + mSize = std::min(aInFrames, MAXSIZE); + if (inBuffer) { + PodCopy(Buffer(), inBuffer + aInFrames - mSize, mSize); } else { - PodCopy(Buffer(), aInBuffer, aInFrames); - mSize = aInFrames; + std::fill_n(Buffer(), mSize, static_cast(0)); } } uint32_t Length() { return mSize; } - static const uint32_t MAXSIZE = 20; + static constexpr uint32_t MAXSIZE = 20; private: float mBuffer[MAXSIZE] = {}; diff --git a/dom/media/driftcontrol/gtest/TestAudioDriftCorrection.cpp b/dom/media/driftcontrol/gtest/TestAudioDriftCorrection.cpp index 90f8a29ba025..ce23497c5d31 100644 --- a/dom/media/driftcontrol/gtest/TestAudioDriftCorrection.cpp +++ b/dom/media/driftcontrol/gtest/TestAudioDriftCorrection.cpp @@ -459,29 +459,46 @@ TEST(TestAudioDriftCorrection, DriftStepResponseUnderrunHighLatencyInput) constexpr uint32_t iterations = 200; const PrincipalHandle testPrincipal = MakePrincipalHandle(nsContentUtils::GetSystemPrincipal()); - uint32_t inputRate = nominalRate * 1005 / 1000; // 0.5% drift - uint32_t inputInterval = inputRate; + uint32_t inputRate1 = nominalRate * 1005 / 1000; // 0.5% drift + uint32_t inputInterval1 = inputRate1; AudioGenerator tone(1, nominalRate, 440); AudioDriftCorrection ad(nominalRate, nominalRate, testPrincipal); for (uint32_t i = 0; i < interval * iterations; i += interval / 100) { AudioSegment inSegment; if (i > 0 && i % interval == 0) { - tone.Generate(inSegment, inputInterval); + tone.Generate(inSegment, inputInterval1); } ad.RequestFrames(inSegment, interval / 100); } - inputRate = nominalRate * 995 / 1000; // -0.5% drift - inputInterval = inputRate; + uint32_t inputRate2 = nominalRate * 995 / 1000; // -0.5% drift + uint32_t inputInterval2 = inputRate2; for (uint32_t i = 0; i < interval * iterations; i += interval / 100) { AudioSegment inSegment; + // The first segment is skipped to cause an underrun. if (i > 0 && i % interval == 0) { - tone.Generate(inSegment, inputInterval); + tone.Generate(inSegment, inputInterval2); } ad.RequestFrames(inSegment, interval / 100); + if (i >= interval / 10 && i < interval) { + // While the DynamicResampler has not set its pre-buffer after the + // underrun, InFramesBuffered() reports the pre-buffer size. + // The initial desired buffer and pre-buffer size was + // inputInterval1 * 11 / 10 to accomodate the large input block size. + // This was doubled when the underrun occurred. + EXPECT_EQ(ad.CurrentBuffering(), inputInterval1 * 11 / 10 * 2) + << "for i=" << i; + } else if (i == interval) { + // After the pre-buffer was set and used to generate the first output + // block, the actual number of frames buffered almost matches the + // pre-buffer size, with some rounding from output to input frame count + // conversion. + EXPECT_EQ(ad.CurrentBuffering(), inputInterval1 * 11 / 10 * 2 - 1) + << "after first input after underrun"; + } } - EXPECT_EQ(ad.BufferSize(), 220800U); + EXPECT_EQ(ad.BufferSize(), 110400U); EXPECT_EQ(ad.NumUnderruns(), 1u); }