Skip to content

Commit

Permalink
Have HTMLVideoElement manage syschronisation of mediaPlayerRenderingC…
Browse files Browse the repository at this point in the history
…anBeAccelerated 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
  • Loading branch information
jyavenard committed Feb 22, 2024
1 parent 50f43b7 commit 48c9403
Show file tree
Hide file tree
Showing 11 changed files with 149 additions and 43 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@

RUN(source = new ManagedMediaSource())
RUN(video.src = URL.createObjectURL(source))
EVENT(sourceopen)
RUN(sourceBuffer = source.addSourceBuffer(loader.type()))
RUN(sourceBuffer.appendBuffer(loader.initSegment()))
EVENT(update)
RUN(sourceBuffer.timestampOffset = 0.012833333333333334)
RUN(sourceBuffer.appendBuffer(loader.mediaSegment(index++)))
EVENT(update)
RUN(sourceBuffer.appendBuffer(loader.mediaSegment(index++)))
EVENT(update)
RUN(sourceBuffer.appendBuffer(loader.mediaSegment(index++)))
EVENT(update)
RUN(sourceBuffer.appendBuffer(loader.mediaSegment(index++)))
EVENT(update)
RUN(sourceBuffer.appendBuffer(loader.mediaSegment(index++)))
EVENT(update)
RUN(sourceBuffer.appendBuffer(loader.mediaSegment(index++)))
EVENT(update)
RUN(sourceBuffer.appendBuffer(loader.mediaSegment(index++)))
EVENT(update)
RUN(sourceBuffer.appendBuffer(loader.mediaSegment(index++)))
EVENT(update)
RUN(sourceBuffer.appendBuffer(loader.mediaSegment(index++)))
EVENT(update)
RUN(sourceBuffer.appendBuffer(loader.mediaSegment(index++)))
EVENT(update)
EXPECTED (video.currentTime >= '1') OK

53 changes: 53 additions & 0 deletions LayoutTests/media/media-source/media-managedmse-poster.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<!DOCTYPE html> <!-- webkit-test-runner [ ManagedMediaSourceEnabled=true ] -->
<html>
<head>
<title>MSE starts when video has a poster.</title>
<script src="../../media/media-source/media-source-loader.js"></script>
<script src="../../media/video-test.js"></script>
<script src="../utilities.js"></script>
<script>

var source;
var sourceBuffer;
var index;
var loader;

function loaderPromise(loader) {
return new Promise((resolve, reject) => {
loader.onload = resolve;
loader.onerror = reject;
});
}

async function init() {
findMediaElement();

loader = new MediaSourceLoader('content/test-fragmented-video-manifest.json');
await loaderPromise(loader);
video.disableRemotePlayback = true;
video.muted = true;
video.autoplay = true;
video.poster = "content/test-fragmented-video.png";
run('source = new ManagedMediaSource()');
run('video.src = URL.createObjectURL(source)');
await waitFor(source, 'sourceopen');
run('sourceBuffer = source.addSourceBuffer(loader.type())');
run('sourceBuffer.appendBuffer(loader.initSegment())');
await waitFor(sourceBuffer, 'update');
run('sourceBuffer.timestampOffset = 0.012833333333333334');
index = 0;
while (loader.mediaSegment(index) != null) {
run('sourceBuffer.appendBuffer(loader.mediaSegment(index++))');
await waitFor(sourceBuffer,'update');
};
await testExpectedEventually('video.currentTime', 1, '>=', 5000);
if (window.testRunner) {
testRunner.notifyDone();
}
};
</script>
</head>
<body onload="init()">
<video playsinline></video>
</body>
</html>
4 changes: 4 additions & 0 deletions LayoutTests/platform/ios-wk2/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ media/modern-media-controls/overflow-support [ Pass ]
http/tests/media/modern-media-controls/overflow-support [ Pass ]
media/modern-media-controls/tracks-support [ Pass ]

media/media-source/media-managedmse-airplay.html [ Pass ]
media/media-source/media-managedmse-idl.html [ Pass ]
webkit.org/b/269897 media/media-source/media-managedmse-poster.html [ Skip ]

