Bug 1382095 - Avoid ref-counting MediaEngineSource itself in setLastPrefs runnables to improve shutdown. r=jesup
authorJan-Ivar Bruaroey <jib@mozilla.com>
Sat, 01 Jul 2017 15:01:25 -0700
changeset 418607 58e5ecbd98b3a60a01707d7e4c713dddc68d3421
parent 418606 05e27c93b3b4157b54ec56295bb0cc733a845272
child 418608 5a82cec969406cbe12caf120e53f96f639a13bc8
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjesup
bugs1382095
milestone56.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 1382095 - Avoid ref-counting MediaEngineSource itself in setLastPrefs runnables to improve shutdown. r=jesup 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)
 {