Bug 1408294 - Set iteration state before calling NotifyInputData. r?padenot draft
authorAndreas Pehrson <pehrsons@mozilla.com>
Tue, 28 Nov 2017 13:32:21 +0100
changeset 717218 597c06c891bd32b7f2e0a396a5bb1fc6fa89ed06
parent 716014 3acb14b949150529ec761f845f9a3d61ee341dac
child 717219 d916e038f6479dc84a76312426b889d201490bf6
push id94604
push userbmo:apehrson@mozilla.com
push dateMon, 08 Jan 2018 14:02:37 +0000
reviewerspadenot
bugs1408294
milestone59.0a1
Bug 1408294 - Set iteration state before calling NotifyInputData. r?padenot In the MediaEngine for microphone capture we want to fall back on feeding silence when the device is stopped. To ensure this doesn't go haywire we check that the invariant that at most one of NotifyInputData and NotifyPull get called in the same iteration. For them to be aligned on which iteration they're in however, the graph needs to report a consistent IterationEnd() to both. This patch fixes this by only calling into NotifyInputData() *after* setting iteration state, which will then be consistent also across OneIteration (which calls into NotifyPull). MozReview-Commit-ID: 4lD4OcdGtM6
dom/media/GraphDriver.cpp
--- a/dom/media/GraphDriver.cpp
+++ b/dom/media/GraphDriver.cpp
@@ -877,18 +877,16 @@ AudioCallbackDriver::AutoInCallback::Aut
 AudioCallbackDriver::AutoInCallback::~AutoInCallback() {
   mDriver->mInCallback = false;
 }
 
 long
 AudioCallbackDriver::DataCallback(const AudioDataValue* aInputBuffer,
                                   AudioDataValue* aOutputBuffer, long aFrames)
 {
-  bool stillProcessing;
-
   // Don't add the callback until we're inited and ready
   if (!mAddedMixer) {
     mGraphImpl->mMixer.AddCallback(this);
     mAddedMixer = true;
   }
 
 #ifdef DEBUG
   // DebugOnly<> doesn't work here... it forces an initialization that will cause
@@ -917,70 +915,71 @@ AudioCallbackDriver::DataCallback(const 
   // duration so there is some damping against sudden changes.
   if (!mIterationDurationMS) {
     mIterationDurationMS = durationMS;
   } else {
     mIterationDurationMS = (mIterationDurationMS*3) + durationMS;
     mIterationDurationMS /= 4;
   }
 
+  mBuffer.SetBuffer(aOutputBuffer, aFrames);
+  // fill part or all with leftover data from last iteration (since we
+  // align to Audio blocks)
+  mScratchBuffer.Empty(mBuffer);
+
+  // State computed time is decided by the audio callback's buffer length. We
+  // compute the iteration start and end from there, trying to keep the amount
+  // of buffering in the graph constant.
+  GraphTime nextStateComputedTime =
+    mGraphImpl->RoundUpToNextAudioBlock(stateComputedTime + mBuffer.Available());
+
+  mIterationStart = mIterationEnd;
+  // inGraph is the number of audio frames there is between the state time and
+  // the current time, i.e. the maximum theoretical length of the interval we
+  // could use as [mIterationStart; mIterationEnd].
+  GraphTime inGraph = stateComputedTime - mIterationStart;
+  // We want the interval [mIterationStart; mIterationEnd] to be before the
+  // interval [stateComputedTime; nextStateComputedTime]. We also want
+  // the distance between these intervals to be roughly equivalent each time, to
+  // ensure there is no clock drift between current time and state time. Since
+  // we can't act on the state time because we have to fill the audio buffer, we
+  // reclock the current time against the state time, here.
+  mIterationEnd = mIterationStart + 0.8 * inGraph;
+
+  LOG(LogLevel::Verbose,
+      ("interval[%ld; %ld] state[%ld; %ld] (frames: %ld) (durationMS: %u) "
+       "(duration ticks: %ld)",
+       (long)mIterationStart,
+       (long)mIterationEnd,
+       (long)stateComputedTime,
+       (long)nextStateComputedTime,
+       (long)aFrames,
+       (uint32_t)durationMS,
+       (long)(nextStateComputedTime - stateComputedTime)));
+
+  mCurrentTimeStamp = TimeStamp::Now();
+
+  if (stateComputedTime < mIterationEnd) {
+    LOG(LogLevel::Warning, ("Media graph global underrun detected"));
+    mIterationEnd = stateComputedTime;
+  }
+
   // Process mic data if any/needed
   if (aInputBuffer) {
     if (mAudioInput) { // for this specific input-only or full-duplex stream
       mAudioInput->NotifyInputData(mGraphImpl, aInputBuffer,
                                    static_cast<size_t>(aFrames),
                                    mSampleRate, mInputChannels);
     }
   }
 
-  mBuffer.SetBuffer(aOutputBuffer, aFrames);
-  // fill part or all with leftover data from last iteration (since we
-  // align to Audio blocks)
-  mScratchBuffer.Empty(mBuffer);
-  // if we totally filled the buffer (and mScratchBuffer isn't empty),
-  // we don't need to run an iteration and if we do so we may overflow.
+  bool stillProcessing;
   if (mBuffer.Available()) {
-
-    // State computed time is decided by the audio callback's buffer length. We
-    // compute the iteration start and end from there, trying to keep the amount
-    // of buffering in the graph constant.
-    GraphTime nextStateComputedTime =
-      mGraphImpl->RoundUpToNextAudioBlock(stateComputedTime + mBuffer.Available());
-
-    mIterationStart = mIterationEnd;
-    // inGraph is the number of audio frames there is between the state time and
-    // the current time, i.e. the maximum theoretical length of the interval we
-    // could use as [mIterationStart; mIterationEnd].
-    GraphTime inGraph = stateComputedTime - mIterationStart;
-    // We want the interval [mIterationStart; mIterationEnd] to be before the
-    // interval [stateComputedTime; nextStateComputedTime]. We also want
-    // the distance between these intervals to be roughly equivalent each time, to
-    // ensure there is no clock drift between current time and state time. Since
-    // we can't act on the state time because we have to fill the audio buffer, we
-    // reclock the current time against the state time, here.
-    mIterationEnd = mIterationStart + 0.8 * inGraph;
-
-    LOG(LogLevel::Verbose,
-        ("interval[%ld; %ld] state[%ld; %ld] (frames: %ld) (durationMS: %u) "
-         "(duration ticks: %ld)",
-         (long)mIterationStart,
-         (long)mIterationEnd,
-         (long)stateComputedTime,
-         (long)nextStateComputedTime,
-         (long)aFrames,
-         (uint32_t)durationMS,
-         (long)(nextStateComputedTime - stateComputedTime)));
-
-    mCurrentTimeStamp = TimeStamp::Now();
-
-    if (stateComputedTime < mIterationEnd) {
-      LOG(LogLevel::Warning, ("Media graph global underrun detected"));
-      mIterationEnd = stateComputedTime;
-    }
-
+    // We totally filled the buffer (and mScratchBuffer isn't empty).
+    // We don't need to run an iteration and if we do so we may overflow.
     stillProcessing = mGraphImpl->OneIteration(nextStateComputedTime);
   } else {
     LOG(LogLevel::Verbose,
         ("DataCallback buffer filled entirely from scratch "
          "buffer, skipping iteration."));
     stillProcessing = true;
   }