Bug 1404977 - Part 12 - Make DeviceChanged() notification thread safe by using the MSG message queue. r?pehrsons draft
authorPaul Adenot <paul@paul.cx>
Thu, 31 May 2018 16:42:24 +0200
changeset 825393 a58d90d41c640c38000bf3f6ca444ee573cc9c5c
parent 825392 5a7fd45932e83797595811040d42e6da7acdf1fb
child 825394 3967e717d24bb55e1a0af7aad6eaa0e628928569
push id118097
push userpaul@paul.cx
push dateWed, 01 Aug 2018 17:03:19 +0000
reviewerspehrsons
bugs1404977
milestone63.0a1
Bug 1404977 - Part 12 - Make DeviceChanged() notification thread safe by using the MSG message queue. r?pehrsons This is slightly slower, especially if the main thread is busy, but it's cleaner and actually safe. MozReview-Commit-ID: 4C2FalxmE3L
dom/media/MediaStreamGraph.cpp
dom/media/MediaStreamGraphImpl.h
--- a/dom/media/MediaStreamGraph.cpp
+++ b/dom/media/MediaStreamGraph.cpp
@@ -974,28 +974,64 @@ MediaStreamGraphImpl::NotifyInputData(co
 #endif
   nsTArray<RefPtr<AudioDataListener>>* listeners = mInputDeviceUsers.GetValue(mInputDeviceID);
   MOZ_ASSERT(listeners);
   for (auto& listener : *listeners) {
     listener->NotifyInputData(this, aBuffer, aFrames, aRate, aChannels);
   }
 }
 
-void MediaStreamGraphImpl::DeviceChanged()
+void MediaStreamGraphImpl::DeviceChangedImpl()
 {
-  MOZ_ASSERT(!OnGraphThread());
+  MOZ_ASSERT(OnGraphThread());
+
   if (!mInputDeviceID) {
     return;
   }
-  nsTArray<RefPtr<AudioDataListener>>* listeners = mInputDeviceUsers.GetValue(mInputDeviceID);
+
+  nsTArray<RefPtr<AudioDataListener>>* listeners =
+    mInputDeviceUsers.GetValue(mInputDeviceID);
   for (auto& listener : *listeners) {
     listener->DeviceChanged();
   }
 }
 
+void MediaStreamGraphImpl::DeviceChanged()
+{
+  // This is safe to be called from any thread: this message comes from an
+  // underlying platform API, and we don't have much guarantees. If it is not
+  // called from the main thread (and it probably will rarely be), it will post
+  // itself to the main thread, and the actual device change message will be ran
+  // and acted upon on the graph thread.
+  if (!NS_IsMainThread()) {
+    RefPtr<nsIRunnable> runnable =
+      WrapRunnable(this,
+                   &MediaStreamGraphImpl::DeviceChanged);
+    mAbstractMainThread->Dispatch(runnable.forget());
+    return;
+  }
+
+  class Message : public ControlMessage {
+  public:
+    explicit Message(MediaStreamGraph* aGraph)
+      : ControlMessage(nullptr)
+      , mGraphImpl(static_cast<MediaStreamGraphImpl*>(aGraph))
+    {}
+    void Run() override
+    {
+      mGraphImpl->DeviceChangedImpl();
+    }
+    // We know that this is valid, because the graph can't shutdown if it has
+    // messages.
+    MediaStreamGraphImpl* mGraphImpl;
+  };
+
+  AppendMessage(MakeUnique<Message>(this));
+}
+
 void MediaStreamGraphImpl::ReevaluateInputDevice()
 {
   MOZ_ASSERT(OnGraphThread());
   bool needToSwitch = false;
 
   if (CurrentDriver()->AsAudioCallbackDriver()) {
     AudioCallbackDriver* audioCallbackDriver = CurrentDriver()->AsAudioCallbackDriver();
     if (audioCallbackDriver->InputChannelCount() != AudioInputChannelCount()) {
--- a/dom/media/MediaStreamGraphImpl.h
+++ b/dom/media/MediaStreamGraphImpl.h
@@ -394,30 +394,33 @@ public:
                            AudioDataListener* aListener);
   /* Called on the main thread when input audio from aID is not needed
    * anymore, for a particular stream. It can be that other streams still need
    * audio from this audio input device. */
   virtual void CloseAudioInput(Maybe<CubebUtils::AudioDeviceID>& aID,
                                AudioDataListener* aListener) override;
   /* Called on the graph thread when the input device settings should be
    * reevaluated, for example, if the channel count of the input stream should
-   * be changed . */
+   * be changed. */
   void ReevaluateInputDevice();
   /* Called on the graph thread when there is new output data for listeners.
    * This is the mixed audio output of this MediaStreamGraph. */
   void NotifyOutputData(AudioDataValue* aBuffer, size_t aFrames,
                         TrackRate aRate, uint32_t aChannels);
   /* Called on the graph thread when there is new input data for listeners. This
    * is the raw audio input for this MediaStreamGraph. */
   void NotifyInputData(const AudioDataValue* aBuffer, size_t aFrames,
                        TrackRate aRate, uint32_t aChannels);
   /* Called every time there are changes to input/output audio devices like
-   * plug/unplug etc.  Depending on the platform, this can be called on
-   * different thread. This function is called with the MSG lock held. */
+   * plug/unplug etc. This can be called on any thread, and posts a message to
+   * the main thread so that it can post a message to the graph thread. */
   void DeviceChanged();
+  /* Called every time there are changes to input/output audio devices. This is
+   * called on the graph thread. */
+  void DeviceChangedImpl();
 
   /**
    * Compute how much stream data we would like to buffer for aStream.
    */
   StreamTime GetDesiredBufferEnd(MediaStream* aStream);
   /**
    * Returns true when there are no active streams.
    */