Bug 1382366 - Disable AudioCallback -> SystemClockDriver fallback before disowning graph. f=pehrsons, r=padenot, a=gchang
authorKarl Tomlinson <karlt+@karlt.net>
Sat, 04 Nov 2017 19:00:46 +1300
changeset 356639 1bfb3d8d451088558a2a274e74f39fed231c567e
parent 356638 084c427ccf990d185117decb4c52003932eb8b94
child 356640 5623e01e63a8c3d812cb3ce1168e63c16f10779b
push id7449
push userryanvm@gmail.com
push dateTue, 19 Dec 2017 15:07:08 +0000
treeherdermozilla-esr52@14a389d40329 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot, gchang
bugs1382366
milestone52.5.3
Bug 1382366 - Disable AudioCallback -> SystemClockDriver fallback before disowning graph. f=pehrsons, r=padenot, a=gchang
dom/media/GraphDriver.cpp
dom/media/GraphDriver.h
--- a/dom/media/GraphDriver.cpp
+++ b/dom/media/GraphDriver.cpp
@@ -54,18 +54,17 @@ struct AutoProfilerUnregisterThread
 
 GraphDriver::GraphDriver(MediaStreamGraphImpl* aGraphImpl)
   : mIterationStart(0),
     mIterationEnd(0),
     mGraphImpl(aGraphImpl),
     mWaitState(WAITSTATE_RUNNING),
     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
@@ -149,22 +148,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)
     : mThread(aThread)
@@ -245,18 +238,17 @@ ThreadedDriver::Start()
 {
   LIFECYCLE_LOG("Starting thread for a SystemClockDriver  %p\n", 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->Dispatch(event, NS_DISPATCH_NORMAL);
-      mScheduled = NS_SUCCEEDED(rv);
+      mThread->Dispatch(event, NS_DISPATCH_NORMAL);
     }
   }
 }
 
 void
 ThreadedDriver::Resume()
 {
   Start();
@@ -583,16 +575,17 @@ AudioCallbackDriver::AudioCallbackDriver
   , mInputChannels(1)
   , mIterationDurationMS(MEDIA_GRAPH_TARGET_PERIOD_MS)
   , mStarted(false)
   , mAudioInput(nullptr)
   , mAudioChannel(aGraphImpl->AudioChannel())
   , mAddedMixer(false)
   , mInCallback(false)
   , mMicrophoneActive(false)
+  , mShouldFallbackIfError(false)
   , mFromFallback(false)
 {
   STREAM_LOG(LogLevel::Debug, ("AudioCallbackDriver ctor for graph %p", aGraphImpl));
 }
 
 AudioCallbackDriver::~AudioCallbackDriver()
 {
   MOZ_ASSERT(mPromisesForOperation.IsEmpty());
@@ -796,23 +789,23 @@ AudioCallbackDriver::Start()
       mPreviousDriver = nullptr;
     }
   }
 
   LIFECYCLE_LOG("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();
 }
 
 void
 AudioCallbackDriver::StartStream()
 {
+  mShouldFallbackIfError = true;
   if (cubeb_stream_start(mAudioStream) != CUBEB_OK) {
     MOZ_CRASH("Could not start cubeb stream for MSG.");
   }
 
   {
     MonitorAutoLock mon(mGraphImpl->GetMonitor());
     mStarted = true;
     mWaitState = WAITSTATE_RUNNING;
@@ -1015,29 +1008,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, ChannelCount);
 
   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;
     }
     STREAM_LOG(LogLevel::Debug, ("Switching to system driver."));
     RemoveCallback();
@@ -1051,25 +1049,18 @@ AudioCallbackDriver::DataCallback(const 
 
   return aFrames;
 }
 
 void
 AudioCallbackDriver::StateCallback(cubeb_state aState)
 {
   STREAM_LOG(LogLevel::Debug, ("AudioCallbackDriver State: %d", aState));
-  if (aState == CUBEB_STATE_ERROR) {
+  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
@@ -141,19 +141,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;
   }
 
@@ -247,19 +244,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.
@@ -538,16 +532,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. */
   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: