Bug 1545079 - Execute revive logic in operation thread. r=padenot
authorAlex Chronopoulos <achronop@gmail.com>
Tue, 14 May 2019 16:00:22 +0000
changeset 474372 70a4ba81a3960e7fdec38063e2608b56b61436b4
parent 474371 17935ce513d2c8909992f187edc68e0ae2e5d941
child 474373 7e8007308d0daeaf79f9ef9efe642e4ac0fb1941
push id113152
push userdluca@mozilla.com
push dateSat, 18 May 2019 10:33:03 +0000
treeherdermozilla-inbound@9b2f851979cb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs1545079
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 1545079 - Execute revive logic in operation thread. r=padenot `IsStarted` may not be updated at the time `Revive()` method is executed since the update of the flag happens on the async operation. This will be the case if `Revive` is executed right after `Start`. When that happens the revive method asserts that the stream is started. Differential Revision: https://phabricator.services.mozilla.com/D30889
dom/media/GraphDriver.cpp
dom/media/GraphDriver.h
--- a/dom/media/GraphDriver.cpp
+++ b/dom/media/GraphDriver.cpp
@@ -437,32 +437,29 @@ 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",
+    case AsyncCubebOperation::REVIVE: {
+      LOG(LogLevel::Debug, ("%p: AsyncCubebOperation::REVIVE driver=%p",
                             mDriver->GraphImpl(), mDriver.get()));
+      if (mDriver->IsStarted()) {
+        mDriver->Stop();
+      }
       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;
@@ -740,29 +737,19 @@ void AudioCallbackDriver::Revive() {
   // 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()));
-    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();
+    RefPtr<AsyncCubebTask> reviveEvent =
+        new AsyncCubebTask(this, AsyncCubebOperation::REVIVE);
+    reviveEvent->Dispatch();
   }
 }
 
 void AudioCallbackDriver::RemoveMixerCallback() {
   MOZ_ASSERT(OnGraphThread() || !ThreadRunning());
 
   if (mAddedMixer) {
     GraphImpl()->mMixer.RemoveCallback(this);
@@ -980,19 +967,30 @@ long AudioCallbackDriver::DataCallback(c
     // Returning less than aFrames starts the draining and eventually stops the
     // audio thread. This function will never get called again.
     return aFrames - 1;
   }
 
   return aFrames;
 }
 
+static const char* StateToString(cubeb_state aState) {
+  switch (aState) {
+    case CUBEB_STATE_STARTED: return "STARTED";
+    case CUBEB_STATE_STOPPED: return "STOPPED";
+    case CUBEB_STATE_DRAINED: return "DRAINED";
+    case CUBEB_STATE_ERROR: return "ERROR";
+    default:
+      MOZ_CRASH("Unexpected state!");
+  }
+}
+
 void AudioCallbackDriver::StateCallback(cubeb_state aState) {
   MOZ_ASSERT(!OnGraphThread());
-  LOG(LogLevel::Debug, ("AudioCallbackDriver State: %d", aState));
+  LOG(LogLevel::Debug, ("AudioCallbackDriver State: %s", StateToString(aState)));
 
   // Clear the flag for the not running
   // states: stopped, drained, error.
   mAudioThreadRunning = (aState == CUBEB_STATE_STARTED);
 
   if (aState == CUBEB_STATE_ERROR && mShouldFallbackIfError) {
     MOZ_ASSERT(!ThreadRunning());
     mShouldFallbackIfError = false;
--- a/dom/media/GraphDriver.h
+++ b/dom/media/GraphDriver.h
@@ -7,16 +7,17 @@
 #define GRAPHDRIVER_H_
 
 #include "nsAutoRef.h"
 #include "AudioBufferUtils.h"
 #include "AudioMixer.h"
 #include "AudioSegment.h"
 #include "SelfRef.h"
 #include "mozilla/Atomics.h"
+#include "mozilla/dom/AudioContext.h"
 #include "mozilla/SharedThreadPool.h"
 #include "mozilla/StaticPtr.h"
 
 #include <thread>
 
 #if defined(XP_WIN)
 #  include "mozilla/audio/AudioNotificationReceiver.h"
 #endif
@@ -317,17 +318,17 @@ struct StreamAndPromiseForOperation {
                                dom::AudioContextOperation aOperation,
                                dom::AudioContextOperationFlags aFlags);
   RefPtr<MediaStream> mStream;
   void* mPromise;
   dom::AudioContextOperation mOperation;
   dom::AudioContextOperationFlags mFlags;
 };
 
-enum class AsyncCubebOperation { INIT, START, STOP, SHUTDOWN };
+enum class AsyncCubebOperation { INIT, REVIVE, 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: