-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Apply dynamic-range-limit to videos #41266
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
Apply dynamic-range-limit to videos #41266
Conversation
|
EWS run on previous version of this PR (hash f8d0c4d) Details |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should not this function just be:
if (RefPtr player = m_player)
player->setPlatformDynamicRangeLimit(platformDynamicRangeLimit);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, but I was copying the style of the ones above. Happy to be inconsistent in this case. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we store this property here? I do not see it is used in HTMLMediaElement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good eyes!
I need it in the next patch where it will be combined with an external notification.
But I suppose I could move this to the other patch, just in case we change tactics there...
[edit: we're both wrong! See next comment.]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually it's used at line 7951, when a player is created after this call here, so that we can give the new player the correct DRLimit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this function should be renamed something like setPlayerDynamicRangeLimit() or dynamicRangeLimitDidChange().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll go with dynamicRangeLimitDidChange(), it matches the one call from RenderMedia::styleDidChange().
f8d0c4d to
0a3f1be
Compare
|
EWS run on previous version of this PR (hash 0a3f1be) Details |
0a3f1be to
3db96ae
Compare
|
EWS run on current version of this PR (hash 3db96ae) Details |
https://bugs.webkit.org/show_bug.cgi?id=288248 rdar://145326880 Reviewed by Jer Noble. HTMLMediaElement now forwards the `dynamic-range-limit` value from RenderMedia to MediaPlayer/content to MediaPlayer/GPU to MediaPlayerPrivate, with the implementation in ObjC derived classes updating the video layer's preferred dynamic range as needed. * LayoutTests/media/video-dynamic-range-limit-expected.html: Added. * LayoutTests/media/video-dynamic-range-limit.html: Added. Minimal test to exercise the new code paths, but it doesn't verify the actual rendering of HDR un/constrained videos. * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/html/HTMLMediaElement.cpp: (WebCore::HTMLMediaElement::dynamicRangeLimitDidChange): * Source/WebCore/html/HTMLMediaElement.h: * Source/WebCore/platform/graphics/MediaPlayer.cpp: (WebCore::MediaPlayer::setPlatformDynamicRangeLimit): * Source/WebCore/platform/graphics/MediaPlayer.h: * Source/WebCore/platform/graphics/MediaPlayerPrivate.h: (WebCore::MediaPlayerPrivateInterface::setPlatformDynamicRangeLimit): * Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h: * Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm: (WebCore::MediaPlayerPrivateAVFoundationObjC::createAVPlayerLayer): (WebCore::MediaPlayerPrivateAVFoundationObjC::setPlatformDynamicRangeLimit): * Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h: * Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm: (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::ensureLayer): (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::setPlatformDynamicRangeLimit): * Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.h: * Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm: (WebCore::MediaPlayerPrivateMediaStreamAVFObjC::layersAreInitialized): (WebCore::MediaPlayerPrivateMediaStreamAVFObjC::setPlatformDynamicRangeLimit): * Source/WebCore/platform/graphics/ca/cocoa/PlatformDynamicRangeLimitCocoa.h: Added. (WebCore::platformDynamicRangeLimitString): * Source/WebCore/rendering/RenderMedia.cpp: (WebCore::RenderMedia::styleDidChange): * Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp: (WebKit::RemoteMediaPlayerProxy::prepareForPlayback): (WebKit::RemoteMediaPlayerProxy::setPlatformDynamicRangeLimit): * Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.h: * Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.messages.in: * Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp: (WebKit::MediaPlayerPrivateRemote::prepareForPlayback): (WebKit::MediaPlayerPrivateRemote::setPlatformDynamicRangeLimit): * Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h: Canonical link: https://commits.webkit.org/291217@main
3db96ae to
8f3d426
Compare
|
Committed 291217@main (8f3d426): https://commits.webkit.org/291217@main Reviewed commits have been landed. Closing PR #41266 and removing active labels. |
8f3d426
3db96ae
🧪 mac-wk2🧪 mac-intel-wk2