webkit.org/b/231187 media/modern-media-controls/overflow-support/button-state.html [ Skip ]
webkit.org/b/231187 media/modern-media-controls/overflow-support/playback-speed.html [ Skip ]
webkit.org/b/231187 media/modern-media-controls/overflow-support/chapters.html [ Skip ]
Expand Down
5 changes: 4 additions & 1 deletion LayoutTests/platform/mac-wk1/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -1055,7 +1055,10 @@ media/media-sources-selection.html [ ImageOnlyFailure ]
media/media-source/media-source-webm-configuration-change.html [ Failure ]
media/media-source/media-source-webm-configuration-framerate.html [ Failure ]
media/media-source/media-source-webm-configuration-vp9-header-color.html [ Failure ]
media/media-source/media-managedmse-multipletracks-bufferedchange.html [ Failure ]

# Managed MediaSource isn't enabled on WK1
media/media-source/media-managedmse-multipletracks-bufferedchange.html [ Skip ]
media/media-source/media-managedmse-poster.html [ Skip ]

# CRABS WebM isn't supported in WK1
media/media-webm-opus-buffered.html [ Failure ]
Expand Down
29 changes: 2 additions & 27 deletions Source/WebCore/html/HTMLMediaElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@
#include "Quirks.h"
#include "RegistrableDomain.h"
#include "RenderBoxInlines.h"
#include "RenderLayerCompositor.h"
#include "RenderTheme.h"
#include "RenderVideo.h"
#include "RenderView.h"
Expand Down Expand Up @@ -1011,8 +1010,7 @@ void HTMLMediaElement::didDetachRenderers()
// If we detach a media element from a renderer, we may no longer need the MediaPlayerPrivate
// to vend a PlatformLayer. However, the renderer may be torn down and re-attached during a
// single run-loop as a result of layout or due to the element being re-parented.
if (!renderer() && m_player)
RefPtr { m_player }->acceleratedRenderingStateChanged();
computeAcceleratedRenderingStateAndUpdateMediaPlayer();
});
}

Expand Down Expand Up @@ -5715,28 +5713,6 @@ void HTMLMediaElement::mediaPlayerSizeChanged()
endProcessingMediaPlayerCallback();
}

bool HTMLMediaElement::mediaPlayerRenderingCanBeAccelerated()
{
#if ENABLE(VIDEO_PRESENTATION_MODE)
// This function must return "true" when the video is playing in the
// picture-in-picture window or if it is in fullscreen.
// Otherwise, the MediaPlayerPrivate* may destroy the video layer if
// the no longer in the DOM.
if (m_videoFullscreenMode != VideoFullscreenModeNone)
return true;
#endif
auto* renderer = dynamicDowncast<RenderVideo>(this->renderer());
return renderer && renderer->view().compositor().canAccelerateVideoRendering(*renderer);
}

void HTMLMediaElement::mediaPlayerRenderingModeChanged()
{
ALWAYS_LOG(LOGIDENTIFIER);

// Kick off a fake recalcStyle that will update the compositing tree.
invalidateStyleAndLayerComposition();
}

bool HTMLMediaElement::mediaPlayerAcceleratedCompositingEnabled()
{
return document().settings().acceleratedCompositingEnabled();
Expand Down Expand Up @@ -9064,8 +9040,7 @@ void HTMLMediaElement::setFullscreenMode(VideoFullscreenMode mode)
visibilityStateChanged();
schedulePlaybackControlsManagerUpdate();

if (RefPtr player = m_player)
player->acceleratedRenderingStateChanged();
computeAcceleratedRenderingStateAndUpdateMediaPlayer();
}

#if !RELEASE_LOG_DISABLED
Expand Down
7 changes: 2 additions & 5 deletions Source/WebCore/html/HTMLMediaElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,6 @@ class HTMLMediaElement
RefPtr<MediaPlayer> protectedPlayer() const { return m_player; }
WEBCORE_EXPORT std::optional<MediaPlayerIdentifier> playerIdentifier() const;

