Bug 1541290 - Restart driver properly on revive method. r=padenot
authorAlex Chronopoulos <achronop@gmail.com>
Fri, 19 Apr 2019 08:34:22 +0000
changeset 470199 510178f0dc32ee3c768c6c8f413bf16905c3e422
parent 470198 80d7bc0880915bb804044007d0fe7a23642ead6c
child 470200 f75aabcf81d0022ac11c173485c1aab2091193d1
push id83585
push userachronopoulos@mozilla.com
push dateFri, 19 Apr 2019 08:36:27 +0000
treeherderautoland@f75aabcf81d0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs1541290
milestone68.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 1541290 - Restart driver properly on revive method. r=padenot Revive method of AudioCallbackDriver was wrong because it was re-initializing an already initialized driver. That was hitting an assert. Instead of that it should stop the drained stream and start it again. Also, mStarted member should be reset properly on the stop method. Differential Revision: https://phabricator.services.mozilla.com/D26678
dom/media/GraphDriver.cpp
dom/media/GraphDriver.h
--- a/dom/media/GraphDriver.cpp
+++ b/dom/media/GraphDriver.cpp
@@ -414,18 +414,19 @@ TimeDuration OfflineClockDriver::WaitInt
 }
 
 AsyncCubebTask::AsyncCubebTask(AudioCallbackDriver* aDriver,
                                AsyncCubebOperation aOperation)
     : Runnable("AsyncCubebTask"),
       mDriver(aDriver),
       mOperation(aOperation),
       mShutdownGrip(aDriver->GraphImpl()) {
-  NS_WARNING_ASSERTION(mDriver->mAudioStream || aOperation == INIT,
-                       "No audio stream!");
+  NS_WARNING_ASSERTION(
+      mDriver->mAudioStream || aOperation == AsyncCubebOperation::INIT,
+      "No audio stream!");
 }
 
 AsyncCubebTask::~AsyncCubebTask() {}
 
 NS_IMETHODIMP
 AsyncCubebTask::Run() {
   MOZ_ASSERT(mDriver);
 
@@ -436,16 +437,32 @@ AsyncCubebTask::Run() {
       if (!mDriver->Init()) {
         LOG(LogLevel::Warning,
             ("AsyncCubebOperation::INIT failed for driver=%p", mDriver.get()));
         return NS_ERROR_FAILURE;
       }
       mDriver->CompleteAudioContextOperations(mOperation);
       break;
     }
+    case AsyncCubebOperation::START: {
+      LOG(LogLevel::Debug, ("%p: AsyncCubebOperation::START driver=%p",
+                            mDriver->GraphImpl(), mDriver.get()));
+      if (!mDriver->StartStream()) {
+        LOG(LogLevel::Warning,
+            ("%p: AsyncCubebOperation couldn't start the driver=%p.",
+             mDriver->GraphImpl(), mDriver.get()));
+      }
+      break;
+    }
+    case AsyncCubebOperation::STOP: {
+      LOG(LogLevel::Debug, ("%p: AsyncCubebOperation::STOP driver=%p",
+                            mDriver->GraphImpl(), mDriver.get()));
+      mDriver->Stop();
+      break;
+    }
     case AsyncCubebOperation::SHUTDOWN: {
       LOG(LogLevel::Debug, ("%p: AsyncCubebOperation::SHUTDOWN driver=%p",
                             mDriver->GraphImpl(), mDriver.get()));
       mDriver->Stop();
 
       mDriver->CompleteAudioContextOperations(mOperation);
 
       mDriver = nullptr;
@@ -708,34 +725,42 @@ bool AudioCallbackDriver::StartStream() 
   return true;
 }
 
 void AudioCallbackDriver::Stop() {
   MOZ_ASSERT(OnCubebOperationThread());
   if (cubeb_stream_stop(mAudioStream) != CUBEB_OK) {
     NS_WARNING("Could not stop cubeb stream for MSG.");
   }
+  mStarted = false;
 }
 
 void AudioCallbackDriver::Revive() {
   MOZ_ASSERT(NS_IsMainThread() && !ThreadRunning());
   // Note: only called on MainThread, without monitor
   // We know were weren't in a running state
   LOG(LogLevel::Debug, ("%p: AudioCallbackDriver reviving.", GraphImpl()));
   // If we were switching, switch now. Otherwise, start the audio thread again.
   MonitorAutoLock mon(GraphImpl()->GetMonitor());
   if (NextDriver()) {
     SwitchToNextDriver();
   } else {
     LOG(LogLevel::Debug,
         ("Starting audio threads for MediaStreamGraph %p from a new thread.",
          mGraphImpl.get()));
-    RefPtr<AsyncCubebTask> initEvent =
-        new AsyncCubebTask(this, AsyncCubebOperation::INIT);
-    initEvent->Dispatch();
+    if (IsStarted()) {
+      RefPtr<AsyncCubebTask> stopEvent =
+          new AsyncCubebTask(this, AsyncCubebOperation::STOP);
+      // This dispatches to a thread pool with a maximum of one thread thus it
+      // is guaranteed to be executed before the start event, right below.
+      stopEvent->Dispatch();
+    }
+    RefPtr<AsyncCubebTask> startEvent =
+        new AsyncCubebTask(this, AsyncCubebOperation::START);
+    startEvent->Dispatch();
   }
 }
 
 void AudioCallbackDriver::RemoveMixerCallback() {
   MOZ_ASSERT(OnGraphThread() || !ThreadRunning());
 
   if (mAddedMixer) {
     GraphImpl()->mMixer.RemoveCallback(this);
--- a/dom/media/GraphDriver.h
+++ b/dom/media/GraphDriver.h
@@ -317,17 +317,17 @@ struct StreamAndPromiseForOperation {
                                dom::AudioContextOperation aOperation,
                                dom::AudioContextOperationFlags aFlags);
   RefPtr<MediaStream> mStream;
   void* mPromise;
   dom::AudioContextOperation mOperation;
   dom::AudioContextOperationFlags mFlags;
 };
 
-enum AsyncCubebOperation { INIT, SHUTDOWN };
+enum class AsyncCubebOperation { INIT, START, STOP, SHUTDOWN  };
 enum class AudioInputType { Unknown, Voice };
 
 /**
  * This is a graph driver that is based on callback functions called by the
  * audio api. This ensures minimal audio latency, because it means there is no
  * buffering happening: the audio is generated inside the callback.
  *
  * This design is less flexible than running our own thread: