Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 48c9403704117925f1d17498719b4fffbc0af0cf
      
https://github.com/WebKit/WebKit/commit/48c9403704117925f1d17498719b4fffbc0af0cf
  Author: Jean-Yves Avenard <j...@apple.com>
  Date:   2024-02-22 (Thu, 22 Feb 2024)

  Changed paths:
    A LayoutTests/media/media-source/media-managedmse-poster-expected.txt
    A LayoutTests/media/media-source/media-managedmse-poster.html
    M LayoutTests/platform/ios-wk2/TestExpectations
    M LayoutTests/platform/mac-wk1/TestExpectations
    M Source/WebCore/html/HTMLMediaElement.cpp
    M Source/WebCore/html/HTMLMediaElement.h
    M Source/WebCore/html/HTMLVideoElement.cpp
    M Source/WebCore/html/HTMLVideoElement.h
    M Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
    M Source/WebCore/rendering/RenderVideo.cpp
    M Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp

  Log Message:
  -----------
  Have HTMLVideoElement manage syschronisation of 
mediaPlayerRenderingCanBeAccelerated states
https://bugs.webkit.org/show_bug.cgi?id=232125
rdar://84531384

Reviewed by Youenn Fablet and Philippe Normand.

There are three conditions controlling if an accelerated layer is usable with a 
decoded video:
1- Does the media player supports hardware accelerated rendering
2- Is the renderer/compositor hardware accelerated
3- Should the video playing in this renderer be accelerated (such as having a 
MediaPlayer, having a poster displayed etc)

The information was contained at various levels and dealt as follow:
- The MediaPlayerPrivate contains the information related to 1.
- When the MediaPlayerPrivate needed to know 3) for the purpose of passing the 
value of 2) to the GPUP's MediaPlayer, it will would query the MediaPlayer, 
which queried the HTMLMediaElement client for 2), which itself queried 
RenderLayerCompositor which returned false if accelerated rendering was 
disabled and if not querid the RenderVideo, which itself queried the 
HTMLMediaElement which queried the MediaPlayer which queried the 
MediaElementPrivate to determine if the player itself supported accelerated 
rendering.
It was also up to the MediaPlayerPrivateRemote to query the value during 
regular operations in order to make sure the value didn't change since it was 
last checked.

This latter requirement or lack of check at the right time was the source of 
multiple bugs (232124, 230495, 21594, 220375, 267661, 268423 and most recently 
269684).
Each time the reason was the same, a failure to synchronise 1) 2) and 3) data, 
and each time the fix was calling `MediaPlaier::renderingModeChanged()`
in various places.
Calling renderingModeChanged() unnecessarily will cause the video layer in the 
MediaElement to be deleted and reconstructed, following
by a full re-layout.

We change this so that each element of the tree is the guardian of the 
information that matters to itself:
1- The MediaPlayerPrivate knows 1)
2- The Compositor knows 2)
3- The HTMLMediaElement knows 3.

The HTMLMediaElement is now the coordinator between the MediaPlayerPrivate and 
the Compositor/RenderVideo.
The RenderVideo no longer queries directly the MediaPlayer, only the 
HTMLMediaElement.
We change to a push model, where:
1- the RenderVideo will notify the HTMLMediaElement if accelerated rendering
status has changed.
2- The MediaPlayerPrivate will notify the HTMLMediaElement if it supports 
accelerated rendering.
3- The HTMLMediaElement will notify either the Compositor or the MediaPlayer 
when the rendering
state relevant to them has changed and will manage to force a re-layout as 
needed.

The requirement for the MediaPlayerPrivate to call "renderingModeChanged" is 
still there in order to
minimise the extent of the changes; should it fail to call it, the side affects 
are minimised
by checking the state once the MediaPlayer has been created.
In a similar fashion, once the MediaPlayerPrivate gets notified by the 
HTMLMediaElement that
the rendering state has changed, it has to call back into the HTMLMediaElement 
through a call
to `HTMLMediaElement::renderingCanBeAccelerated()` in order to minimise the 
patch size.
We could have instead pass the new rendering status as an argument.

We move the handling of layers and its acceleration status to HTMLVideoElement 
class
as if it was an audio element, repainting when the player change would be 
unnecessary
and harmful to performance.

Added test.

* LayoutTests/media/media-source/media-managedmse-poster-expected.txt: Added.
* LayoutTests/media/media-source/media-managedmse-poster.html: Added.
We use ManagedMSE instead of plain MSE so that we can test on iOS.
* LayoutTests/platform/ios/TestExpectations: Make new test runs on iOS as 
media-source tests are disabled by default
* LayoutTests/platform/mac-wk1/TestExpectations: Skip test as ManagedMSE isn't 
enabled on wk1
* Source/WebCore/html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::didDetachRenderers):
(WebCore::HTMLMediaElement::stopPeriodicTimers):
(WebCore::HTMLMediaElement::setFullscreenMode):
(WebCore::HTMLMediaElement::mediaPlayerRenderingCanBeAccelerated): Deleted.
(WebCore::HTMLMediaElement::mediaPlayerRenderingModeChanged): Deleted.
* Source/WebCore/html/HTMLMediaElement.h:
(WebCore::HTMLMediaElement::maybeSyncAcceleratedRenderingState):
(WebCore::HTMLMediaElement::supportsAcceleratedRendering const): Deleted.
* Source/WebCore/html/HTMLVideoElement.cpp:
(WebCore::HTMLVideoElement::acceleratedRenderingStateChanged):
(WebCore::HTMLVideoElement::supportsAcceleratedRendering const):
(WebCore::HTMLVideoElement::mediaPlayerRenderingModeChanged):
(WebCore::HTMLVideoElement::maybeSyncAcceleratedRenderingState):
(WebCore::HTMLVideoElement::mediaPlayerEngineUpdated):
* Source/WebCore/html/HTMLVideoElement.h:
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::createVideoSink): Call 
renderingModeChanged as the creation
of the video sink can change the status of 
MediaPlayer::supportsAcceleratedRendering.
* Source/WebCore/rendering/RenderVideo.cpp:
(WebCore::RenderVideo::acceleratedRenderingStateChanged):
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:
(WebKit::MediaPlayerPrivateRemote::updateVideoFullscreenInlineImage):
(WebKit::MediaPlayerPrivateRemote::setVideoFullscreenLayer): We no longer need
to refresh the value of acceleratedRendering. This will be done by the 
HTMLMediaElement as needed.
* Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:
(WebKit::MediaPlayerPrivateRemote::updateConfiguration): We no longer need to
check the value of `supportsAcceleratedRendering`; this is managed by the 
HTMLMediaElement

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



To unsubscribe from these emails, change your notification settings at 
https://github.com/WebKit/WebKit/settings/notifications
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to