Bug 1844181 - Remove the media.clockdrift.buffering pref. r=padenot

Differential Revision: https://phabricator.services.mozilla.com/D188210

dom/media/driftcontrol/AudioDriftCorrection.cpp

@@ -9,7 +9,6 @@
 
 #include "AudioResampler.h"
 #include "DriftController.h"
-#include "mozilla/StaticPrefs_media.h"
 
 namespace mozilla {
 
@@ -23,11 +22,8 @@ extern LazyLogModule gMediaTrackGraphLog;
 static media::TimeUnit DesiredBuffering(media::TimeUnit aSourceLatency) {
   constexpr media::TimeUnit kMinBuffer(10, MSECS_PER_S);
   constexpr media::TimeUnit kMaxBuffer(2500, MSECS_PER_S);
-  const media::TimeUnit bufferingByPref(
-      StaticPrefs::media_clockdrift_buffering(), MSECS_PER_S);
 
-  const auto clamped = std::clamp(std::max(aSourceLatency, bufferingByPref),
-                                  kMinBuffer, kMaxBuffer);
+  const auto clamped = std::clamp(aSourceLatency, kMinBuffer, kMaxBuffer);
 
   // Ensure the base is the source's sampling rate.
   return clamped.ToBase(aSourceLatency);
@@ -51,7 +47,17 @@ AudioSegment AudioDriftCorrection::RequestFrames(const AudioSegment& aInput,
   const media::TimeUnit outputDuration(aOutputFrames, mTargetRate);
 
   if (inputDuration.IsPositive()) {
-    if (mDesiredBuffering.IsZero() || inputDuration > mDesiredBuffering) {
+    if (mDesiredBuffering.IsZero()) {
+      // Start with the desired buffering at at least 50ms, since the drift is
+      // still unknown. It may be adjust downward later on, when we have adapted
+      // to the drift more.
+      const media::TimeUnit desiredBuffering = DesiredBuffering(std::max(
+          inputDuration * 11 / 10, media::TimeUnit::FromSeconds(0.05)));
+      LOG_CONTROLLER(LogLevel::Info, mDriftController.get(),
+                     "Initial desired buffering %.2fms",
+                     desiredBuffering.ToSeconds() * 1000.0);
+      SetDesiredBuffering(desiredBuffering);
+    } else if (inputDuration > mDesiredBuffering) {
       // Input latency is higher than the desired buffering. Increase the
       // desired buffering to try to avoid underruns.
       if (inputDuration > mSourceLatency) {

dom/media/driftcontrol/gtest/TestAudioDriftCorrection.cpp

@@ -8,7 +8,6 @@
 #include "AudioDriftCorrection.h"
 #include "AudioGenerator.h"
 #include "AudioVerifier.h"
-#include "mozilla/StaticPrefs_media.h"
 #include "nsContentUtils.h"
 
 using namespace mozilla;
@@ -86,10 +85,12 @@ void testAudioCorrection(int32_t aSourceRate, int32_t aTargetRate,
     }
   }
 
-  const int32_t expectedBuffering =
-      StaticPrefs::media_clockdrift_buffering() * aSourceRate / 1000 -
-      aSourceRate / 100 /* 10ms */;
-  EXPECT_NEAR(ad.CurrentBuffering(), expectedBuffering, 512);
+  // Initial buffering is 50ms, which is then expected to be reduced as the
+  // drift adaptation stabilizes.
+  EXPECT_LT(ad.CurrentBuffering(), aSourceRate * 50U / 1000);
+  // Desired buffering should not go lower than some 130% of the source buffer
+  // size per-iteration.
+  EXPECT_GT(ad.CurrentBuffering(), aSourceRate * 10U / 1000);
 
   EXPECT_EQ(ad.NumUnderruns(), 0U);
 
@@ -98,10 +99,18 @@ void testAudioCorrection(int32_t aSourceRate, int32_t aTargetRate,
   EXPECT_EQ(inToneVerifier.CountDiscontinuities(), 0U);
 
   EXPECT_NEAR(outToneVerifier.EstimatedFreq(), tone.mFrequency, 1.0f);
-  // The expected pre-silence is 50ms plus the resampling, minus the size of the
-  // first resampled segment.
-  EXPECT_GE(outToneVerifier.PreSilenceSamples(),
-            aTargetRate * 50 / 1000U - aTargetRate * 102 / 100 / 100);
+  // The expected pre-silence is equal to the initial desired buffering (50ms)
+  // minus what is left after resampling the first input segment.
+  const auto buffering = media::TimeUnit::FromSeconds(0.05);
+  const auto sourceStep =
+      media::TimeUnit(aSourceRate * 1002 / 1000 / 100, aSourceRate);
+  const auto targetStep = media::TimeUnit(aTargetRate / 100, aTargetRate);
+  EXPECT_NEAR(static_cast<int64_t>(outToneVerifier.PreSilenceSamples()),
+              (targetStep + buffering - sourceStep)
+                  .ToBase(aSourceRate)
+                  .ToBase<media::TimeUnit::CeilingPolicy>(aTargetRate)
+                  .ToTicksAtRate(aTargetRate),
+              1U);
   EXPECT_EQ(outToneVerifier.CountDiscontinuities(), 0U);
 }
 
@@ -347,7 +356,7 @@ TEST(TestAudioDriftCorrection, DynamicInputBufferSizeChanges)
   EXPECT_EQ(ad.NumUnderruns(), 1U);
 
   // Adapting to the new input block size should have stabilized.
-  EXPECT_GT(ad.BufferSize(), 4800U);
+  EXPECT_GT(ad.BufferSize(), transmitterBlockSize2);
   produceSomeData(transmitterBlockSize2, 10 * sampleRate);
   EXPECT_EQ(ad.NumCorrectionChanges(), numCorrectionChanges);
   EXPECT_EQ(ad.NumUnderruns(), 1U);
@@ -360,8 +369,8 @@ TEST(TestAudioDriftCorrection, DynamicInputBufferSizeChanges)
 
   // Adapting to the new input block size should have stabilized.
   EXPECT_EQ(ad.BufferSize(), 9600U);
-  produceSomeData(transmitterBlockSize1, 10 * sampleRate);
-  EXPECT_EQ(ad.NumCorrectionChanges(), numCorrectionChanges);
+  produceSomeData(transmitterBlockSize1, 20 * sampleRate);
+  EXPECT_NEAR(ad.NumCorrectionChanges(), numCorrectionChanges, 1U);
   EXPECT_EQ(ad.NumUnderruns(), 1U);
 
   EXPECT_NEAR(inToneVerifier.EstimatedFreq(), tone.mFrequency, 1.0f);
@@ -418,7 +427,6 @@ TEST(TestAudioDriftCorrection, DriftStepResponseUnderrun)
       MakePrincipalHandle(nsContentUtils::GetSystemPrincipal());
   uint32_t inputRate = nominalRate * 1005 / 1000;  // 0.5% drift
   uint32_t inputInterval = inputRate;
-  Preferences::SetUint("media.clockdrift.buffering", 10);
   AudioGenerator<AudioDataValue> tone(1, nominalRate, 440);
   AudioDriftCorrection ad(nominalRate, nominalRate, testPrincipal);
   for (uint32_t i = 0; i < interval * iterations; i += interval / 100) {
@@ -437,7 +445,6 @@ TEST(TestAudioDriftCorrection, DriftStepResponseUnderrun)
 
   EXPECT_EQ(ad.BufferSize(), 4800U);
   EXPECT_EQ(ad.NumUnderruns(), 1u);
-  Preferences::ClearUser("media.clockdrift.buffering");
 }
 
 /**
@@ -454,7 +461,6 @@ TEST(TestAudioDriftCorrection, DriftStepResponseUnderrunHighLatencyInput)
       MakePrincipalHandle(nsContentUtils::GetSystemPrincipal());
   uint32_t inputRate = nominalRate * 1005 / 1000;  // 0.5% drift
   uint32_t inputInterval = inputRate;
-  Preferences::SetUint("media.clockdrift.buffering", 10);
   AudioGenerator<AudioDataValue> tone(1, nominalRate, 440);
   AudioDriftCorrection ad(nominalRate, nominalRate, testPrincipal);
   for (uint32_t i = 0; i < interval * iterations; i += interval / 100) {
@@ -477,7 +483,6 @@ TEST(TestAudioDriftCorrection, DriftStepResponseUnderrunHighLatencyInput)
 
   EXPECT_EQ(ad.BufferSize(), 220800U);
   EXPECT_EQ(ad.NumUnderruns(), 1u);
-  Preferences::ClearUser("media.clockdrift.buffering");
 }
 
 /**

dom/media/gtest/TestAudioTrackGraph.cpp

@@ -18,11 +18,8 @@
 #include "mozilla/gtest/WaitFor.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/SpinEventLoopUntil.h"
-#include "mozilla/StaticPrefs_media.h"
 #include "WavDumper.h"
 
-#define DRIFT_BUFFERING_PREF "media.clockdrift.buffering"
-
 using namespace mozilla;
 
 // Short-hand for InvokeAsync on the current thread.
@@ -2545,12 +2542,12 @@ void TestCrossGraphPort(uint32_t aInputRate, uint32_t aOutputRate,
   // side. There is one block buffered in NativeInputTrack. Then
   // AudioDriftCorrection sets its pre-buffering so that *after* the first
   // resample of real input data, the buffer contains enough data to match the
-  // desired level. I.e. silence = buffering - inputStep + outputStep. Note that
-  // the steps here are rounded up to block size.
+  // desired level, which is initially 50ms. I.e. silence = buffering -
+  // inputStep + outputStep. Note that the steps here are rounded up to block
+  // size.
   const media::TimeUnit inputBuffering(WEBAUDIO_BLOCK_SIZE, aInputRate);
   const media::TimeUnit buffering =
-      media::TimeUnit(Preferences::GetInt(DRIFT_BUFFERING_PREF), MSECS_PER_S)
-          .ToBase(aInputRate);
+      media::TimeUnit::FromSeconds(0.05).ToBase(aInputRate);
   const media::TimeUnit inputStepSize(
       MediaTrackGraphImpl::RoundUpToEndOfAudioBlock(
           step.ToTicksAtRate(aInputRate)),
@@ -2563,7 +2560,8 @@ void TestCrossGraphPort(uint32_t aInputRate, uint32_t aOutputRate,
                       aOutputRate)
           .ToBase(aInputRate);
   const uint32_t expectedPreSilence =
-      (inputBuffering + buffering - inputStepSize + outputStepSize)
+      (outputStepSize + inputBuffering + buffering - inputStepSize)
+          .ToBase(aInputRate)
           .ToBase<media::TimeUnit::CeilingPolicy>(aOutputRate)
           .ToTicksAtRate(aOutputRate);
   // Use a margin of 0.1% of the expected pre-silence, since the resampler is
@@ -2618,17 +2616,6 @@ TEST(TestAudioTrackGraph, CrossGraphPortUnderrun)
   TestCrossGraphPort(52110, 17781, 1.01, 30, 1);
   TestCrossGraphPort(52110, 17781, 1.03, 40, 3);
 }
-
-TEST(TestAudioTrackGraph, CrossGraphPortLargeBuffer)
-{
-  const int32_t longBuffering = 2500;
-  Preferences::SetInt(DRIFT_BUFFERING_PREF, longBuffering);
-
-  TestCrossGraphPort(44100, 48000, 1.05, 60);
-  TestCrossGraphPort(52110, 17781, 0.95, 60);
-
-  Preferences::ClearUser(DRIFT_BUFFERING_PREF);
-}
 #endif  // MOZ_WEBRTC
 
 #undef Invoke

modules/libpref/init/StaticPrefList.yaml

@@ -9379,12 +9379,6 @@
   mirror: always
   value: 0
 
-# ClockDrift desired buffering in milliseconds
-- name: media.clockdrift.buffering
-  type: RelaxedAtomicUint32
-  mirror: always
-  value: 50
-
 # If a resource is known to be smaller than this size (in kilobytes), a
 # memory-backed MediaCache may be used; otherwise the (single shared global)
 # file-backed MediaCache is used.