Allow calling VideoSampleObserver::videoSampleAvailable from a background thread
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 May 2020 07:54:05 +0000 (07:54 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 May 2020 07:54:05 +0000 (07:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=212024

Reviewed by Eric Carlson.

Allow RealtimeMediaSource::videoSampleAvailable to be called on a background thread, typically the capture thread.
Make WebRTC remote sources and mock capture sources do that.

RealtimeMediaSource is then updating its intrinsic size from the generation thread while updating its size in the main thread.
The size() getter can be called from both threads.

Existing consumers do the following:
- media player will hop to the main thread.
- media recorder will do processing from the background thread.
- WebRTC sender will do processing from the background thread, except when sending black frames where this will still be done on the main thread.
This is ok as we ensure either we send black frames on the main thread (and we do not observe the source) or we observe the source to send.

Follow-ups will migrate the real capture sources as well as migrating media player processing out of the main thread.
Covered by existing tests.

* platform/MediaSample.h:
* platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:
(WebCore::MediaPlayerPrivateMediaStreamAVFObjC::MediaPlayerPrivateMediaStreamAVFObjC):
(WebCore::MediaPlayerPrivateMediaStreamAVFObjC::videoSampleAvailable):
* platform/mediastream/RealtimeMediaSource.cpp:
(WebCore::RealtimeMediaSource::videoSampleAvailable):
(WebCore::RealtimeMediaSource::setIntrinsicSize):
* platform/mediastream/RealtimeMediaSource.h:
* platform/mediastream/mac/MockRealtimeVideoSourceMac.h:
* platform/mediastream/mac/MockRealtimeVideoSourceMac.mm:
(WebCore::MockRealtimeVideoSourceMac::MockRealtimeVideoSourceMac):
(WebCore::MockRealtimeVideoSourceMac::updateSampleBuffer):
* platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.h:
* platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.mm:
(WebCore::RealtimeIncomingVideoSourceCocoa::OnFrame):
(WebCore::RealtimeIncomingVideoSourceCocoa::processNewSample): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@261908 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/MediaSample.h
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.h
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm
Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp
Source/WebCore/platform/mediastream/RealtimeMediaSource.h
Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.h
Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm
Source/WebCore/platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.h
Source/WebCore/platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.mm

index 2635721..69a9cac 100644 (file)
@@ -1,3 +1,42 @@
+2020-05-20  Youenn Fablet  <youenn@apple.com>
+
+        Allow calling VideoSampleObserver::videoSampleAvailable from a background thread
+        https://bugs.webkit.org/show_bug.cgi?id=212024
+
+        Reviewed by Eric Carlson.
+
+        Allow RealtimeMediaSource::videoSampleAvailable to be called on a background thread, typically the capture thread.
+        Make WebRTC remote sources and mock capture sources do that.
+
+        RealtimeMediaSource is then updating its intrinsic size from the generation thread while updating its size in the main thread.
+        The size() getter can be called from both threads.
+
+        Existing consumers do the following:
+        - media player will hop to the main thread.
+        - media recorder will do processing from the background thread.
+        - WebRTC sender will do processing from the background thread, except when sending black frames where this will still be done on the main thread.
+        This is ok as we ensure either we send black frames on the main thread (and we do not observe the source) or we observe the source to send.
+
+        Follow-ups will migrate the real capture sources as well as migrating media player processing out of the main thread.
+        Covered by existing tests.
+
+        * platform/MediaSample.h:
+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:
+        (WebCore::MediaPlayerPrivateMediaStreamAVFObjC::MediaPlayerPrivateMediaStreamAVFObjC):
+        (WebCore::MediaPlayerPrivateMediaStreamAVFObjC::videoSampleAvailable):
+        * platform/mediastream/RealtimeMediaSource.cpp:
+        (WebCore::RealtimeMediaSource::videoSampleAvailable):
+        (WebCore::RealtimeMediaSource::setIntrinsicSize):
+        * platform/mediastream/RealtimeMediaSource.h:
+        * platform/mediastream/mac/MockRealtimeVideoSourceMac.h:
+        * platform/mediastream/mac/MockRealtimeVideoSourceMac.mm:
+        (WebCore::MockRealtimeVideoSourceMac::MockRealtimeVideoSourceMac):
+        (WebCore::MockRealtimeVideoSourceMac::updateSampleBuffer):
+        * platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.h:
+        * platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.mm:
+        (WebCore::RealtimeIncomingVideoSourceCocoa::OnFrame):
+        (WebCore::RealtimeIncomingVideoSourceCocoa::processNewSample): Deleted.
+
 2020-05-20  Oriol Brufau  <obrufau@igalia.com>
 
         Fix computeMarginLogicalSizeForChild to check auto margins in the right axis
index 1e44279..ecd334e 100644 (file)
@@ -30,7 +30,7 @@
 #include <JavaScriptCore/TypedArrays.h>
 #include <wtf/EnumTraits.h>
 #include <wtf/MediaTime.h>
-#include <wtf/RefCounted.h>
+#include <wtf/ThreadSafeRefCounted.h>
 #include <wtf/text/AtomString.h>
 
 typedef struct opaqueCMSampleBuffer *CMSampleBufferRef;
@@ -54,7 +54,7 @@ struct PlatformSample {
     } sample;
 };
 
-class MediaSample : public RefCounted<MediaSample> {
+class MediaSample : public ThreadSafeRefCounted<MediaSample> {
 public:
     virtual ~MediaSample() = default;
 
index 9663079..e06d6b3 100644 (file)
@@ -139,7 +139,6 @@ private:
 
     MediaTime calculateTimelineOffset(const MediaSample&, double);
     
-    void enqueueVideoSample(MediaSample&);
     void enqueueCorrectedVideoSample(MediaSample&);
     void requestNotificationWhenReadyForVideoData();
 
index 01992a5..ba6d253 100644 (file)
@@ -141,6 +141,7 @@ MediaPlayerPrivateMediaStreamAVFObjC::MediaPlayerPrivateMediaStreamAVFObjC(Media
     , m_videoLayerManager(makeUnique<VideoLayerManagerObjC>(m_logger, m_logIdentifier))
 {
     INFO_LOG(LOGIDENTIFIER);
+    // MediaPlayerPrivateMediaStreamAVFObjC::videoSampleAvailable expects a weak pointer to be created in the constructor.
     m_boundsChangeListener = adoptNS([[WebRootSampleBufferBoundsChangeListener alloc] initWithCallback:[this, weakThis = makeWeakPtr(this)] {
         if (!weakThis)
             return;
@@ -281,8 +282,19 @@ void MediaPlayerPrivateMediaStreamAVFObjC::enqueueCorrectedVideoSample(MediaSamp
     }
 }
 
-void MediaPlayerPrivateMediaStreamAVFObjC::enqueueVideoSample(MediaSample& sample)
+void MediaPlayerPrivateMediaStreamAVFObjC::videoSampleAvailable(MediaSample& sample)
 {
+    if (!isMainThread()) {
+        callOnMainThread([weakThis = makeWeakPtr(this), sample = makeRef(sample)]() mutable {
+            if (weakThis)
+                weakThis->videoSampleAvailable(sample.get());
+        });
+        return;
+    }
+
+    if (!m_activeVideoTrack)
+        return;
+
     if (!m_imagePainter.mediaSample || m_displayMode != PausedImage) {
         m_imagePainter.mediaSample = &sample;
         m_imagePainter.cgImage = nullptr;
@@ -735,14 +747,6 @@ void MediaPlayerPrivateMediaStreamAVFObjC::didRemoveTrack(MediaStreamTrackPrivat
     updateTracks();
 }
 
-void MediaPlayerPrivateMediaStreamAVFObjC::videoSampleAvailable(MediaSample& mediaSample)
-{
-    if (streamTime().toDouble() < 0)
-        return;
-
-    enqueueVideoSample(mediaSample);
-}
-
 void MediaPlayerPrivateMediaStreamAVFObjC::readyStateChanged(MediaStreamTrackPrivate&)
 {
     scheduleDeferredTask([this] {
index f7d9096..869ef2f 100644 (file)
@@ -187,8 +187,6 @@ void RealtimeMediaSource::updateHasStartedProducingData()
 
 void RealtimeMediaSource::videoSampleAvailable(MediaSample& mediaSample)
 {
-    // FIXME: Migrate RealtimeMediaSource clients to non main thread processing.
-    ASSERT(isMainThread());
 #if !RELEASE_LOG_DISABLED
     ++m_frameCount;
 
@@ -966,8 +964,11 @@ void RealtimeMediaSource::setIntrinsicSize(const IntSize& size)
     auto currentSize = this->size();
     m_intrinsicSize = size;
 
-    if (currentSize != this->size())
-        notifySettingsDidChangeObservers({ RealtimeMediaSourceSettings::Flag::Width, RealtimeMediaSourceSettings::Flag::Height });
+    if (currentSize != this->size()) {
+        scheduleDeferredTask([this] {
+            notifySettingsDidChangeObservers({ RealtimeMediaSourceSettings::Flag::Width, RealtimeMediaSourceSettings::Flag::Height });
+        });
+    }
 }
 
 const IntSize RealtimeMediaSource::intrinsicSize() const
index 1db8114..35fdf42 100644 (file)
@@ -102,7 +102,8 @@ public:
     public:
         virtual ~VideoSampleObserver() = default;
 
-        virtual void videoSampleAvailable(MediaSample&) { }
+        // May be called on a background thread.
+        virtual void videoSampleAvailable(MediaSample&) = 0;
     };
 
     virtual ~RealtimeMediaSource() = default;
@@ -278,7 +279,9 @@ private:
     mutable RecursiveLock m_videoSampleObserversLock;
     HashSet<VideoSampleObserver*> m_videoSampleObservers;
 
+    // Set on the main thread from constraints.
     IntSize m_size;
+    // Set on sample generation thread.
     IntSize m_intrinsicSize;
     double m_frameRate { 30 };
     double m_aspectRatio { 0 };
index 81ed997..95cf35c 100644 (file)
@@ -34,6 +34,7 @@
 
 #include "MockRealtimeVideoSource.h"
 #include "PixelBufferConformerCV.h"
+#include <wtf/WorkQueue.h>
 
 typedef struct __CVBuffer *CVBufferRef;
 typedef CVBufferRef CVImageBufferRef;
@@ -60,6 +61,7 @@ private:
 
     std::unique_ptr<ImageTransferSessionVT> m_imageTransferSession;
     IntSize m_presetSize;
+    Ref<WorkQueue> m_workQueue;
 };
 
 } // namespace WebCore
index b84e0b0..3f6cb6e 100644 (file)
@@ -78,6 +78,7 @@ Ref<MockRealtimeVideoSource> MockRealtimeVideoSourceMac::createForMockDisplayCap
 
 MockRealtimeVideoSourceMac::MockRealtimeVideoSourceMac(String&& deviceID, String&& name, String&& hashSalt)
     : MockRealtimeVideoSource(WTFMove(deviceID), WTFMove(name), WTFMove(hashSalt))
+    , m_workQueue(WorkQueue::create("MockRealtimeVideoSource Render Queue", WorkQueue::Type::Serial, WorkQueue::QOS::UserInteractive))
 {
 }
 
@@ -95,7 +96,9 @@ void MockRealtimeVideoSourceMac::updateSampleBuffer()
     if (!sampleBuffer)
         return;
 
-    dispatchMediaSampleToObservers(*sampleBuffer);
+    m_workQueue->dispatch([this, protectedThis = makeRef(*this), sampleBuffer = WTFMove(sampleBuffer)]() mutable {
+        dispatchMediaSampleToObservers(*sampleBuffer);
+    });
 }
 
 } // namespace WebCore
index 786b550..f5523d3 100644 (file)
@@ -43,7 +43,6 @@ public:
 
 private:
     RealtimeIncomingVideoSourceCocoa(rtc::scoped_refptr<webrtc::VideoTrackInterface>&&, String&&);
-    void processNewSample(CMSampleBufferRef, unsigned, unsigned, MediaSample::VideoRotation);
     RetainPtr<CVPixelBufferRef> pixelBufferFromVideoFrame(const webrtc::VideoFrame&);
     CVPixelBufferPoolRef pixelBufferPool(size_t width, size_t height);
 
index e80ecc0..ee9836b 100644 (file)
@@ -193,18 +193,8 @@ void RealtimeIncomingVideoSourceCocoa::OnFrame(const webrtc::VideoFrame& frame)
         break;
     }
 
-    callOnMainThread([protectedThis = makeRef(*this), sample = WTFMove(sample), width, height, rotation] {
-        protectedThis->processNewSample(sample.get(), width, height, rotation);
-    });
-}
-
-void RealtimeIncomingVideoSourceCocoa::processNewSample(CMSampleBufferRef sample, unsigned width, unsigned height, MediaSample::VideoRotation rotation)
-{
-    auto size = this->size();
-    if (WTF::safeCast<int>(width) != size.width() || WTF::safeCast<int>(height) != size.height())
-        setIntrinsicSize(IntSize(width, height));
-
-    videoSampleAvailable(MediaSampleAVFObjC::create(sample, rotation));
+    setIntrinsicSize(IntSize(width, height));
+    videoSampleAvailable(MediaSampleAVFObjC::create(sample.get(), rotation));
 }
 
 } // namespace WebCore