bool supportsAcceleratedRendering() const { return m_player && m_player->supportsAcceleratedRendering(); }

virtual bool isVideo() const { return false; }
bool hasVideo() const override { return false; }
bool hasAudio() const override;
Expand Down Expand Up @@ -628,8 +626,6 @@ class HTMLMediaElement
using EventTarget::dispatchEvent;
void dispatchEvent(Event&) override;

WEBCORE_EXPORT bool mediaPlayerRenderingCanBeAccelerated() final;

#if USE(AUDIO_SESSION)
AudioSessionCategory categoryAtMostRecentPlayback() const { return m_categoryAtMostRecentPlayback; }
AudioSessionMode modeAtMostRecentPlayback() const { return m_modeAtMostRecentPlayback; }
Expand Down Expand Up @@ -728,7 +724,6 @@ class HTMLMediaElement
void mediaPlayerResourceNotSupported() final;
void mediaPlayerRepaint() final;
void mediaPlayerSizeChanged() final;
void mediaPlayerRenderingModeChanged() final;
bool mediaPlayerAcceleratedCompositingEnabled() final;
void mediaPlayerWillInitializeMediaEngine() final;
void mediaPlayerDidInitializeMediaEngine() final;
Expand Down Expand Up @@ -1054,6 +1049,8 @@ class HTMLMediaElement
void playPlayer();
void pausePlayer();

virtual void computeAcceleratedRenderingStateAndUpdateMediaPlayer() { }

struct RemotePlaybackConfiguration {
MediaTime currentTime;
double rate;
Expand Down
45 changes: 44 additions & 1 deletion Source/WebCore/html/HTMLVideoElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@
#include "Performance.h"
#include "PictureInPictureSupport.h"
#include "RenderImage.h"
#include "RenderLayerCompositor.h"
#include "RenderVideo.h"
#include "RenderView.h"
#include "ScriptController.h"
#include "Settings.h"
#include "VideoFrameMetadata.h"
Expand Down Expand Up @@ -113,6 +115,47 @@ void HTMLVideoElement::didAttachRenderers()
}
}

void HTMLVideoElement::acceleratedRenderingStateChanged()
{
computeAcceleratedRenderingStateAndUpdateMediaPlayer();
}

bool HTMLVideoElement::supportsAcceleratedRendering() const
{
return RefPtr { player() } && player()->supportsAcceleratedRendering();
}

void HTMLVideoElement::mediaPlayerRenderingModeChanged()
{
ALWAYS_LOG(LOGIDENTIFIER);

// Kick off a fake recalcStyle that will update the compositing tree.
computeAcceleratedRenderingStateAndUpdateMediaPlayer();
invalidateStyleAndLayerComposition();
}

void HTMLVideoElement::computeAcceleratedRenderingStateAndUpdateMediaPlayer()
{
RefPtr player = this->player();
if (!player)
return;
#if ENABLE(VIDEO_PRESENTATION_MODE)
// This function must return "true" when the video is playing in the
// picture-in-picture window or if it is in fullscreen.
// Otherwise, the MediaPlayerPrivate* may destroy the video layer if
// it is no longer in the DOM.
bool isInFullScreen = fullscreenMode() != VideoFullscreenModeNone;
#else
bool isInFullScreen = false;
#endif
auto* renderer = this->renderer();
bool canBeAccelerated = player->supportsAcceleratedRendering() && (isInFullScreen || (renderer && renderer->view().compositor().hasAcceleratedCompositing()));
if (canBeAccelerated == m_renderingCanBeAccelerated)
return;
m_renderingCanBeAccelerated = canBeAccelerated;
player->acceleratedRenderingStateChanged(); // This call will trigger a call back to `mediaPlayerRenderingCanBeAccelerated()` from the MediaPlayer.
}

