Bug 1404977 - Part 15 - Disconnect the MediaManager and MediaStreamGraph part of MediaEngineWebRTCMicrophoneSource on the MSG thread. r?pehrsons draft
authorPaul Adenot <paul@paul.cx>
Tue, 05 Jun 2018 18:57:13 +0200
changeset 804253 8952010324b9f4e7f1ed866f4ec183b30b9e9b3f
parent 804252 16d1d5fcf5d997e0d94306def57be304ad2a06b3
push id112326
push userpaul@paul.cx
push dateTue, 05 Jun 2018 17:03:32 +0000
reviewerspehrsons
bugs1404977
milestone62.0a1
Bug 1404977 - Part 15 - Disconnect the MediaManager and MediaStreamGraph part of MediaEngineWebRTCMicrophoneSource on the MSG thread. r?pehrsons This makes the lifetime and various thread access clearer. MozReview-Commit-ID: ExDEB0hcQ92
dom/media/MediaStreamGraph.cpp
dom/media/MediaStreamGraph.h
dom/media/webrtc/MediaEngineWebRTC.h
dom/media/webrtc/MediaEngineWebRTCAudio.cpp
--- a/dom/media/MediaStreamGraph.cpp
+++ b/dom/media/MediaStreamGraph.cpp
@@ -873,16 +873,20 @@ MediaStreamGraphImpl::CloseAudioInputImp
     MOZ_ASSERT(aID.isSome(), "Closing an audio input that was not opened.");
   }
 
   nsTArray<RefPtr<AudioDataListener>>* listeners = mInputDeviceUsers.GetValue(aID.value());
 
   MOZ_ASSERT(listeners);
   DebugOnly<bool> wasPresent = listeners->RemoveElement(aListener);
   MOZ_ASSERT(wasPresent);
+
+  // Breaks the cycle between the MSG and the listener.
+  aListener->Disconnect(this);
+
   if (!listeners->IsEmpty()) {
     // There is still a consumer for this audio input device
     return;
   }
 
   mInputDeviceID = nullptr; // reset to default
   mInputDeviceUsers.Remove(aID.value());
 
--- a/dom/media/MediaStreamGraph.h
+++ b/dom/media/MediaStreamGraph.h
@@ -121,16 +121,21 @@ public:
    * Number of audio input channels.
    */
   virtual uint32_t InputChannelCount(MediaStreamGraph* aGraph) = 0;
 
   /**
    * Called when the underlying audio device has changed.
    */
   virtual void DeviceChanged(MediaStreamGraph* aGraph) = 0;
+
+  /**
+   * Called when the underlying audio device is being closed.
+   */
+  virtual void Disconnect(MediaStreamGraph* aGraph) = 0;
 };
 
 class AudioDataListener : public AudioDataListenerInterface {
 protected:
   // Protected destructor, to discourage deletion outside of Release():
   virtual ~AudioDataListener() {}
 
 public:
--- a/dom/media/webrtc/MediaEngineWebRTC.h
+++ b/dom/media/webrtc/MediaEngineWebRTC.h
@@ -155,26 +155,27 @@ private:
   Mutex mMutex;
   nsTArray<RefPtr<AudioDeviceInfo>> mDevices;
   // If mManualInvalidation is true, then it is necessary to query the device
   // list each time instead of relying on automatic invalidation of the cache by
   // cubeb itself. Set in the constructor and then can be access on any thread.
   bool mManualInvalidation;
 };
 
+// This class is instantiated on the MediaManager thread, and is then sent and
+// only ever access again on the MediaStreamGraph.
 class WebRTCAudioDataListener : public AudioDataListener
 {
 protected:
   // Protected destructor, to discourage deletion outside of Release():
   virtual ~WebRTCAudioDataListener() {}
 
 public:
   explicit WebRTCAudioDataListener(MediaEngineWebRTCMicrophoneSource* aAudioSource)
-    : mMutex("WebRTCAudioDataListener::mMutex")
-    , mAudioSource(aAudioSource)
+    : mAudioSource(aAudioSource)
   {}
 
   // AudioDataListenerInterface methods
   void NotifyOutputData(MediaStreamGraph* aGraph,
                         AudioDataValue* aBuffer,
                         size_t aFrames,
                         TrackRate aRate,
                         uint32_t aChannels) override;
@@ -184,20 +185,19 @@ public:
                        size_t aFrames,
                        TrackRate aRate,
                        uint32_t aChannels) override;
 
   uint32_t InputChannelCount(MediaStreamGraph* aGraph) override;
 
   void DeviceChanged(MediaStreamGraph* aGraph) override;
 
-  void Shutdown();
+  void Disconnect(MediaStreamGraph* aGraph) override;
 
 private:
-  Mutex mMutex;
   RefPtr<MediaEngineWebRTCMicrophoneSource> mAudioSource;
 };
 
 class MediaEngineWebRTCMicrophoneSource : public MediaEngineSource,
                                           public AudioDataListenerInterface
 {
 public:
   MediaEngineWebRTCMicrophoneSource(RefPtr<AudioDeviceInfo> aInfo,
@@ -256,16 +256,18 @@ public:
 
   void DeviceChanged(MediaStreamGraph* aGraph) override;
 
   uint32_t InputChannelCount(MediaStreamGraph* aGraph) override
   {
     return GetRequestedInputChannelCount();
   }
 
+  void Disconnect(MediaStreamGraph* aGraph) override;
+
   dom::MediaSourceEnum GetMediaSource() const override
   {
     return dom::MediaSourceEnum::Microphone;
   }
 
   nsresult TakePhoto(MediaEnginePhotoCallback* aCallback) override
   {
     return NS_ERROR_NOT_IMPLEMENTED;
@@ -376,19 +378,24 @@ private:
   // Graph thread only.
   bool PassThrough() const;
 
   // Graph thread only.
   void SetPassThrough(bool aPassThrough);
   uint32_t GetRequestedInputChannelCount();
   void SetRequestedInputChannelCount(uint32_t aRequestedInputChannelCount);
 
-  // Owning thread only.
+  // mListener is created on the MediaManager thread, and then sent to the MSG
+  // thread. On shutdown, we send this pointer to the MSG thread again, telling
+  // it to clean up.
   RefPtr<WebRTCAudioDataListener> mListener;
+
+  // mEnumerator is thread safe.
   mozilla::CubebDeviceEnumerator mEnumerator;
+
   const RefPtr<AudioDeviceInfo> mDeviceInfo;
 
   const UniquePtr<webrtc::AudioProcessing> mAudioProcessing;
 
   // accessed from the GraphDriver thread except for deletion.
   nsAutoPtr<AudioPacketizer<AudioDataValue, float>> mPacketizerInput;
   nsAutoPtr<AudioPacketizer<AudioDataValue, float>> mPacketizerOutput;
 
--- a/dom/media/webrtc/MediaEngineWebRTCAudio.cpp
+++ b/dom/media/webrtc/MediaEngineWebRTCAudio.cpp
@@ -92,19 +92,23 @@ WebRTCAudioDataListener::InputChannelCou
   MOZ_ASSERT(static_cast<MediaStreamGraphImpl*>(aGraph)->CurrentDriver()->OnThread());
   if (mAudioSource) {
     return mAudioSource->InputChannelCount(aGraph);
   }
   return 0;
 }
 
 void
-WebRTCAudioDataListener::Shutdown()
+WebRTCAudioDataListener::Disconnect(MediaStreamGraph* aGraph)
 {
-  mAudioSource = nullptr;
+  MOZ_ASSERT(static_cast<MediaStreamGraphImpl*>(aGraph)->CurrentDriver()->OnThread());
+  if (mAudioSource) {
+    mAudioSource->Disconnect(aGraph);
+    mAudioSource = nullptr;
+  }
 }
 
 /**
  * WebRTC Microphone MediaEngineSource.
  */
 
 MediaEngineWebRTCMicrophoneSource::Allocation::Allocation(
     const RefPtr<AllocationHandle>& aHandle)
@@ -493,19 +497,20 @@ MediaEngineWebRTCMicrophoneSource::Updat
   return NS_OK;
 }
 
 #undef HANDLE_APM_ERROR
 
 bool
 MediaEngineWebRTCMicrophoneSource::PassThrough() const
 {
-  MOZ_ASSERT(!mAllocations.IsEmpty() &&
+  MOZ_ASSERT(mAllocations.IsEmpty() ||
+             (!mAllocations.IsEmpty() &&
              mAllocations[0].mStream &&
-             mAllocations[0].mStream->GraphImpl()->CurrentDriver()->OnThread());
+             mAllocations[0].mStream->GraphImpl()->CurrentDriver()->OnThread()));
   return mSkipProcessing;
 }
 void
 MediaEngineWebRTCMicrophoneSource::SetPassThrough(bool aPassThrough)
 {
   if (mAllocations.IsEmpty()) {
     return;
   }
@@ -762,32 +767,27 @@ MediaEngineWebRTCMicrophoneSource::Stop(
 
   {
     // This spans setting both the enabled state and mState.
     MutexAutoLock lock(mMutex);
     allocation.mEnabled = false;
 
     CubebUtils::AudioDeviceID deviceID = mDeviceInfo->DeviceID();
     allocation.mStream->CloseAudioInput(Some(deviceID), mListener);
+    mListener = nullptr;
 
     if (HasEnabledTrack()) {
       // Another track is keeping us from stopping
       return NS_OK;
     }
 
     MOZ_ASSERT(mState == kStarted, "Should be started when stopping");
     mState = kStopped;
   }
 
-  if (mListener) {
-    // breaks a cycle, since the WebRTCAudioDataListener has a RefPtr to us
-    mListener->Shutdown();
-    mListener = nullptr;
-  }
-
   return NS_OK;
 }
 
 void
 MediaEngineWebRTCMicrophoneSource::GetSettings(dom::MediaTrackSettings& aOutSettings) const
 {
   MOZ_ASSERT(NS_IsMainThread());
   aOutSettings = *mSettings;
@@ -1228,27 +1228,28 @@ MediaEngineWebRTCMicrophoneSource::Devic
              mAllocations[0].mStream->GraphImpl()->CurrentDriver()->OnThread());
   // Reset some processing
   ResetProcessingIfNeeded(gain_control);
   ResetProcessingIfNeeded(echo_cancellation);
   ResetProcessingIfNeeded(noise_suppression);
 }
 
 void
+MediaEngineWebRTCMicrophoneSource::Disconnect(MediaStreamGraph* aGraph)
+{
+  // This method is just for asserts.
+  MOZ_ASSERT(static_cast<MediaStreamGraphImpl*>(aGraph)->CurrentDriver()->OnThread());
+  MOZ_ASSERT(!mListener);
+}
+
+void
 MediaEngineWebRTCMicrophoneSource::Shutdown()
 {
   AssertIsOnOwningThread();
 
-  if (mListener) {
-    // breaks a cycle, since the WebRTCAudioDataListener has a RefPtr to us
-    mListener->Shutdown();
-    // Don't release the webrtc.org pointers yet until the Listener is (async) shutdown
-    mListener = nullptr;
-  }
-
   if (mState == kStarted) {
     for (const Allocation& allocation : mAllocations) {
       if (allocation.mEnabled) {
         Stop(allocation.mHandle);
       }
     }
     MOZ_ASSERT(mState == kStopped);
   }