Bug 1404997 - P2. Use AutoTaskQueue in VideoFrameConverter. r=pehrsons
authorJean-Yves Avenard <jyavenard@mozilla.com>
Thu, 30 Nov 2017 16:23:28 +0100
changeset 448404 3f7963b6920423a3f1ab6a5052d80c38acb8aea0
parent 448403 a306e023eb1ca973623d41a1503b16d8d1a09714
child 448405 844325b36ac8659fc8df3d3a46d75cadbd16bc62
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspehrsons
bugs1404997
milestone59.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 1404997 - P2. Use AutoTaskQueue in VideoFrameConverter. r=pehrsons It removes the need to explicitly shutdown the taskqueue and wait on the taskqueue to have run all dispatched task. We do want to enforce that no listeners are being called once the VideoFrameConverter's owner has been destroyed as it could potentially lead to a UAF. For now, access is okay as all operations are performed on the MSG's thread. However, this will change in follow up patches. The SourceMediaStream keeps a raw pointer to the MSG, and check if it's value isn't null to determine if the MSG has been shutdown or not, however SourceMediaStream::mGraph isn't thread safe as its access isn't protected by a mutex/monitor. MozReview-Commit-ID: 1QsJAzPuE6L
media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
--- a/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
+++ b/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
@@ -24,31 +24,31 @@
 #include "MediaStreamListener.h"
 #include "MediaStreamVideoSink.h"
 #include "VideoUtils.h"
 #include "VideoStreamTrack.h"
 #include "MediaEngine.h"
 
 #include "nsError.h"
 #include "AudioSegment.h"
+#include "AutoTaskQueue.h"
 #include "MediaSegment.h"
 #include "MediaPipelineFilter.h"
 #include "RtpLogger.h"
 #include "databuffer.h"
 #include "transportflow.h"
 #include "transportlayer.h"
 #include "transportlayerdtls.h"
 #include "transportlayerice.h"
 #include "runnable_utils.h"
 #include "libyuv/convert.h"
 #include "mozilla/dom/RTCStatsReportBinding.h"
 #include "mozilla/SharedThreadPool.h"
 #include "mozilla/PeerIdentity.h"
 #include "mozilla/Preferences.h"
-#include "mozilla/TaskQueue.h"
 #include "mozilla/gfx/Point.h"
 #include "mozilla/gfx/Types.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/UniquePtrExtensions.h"
 #include "mozilla/Sprintf.h"
 
 #include "webrtc/common_types.h"
 #include "webrtc/common_video/libyuv/include/webrtc_libyuv.h"
@@ -115,29 +115,26 @@ protected:
 // thread whenever a frame is converted.
 class VideoFrameConverter
 {
 public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(VideoFrameConverter)
 
   VideoFrameConverter()
     : mLength(0)
+    , mTaskQueue(new AutoTaskQueue(
+        SharedThreadPool::Get(NS_LITERAL_CSTRING("VideoFrameConverter"))))
     , last_img_(-1) // -1 is not a guaranteed invalid serial. See bug 1262134.
 #ifdef DEBUG
     , mThrottleCount(0)
     , mThrottleRecord(0)
 #endif
     , mMutex("VideoFrameConverter")
   {
     MOZ_COUNT_CTOR(VideoFrameConverter);
-
-    RefPtr<SharedThreadPool> pool =
-      SharedThreadPool::Get(NS_LITERAL_CSTRING("VideoFrameConverter"));
-
-    mTaskQueue = MakeAndAddRef<TaskQueue>(pool.forget());
   }
 
   void QueueVideoChunk(VideoChunk& aChunk, bool aForceBlack)
   {
     if (aChunk.IsNull()) {
       return;
     }
 
@@ -236,18 +233,18 @@ public:
   {
     MutexAutoLock lock(mMutex);
 
     return mListeners.RemoveElement(aListener);
   }
 
   void Shutdown()
   {
-    mTaskQueue->BeginShutdown();
-    mTaskQueue->AwaitShutdownAndIdle();
+    MutexAutoLock lock(mMutex);
+    mListeners.Clear();
   }
 
 protected:
   virtual ~VideoFrameConverter() { MOZ_COUNT_DTOR(VideoFrameConverter); }
 
   static void DeleteBuffer(uint8* data) { delete[] data; }
 
   // This takes ownership of the buffer and attached it to the VideoFrame we
@@ -471,17 +468,17 @@ protected:
                         buffer_size.value(),
                         size.width,
                         size.height,
                         mozilla::kVideoI420,
                         0);
   }
 
   Atomic<int32_t, Relaxed> mLength;
-  RefPtr<TaskQueue> mTaskQueue;
+  RefPtr<AutoTaskQueue> mTaskQueue;
 
   // Written and read from the queueing thread (normally MSG).
   int32_t last_img_;              // serial number of last Image
   TimeStamp disabled_frame_sent_; // The time we sent the last disabled frame.
 #ifdef DEBUG
   uint32_t mThrottleCount;
   uint32_t mThrottleRecord;
 #endif