Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[macOS] REGRESSION(275831@main): Netflix can error out while scrubbing #27791

Conversation

jernoble
Copy link
Contributor

@jernoble jernoble commented Apr 26, 2024

c7be93f

[macOS] REGRESSION(275831@main): Netflix can error out while scrubbing
https://bugs.webkit.org/show_bug.cgi?id=273306
rdar://126654352

Reviewed by Eric Carlson.

A race condition can occur where the MediaSource object in the WebContent process
does not check the buffered ranges for the new currentTime after a seek, which leads
to the readyState remaining at HAVE_METADATA, and the seeked event failing to fire.

Ensure that the MediaSource checks its buffered ranges after a seek by adding an
explicit "seeked()" method to MediaSourcePrivateClient to be used when a seek completes
in a MediaPlayerPrivate, to notify the MediaSource to revalidate its buffered ranges.

To avaid adding a new race condition, call this new "seeked()" method from
MediaPlayerPrivateRemote, rather than from MediaSourcePrivateRemote, to ensure that
the "official" time is fully updated before the MediaSource monitiors its source buffers.

* Source/WebCore/Modules/mediasource/MediaSource.cpp:
(WebCore::MediaSource::failedToCreateRenderer):
(WebCore::MediaSource::seeked):
* Source/WebCore/Modules/mediasource/MediaSource.h:
* Source/WebCore/platform/graphics/MediaSourcePrivate.cpp:
(WebCore::MediaSourcePrivate::seeked):
* Source/WebCore/platform/graphics/MediaSourcePrivate.h:
* Source/WebCore/platform/graphics/MediaSourcePrivateClient.h:
* Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.h:
* Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm:
(WebCore::MediaSourcePrivateAVFObjC::seeked):
* Source/WebKit/GPUProcess/media/RemoteMediaSourceProxy.h:
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:
(WebKit::MediaPlayerPrivateRemote::seeked):

Canonical link: https://commits.webkit.org/278060@main

02a26a1

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac   πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug   πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac   πŸ§ͺ api-wpe
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 βœ… πŸ›  wpe-skia
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ›  gtk
βœ… πŸ›  tv   πŸ§ͺ mac-AS-debug-wk2   πŸ§ͺ gtk-wk2
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-wk2-stress   πŸ§ͺ api-gtk
βœ… πŸ›  πŸ§ͺ merge   πŸ›  watch
βœ… πŸ›  watch-sim

@jernoble jernoble requested a review from cdumez as a code owner April 26, 2024 07:28
@jernoble jernoble self-assigned this Apr 26, 2024
@jernoble jernoble added the Media Bugs related to the HTML 5 Media elements. label Apr 26, 2024
@@ -508,6 +508,12 @@ void MediaPlayerPrivateRemote::seeked(MediaTimeUpdateData&& timeData)
m_currentTimeEstimator.setTime(timeData);
if (auto player = m_player.get())
player->seeked(timeData.currentTime);
#if ENABLE(MEDIA_SOURCE)
// This message may well have been handled by the MediaSource object before
// theis message, which would result in the wrong
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this

@jernoble jernoble added the merge-queue Applied to send a pull request to merge-queue label Apr 26, 2024
// This message may well have been handled by the MediaSource object before
// theis message, which would result in the wrong
if (m_mediaSourcePrivate)
return m_mediaSourcePrivate->seeked(timeData.currentTime);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be:

if (RefPtr mediaSourcePrivate = m_mediaSourcePrivate)
    return mediaSourcePrivate->seeked(timeData.currentTime);

@jernoble jernoble removed the merge-queue Applied to send a pull request to merge-queue label Apr 26, 2024
@jernoble jernoble force-pushed the eng/macOS-REGRESSION275831main-Netflix-can-error-out-while-scrubbing branch from 34342ce to 02a26a1 Compare April 26, 2024 21:05
@jernoble jernoble added the merge-queue Applied to send a pull request to merge-queue label Apr 26, 2024
https://bugs.webkit.org/show_bug.cgi?id=273306
rdar://126654352

Reviewed by Eric Carlson.

A race condition can occur where the MediaSource object in the WebContent process
does not check the buffered ranges for the new currentTime after a seek, which leads
to the readyState remaining at HAVE_METADATA, and the seeked event failing to fire.

Ensure that the MediaSource checks its buffered ranges after a seek by adding an
explicit "seeked()" method to MediaSourcePrivateClient to be used when a seek completes
in a MediaPlayerPrivate, to notify the MediaSource to revalidate its buffered ranges.

To avaid adding a new race condition, call this new "seeked()" method from
MediaPlayerPrivateRemote, rather than from MediaSourcePrivateRemote, to ensure that
the "official" time is fully updated before the MediaSource monitiors its source buffers.

* Source/WebCore/Modules/mediasource/MediaSource.cpp:
(WebCore::MediaSource::failedToCreateRenderer):
(WebCore::MediaSource::seeked):
* Source/WebCore/Modules/mediasource/MediaSource.h:
* Source/WebCore/platform/graphics/MediaSourcePrivate.cpp:
(WebCore::MediaSourcePrivate::seeked):
* Source/WebCore/platform/graphics/MediaSourcePrivate.h:
* Source/WebCore/platform/graphics/MediaSourcePrivateClient.h:
* Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.h:
* Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm:
(WebCore::MediaSourcePrivateAVFObjC::seeked):
* Source/WebKit/GPUProcess/media/RemoteMediaSourceProxy.h:
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:
(WebKit::MediaPlayerPrivateRemote::seeked):

Canonical link: https://commits.webkit.org/278060@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/macOS-REGRESSION275831main-Netflix-can-error-out-while-scrubbing branch from 02a26a1 to c7be93f Compare April 26, 2024 22:02
@webkit-commit-queue
Copy link
Collaborator

Committed 278060@main (c7be93f): https://commits.webkit.org/278060@main

Reviewed commits have been landed. Closing PR #27791 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit c7be93f into WebKit:main Apr 26, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Media Bugs related to the HTML 5 Media elements.
Projects
None yet
5 participants