[visionOS] Changing video.src while in video fullscreen results in black video #28394
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
EWS run on previous version of this PR (hash 62e72d3) |
webkit-ews-buildbot
added
the
merging-blocked
jernoble
removed
the
merging-blocked
jernoble
force-pushed
the
eng/visionOS-Changing-video-src-while-in-video-fullscreen-results-in-black-video
branch
from
62e72d3
to
696c9d8
Compare
|
EWS run on previous version of this PR (hash 696c9d8) |
webkit-ews-buildbot
added
the
merging-blocked
jernoble
removed
the
merging-blocked
jernoble
force-pushed
the
eng/visionOS-Changing-video-src-while-in-video-fullscreen-results-in-black-video
branch
from
696c9d8
to
415dbd7
Compare
|
EWS run on previous version of this PR (hash 415dbd7) |
webkit-ews-buildbot
added
the
merging-blocked
jernoble
removed
the
merging-blocked
jernoble
force-pushed
the
eng/visionOS-Changing-video-src-while-in-video-fullscreen-results-in-black-video
branch
from
415dbd7
to
951e03e
Compare
|
EWS run on previous version of this PR (hash 951e03e) |
jernoble
force-pushed
the
eng/visionOS-Changing-video-src-while-in-video-fullscreen-results-in-black-video
branch
from
951e03e
to
8a20ec3
Compare
|
EWS run on previous version of this PR (hash 8a20ec3) |
aestes
approved these changes
May 10, 2024
|
|
||
| #if ENABLE(LINEAR_MEDIA_PLAYER) | ||
|
|
||
| #include <pal/cocoa/MediaToolboxSoftLink.h> |
|
|
||
| #include "VideoReceiverEndpointMessage.h" | ||
| #include <WebCore/MediaPlayerPrivate.h> | ||
| #include <WebCore/VideoTargetFactory.h> |
| return; | ||
| } | ||
|
|
||
| // An cached entry implies a MediaPlayer has already been given this endpoint. |
|
|
||
| // If nothing has actually changed, bail. | ||
| if (cachedPlayerIdentifier == endpointMessage.playerIdentifier() | ||
| && cachedEndpoint.get() == endpointMessage.endpoint().get()) |
There was a problem hiding this comment.
This check seems harmless, but I don't think it's possible for cachedEndpoint to ever equal endpointMessage.endpoint() since there will be a new instance of an xpc_object_t each time endpointMessage is deserialized.
| && cachedEndpoint.get() == endpointMessage.endpoint().get()) | ||
| return; | ||
|
|
||
| RefPtr cachedPlayer = this->mediaPlayer(cachedPlayerIdentifier); |
| cachedPlayer->setVideoTarget(nullptr); | ||
| } | ||
|
|
||
| // Then set the new endpoint, which may have changed, on the specified MediaPlayer. |
There was a problem hiding this comment.
Nit: Then set the new target, which may have changed, ...
webkit-ews-buildbot
added
the
merging-blocked
jernoble
removed
the
merging-blocked
jernoble
force-pushed
the
eng/visionOS-Changing-video-src-while-in-video-fullscreen-results-in-black-video
branch
from
8a20ec3
to
ba8d535
Compare
|
EWS run on previous version of this PR (hash ba8d535) |
jernoble
force-pushed
the
eng/visionOS-Changing-video-src-while-in-video-fullscreen-results-in-black-video
branch
from
ba8d535
to
87ef343
Compare
|
EWS run on previous version of this PR (hash 87ef343) |
jernoble
added
merge-queue
jernoble
force-pushed
the
eng/visionOS-Changing-video-src-while-in-video-fullscreen-results-in-black-video
branch
from
87ef343
to
b2c885f
Compare
|
EWS run on current version of this PR (hash b2c885f) |
webkit-ews-buildbot
added
the
merging-blocked
jernoble
added
merge-queue
…ack video https://bugs.webkit.org/show_bug.cgi?id=274002 rdar://125926759 Reviewed by Andy Estes. A FigVideoTarget can only be used with one renderer at a time, and once a target has been created from an XPC endpoint, it cannot be re-used to create another FigVideoTarget. The combination of these requirements means that once a FigVideoTarget has been created for a given HTMLMediaElement, it must be cached and re-used for subsequent MediaPlayers created by that HTMLMediaElement. To allow the WebCore/platform layer to identify the owner of a MediaPlayer (typically a HTMLMediaElement, but in WebKit this will be a RemoteMediaPlayerProxy), add a new MediaPlayerClientIdentifier and make the existing HTMLMediaElementIdentifier an alias for that new identifier. Add a utility class/namespace to convert XPC endpoints to a FigVideoTarget, and add a new PlatformVideoTarget type to wrap FigVideoTarget. Add methods to MediaPlayer to allow MediaPlayerPrivates to query for an existing PlatformVideoTarget. Rename MediaPlayer setVideoEndpoint() methods to setVideoTarget(). Add a cache of PlatformVideoTargets (and endpoints, and MediaPlayerIdentifiers) in RemoteMediaPlayerManagerProxy. Having the cache exist on this object means that it will automatically be cleared when the associated WebContent process exits. Add new messaging from the UIProcess to explicitly clear cached PlatformVideoTargets when the video interface in the UIProcess is closed. * Source/WebCore/Headers.cmake: * Source/WebCore/SourcesCocoa.txt: * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/html/HTMLMediaElement.h: * Source/WebCore/html/HTMLMediaElementIdentifier.h: * Source/WebCore/platform/VideoReceiverEndpoint.h: * Source/WebCore/platform/graphics/MediaPlayer.h: (WebCore::MediaPlayerClient::mediaPlayerVideoTarget const): (WebCore::MediaPlayerClient::mediaPlayerClientIdentifier const): * Source/WebCore/platform/graphics/MediaPlayerClientIdentifier.h: Copied from Source/WebCore/html/HTMLMediaElementIdentifier.h. * Source/WebCore/platform/graphics/MediaPlayerPrivate.h: (WebCore::MediaPlayerPrivateInterface::setVideoTarget): (WebCore::MediaPlayerPrivateInterface::setVideoReceiverEndpoint): Deleted. * Source/WebCore/platform/graphics/VideoTarget.h: Copied from Source/WebCore/platform/VideoReceiverEndpoint.h. * Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h: * Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm: (WebCore::MediaPlayerPrivateAVFoundationObjC::MediaPlayerPrivateAVFoundationObjC): (WebCore::MediaPlayerPrivateAVFoundationObjC::cancelLoad): (WebCore::MediaPlayerPrivateAVFoundationObjC::createAVPlayer): (WebCore::MediaPlayerPrivateAVFoundationObjC::setVideoTarget): (WebCore::MediaPlayerPrivateAVFoundationObjC::isInFullscreenOrPictureInPictureChanged): (WebCore::MediaPlayerPrivateAVFoundationObjC::setVideoReceiverEndpoint): Deleted. * Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h: * Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm: (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::MediaPlayerPrivateMediaSourceAVFObjC): (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::destroyVideoRenderer): (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::setVideoTarget): (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::setVideoReceiverEndpoint): Deleted. * Source/WebCore/platform/graphics/cocoa/MediaPlayerCocoa.mm: (WebCore::MediaPlayer::setVideoTarget): (WebCore::MediaPlayer::setVideoReceiverEndpoint): Deleted. * Source/WebCore/platform/graphics/cocoa/VideoTargetFactory.h: Copied from Source/WebCore/platform/VideoReceiverEndpoint.h. * Source/WebCore/platform/graphics/cocoa/VideoTargetFactory.mm: Copied from Source/WebCore/platform/VideoReceiverEndpoint.h. (WebCore::VideoTargetFactory::createTargetFromEndpoint): * Source/WebKit/GPUProcess/media/RemoteMediaPlayerManagerProxy.cpp: (WebKit::RemoteMediaPlayerManagerProxy::createMediaPlayer): * Source/WebKit/GPUProcess/media/RemoteMediaPlayerManagerProxy.h: * Source/WebKit/GPUProcess/media/RemoteMediaPlayerManagerProxy.messages.in: * Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp: (WebKit::RemoteMediaPlayerProxy::create): (WebKit::RemoteMediaPlayerProxy::RemoteMediaPlayerProxy): (WebKit::RemoteMediaPlayerProxy::mediaPlayerVideoTarget const): * Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.h: * Source/WebKit/GPUProcess/media/cocoa/RemoteMediaPlayerManagerProxyCocoa.mm: Added. (WebKit::RemoteMediaPlayerManagerProxy::videoTargetForMediaElementIdentifier): (WebKit::RemoteMediaPlayerManagerProxy::handleVideoReceiverEndpointMessage): * Source/WebKit/Platform/ios/VideoReceiverEndpointMessage.h: (WebKit::VideoReceiverEndpointMessage::mediaElementIdentifier const): * Source/WebKit/Platform/ios/VideoReceiverEndpointMessage.mm: (WebKit::VideoReceiverEndpointMessage::VideoReceiverEndpointMessage): (WebKit::VideoReceiverEndpointMessage::decode): (WebKit::VideoReceiverEndpointMessage::encode const): * Source/WebKit/Scripts/webkit/messages.py: (serialized_identifiers): * Source/WebKit/Shared/EntryPointUtilities/Cocoa/XPCService/XPCEndpointMessages.mm: (WebKit::handleVideoReceiverEndpointMessage): * Source/WebKit/Shared/WTFArgumentCoders.serialization.in: * Source/WebKit/SourcesCocoa.txt: * Source/WebKit/UIProcess/Cocoa/PlaybackSessionManagerProxy.h: * Source/WebKit/UIProcess/Cocoa/PlaybackSessionManagerProxy.mm: (WebKit::PlaybackSessionModelContext::~PlaybackSessionModelContext): (WebKit::PlaybackSessionModelContext::setVideoReceiverEndpoint): (WebKit::PlaybackSessionManagerProxy::setVideoReceiverEndpoint): (WebKit::PlaybackSessionManagerProxy::uncacheVideoReceiverEndpoint): * Source/WebKit/UIProcess/WebPageProxy.h: * Source/WebKit/WebKit.xcodeproj/project.pbxproj: * Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.cpp: (WebKit::RemoteMediaPlayerManager::createRemoteMediaPlayer): Canonical link: https://commits.webkit.org/278712@main
webkit-commit-queue
force-pushed
the
eng/visionOS-Changing-video-src-while-in-video-fullscreen-results-in-black-video
branch
from
b2c885f
to
159e1c8
Compare
|
Committed 278712@main (159e1c8): https://commits.webkit.org/278712@main Reviewed commits have been landed. Closing PR #28394 and removing active labels. |
webkit-commit-queue
removed
the
merge-queue
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
159e1c8
b2c885f