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 705084 1571dfc0fe48efff799fa0f91eca0ddf359ccc06
parent 700722 eac419c4e8d6be6a44360e2baebbe890b22f96be
child 705085 aa0d667d559b9eb64b4c1b8952ef19bf808a1aa0
push id91355
push userbmo:apehrson@mozilla.com
push dateWed, 29 Nov 2017 13:15:51 +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
@@ -892,18 +892,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
@@ -932,70 +930,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;
   }