Attachment 373648 Details for Bug 199581 – Patch

[patch] Patch

bug-199581-20190708112758.patch (text/plain), 6.14 KB, created by youenn fablet on 2019-07-08 11:27:59 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-07-08 11:27:59 PDT

Size: 6.14 KB

patch

obsolete

>Subversion Revision: 247200
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index d5c718c77389fc23a70b63430a9eebeeb194008b..1b7b79632e6b5c86f792e79799d074b2d68b8783 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,22 @@
>+2019-07-08  Youenn Fablet  <youenn@apple.com>
>+
>+        Hop explicitly to the main thread after generating a frame in ScreenDisplayCaptureSourceMac
>+        https://bugs.webkit.org/show_bug.cgi?id=199581
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Instead of locking and setting the current frame from a background thread, hop to the main thread.
>+        This also makes sure the weakThis check is done in the main thread.
>+        Manually tested.
>+
>+        * platform/mediastream/mac/ScreenDisplayCaptureSourceMac.h:
>+        (WebCore::ScreenDisplayCaptureSourceMac::DisplaySurface::DisplaySurface):
>+        * platform/mediastream/mac/ScreenDisplayCaptureSourceMac.mm:
>+        (WebCore::ScreenDisplayCaptureSourceMac::createDisplayStream):
>+        (WebCore::ScreenDisplayCaptureSourceMac::generateFrame):
>+        (WebCore::ScreenDisplayCaptureSourceMac::newFrame):
>+        (WebCore::ScreenDisplayCaptureSourceMac::frameAvailable): Deleted.
>+
> 2019-07-07  Youenn Fablet  <youenn@apple.com>
> 
>         Register a MediaStreamTrack as media producer only if it is a capture track
>diff --git a/Source/WebCore/platform/mediastream/mac/ScreenDisplayCaptureSourceMac.h b/Source/WebCore/platform/mediastream/mac/ScreenDisplayCaptureSourceMac.h
>index b82feafad1e174b87e9917006e40dcd65b7bccd9..577902030d766df6224fb7c7065ca0905fea64d2 100644
>--- a/Source/WebCore/platform/mediastream/mac/ScreenDisplayCaptureSourceMac.h
>+++ b/Source/WebCore/platform/mediastream/mac/ScreenDisplayCaptureSourceMac.h
>@@ -54,8 +54,6 @@ private:
> 
>     void displayWasReconfigured(CGDirectDisplayID, CGDisplayChangeSummaryFlags);
> 
>-    void frameAvailable(CGDisplayStreamFrameStatus, uint64_t, IOSurfaceRef, CGDisplayStreamUpdateRef);
>-
>     DisplayCaptureSourceCocoa::DisplayFrameType generateFrame() final;
>     RealtimeMediaSourceSettings::DisplaySurfaceType surfaceType() const final { return RealtimeMediaSourceSettings::DisplaySurfaceType::Monitor; }
> 
>@@ -74,6 +72,13 @@ private:
>     class DisplaySurface {
>     public:
>         DisplaySurface() = default;
>+        explicit DisplaySurface(IOSurfaceRef surface)
>+            : m_surface(surface)
>+        {
>+            if (m_surface)
>+                IOSurfaceIncrementUseCount(m_surface.get());
>+        }
>+
>         ~DisplaySurface()
>         {
>             if (m_surface)
>@@ -96,7 +101,8 @@ private:
>         RetainPtr<IOSurfaceRef> m_surface;
>     };
> 
>-    mutable Lock m_currentFrameMutex;
>+    void newFrame(CGDisplayStreamFrameStatus, DisplaySurface&&);
>+
>     DisplaySurface m_currentFrame;
>     RetainPtr<CGDisplayStreamRef> m_displayStream;
>     OSObjectPtr<dispatch_queue_t> m_captureQueue;
>diff --git a/Source/WebCore/platform/mediastream/mac/ScreenDisplayCaptureSourceMac.mm b/Source/WebCore/platform/mediastream/mac/ScreenDisplayCaptureSourceMac.mm
>index 087ad0a829e0f93d5986493f4aad7c424203da96..2cbf8ae8e68bf1305d727ddbf781b760e28a62eb 100644
>--- a/Source/WebCore/platform/mediastream/mac/ScreenDisplayCaptureSourceMac.mm
>+++ b/Source/WebCore/platform/mediastream/mac/ScreenDisplayCaptureSourceMac.mm
>@@ -154,10 +154,20 @@ bool ScreenDisplayCaptureSourceMac::createDisplayStream()
> 
>         auto weakThis = makeWeakPtr(*this);
>         auto frameAvailableBlock = ^(CGDisplayStreamFrameStatus status, uint64_t displayTime, IOSurfaceRef frameSurface, CGDisplayStreamUpdateRef updateRef) {
>-            if (!weakThis)
>+
>+            if (!frameSurface || !displayTime)
>+                return;
>+
>+            size_t count;
>+            auto* rects = CGDisplayStreamUpdateGetRects(updateRef, kCGDisplayStreamUpdateDirtyRects, &count);
>+            if (!rects || !count)
>                 return;
> 
>-            weakThis->frameAvailable(status, displayTime, frameSurface, updateRef);
>+            RunLoop::main().dispatch([weakThis, status, frame = DisplaySurface { frameSurface }]() mutable {
>+                if (!weakThis)
>+                    return;
>+                weakThis->newFrame(status, WTFMove(frame));
>+            });
>         };
> 
>         m_displayStream = adoptCF(CGDisplayStreamCreateWithDispatchQueue(m_displayID, screenWidth, screenHeight, preferedPixelBufferFormat(), (__bridge CFDictionaryRef)streamOptions, m_captureQueue.get(), frameAvailableBlock));
>@@ -203,13 +213,7 @@ void ScreenDisplayCaptureSourceMac::stopProducingData()
> 
> DisplayCaptureSourceCocoa::DisplayFrameType ScreenDisplayCaptureSourceMac::generateFrame()
> {
>-    DisplaySurface currentFrame;
>-    {
>-        LockHolder lock(m_currentFrameMutex);
>-        currentFrame = m_currentFrame.ioSurface();
>-    }
>-
>-    return DisplayCaptureSourceCocoa::DisplayFrameType { RetainPtr<IOSurfaceRef> { currentFrame.ioSurface() } };
>+    return DisplayCaptureSourceCocoa::DisplayFrameType { RetainPtr<IOSurfaceRef> { m_currentFrame.ioSurface() } };
> }
> 
> void ScreenDisplayCaptureSourceMac::startDisplayStream()
>@@ -253,7 +257,7 @@ void ScreenDisplayCaptureSourceMac::displayReconfigurationCallBack(CGDirectDispl
>         reinterpret_cast<ScreenDisplayCaptureSourceMac *>(userInfo)->displayWasReconfigured(display, flags);
> }
> 
>-void ScreenDisplayCaptureSourceMac::frameAvailable(CGDisplayStreamFrameStatus status, uint64_t displayTime, IOSurfaceRef frameSurface, CGDisplayStreamUpdateRef updateRef)
>+void ScreenDisplayCaptureSourceMac::newFrame(CGDisplayStreamFrameStatus status, DisplaySurface&& newFrame)
> {
>     switch (status) {
>     case kCGDisplayStreamFrameStatusFrameComplete:
>@@ -271,16 +275,7 @@ void ScreenDisplayCaptureSourceMac::frameAvailable(CGDisplayStreamFrameStatus st
>         break;
>     }
> 
>-    if (!frameSurface || !displayTime)
>-        return;
>-
>-    size_t count;
>-    auto* rects = CGDisplayStreamUpdateGetRects(updateRef, kCGDisplayStreamUpdateDirtyRects, &count);
>-    if (!rects || !count)
>-        return;
>-
>-    LockHolder lock(m_currentFrameMutex);
>-    m_currentFrame = frameSurface;
>+    m_currentFrame = WTFMove(newFrame);
> }
> 
> Optional<CaptureDevice> ScreenDisplayCaptureSourceMac::screenCaptureDeviceWithPersistentID(const String& deviceID)

Actions: View | Formatted Diff | Diff

Attachments on bug 199581: 373648