Skip to content

Commit

Permalink
ScreenCaptureKitCaptureSource should not reconfigure if video effect …
Browse files Browse the repository at this point in the history
…is on but overlay rect is not filled

rdar://127882407
https://bugs.webkit.org/show_bug.cgi?id=274076

Reviewed by Eric Carlson and Jean-Yves Avenard.

In rdar://125925090, https://commits.webkit.org/278390@main, we started detecting presenter overlay mode by looking at SCStreamFrameInfoPresenterOverlayContentRect.
We were checking whether the origin was not (0, 0) and not (Inf, Inf).
(0, 0) is to ensure we are not in small overlay mode.
(Inf, Inf) was to ensure there was some overlay.

When starting large presenter overlay mode, (Inf, Inf) may be used for a couple of video frames, in particular when going from small to large overlay mode.
This triggered sometimes reconfiguration, which was further delaying a correct overlay, ending in a loop of reconfiguration.

To prevent this, we only allow reconfiguration when we are sure we are in small presenter overlay mode, thus origin is (0, 0).
We rename hasLargePresenterOverlay to shouldDisallowReconfiguration as this is more consistent.
Manually tested.

* Source/WebCore/platform/mediastream/mac/ScreenCaptureKitCaptureSource.mm:
(WebCore::ScreenCaptureKitCaptureSource::streamDidOutputVideoSampleBuffer):

Canonical link: https://commits.webkit.org/278685@main
  • Loading branch information
youennf committed May 13, 2024
1 parent 000602e commit ec6f4da
Showing 1 changed file with 3 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ - (void)outputVideoEffectDidStopForStream:(SCStream *)stream
double contentScale = 1;
double scaleFactor = 1;
FloatRect contentRect;
auto hasLargePresenterOverlay = false;
bool shouldDisallowReconfiguration = false;
#if HAVE(SC_CONTENT_SHARING_PICKER)
auto canCheckForOverlayMode = PAL::canLoad_ScreenCaptureKit_SCStreamFrameInfoPresenterOverlayContentRect();
#endif
Expand All @@ -544,7 +544,7 @@ - (void)outputVideoEffectDidStopForStream:(SCStream *)stream
if (auto overlayRectDictionary = (CFDictionaryRef)attachment[SCStreamFrameInfoPresenterOverlayContentRect]) {
CGRect overlayRect;
if (CGRectMakeWithDictionaryRepresentation(overlayRectDictionary, &overlayRect))
hasLargePresenterOverlay = overlayRect.origin.x > 0 && overlayRect.origin.x != std::numeric_limits<double>::infinity() && overlayRect.origin.y > 0 && overlayRect.origin.y != std::numeric_limits<double>::infinity();
shouldDisallowReconfiguration = overlayRect.origin.x && overlayRect.origin.y;
}
}
#endif
Expand Down Expand Up @@ -578,7 +578,7 @@ - (void)outputVideoEffectDidStopForStream:(SCStream *)stream

// FIXME: for now we will rely on cropping to handle large presenter overlay.
// We might further want to reduce calling updateStreamConfiguration once we crop when user is resizing.
if (m_contentSize != scaledContentRect.size() && !hasLargePresenterOverlay) {
if (m_contentSize != scaledContentRect.size() && !shouldDisallowReconfiguration) {
m_contentSize = scaledContentRect.size();
m_streamConfiguration = nullptr;
updateStreamConfiguration();
Expand Down

0 comments on commit ec6f4da

Please sign in to comment.