Changeset 247238 in webkit

Timestamp:

Jul 8, 2019 4:51:43 PM (5 years ago)

Author:

youenn@apple.com

Message:

Hop explicitly to the main thread after generating a frame in ScreenDisplayCaptureSourceMac
​https://bugs.webkit.org/show_bug.cgi?id=199581

Reviewed by Eric Carlson.

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.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/mediastream/mac/ScreenDisplayCaptureSourceMac.h (modified) (3 diffs)
• platform/mediastream/mac/ScreenDisplayCaptureSourceMac.mm (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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 Eric Carlson.
+
+        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-08  Daniel Bates  <dabates@apple.com>
 

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

@@ -55 +55 @@
     void displayWasReconfigured(CGDirectDisplayID, CGDisplayChangeSummaryFlags);
 
-    void frameAvailable(CGDisplayStreamFrameStatus, uint64_t, IOSurfaceRef, CGDisplayStreamUpdateRef);
-
     DisplayCaptureSourceCocoa::DisplayFrameType generateFrame() final;
     RealtimeMediaSourceSettings::DisplaySurfaceType surfaceType() const final { return RealtimeMediaSourceSettings::DisplaySurfaceType::Monitor; }
@@ -75 +73 @@
     public:
         DisplaySurface() = default;
+        explicit DisplaySurface(IOSurfaceRef surface)
+            : m_surface(surface)
+        {
+            if (m_surface)
+                IOSurfaceIncrementUseCount(m_surface.get());
+        }
+
         ~DisplaySurface()
         {
@@ -97 +102 @@
     };
 
-    mutable Lock m_currentFrameMutex;
+    void newFrame(CGDisplayStreamFrameStatus, DisplaySurface&&);
+
     DisplaySurface m_currentFrame;
     RetainPtr<CGDisplayStreamRef> m_displayStream;

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

@@ -155 +155 @@
         auto weakThis = makeWeakPtr(*this);
         auto frameAvailableBlock = ^(CGDisplayStreamFrameStatus status, uint64_t displayTime, IOSurfaceRef frameSurface, CGDisplayStreamUpdateRef updateRef) {
-            if (!weakThis)
+
+            if (!frameSurface || !displayTime)
                 return;
 
-            weakThis->frameAvailable(status, displayTime, frameSurface, updateRef);
+            size_t count;
+            auto* rects = CGDisplayStreamUpdateGetRects(updateRef, kCGDisplayStreamUpdateDirtyRects, &count);
+            if (!rects || !count)
+                return;
+
+            RunLoop::main().dispatch([weakThis, status, frame = DisplaySurface { frameSurface }]() mutable {
+                if (!weakThis)
+                    return;
+                weakThis->newFrame(status, WTFMove(frame));
+            });
         };
 
@@ -204 +214 @@
 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() } };
 }
 
@@ -254 +258 @@
 }
 
-void ScreenDisplayCaptureSourceMac::frameAvailable(CGDisplayStreamFrameStatus status, uint64_t displayTime, IOSurfaceRef frameSurface, CGDisplayStreamUpdateRef updateRef)
+void ScreenDisplayCaptureSourceMac::newFrame(CGDisplayStreamFrameStatus status, DisplaySurface&& newFrame)
 {
     switch (status) {
@@ -272 +276 @@
     }
 
-    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);
 }