bug 1382366 disable AudioCallback -> SystemClockDriver fallback before disowning graph f=pehrsons r=padenot
authorKarl Tomlinson <karlt+@karlt.net>
Sat, 04 Nov 2017 19:00:46 +1300
changeset 393922 53f055298fa9d2446fd90ad8a51fefd3b15b16c1
parent 393921 42564e015369c3206e004ce7287d338a489e3291
child 393923 3e5f310f67e81dcc6af412e1587b4e5afe46a2a8
push id32985
push usercbrindusan@mozilla.com
push dateTue, 28 Nov 2017 09:51:26 +0000
treeherdermozilla-central@5b33b070378a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs1382366
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 1382366 disable AudioCallback -> SystemClockDriver fallback before disowning graph f=pehrsons r=padenot MozReview-Commit-ID: ESr6yxjPVWf
dom/media/GraphDriver.cpp
dom/media/GraphDriver.h
--- a/dom/media/GraphDriver.cpp
+++ b/dom/media/GraphDriver.cpp
@@ -27,18 +27,17 @@ extern mozilla::LazyLogModule gMediaStre
 namespace mozilla {
 
 GraphDriver::GraphDriver(MediaStreamGraphImpl* aGraphImpl)
   : mIterationStart(0),
     mIterationEnd(0),
     mGraphImpl(aGraphImpl),
     mCurrentTimeStamp(TimeStamp::Now()),
     mPreviousDriver(nullptr),
-    mNextDriver(nullptr),
-    mScheduled(false)
+    mNextDriver(nullptr)
 { }
 
 void GraphDriver::SetGraphTime(GraphDriver* aPreviousDriver,
                                GraphTime aLastSwitchNextIterationStart,
                                GraphTime aLastSwitchNextIterationEnd)
 {
   GraphImpl()->GetMonitor().AssertCurrentThreadOwns();
   // We set mIterationEnd here, because the first thing a driver do when it
@@ -115,22 +114,16 @@ void GraphDriver::SetNextDriver(GraphDri
 }
 
 void GraphDriver::SetPreviousDriver(GraphDriver* aPreviousDriver)
 {
   GraphImpl()->GetMonitor().AssertCurrentThreadOwns();
   mPreviousDriver = aPreviousDriver;
 }
 
-bool GraphDriver::Scheduled()
-{
-  GraphImpl()->GetMonitor().AssertCurrentThreadOwns();
-  return mScheduled;
-}
-
 ThreadedDriver::ThreadedDriver(MediaStreamGraphImpl* aGraphImpl)
   : GraphDriver(aGraphImpl)
 { }
 
 class MediaStreamGraphShutdownThreadRunnable : public Runnable {
 public:
   explicit MediaStreamGraphShutdownThreadRunnable(
     already_AddRefed<nsIThread> aThread)
@@ -210,18 +203,17 @@ ThreadedDriver::Start()
   LOG(LogLevel::Debug,
       ("Starting thread for a SystemClockDriver  %p", mGraphImpl));
   Unused << NS_WARN_IF(mThread);
   if (!mThread) { // Ensure we haven't already started it
     nsCOMPtr<nsIRunnable> event = new MediaStreamGraphInitThreadRunnable(this);
     // Note: mThread may be null during event->Run() if we pass to NewNamedThread!  See AudioInitTask
     nsresult rv = NS_NewNamedThread("MediaStreamGrph", getter_AddRefs(mThread));
     if (NS_SUCCEEDED(rv)) {
-      rv = mThread->EventTarget()->Dispatch(event.forget(), NS_DISPATCH_NORMAL);
-      mScheduled = NS_SUCCEEDED(rv);
+      mThread->EventTarget()->Dispatch(event.forget(), NS_DISPATCH_NORMAL);
     }
   }
 }
 
 void
 ThreadedDriver::Revive()
 {
   // Note: only called on MainThread, without monitor
@@ -540,16 +532,17 @@ AudioCallbackDriver::AudioCallbackDriver
   , mSampleRate(0)
   , mInputChannels(1)
   , mIterationDurationMS(MEDIA_GRAPH_TARGET_PERIOD_MS)
   , mStarted(false)
   , mAudioInput(nullptr)
   , mAddedMixer(false)
   , mInCallback(false)
   , mMicrophoneActive(false)
+  , mShouldFallbackIfError(false)
   , mFromFallback(false)
 {
   LOG(LogLevel::Debug, ("AudioCallbackDriver ctor for graph %p", aGraphImpl));
 #if defined(XP_WIN)
   if (XRE_IsContentProcess()) {
     audio::AudioNotificationReceiver::Register(this);
   }
 #endif
@@ -751,23 +744,23 @@ AudioCallbackDriver::Start()
     }
   }
 
   LOG(LogLevel::Debug,
       ("Starting new audio driver off main thread, "
        "to ensure it runs after previous shutdown."));
   RefPtr<AsyncCubebTask> initEvent =
     new AsyncCubebTask(AsAudioCallbackDriver(), AsyncCubebOperation::INIT);
-  nsresult rv = initEvent->Dispatch();
-  mScheduled = NS_SUCCEEDED(rv);
+  initEvent->Dispatch();
 }
 
 bool
 AudioCallbackDriver::StartStream()
 {
+  mShouldFallbackIfError = true;
   if (cubeb_stream_start(mAudioStream) != CUBEB_OK) {
     NS_WARNING("Could not start cubeb stream for MSG.");
     return false;
   }
 
   {
     MonitorAutoLock mon(mGraphImpl->GetMonitor());
     mStarted = true;
@@ -1000,29 +993,34 @@ AudioCallbackDriver::DataCallback(const 
   // (maybe) of these will be full-duplex, the others will get their input
   // data off separate cubeb callbacks.  Take care with how stuff is
   // removed/added to this list and TSAN issues, but input and output will
   // use separate callback methods.
   mGraphImpl->NotifyOutputData(aOutputBuffer, static_cast<size_t>(aFrames),
                                mSampleRate, mOutputChannels);
 
   if (!stillProcessing) {
+    // About to hand over control of the graph.  Do not start a new driver if
+    // StateCallback() receives an error for this stream while the main thread
+    // or another driver has control of the graph.
+    mShouldFallbackIfError = false;
     // Enter shutdown mode. The stable-state handler will detect this
     // and complete shutdown if the graph does not get restarted.
     mGraphImpl->SignalMainThreadCleanup();
     return aFrames - 1;
   }
 
   bool switching = false;
   {
     MonitorAutoLock mon(mGraphImpl->GetMonitor());
     switching = !!NextDriver();
   }
 
   if (switching) {
+    mShouldFallbackIfError = false;
     // If the audio stream has not been started by the previous driver or
     // the graph itself, keep it alive.
     MonitorAutoLock mon(mGraphImpl->GetMonitor());
     if (!IsStarted()) {
       return aFrames;
     }
     LOG(LogLevel::Debug, ("Switching to system driver."));
     RemoveCallback();
@@ -1037,32 +1035,18 @@ AudioCallbackDriver::DataCallback(const 
   return aFrames;
 }
 
 void
 AudioCallbackDriver::StateCallback(cubeb_state aState)
 {
   LOG(LogLevel::Debug, ("AudioCallbackDriver State: %d", aState));
 
-  if (aState == CUBEB_STATE_ERROR) {
-    if (!mAudioStream) {
-      // If we don't have an audio stream here, this means that the stream
-      // initialization has failed. A fallback on a SystemCallDriver will happen at
-      // the callsite of `cubeb_stream_init`.
-      return;
-    }
-
+  if (aState == CUBEB_STATE_ERROR && mShouldFallbackIfError) {
     MonitorAutoLock lock(GraphImpl()->GetMonitor());
-
-    if (NextDriver() && NextDriver()->Scheduled()) {
-      // We are switching to another driver that has already been scheduled
-      // to be initialized and started. There's nothing for us to do here.
-      return;
-    }
-
     // Fall back to a driver using a normal thread. If needed,
     // the graph will try to re-open an audio stream later.
     SystemClockDriver* nextDriver = new SystemClockDriver(GraphImpl());
     SetNextDriver(nextDriver);
     RemoveCallback();
     nextDriver->MarkAsFallback();
     nextDriver->SetGraphTime(this, mIterationStart, mIterationEnd);
     // We're not using SwitchAtNextIteration here, because there
--- a/dom/media/GraphDriver.h
+++ b/dom/media/GraphDriver.h
@@ -140,19 +140,16 @@ public:
 
   // Those are simply or setting the associated pointer, but assert that the
   // lock is held.
   GraphDriver* NextDriver();
   GraphDriver* PreviousDriver();
   void SetNextDriver(GraphDriver* aNextDriver);
   void SetPreviousDriver(GraphDriver* aPreviousDriver);
 
-  /* Return whether we have been scheduled to start. */
-  bool Scheduled();
-
   /**
    * If we are running a real time graph, get the current time stamp to schedule
    * video frames. This has to be reimplemented by real time drivers.
    */
   virtual TimeStamp GetCurrentTimeStamp() {
     return mCurrentTimeStamp;
   }
 
@@ -230,19 +227,16 @@ protected:
   // check whether we're changing driver (in Switching()), from the graph
   // thread.
   // This must be accessed using the {Set,Get}PreviousDriver methods.
   RefPtr<GraphDriver> mPreviousDriver;
   // This is non-null only when this driver is going to switch to an other
   // driver at the end of this iteration.
   // This must be accessed using the {Set,Get}NextDriver methods.
   RefPtr<GraphDriver> mNextDriver;
-  // This is initially false, but set to true as soon the driver has been
-  // scheduled to start through GraphDriver::Start().
-  bool mScheduled;
   virtual ~GraphDriver()
   { }
 };
 
 class MediaStreamGraphInitThreadRunnable;
 
 /**
  * This class is a driver that manages its own thread.
@@ -552,16 +546,24 @@ private:
 
   /* This is atomic and is set by the audio callback thread. It can be read by
    * any thread safely. */
   Atomic<bool> mInCallback;
   /**
    * True if microphone is being used by this process. This is synchronized by
    * the graph's monitor. */
   Atomic<bool> mMicrophoneActive;
+  /* Indication of whether a fallback SystemClockDriver should be started if
+   * StateCallback() receives an error.  No mutex need be held during access.
+   * The transition to true happens before cubeb_stream_start() is called.
+   * After transitioning to false on the last DataCallback(), the stream is
+   * not accessed from another thread until the graph thread either signals
+   * main thread cleanup or dispatches an event to switch to another
+   * driver. */
+  bool mShouldFallbackIfError;
   /* True if this driver was created from a driver created because of a previous
    * AudioCallbackDriver failure. */
   bool mFromFallback;
 };
 
 class AsyncCubebTask : public Runnable
 {
 public: