Bug 825526: Improve lifetime control of SourceMediaStream in gUM r=anant
authorRandell Jesup <rjesup@jesup.org>
Mon, 31 Dec 2012 18:12:15 -0500
changeset 117286 b3993179ea52b5a65c95450d22613e9adb256d72
parent 117285 8c3c368478a07cfba87a07c053975109e2733381
child 117287 a3fcdb9c37f4c9bff79a5dad724fd1a79e7cf36a
push id24093
push userryanvm@gmail.com
push dateTue, 01 Jan 2013 17:24:16 +0000
treeherdermozilla-central@a812ef63de87 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersanant
bugs825526
milestone20.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 825526: Improve lifetime control of SourceMediaStream in gUM r=anant
dom/media/MediaManager.cpp
dom/media/MediaManager.h
media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
--- a/dom/media/MediaManager.cpp
+++ b/dom/media/MediaManager.cpp
@@ -1094,9 +1094,22 @@ MediaManager::GetActiveMediaCaptureWindo
     return rv;
 
   mActiveWindows.EnumerateRead(WindowsHashToArrayFunc, array);
 
   *aArray = array;
   return NS_OK;
 }
 
+void
+GetUserMediaCallbackMediaStreamListener::Invalidate()
+{
+  nsRefPtr<MediaOperationRunnable> runnable;
+  // We can't take a chance on blocking here, so proxy this to another
+  // thread.
+  // Pass a ref to us (which is threadsafe) so it can query us for the
+  // source stream info.
+  runnable = new MediaOperationRunnable(MEDIA_STOP,
+                                        this, mAudioSource, mVideoSource);
+  mMediaThread->Dispatch(runnable, NS_DISPATCH_NORMAL);
+}
+
 } // namespace mozilla
--- a/dom/media/MediaManager.h
+++ b/dom/media/MediaManager.h
@@ -56,136 +56,16 @@ class GetUserMediaNotificationEvent: pub
       }
       return NS_OK;
     }
 
   protected:
     GetUserMediaStatus mStatus;
 };
 
-typedef enum {
-  MEDIA_START,
-  MEDIA_STOP
-} MediaOperation;
-
-class GetUserMediaCallbackMediaStreamListener;
-
-// Generic class for running long media operations like Start off the main
-// thread, and then (because nsDOMMediaStreams aren't threadsafe),
-// ProxyReleases mStream since it's cycle collected.
-class MediaOperationRunnable : public nsRunnable
-{
-public:
-  MediaOperationRunnable(MediaOperation aType,
-    nsDOMMediaStream* aStream,
-    MediaEngineSource* aAudioSource,
-    MediaEngineSource* aVideoSource)
-    : mType(aType)
-    , mAudioSource(aAudioSource)
-    , mVideoSource(aVideoSource)
-    , mStream(aStream)
-    {}
-
-  MediaOperationRunnable(MediaOperation aType,
-    SourceMediaStream* aStream,
-    MediaEngineSource* aAudioSource,
-    MediaEngineSource* aVideoSource)
-    : mType(aType)
-    , mAudioSource(aAudioSource)
-    , mVideoSource(aVideoSource)
-    , mStream(nullptr)
-    , mSourceStream(aStream)
-    {}
-
-  ~MediaOperationRunnable()
-  {
-    // nsDOMMediaStreams are cycle-collected and thus main-thread-only for
-    // refcounting and releasing
-    if (mStream) {
-      nsCOMPtr<nsIThread> mainThread = do_GetMainThread();
-      nsDOMMediaStream *stream;
-      mStream.forget(&stream);
-      NS_ProxyRelease(mainThread, stream, true);
-    }
-  }
-
-  NS_IMETHOD
-  Run()
-  {
-    // No locking between these is required as all the callbacks for the
-    // same MediaStream will occur on the same thread.
-    if (mStream) {
-      mSourceStream = mStream->GetStream()->AsSourceStream();
-    }
-    switch (mType) {
-      case MEDIA_START:
-        {
-          NS_ASSERTION(!NS_IsMainThread(), "Never call on main thread");
-          nsresult rv;
-
-          mSourceStream->SetPullEnabled(true);
-
-          if (mAudioSource) {
-            rv = mAudioSource->Start(mSourceStream, kAudioTrack);
-            if (NS_FAILED(rv)) {
-              MM_LOG(("Starting audio failed, rv=%d",rv));
-            }
-          }
-          if (mVideoSource) {
-            rv = mVideoSource->Start(mSourceStream, kVideoTrack);
-            if (NS_FAILED(rv)) {
-              MM_LOG(("Starting video failed, rv=%d",rv));
-            }
-          }
-
-          MM_LOG(("started all sources"));
-          nsRefPtr<GetUserMediaNotificationEvent> event =
-            new GetUserMediaNotificationEvent(GetUserMediaNotificationEvent::STARTING);
-
-
-          NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL);
-        }
-        break;
-
-      case MEDIA_STOP:
-        {
-          NS_ASSERTION(!NS_IsMainThread(), "Never call on main thread");
-          if (mAudioSource) {
-            mAudioSource->Stop(mSourceStream, kAudioTrack);
-            mAudioSource->Deallocate();
-          }
-          if (mVideoSource) {
-            mVideoSource->Stop(mSourceStream, kVideoTrack);
-            mVideoSource->Deallocate();
-          }
-          // Do this after stopping all tracks with EndTrack()
-          mSourceStream->Finish();
-
-          nsRefPtr<GetUserMediaNotificationEvent> event =
-            new GetUserMediaNotificationEvent(GetUserMediaNotificationEvent::STOPPING);
-
-          NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL);
-        }
-        break;
-
-      default:
-        MOZ_ASSERT(false,"invalid MediaManager operation");
-        break;
-    }
-    return NS_OK;
-  }
-
-private:
-  MediaOperation mType;
-  nsRefPtr<MediaEngineSource> mAudioSource; // threadsafe
-  nsRefPtr<MediaEngineSource> mVideoSource; // threadsafe
-  nsRefPtr<nsDOMMediaStream> mStream;       // not threadsafe
-  SourceMediaStream *mSourceStream;
-};
-
 /**
  * This class is an implementation of MediaStreamListener. This is used
  * to Start() and Stop() the underlying MediaEngineSource when MediaStreams
  * are assigned and deassigned in content.
  */
 class GetUserMediaCallbackMediaStreamListener : public MediaStreamListener
 {
 public:
@@ -194,50 +74,37 @@ public:
     MediaEngineSource* aAudioSource,
     MediaEngineSource* aVideoSource)
     : mMediaThread(aThread)
     , mAudioSource(aAudioSource)
     , mVideoSource(aVideoSource)
     , mStream(aStream)
     , mSourceStream(aStream->GetStream()->AsSourceStream())
     , mLastEndTimeAudio(0)
-    , mLastEndTimeVideo(0) {}
+    , mLastEndTimeVideo(0) { MOZ_ASSERT(mSourceStream); }
 
   ~GetUserMediaCallbackMediaStreamListener()
   {
     // In theory this could be released from the MediaStreamGraph thread (RemoveListener)
     if (mStream) {
       nsCOMPtr<nsIThread> mainThread = do_GetMainThread();
       nsDOMMediaStream *stream;
       mStream.forget(&stream);
       // Releases directly if on MainThread already
       NS_ProxyRelease(mainThread, stream, false);
     }
   }
 
-  void
-  Invalidate()
+  SourceMediaStream *GetSourceStream()
   {
-    nsRefPtr<MediaOperationRunnable> runnable;
+    return mStream->GetStream()->AsSourceStream();
+  }
 