void HTMLVideoElement::collectPresentationalHintsForAttribute(const QualifiedName& name, const AtomString& value, MutableStyleProperties& style)
{
if (name == widthAttr) {
Expand Down Expand Up @@ -681,7 +724,7 @@ void HTMLVideoElement::mediaPlayerEngineUpdated()
player->startVideoFrameMetadataGathering();
}
// If the RenderLayerCompositor had queried the element's MediaPlayer::supportsAcceleratedRendering prior the player having been created it would have been set to false.
HTMLMediaElement::mediaPlayerRenderingModeChanged();
mediaPlayerRenderingModeChanged();
}

} // namespace WebCore
Expand Down
8 changes: 8 additions & 0 deletions Source/WebCore/html/HTMLVideoElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ class HTMLVideoElement final : public HTMLMediaElement, public Supplementable<HT
#endif

RenderVideo* renderer() const;
void acceleratedRenderingStateChanged();
bool supportsAcceleratedRendering() const;

bool shouldServiceRequestVideoFrameCallbacks() const { return !m_videoFrameRequests.isEmpty(); }
void serviceRequestVideoFrameCallbacks(ReducedResolutionSeconds);
Expand Down Expand Up @@ -142,14 +144,20 @@ class HTMLVideoElement final : public HTMLMediaElement, public Supplementable<HT

PlatformMediaSession::MediaType presentationType() const final { return PlatformMediaSession::MediaType::Video; }

bool mediaPlayerRenderingCanBeAccelerated() final { return m_renderingCanBeAccelerated; }
void mediaPlayerRenderingModeChanged() final;
void mediaPlayerEngineUpdated() final;

void computeAcceleratedRenderingStateAndUpdateMediaPlayer() final;

std::unique_ptr<HTMLImageLoader> m_imageLoader;

AtomString m_defaultPosterURL;

FloatSize m_lastReportedNaturalSize { };

bool m_renderingCanBeAccelerated { false };

#if ENABLE(VIDEO_PRESENTATION_MODE)
bool m_enteringPictureInPicture { false };
bool m_exitingPictureInPicture { false };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4268,6 +4268,7 @@ GstElement* MediaPlayerPrivateGStreamer::createVideoSink()
m_videoSink = gst_element_factory_make("fakesink", nullptr);
g_object_set(m_videoSink.get(), "sync", TRUE, nullptr);
}
player->renderingModeChanged();

return m_videoSink.get();
}
Expand Down
3 changes: 1 addition & 2 deletions Source/WebCore/rendering/RenderVideo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -324,8 +324,7 @@ bool RenderVideo::supportsAcceleratedRendering() const

void RenderVideo::acceleratedRenderingStateChanged()
{
if (RefPtr player = videoElement().player())
player->acceleratedRenderingStateChanged();
videoElement().acceleratedRenderingStateChanged();
}

bool RenderVideo::requiresImmediateCompositing() const
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -580,11 +580,7 @@ void MediaPlayerPrivateRemote::acceleratedRenderingStateChanged()

void MediaPlayerPrivateRemote::updateConfiguration(RemoteMediaPlayerConfiguration&& configuration)
{
bool oldSupportsAcceleraterRendering = supportsAcceleratedRendering();
m_configuration = WTFMove(configuration);
// player->renderingCanBeAccelerated() result is dependent on m_configuration.supportsAcceleratedRendering value.
if (RefPtr player = m_player.get(); player && oldSupportsAcceleraterRendering != supportsAcceleratedRendering())
player->renderingModeChanged();
}

#if ENABLE(WIRELESS_PLAYBACK_TARGET)
Expand Down Expand Up @@ -1020,9 +1016,6 @@ void MediaPlayerPrivateRemote::setVideoFullscreenLayer(PlatformLayer* videoFulls
#if PLATFORM(COCOA)
m_videoLayerManager->setVideoFullscreenLayer(videoFullscreenLayer, WTFMove(completionHandler), nullptr);
#endif
// A Fullscreen layer has been set, this could update the value returned by player->renderingCanBeAccelerated().
if (RefPtr player = m_player.get())
player->renderingModeChanged();
}

void MediaPlayerPrivateRemote::updateVideoFullscreenInlineImage()
Expand Down

0 comments on commit 48c9403

Please sign in to comment.