Bug 1586370 - Move EnsureNextIteration logic into ThreadedDriver . r=padenot

This removes some driver-graph interdependencies.

It also changes the logic of EnsureNextIteration() so that it no longer blindly
wakes the driver up, should it be sleeping. Instead it waits until the driver's
next iteration would have occurred anyway, as that seems more expected, and
closer to how an AudioClockDriver works.

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

--HG--
extra : moz-landing-system : lando

dom/media/GraphDriver.cpp

@@ -285,8 +285,8 @@ void ThreadedDriver::RunThread() {
       break;
     }
     mStateComputedTime = nextStateComputedTime;
-    MonitorAutoLock lock(GraphImpl()->GetMonitor());
     WaitForNextIteration();
+    MonitorAutoLock lock(GraphImpl()->GetMonitor());
     if (NextDriver()) {
       LOG(LogLevel::Debug,
           ("%p: Switching to AudioCallbackDriver", GraphImpl()));
@@ -313,47 +313,19 @@ MediaTime SystemClockDriver::GetIntervalForIteration() {
   return interval;
 }
 
-void ThreadedDriver::WaitForNextIteration() {
-  GraphImpl()->GetMonitor().AssertCurrentThreadOwns();
-
-  TimeDuration timeout = TimeDuration::Forever();
-
-  // This lets us avoid hitting the Atomic twice when we know we won't sleep
-  bool another = GraphImpl()->mNeedAnotherIteration;  // atomic
-  if (!another) {
-    GraphImpl()->mGraphDriverAsleep = true;  // atomic
-  }
-  // NOTE: mNeedAnotherIteration while also atomic may have changed before
-  // we could set mGraphDriverAsleep, so we must re-test it.
-  // (GraphImpl::EnsureNextIteration() sets mNeedAnotherIteration, then tests
-  // mGraphDriverAsleep)
-  if (another || GraphImpl()->mNeedAnotherIteration) {  // atomic
-    timeout = WaitInterval();
-    if (!another) {
-      GraphImpl()->mGraphDriverAsleep = false;  // atomic
-      another = true;
-    }
-  }
-  if (!timeout.IsZero()) {
-    CVStatus status = GraphImpl()->GetMonitor().Wait(timeout);
-    LOG(LogLevel::Verbose,
-        ("%p: Resuming after %s", GraphImpl(),
-         status == CVStatus::Timeout ? "timeout" : "wake-up"));
-  }
-
-  if (!another) {
-    GraphImpl()->mGraphDriverAsleep = false;  // atomic
-  }
-  GraphImpl()->mNeedAnotherIteration = false;  // atomic
+void ThreadedDriver::EnsureNextIteration() {
+  mWaitHelper.EnsureNextIteration();
 }
 
-void ThreadedDriver::WakeUp() {
-  GraphImpl()->GetMonitor().AssertCurrentThreadOwns();
-  GraphImpl()->mGraphDriverAsleep = false;  // atomic
-  GraphImpl()->GetMonitor().Notify();
+void ThreadedDriver::WaitForNextIteration() {
+  MOZ_ASSERT(mThread);
+  MOZ_ASSERT(OnThread());
+  mWaitHelper.WaitForNextIterationAtLeast(WaitInterval());
 }
 
 TimeDuration SystemClockDriver::WaitInterval() {
+  MOZ_ASSERT(mThread);
+  MOZ_ASSERT(OnThread());
   TimeStamp now = TimeStamp::Now();
   int64_t timeoutMS = MEDIA_GRAPH_TARGET_PERIOD_MS -
                       int64_t((now - mCurrentTimeStamp).ToMilliseconds());
@@ -377,11 +349,6 @@ MediaTime OfflineClockDriver::GetIntervalForIteration() {
   return GraphImpl()->MillisecondsToMediaTime(mSlice);
 }
 
-TimeDuration OfflineClockDriver::WaitInterval() {
-  // We want to go as fast as possible when we are offline
-  return 0;
-}
-
 AsyncCubebTask::AsyncCubebTask(AudioCallbackDriver* aDriver,
                                AsyncCubebOperation aOperation)
     : Runnable("AsyncCubebTask"),
@@ -719,8 +686,6 @@ void AudioCallbackDriver::AddMixerCallback() {
   }
 }
 
-void AudioCallbackDriver::WakeUp() {}
-
 void AudioCallbackDriver::Shutdown() {
   MOZ_ASSERT(NS_IsMainThread());
   LOG(LogLevel::Debug,

dom/media/GraphDriver.h

@@ -120,8 +120,6 @@ class GraphDriver {
   explicit GraphDriver(MediaTrackGraphImpl* aGraphImpl);
 
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(GraphDriver);
-  /* Wakes up the graph if it is waiting for the next iteration. */
-  virtual void WakeUp() = 0;
   /* Start the graph, init the driver, start the thread.
    * A driver cannot be started twice, it must be shutdown
    * before being started again. */
@@ -135,6 +133,12 @@ class GraphDriver {
    * number of buffers of this audio backend: say we have four buffers, and 40ms
    * latency, we will get a callback approximately every 10ms. */
   virtual uint32_t IterationDuration() = 0;
+  /*
+   * Signaled by the graph when it needs another iteration. Goes unhandled for
+   * GraphDrivers that are not able to sleep indefinitely (i.e., all drivers but
+   * ThreadedDriver). Can be called on any thread.
+   */
+  virtual void EnsureNextIteration() {}
 
   /* Return whether we are switching or not. */
   bool Switching();
@@ -159,7 +163,7 @@ class GraphDriver {
    * Tell the driver it has to stop and return the current time of the graph, so
    * another driver can start from the right point in time.
    */
-  virtual void SwitchAtNextIteration(GraphDriver* aDriver);
+  void SwitchAtNextIteration(GraphDriver* aDriver);
 
   /**
    * Set the state of the driver so it can start at the right point in time,
@@ -217,14 +221,55 @@ class MediaTrackGraphInitThreadRunnable;
  * This class is a driver that manages its own thread.
  */
 class ThreadedDriver : public GraphDriver {
+  class IterationWaitHelper {
+    Monitor mMonitor;
+    // The below members are guarded by mMonitor.
+    bool mNeedAnotherIteration = false;
+    TimeStamp mWakeTime;
+
+   public:
+    IterationWaitHelper() : mMonitor("IterationWaitHelper::mMonitor") {}
+
+    /**
+     * If another iteration is needed we wait for aDuration, otherwise we wait
+     * for a wake-up. If a wake-up occurs before aDuration time has passed, we
+     * wait for aDuration nonetheless.
+     */
+    void WaitForNextIterationAtLeast(TimeDuration aDuration) {
+      MonitorAutoLock lock(mMonitor);
+      TimeStamp now = TimeStamp::Now();
+      mWakeTime = now + aDuration;
+      while (true) {
+        if (mNeedAnotherIteration && now >= mWakeTime) {
+          break;
+        }
+        if (mNeedAnotherIteration) {
+          lock.Wait(mWakeTime - now);
+        } else {
+          lock.Wait(TimeDuration::Forever());
+        }
+        now = TimeStamp::Now();
+      }
+      mWakeTime = TimeStamp();
+      mNeedAnotherIteration = false;
+    }
+
+    /**
+     * Sets mNeedAnotherIteration to true and notifies the monitor, in case a
+     * driver is currently waiting.
+     */
+    void EnsureNextIteration() {
+      MonitorAutoLock lock(mMonitor);
+      mNeedAnotherIteration = true;
+      lock.Notify();
+    }
+  };
+
  public:
   explicit ThreadedDriver(MediaTrackGraphImpl* aGraphImpl);
   virtual ~ThreadedDriver();
-  /**
-   * Waits until it's time to process more data.
-   * */
-  void WaitForNextIteration();
-  void WakeUp() override;
+
+  void EnsureNextIteration() override;
   void Start() override;
   MOZ_CAN_RUN_SCRIPT void Shutdown() override;
   /**
@@ -242,9 +287,12 @@ class ThreadedDriver : public GraphDriver {
   }
 
   bool ThreadRunning() override { return mThreadRunning; }
-  /*
-   * Return the TimeDuration to wait before the next rendering iteration.
-   */
+
+ protected:
+  /* Waits until it's time to process more data. */
+  void WaitForNextIteration();
+  /* Implementation dependent time the ThreadedDriver should wait between
+   * iterations. */
   virtual TimeDuration WaitInterval() = 0;
   /* When the graph wakes up to do an iteration, implementations return the
    * range of time that will be processed.  This is called only once per
@@ -252,13 +300,15 @@ class ThreadedDriver : public GraphDriver {
    * call. */
   virtual MediaTime GetIntervalForIteration() = 0;
 
- protected:
   nsCOMPtr<nsIThread> mThread;
 
  private:
   // This is true if the thread is running. It is false
   // before starting the thread and after stopping it.
   Atomic<bool> mThreadRunning;
+
+  // Any thread.
+  IterationWaitHelper mWaitHelper;
 };
 
 /**
@@ -271,11 +321,14 @@ class SystemClockDriver : public ThreadedDriver {
   explicit SystemClockDriver(MediaTrackGraphImpl* aGraphImpl,
                              FallbackMode aFallback = FallbackMode::Regular);
   virtual ~SystemClockDriver();
-  TimeDuration WaitInterval() override;
-  MediaTime GetIntervalForIteration() override;
   bool IsFallback();
   SystemClockDriver* AsSystemClockDriver() override { return this; }
 
+ protected:
+  /* Return the TimeDuration to wait before the next rendering iteration. */
+  TimeDuration WaitInterval() override;
+  MediaTime GetIntervalForIteration() override;
+
  private:
   // Those are only modified (after initialization) on the graph thread. The
   // graph thread does not run during the initialization.
@@ -283,8 +336,8 @@ class SystemClockDriver : public ThreadedDriver {
   TimeStamp mCurrentTimeStamp;
   TimeStamp mLastTimeStamp;
 
-  // This is true if this SystemClockDriver runs the graph because we could not
-  // open an audio stream.
+  // This is true if this SystemClockDriver runs the graph because we could
+  // not open an audio stream.
   const bool mIsFallback;
 };
 
@@ -296,10 +349,12 @@ class OfflineClockDriver : public ThreadedDriver {
  public:
   OfflineClockDriver(MediaTrackGraphImpl* aGraphImpl, GraphTime aSlice);
   virtual ~OfflineClockDriver();
-  TimeDuration WaitInterval() override;
-  MediaTime GetIntervalForIteration() override;
   OfflineClockDriver* AsOfflineClockDriver() override { return this; }
 
+ protected:
+  TimeDuration WaitInterval() override { return 0; }
+  MediaTime GetIntervalForIteration() override;
+
  private:
   // Time, in GraphTime, for each iteration
   GraphTime mSlice;
@@ -353,7 +408,6 @@ class AudioCallbackDriver : public GraphDriver,
   virtual ~AudioCallbackDriver();
 
   void Start() override;
-  void WakeUp() override;
   MOZ_CAN_RUN_SCRIPT void Shutdown() override;
 #if defined(XP_WIN)
   void ResetDefaultDevice() override;

dom/media/MediaTrackGraph.cpp

@@ -978,11 +978,11 @@ bool MediaTrackGraphImpl::ShouldUpdateMainThread() {
   }
 
   TimeStamp now = TimeStamp::Now();
-  // For offline graphs, update now if there is no pending iteration or if it
-  // has been long enough since the last update.
-  if (!mNeedAnotherIteration ||
-      ((now - mLastMainThreadUpdate).ToMilliseconds() >
-       CurrentDriver()->IterationDuration())) {
+  // For offline graphs, update now if it has been long enough since the last
+  // update, or if it has reached the end.
+  if ((now - mLastMainThreadUpdate).ToMilliseconds() >
+          CurrentDriver()->IterationDuration() ||
+      mStateComputedTime >= mEndTime) {
     mLastMainThreadUpdate = now;
     return true;
   }
@@ -1662,7 +1662,7 @@ void MediaTrackGraphImpl::RunInStableState(bool aSourceIsMTG) {
       if (LifecycleStateRef() <= LIFECYCLE_WAITING_FOR_MAIN_THREAD_CLEANUP) {
         MessageBlock* block = mBackMessageQueue.AppendElement();
         block->mMessages.SwapElements(mCurrentTaskMessageQueue);
-        EnsureNextIterationLocked();
+        EnsureNextIteration();
       }
 
       // If this MediaTrackGraph has entered regular (non-forced) shutdown it
@@ -2560,7 +2560,10 @@ TrackTime SourceMediaTrack::AppendData(MediaSegment* aSegment,
   NotifyDirectConsumers(aRawSegment ? aRawSegment : aSegment);
   appended = aSegment->GetDuration();
   mUpdateTrack->mData->AppendFrom(aSegment);  // note: aSegment is now dead
-  graph->EnsureNextIteration();
+  {
+    MonitorAutoLock lock(graph->GetMonitor());
+    graph->EnsureNextIteration();
+  }
 
   return appended;
 }
@@ -2673,6 +2676,7 @@ void SourceMediaTrack::End() {
   }
   mUpdateTrack->mEnded = true;
   if (auto graph = GraphImpl()) {
+    MonitorAutoLock lock(graph->GetMonitor());
     graph->EnsureNextIteration();
   }
 }
@@ -2882,8 +2886,6 @@ MediaTrackGraphImpl::MediaTrackGraphImpl(GraphDriverType aDriverRequested,
       mPortCount(0),
       mInputDeviceID(nullptr),
       mOutputDeviceID(nullptr),
-      mNeedAnotherIteration(false),
-      mGraphDriverAsleep(false),
       mMonitor("MediaTrackGraphImpl"),
       mLifecycleState(LIFECYCLE_THREAD_NOT_STARTED),
       mForceShutDown(false),

dom/media/MediaTrackGraphImpl.h

@@ -283,7 +283,7 @@ class MediaTrackGraphImpl : public MediaTrackGraph,
     MOZ_ASSERT(mFrontMessageQueue.IsEmpty());
     mFrontMessageQueue.SwapElements(mBackMessageQueue);
     if (!mFrontMessageQueue.IsEmpty()) {
-      EnsureNextIterationLocked();
+      EnsureNextIteration();
     }
   }
   /**
@@ -598,25 +598,7 @@ class MediaTrackGraphImpl : public MediaTrackGraph,
 
   Monitor& GetMonitor() { return mMonitor; }
 
-  void EnsureNextIteration() {
-    mNeedAnotherIteration = true;  // atomic
-    // Note: GraphDriver must ensure that there's no race on setting
-    // mNeedAnotherIteration and mGraphDriverAsleep -- see
-    // WaitForNextIteration()
-    if (mGraphDriverAsleep) {  // atomic
-      MonitorAutoLock mon(mMonitor);
-      CurrentDriver()
-          ->WakeUp();  // Might not be the same driver; might have woken already
-    }
-  }
-
-  void EnsureNextIterationLocked() {
-    mNeedAnotherIteration = true;  // atomic
-    if (mGraphDriverAsleep) {      // atomic
-      CurrentDriver()
-          ->WakeUp();  // Might not be the same driver; might have woken already
-    }
-  }
+  void EnsureNextIteration() { CurrentDriver()->EnsureNextIteration(); }
 
   // Capture API. This allows to get a mixed-down output for a window.
   void RegisterCaptureTrackForWindow(uint64_t aWindowId,
@@ -779,11 +761,6 @@ class MediaTrackGraphImpl : public MediaTrackGraph,
   nsDataHashtable<nsVoidPtrHashKey, nsTArray<RefPtr<AudioDataListener>>>
       mInputDeviceUsers;
 
-  // True if the graph needs another iteration after the current iteration.
-  Atomic<bool> mNeedAnotherIteration;
-  // GraphDriver may need a WakeUp() if something changes
-  Atomic<bool> mGraphDriverAsleep;
-
   // mMonitor guards the data below.
   // MediaTrackGraph normally does its work without holding mMonitor, so it is
   // not safe to just grab mMonitor from some thread and start monkeying with