-    // We can't take a chance on blocking here, so proxy this to another
-    // thread.
-    // XXX FIX! I'm cheating and passing a raw pointer to the sourcestream
-    // which is valid as long as the mStream pointer here is.
-    // Solutions (see bug 825235):
-    // a) if on MainThread, pass mStream and let it addref
-    //    (MediaOperation will need to ProxyRelease however)
-    // b) if on MediaStreamGraph thread, dispatch a runnable to MainThread
-    //    to call Invalidate() (with a strong ref to this listener)
-    runnable = new MediaOperationRunnable(MEDIA_STOP,
-                                          mSourceStream, mAudioSource, mVideoSource);
-    mMediaThread->Dispatch(runnable, NS_DISPATCH_NORMAL);
-
-    return;
-  }
+  void
+  Invalidate(); // implement in .cpp to avoid circular dependency with MediaOperationRunnable
 
   void
   Remove()
   {
     NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
     // Caller holds strong reference to us, so no death grip required
     mStream->GetStream()->RemoveListener(this);
   }
@@ -268,16 +135,141 @@ private:
   nsRefPtr<MediaEngineSource> mAudioSource;
   nsRefPtr<MediaEngineSource> mVideoSource;
   nsRefPtr<nsDOMMediaStream> mStream;
   SourceMediaStream *mSourceStream; // mStream controls ownership
   TrackTicks mLastEndTimeAudio;
   TrackTicks mLastEndTimeVideo;
 };
 
