Changeset 261893 in webkit

Timestamp:

May 19, 2020 4:35:58 PM (4 years ago)

Author:

Jacob Uphoff

Message:

Unreviewed, reverting r261856.

This caused internal assertion failures.

Reverted changeset:

"Allow calling VideoSampleObserver::videoSampleAvailable from
a background thread"
​https://bugs.webkit.org/show_bug.cgi?id=212024
​https://trac.webkit.org/changeset/261856

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/MediaSample.h (modified) (2 diffs)
• platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.h (modified) (1 diff)
• platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm (modified) (3 diffs)
• platform/mediastream/RealtimeMediaSource.cpp (modified) (2 diffs)
• platform/mediastream/RealtimeMediaSource.h (modified) (2 diffs)
• platform/mediastream/mac/MockRealtimeVideoSourceMac.h (modified) (2 diffs)
• platform/mediastream/mac/MockRealtimeVideoSourceMac.mm (modified) (2 diffs)
• platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.h (modified) (1 diff)
• platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.mm (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-05-19  Jacob Uphoff  <jacob_uphoff@apple.com>
+
+        Unreviewed, reverting r261856.
+
+        This caused internal assertion failures.
+
+        Reverted changeset:
+
+        "Allow calling VideoSampleObserver::videoSampleAvailable from
+        a background thread"
+        https://bugs.webkit.org/show_bug.cgi?id=212024
+        https://trac.webkit.org/changeset/261856
+
 2020-05-19  David Kilzer  <ddkilzer@apple.com>
 

trunk/Source/WebCore/platform/MediaSample.h

@@ -31 +31 @@
 #include <wtf/EnumTraits.h>
 #include <wtf/MediaTime.h>
-#include <wtf/ThreadSafeRefCounted.h>
+#include <wtf/RefCounted.h>
 #include <wtf/text/AtomString.h>
 
@@ -55 +55 @@
 };
 
-class MediaSample : public ThreadSafeRefCounted<MediaSample> {
+class MediaSample : public RefCounted<MediaSample> {
 public:
     virtual ~MediaSample() = default;

trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.h

@@ -140 +140 @@
     MediaTime calculateTimelineOffset(const MediaSample&, double);
     
+    void enqueueVideoSample(MediaSample&);
     void enqueueCorrectedVideoSample(MediaSample&);
     void requestNotificationWhenReadyForVideoData();

trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm

@@ -142 +142 @@
 {
     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)
@@ -283 +282 @@
 }
 
-void MediaPlayerPrivateMediaStreamAVFObjC::videoSampleAvailable(MediaSample& sample)
-{
-    if (!isMainThread()) {
-        callOnMainThread([weakThis = makeWeakPtr(this), sample = makeRef(sample)]() mutable {
-            if (weakThis)
-                weakThis->videoSampleAvailable(sample.get());
-        });
-        return;
-    }
-
+void MediaPlayerPrivateMediaStreamAVFObjC::enqueueVideoSample(MediaSample& sample)
+{
     if (!m_imagePainter.mediaSample || m_displayMode != PausedImage) {
         m_imagePainter.mediaSample = &sample;
@@ -743 +734 @@
 {
     updateTracks();
+}
+
+void MediaPlayerPrivateMediaStreamAVFObjC::videoSampleAvailable(MediaSample& mediaSample)
+{
+    if (streamTime().toDouble() < 0)
+        return;
+
+    enqueueVideoSample(mediaSample);
 }
 

trunk/Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp

@@ -188 +188 @@
 void RealtimeMediaSource::videoSampleAvailable(MediaSample& mediaSample)
 {
+    // FIXME: Migrate RealtimeMediaSource clients to non main thread processing.
+    ASSERT(isMainThread());
 #if !RELEASE_LOG_DISABLED
     ++m_frameCount;
@@ -965 +967 @@
     m_intrinsicSize = size;
 
-    if (currentSize != this->size()) {
-        scheduleDeferredTask([this] {
-            notifySettingsDidChangeObservers({ RealtimeMediaSourceSettings::Flag::Width, RealtimeMediaSourceSettings::Flag::Height });
-        });
-    }
+    if (currentSize != this->size())
+        notifySettingsDidChangeObservers({ RealtimeMediaSourceSettings::Flag::Width, RealtimeMediaSourceSettings::Flag::Height });
 }
 

trunk/Source/WebCore/platform/mediastream/RealtimeMediaSource.h

@@ -103 +103 @@
         virtual ~VideoSampleObserver() = default;
 
-        // May be called on a background thread.
-        virtual void videoSampleAvailable(MediaSample&) = 0;
+        virtual void videoSampleAvailable(MediaSample&) { }
     };
 
@@ -280 +279 @@
     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 };

trunk/Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.h

@@ -35 +35 @@
 #include "MockRealtimeVideoSource.h"
 #include "PixelBufferConformerCV.h"
-#include <wtf/WorkQueue.h>
 
 typedef struct __CVBuffer *CVBufferRef;
@@ -62 +61 @@
     std::unique_ptr<ImageTransferSessionVT> m_imageTransferSession;
     IntSize m_presetSize;
-    Ref<WorkQueue> m_workQueue;
 };
 

trunk/Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm

@@ -79 +79 @@
 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))
 {
 }
@@ -97 +96 @@
         return;
 
-    m_workQueue->dispatch([this, protectedThis = makeRef(*this), sampleBuffer = WTFMove(sampleBuffer)]() mutable {
-        dispatchMediaSampleToObservers(*sampleBuffer);
-    });
+    dispatchMediaSampleToObservers(*sampleBuffer);
 }
 

trunk/Source/WebCore/platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.h

@@ -44 +44 @@
 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);

trunk/Source/WebCore/platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.mm

@@ -194 +194 @@
     }
 
-    setIntrinsicSize(IntSize(width, height));
-    videoSampleAvailable(MediaSampleAVFObjC::create(sample.get(), rotation));
+    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));
 }