Bug 1382095 - Avoid ref-counting MediaEngineSource itself in setLastPrefs runnables to improve shutdown. draft
authorJan-Ivar Bruaroey <jib@mozilla.com>
Sat, 01 Jul 2017 15:01:25 -0700
changeset 610989 e946dc6555af0b26eab20d60bbc18130b6d91d81
parent 606958 6fec4855b5345eb63fef57089e61829b88f5f4eb
child 610990 a1e23adeb54a8753dac6313eed0d6a30cb5c2a88
push id69080
push userjbruaroey@mozilla.com
push dateWed, 19 Jul 2017 02:43:31 +0000
bugs1382095
milestone56.0a1
Bug 1382095 - Avoid ref-counting MediaEngineSource itself in setLastPrefs runnables to improve shutdown. MozReview-Commit-ID: LyMIXG9ClRJ
dom/media/webrtc/MediaEngine.h
dom/media/webrtc/MediaEngineCameraVideoSource.h
dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
dom/media/webrtc/MediaEngineWebRTCAudio.cpp
--- a/dom/media/webrtc/MediaEngine.h
+++ b/dom/media/webrtc/MediaEngine.h
@@ -349,24 +349,25 @@ public:
 
   virtual uint32_t GetBestFitnessDistance(
       const nsTArray<const NormalizedConstraintSet*>& aConstraintSets,
       const nsString& aDeviceId) const = 0;
 
   void GetSettings(dom::MediaTrackSettings& aOutSettings)
   {
     MOZ_ASSERT(NS_IsMainThread());
-    aOutSettings = mSettings;
+    aOutSettings = *mSettings;
   }
 
 protected:
   // Only class' own members can be initialized in constructor initializer list.
   explicit MediaEngineSource(MediaEngineState aState)
     : mState(aState)
     , mInShutdown(false)
+    , mSettings(MakeRefPtr<media::Refcountable<dom::MediaTrackSettings>>())
   {}
 
   /* UpdateSingleSource - Centralized abstract function to implement in those
    * cases where a single device is being shared between users. Should apply net
    * constraints and restart the device as needed.
    *
    * aHandle           - New or existing handle, or null to update after removal.
    * aNetConstraints   - Net constraints to be applied to the single device.
@@ -443,18 +444,20 @@ protected:
 
   MediaEngineState mState;
 
   NS_DECL_OWNINGTHREAD
 
   nsTArray<RefPtr<AllocationHandle>> mRegisteredHandles;
   bool mInShutdown;
 
-  // Main-thread only:
-  dom::MediaTrackSettings mSettings;
+  // The following is accessed on main-thread only. It has its own ref-count to
+  // avoid ref-counting MediaEngineSource itself in runnables.
+  // (MediaEngineSource subclasses balk on ref-counts too late during shutdown.)
+  RefPtr<media::Refcountable<dom::MediaTrackSettings>> mSettings;
 };
 
 class MediaEngineVideoSource : public MediaEngineSource
 {
 public:
   virtual ~MediaEngineVideoSource() {}
 
 protected:
--- a/dom/media/webrtc/MediaEngineCameraVideoSource.h
+++ b/dom/media/webrtc/MediaEngineCameraVideoSource.h
@@ -55,18 +55,16 @@ public:
   {
     return NS_ERROR_NOT_IMPLEMENTED;
   }
 
   uint32_t GetBestFitnessDistance(
       const nsTArray<const NormalizedConstraintSet*>& aConstraintSets,
       const nsString& aDeviceId) const override;
 
-  void Shutdown() override {};
-
 protected:
   struct CapabilityCandidate {
     explicit CapabilityCandidate(uint8_t index, uint32_t distance = 0)
     : mIndex(index), mDistance(distance) {}
 
     size_t mIndex;
     uint32_t mDistance;
   };
--- a/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
+++ b/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp
@@ -31,19 +31,19 @@ MediaEngineRemoteVideoSource::MediaEngin
   int aIndex, mozilla::camera::CaptureEngine aCapEngine,
   dom::MediaSourceEnum aMediaSource, bool aScary, const char* aMonitorName)
   : MediaEngineCameraVideoSource(aIndex, aMonitorName),
     mMediaSource(aMediaSource),
     mCapEngine(aCapEngine),
     mScary(aScary)
 {
   MOZ_ASSERT(aMediaSource != dom::MediaSourceEnum::Other);
-  mSettings.mWidth.Construct(0);
-  mSettings.mHeight.Construct(0);
-  mSettings.mFrameRate.Construct(0);
+  mSettings->mWidth.Construct(0);
+  mSettings->mHeight.Construct(0);
+  mSettings->mFrameRate.Construct(0);
   Init();
 }
 
 void
 MediaEngineRemoteVideoSource::Init()
 {
   LOG((__PRETTY_FUNCTION__));
   char deviceName[kMaxDeviceNameLength];
@@ -323,22 +323,22 @@ MediaEngineRemoteVideoSource::SetLastCap
       // know the real resolution yet, so report min(ideal, max) for now.
       cap.width = std::min(cap.width >> 16, cap.width & 0xffff);
       cap.height = std::min(cap.height >> 16, cap.height & 0xffff);
       break;
 
     default:
       break;
   }
-  RefPtr<MediaEngineRemoteVideoSource> that = this;
+  auto settings = mSettings;
 
-  NS_DispatchToMainThread(media::NewRunnableFrom([that, cap]() mutable {
-    that->mSettings.mWidth.Value() = cap.width;
-    that->mSettings.mHeight.Value() = cap.height;
-    that->mSettings.mFrameRate.Value() = cap.maxFPS;
+  NS_DispatchToMainThread(media::NewRunnableFrom([settings, cap]() mutable {
+    settings->mWidth.Value() = cap.width;
+    settings->mHeight.Value() = cap.height;
+    settings->mFrameRate.Value() = cap.maxFPS;
     return NS_OK;
   }));
 }
 
 void
 MediaEngineRemoteVideoSource::NotifyPull(MediaStreamGraph* aGraph,
                                          SourceMediaStream* aSource,
                                          TrackID aID, StreamTime aDesiredTime,
@@ -366,20 +366,20 @@ MediaEngineRemoteVideoSource::FrameSizeC
   mMonitor.AssertCurrentThreadOwns(); // mWidth and mHeight are protected...
 #endif
   if ((mWidth < 0) || (mHeight < 0) ||
       (w !=  (unsigned int) mWidth) || (h != (unsigned int) mHeight)) {
     LOG(("MediaEngineRemoteVideoSource Video FrameSizeChange: %ux%u was %ux%u", w, h, mWidth, mHeight));
     mWidth = w;
     mHeight = h;
 
-    RefPtr<MediaEngineRemoteVideoSource> that = this;
-    NS_DispatchToMainThread(media::NewRunnableFrom([that, w, h]() mutable {
-      that->mSettings.mWidth.Value() = w;
-      that->mSettings.mHeight.Value() = h;
+    auto settings = mSettings;
+    NS_DispatchToMainThread(media::NewRunnableFrom([settings, w, h]() mutable {
+      settings->mWidth.Value() = w;
+      settings->mHeight.Value() = h;
       return NS_OK;
     }));
   }
 }
 
 int
 MediaEngineRemoteVideoSource::DeliverFrame(uint8_t* aBuffer ,
                                     const camera::VideoFrameProperties& aProps)
--- a/dom/media/webrtc/MediaEngineWebRTCAudio.cpp
+++ b/dom/media/webrtc/MediaEngineWebRTCAudio.cpp
@@ -193,20 +193,20 @@ MediaEngineWebRTCMicrophoneSource::Media
   , mNullTransport(nullptr)
   , mSkipProcessing(false)
 {
   MOZ_ASSERT(aVoiceEnginePtr);
   MOZ_ASSERT(aAudioInput);
   mDeviceName.Assign(NS_ConvertUTF8toUTF16(name));
   mDeviceUUID.Assign(uuid);
   mListener = new mozilla::WebRTCAudioDataListener(this);
-  mSettings.mEchoCancellation.Construct(0);
-  mSettings.mAutoGainControl.Construct(0);
-  mSettings.mNoiseSuppression.Construct(0);
-  mSettings.mChannelCount.Construct(0);
+  mSettings->mEchoCancellation.Construct(0);
+  mSettings->mAutoGainControl.Construct(0);
+  mSettings->mNoiseSuppression.Construct(0);
+  mSettings->mChannelCount.Construct(0);
   // We'll init lazily as needed
 }
 
 void
 MediaEngineWebRTCMicrophoneSource::GetName(nsAString& aName) const
 {
   aName.Assign(mDeviceName);
   return;
@@ -412,20 +412,20 @@ void
 MediaEngineWebRTCMicrophoneSource::SetLastPrefs(
     const MediaEnginePrefs& aPrefs)
 {
   mLastPrefs = aPrefs;
 
   RefPtr<MediaEngineWebRTCMicrophoneSource> that = this;
 
   NS_DispatchToMainThread(media::NewRunnableFrom([that, aPrefs]() mutable {
-    that->mSettings.mEchoCancellation.Value() = aPrefs.mAecOn;
-    that->mSettings.mAutoGainControl.Value() = aPrefs.mAgcOn;
-    that->mSettings.mNoiseSuppression.Value() = aPrefs.mNoiseOn;
-    that->mSettings.mChannelCount.Value() = aPrefs.mChannels;
+    that->mSettings->mEchoCancellation.Value() = aPrefs.mAecOn;
+    that->mSettings->mAutoGainControl.Value() = aPrefs.mAgcOn;
+    that->mSettings->mNoiseSuppression.Value() = aPrefs.mNoiseOn;
+    that->mSettings->mChannelCount.Value() = aPrefs.mChannels;
     return NS_OK;
   }));
 }
 
 
 nsresult
 MediaEngineWebRTCMicrophoneSource::Deallocate(AllocationHandle* aHandle)
 {