+typedef enum {
+  MEDIA_START,
+  MEDIA_STOP
+} MediaOperation;
+
+// Generic class for running long media operations like Start off the main
+// thread, and then (because nsDOMMediaStreams aren't threadsafe),
+// ProxyReleases mStream since it's cycle collected.
+class MediaOperationRunnable : public nsRunnable
+{
+public:
+  MediaOperationRunnable(MediaOperation aType,
+    nsDOMMediaStream* aStream,
+    MediaEngineSource* aAudioSource,
+    MediaEngineSource* aVideoSource)
+    : mType(aType)
+    , mAudioSource(aAudioSource)
+    , mVideoSource(aVideoSource)
+    , mStream(aStream)
+    {}
+
+  MediaOperationRunnable(MediaOperation aType,
+    GetUserMediaCallbackMediaStreamListener* aListener,
+    MediaEngineSource* aAudioSource,
+    MediaEngineSource* aVideoSource)
+    : mType(aType)
+    , mAudioSource(aAudioSource)
+    , mVideoSource(aVideoSource)
+    , mStream(nullptr)
+    , mListener(aListener)
+    {}
+
+  ~MediaOperationRunnable()
+  {
+    // nsDOMMediaStreams are cycle-collected and thus main-thread-only for
+    // refcounting and releasing
+    if (mStream) {
+      nsCOMPtr<nsIThread> mainThread = do_GetMainThread();
+      nsDOMMediaStream *stream;
+      mStream.forget(&stream);
+      NS_ProxyRelease(mainThread, stream, true);
+    }
+  }
+
+  NS_IMETHOD
+  Run()
+  {
+    SourceMediaStream *source;
+    // No locking between these is required as all the callbacks for the
+    // same MediaStream will occur on the same thread.
+    if (mStream) {
+      source = mStream->GetStream()->AsSourceStream();
+    } else {
+      source = mListener->GetSourceStream();
+    }
+    MOZ_ASSERT(source);
+    if (!source)  // paranoia
+      return NS_ERROR_FAILURE;
+
+    switch (mType) {
+      case MEDIA_START:
+        {
+          NS_ASSERTION(!NS_IsMainThread(), "Never call on main thread");
+          nsresult rv;
+
+          source->SetPullEnabled(true);
+
+          if (mAudioSource) {
+            rv = mAudioSource->Start(source, kAudioTrack);
+            if (NS_FAILED(rv)) {
+              MM_LOG(("Starting audio failed, rv=%d",rv));
+            }
+          }
+          if (mVideoSource) {
+            rv = mVideoSource->Start(source, kVideoTrack);
+            if (NS_FAILED(rv)) {
+              MM_LOG(("Starting video failed, rv=%d",rv));
+            }
+          }
+
+          MM_LOG(("started all sources"));
+          nsRefPtr<GetUserMediaNotificationEvent> event =
+            new GetUserMediaNotificationEvent(GetUserMediaNotificationEvent::STARTING);
+
+
+          NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL);
+        }
+        break;
+
+      case MEDIA_STOP:
+        {
+          NS_ASSERTION(!NS_IsMainThread(), "Never call on main thread");
+          if (mAudioSource) {
+            mAudioSource->Stop(source, kAudioTrack);
+            mAudioSource->Deallocate();
+          }
+          if (mVideoSource) {
+            mVideoSource->Stop(source, kVideoTrack);
+            mVideoSource->Deallocate();
+          }
+          // Do this after stopping all tracks with EndTrack()
+          source->Finish();
+
+          nsRefPtr<GetUserMediaNotificationEvent> event =
+            new GetUserMediaNotificationEvent(GetUserMediaNotificationEvent::STOPPING);
+
+          NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL);
+        }
+        break;
+
+      default:
+        MOZ_ASSERT(false,"invalid MediaManager operation");
+        break;
+    }
+    return NS_OK;
+  }
+
+private:
+  MediaOperation mType;
+  nsRefPtr<MediaEngineSource> mAudioSource; // threadsafe
+  nsRefPtr<MediaEngineSource> mVideoSource; // threadsafe
+  nsRefPtr<nsDOMMediaStream> mStream;       // not threadsafe
+  nsRefPtr<GetUserMediaCallbackMediaStreamListener> mListener; // threadsafe
+};
+
 typedef nsTArray<nsRefPtr<GetUserMediaCallbackMediaStreamListener> > StreamListeners;
 typedef nsClassHashtable<nsUint64HashKey, StreamListeners> WindowTable;
 
 class MediaDevice : public nsIMediaDevice
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIMEDIADEVICE
--- a/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
+++ b/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
@@ -480,17 +480,17 @@ nsresult MediaPipelineTransmit::Init() {
   stream_->AddListener(listener_);
 
   return MediaPipeline::Init();
 }
 
 nsresult MediaPipelineTransmit::TransportReady(TransportFlow *flow) {
   // Call base ready function.
   MediaPipeline::TransportReady(flow);
-  
+
   if (flow == rtp_transport_) {
     // TODO(ekr@rtfm.com): Move onto MSG thread.
     listener_->SetActive(true);
   }
 
   return NS_OK;
 }
 
@@ -600,17 +600,17 @@ void MediaPipelineTransmit::PipelineList
 NotifyQueuedTrackChanges(MediaStreamGraph* graph, TrackID tid,
                          TrackRate rate,
                          TrackTicks offset,
                          uint32_t events,
                          const MediaSegment& queued_media) {
   MOZ_MTLOG(PR_LOG_DEBUG, "MediaPipeline::NotifyQueuedTrackChanges()");
 
   if (!active_) {
-    MOZ_MTLOG(PR_LOG_DEBUG, "Discarding packets because transport not ready");    
+    MOZ_MTLOG(PR_LOG_DEBUG, "Discarding packets because transport not ready");
     return;
   }
 
   // TODO(ekr@rtfm.com): For now assume that we have only one
   // track type and it's destined for us
   // See bug 784517
   if (queued_media.GetType() == MediaSegment::AUDIO) {
     if (conduit_->type() != MediaSessionConduit::AUDIO) {
@@ -867,9 +867,8 @@ void MediaPipelineReceiveVideo::Pipeline
 
   segment.AppendFrame(image.forget(), 1, gfxIntSize(width_, height_));
   source->AppendToTrack(1, &(segment));
 #endif
 }
 
 
 }  // end namespace
-