Changeset 284741 in webkit


Ignore:
Timestamp:
Oct 22, 2021 10:58:14 PM (9 months ago)
Author:
Jean-Yves Avenard
Message:

video appears blank with only audio playing if video element isn't appended to the dom tree
​https://bugs.webkit.org/show_bug.cgi?id=232124
rdar://83438282

Reviewed by Eric Carlson.

Source/WebCore:

If the renderer isn't accelerated, the current playback policity is to
not have the video tracks visible on screen.
The HTMLMediaElement could only check if the renderer was accelerated if
it was part of the DOM.
On iPhone, for historical reasons, inline playback isn't allowed but
will play fullscreen instead.

This is a temporary workaround until bug 232125 is comlpeted which would provide
a more elegant and universal solution.

Test: media/video-element-fullscreen-not-in-dom-accelerated-iphone.html

  • html/HTMLMediaElement.cpp:

(WebCore::HTMLMediaElement::mediaPlayerRenderingCanBeAccelerated):

  • html/HTMLMediaElement.h:
  • testing/Internals.cpp:

(WebCore::Internals::mediaPlayerRenderingCanBeAccelerated):

  • testing/Internals.h:
  • testing/Internals.idl:

Source/WebKit:

Ensure that we inform the GPU process whenever
MediaPlayer::renderingCanBeAccelerated value could have changed.

  • WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:

(WebKit::MediaPlayerPrivateRemote::load):
(WebKit::MediaPlayerPrivateRemote::readyStateChanged):
(WebKit::MediaPlayerPrivateRemote::checkAcceleratedRenderingState):
(WebKit::MediaPlayerPrivateRemote::updateConfiguration):
(WebKit::MediaPlayerPrivateRemote::setVideoFullscreenLayer):
(WebKit::MediaPlayerPrivateRemote::setVideoFullscreenGravity):

  • WebProcess/GPU/media/MediaPlayerPrivateRemote.h:

LayoutTests:

  • TestExpectations:
  • media/remove-video-element-in-pip-from-document-expected.txt:
  • media/remove-video-element-in-pip-from-document.html: update test to improve coverage.
  • media/video-element-fullscreen-not-in-dom-accelerated-iphone-expected.txt: Added.
  • media/video-element-fullscreen-not-in-dom-accelerated-iphone.html: Added.
  • platform/ios/TestExpectations:
  • platform/ipad/TestExpectations:
Location:
trunk
Files:
2 added
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r284740 r284741  
     12021-10-22  Jean-Yves Avenard  <jya@apple.com>
     2
     3        video appears blank with only audio playing if video element isn't appended to the dom tree
     4        https://bugs.webkit.org/show_bug.cgi?id=232124
     5        rdar://83438282
     6
     7        Reviewed by Eric Carlson.
     8
     9        * TestExpectations:
     10        * media/remove-video-element-in-pip-from-document-expected.txt:
     11        * media/remove-video-element-in-pip-from-document.html: update test to improve coverage.
     12        * media/video-element-fullscreen-not-in-dom-accelerated-iphone-expected.txt: Added.
     13        * media/video-element-fullscreen-not-in-dom-accelerated-iphone.html: Added.
     14        * platform/ios/TestExpectations:
     15        * platform/ipad/TestExpectations:
     16
    1172021-10-22  Said Abou-Hallawa  <said@apple.com>
    218

  • trunk/LayoutTests/TestExpectations

    r284724 r284741  
    279279
    280280media/remove-video-element-in-pip-from-document.html [ Skip ]
     281# This test is only relevent on iPhone where videos always play in fullscreen.
     282media/video-element-fullscreen-not-in-dom-accelerated-iphone.html [ Skip ]
    281283
    282284# Shared Workers are unsupported

  • trunk/LayoutTests/media/remove-video-element-in-pip-from-document-expected.txt

    r265904 r284741  
    55EVENT(canplaythrough)
    66EVENT(enterpictureinpicture)
     7EXPECTED (window.internals.mediaPlayerRenderingCanBeAccelerated(video) == 'true') OK
    78EXPECTED (pipWindow.width > '0') OK
    89EXPECTED (pipWindow.height > '0') OK

  • trunk/LayoutTests/media/remove-video-element-in-pip-from-document.html

    r265904 r284741  
    2626            waitForEventOnce('enterpictureinpicture', event => {
    2727                window.pipWindow = event.pictureInPictureWindow;
     28                testExpected('window.internals.mediaPlayerRenderingCanBeAccelerated(video)', true);
    2829                testExpected('pipWindow.width', 0, '>');
    2930                testExpected('pipWindow.height', 0, '>');

  • trunk/LayoutTests/platform/ios/TestExpectations

    r284722 r284741  
    224224# This test uses movie matricies, which AVFoundation and QTKit64 do not support.
    225225media/video-size-intrinsic-scale.html [ WontFix ]
     226
     227media/video-element-fullscreen-not-in-dom-accelerated-iphone.html [ Pass ]
    226228
    227229# MediaSource is not currently supported on iOS.

  • trunk/LayoutTests/platform/ipad/TestExpectations

    r284686 r284741  
    4646media/modern-media-controls/media-documents/media-document-audio-ios-sizing.html [ Skip ]
    4747media/modern-media-controls/media-documents/media-document-video-iphone-sizing.html [ Skip ]
     48
     49# This test is designed for iPhone and would timeout on iPad
     50media/video-element-fullscreen-not-in-dom-accelerated-iphone.html [ Skip ]
    4851
    4952# This test is iPad-specific

  • trunk/Source/WebCore/ChangeLog

    r284740 r284741  
     12021-10-22  Jean-Yves Avenard  <jya@apple.com>
     2
     3        video appears blank with only audio playing if video element isn't appended to the dom tree
     4        https://bugs.webkit.org/show_bug.cgi?id=232124
     5        rdar://83438282
     6
     7        Reviewed by Eric Carlson.
     8
     9        If the renderer isn't accelerated, the current playback policity is to
     10        not have the video tracks visible on screen.
     11        The HTMLMediaElement could only check if the renderer was accelerated if
     12        it was part of the DOM.
     13        On iPhone, for historical reasons, inline playback isn't allowed but
     14        will play fullscreen instead.
     15
     16        This is a temporary workaround until bug 232125 is comlpeted which would provide
     17        a more elegant and universal solution.
     18
     19        Test: media/video-element-fullscreen-not-in-dom-accelerated-iphone.html
     20
     21        * html/HTMLMediaElement.cpp:
     22        (WebCore::HTMLMediaElement::mediaPlayerRenderingCanBeAccelerated):
     23        * html/HTMLMediaElement.h:
     24        * testing/Internals.cpp:
     25        (WebCore::Internals::mediaPlayerRenderingCanBeAccelerated):
     26        * testing/Internals.h:
     27        * testing/Internals.idl:
     28
    1292021-10-22  Said Abou-Hallawa  <said@apple.com>
    230

  • trunk/Source/WebCore/html/HTMLMediaElement.cpp

    r284093 r284741  
    52135213bool HTMLMediaElement::mediaPlayerRenderingCanBeAccelerated()
    52145214{
     5215#if ENABLE(VIDEO_PRESENTATION_MODE)
    52155216    // This function must return "true" when the video is playing in the
    5216     // picture-in-picture window. Otherwise, the MediaPlayerPrivate* may
    5217     // destroy the video layer.
    5218     if (m_videoFullscreenMode == VideoFullscreenModePictureInPicture)
     5217    // picture-in-picture window or if it is in fullscreen.
     5218    // Otherwise, the MediaPlayerPrivate* may destroy the video layer if
     5219    // the no longer in the DOM.
     5220    if (m_videoFullscreenLayer)
    52195221        return true;
    5220 
     5222#endif
    52215223    auto* renderer = this->renderer();
    52225224    return is<RenderVideo>(renderer)

  • trunk/Source/WebCore/html/HTMLMediaElement.h

    r284528 r284741  
    604604    void dispatchEvent(Event&) override;
    605605
     606    WEBCORE_EXPORT bool mediaPlayerRenderingCanBeAccelerated() final;
     607
    606608protected:
    607609    HTMLMediaElement(const QualifiedName&, Document&, bool createdByParser);
    … …  
    689691    void mediaPlayerRepaint() final;
    690692    void mediaPlayerSizeChanged() final;
    691     bool mediaPlayerRenderingCanBeAccelerated() final;
    692693    void mediaPlayerRenderingModeChanged() final;
    693694    bool mediaPlayerAcceleratedCompositingEnabled() final;

  • trunk/Source/WebCore/testing/Internals.cpp

    r284693 r284741  
    39603960}
    39613961
     3962ExceptionOr<bool> Internals::mediaPlayerRenderingCanBeAccelerated(HTMLMediaElement& element)
     3963{
     3964    return element.mediaPlayerRenderingCanBeAccelerated();
     3965}
     3966
    39623967bool Internals::elementShouldBufferData(HTMLMediaElement& element)
    39633968{

  • trunk/Source/WebCore/testing/Internals.h

    r284523 r284741  
    660660    void beginSimulatedHDCPError(HTMLMediaElement&);
    661661    void endSimulatedHDCPError(HTMLMediaElement&);
     662    ExceptionOr<bool> mediaPlayerRenderingCanBeAccelerated(HTMLMediaElement&);
    662663
    663664    bool elementShouldBufferData(HTMLMediaElement&);

  • trunk/Source/WebCore/testing/Internals.idl

    r284523 r284741  
    701701    [Conditional=VIDEO] undefined beginSimulatedHDCPError(HTMLMediaElement media);
    702702    [Conditional=VIDEO] undefined endSimulatedHDCPError(HTMLMediaElement media);
     703    [Conditional=VIDEO] boolean mediaPlayerRenderingCanBeAccelerated(HTMLMediaElement media);
    703704
    704705    [Conditional=VIDEO] boolean elementShouldBufferData(HTMLMediaElement media);

  • trunk/Source/WebKit/ChangeLog

    r284712 r284741  
     12021-10-22  Jean-Yves Avenard  <jya@apple.com>
     2
     3        video appears blank with only audio playing if video element isn't appended to the dom tree
     4        https://bugs.webkit.org/show_bug.cgi?id=232124
     5        rdar://83438282
     6
     7        Reviewed by Eric Carlson.
     8
     9        Ensure that we inform the GPU process whenever
     10        MediaPlayer::renderingCanBeAccelerated value could have changed.
     11
     12        * WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:
     13        (WebKit::MediaPlayerPrivateRemote::load):
     14        (WebKit::MediaPlayerPrivateRemote::readyStateChanged):
     15        (WebKit::MediaPlayerPrivateRemote::checkAcceleratedRenderingState):
     16        (WebKit::MediaPlayerPrivateRemote::updateConfiguration):
     17        (WebKit::MediaPlayerPrivateRemote::setVideoFullscreenLayer):
     18        (WebKit::MediaPlayerPrivateRemote::setVideoFullscreenGravity):
     19        * WebProcess/GPU/media/MediaPlayerPrivateRemote.h:
     20
    1212021-10-22  Chris Fleizach  <cfleizach@apple.com>
    222

  • trunk/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp

    r284439 r284741  
    210210            return;
    211211
    212         m_configuration = WTFMove(configuration);
     212        updateConfiguration(WTFMove(configuration));
    213213        player->mediaEngineUpdated();
    214214    }, m_id);
    … …  
    355355    if (RefPtr player = m_player.get()) {
    356356        player->readyStateChanged();
    357         bool renderingCanBeAccelerated = player->renderingCanBeAccelerated();
     357        checkAcceleratedRenderingState();
     358    }
     359}
     360
     361void MediaPlayerPrivateRemote::volumeChanged(double volume)
     362{
     363    m_volume = volume;
     364    if (RefPtr player = m_player.get())
     365        player->volumeChanged(volume);
     366}
     367
     368void MediaPlayerPrivateRemote::muteChanged(bool muted)
     369{
     370    m_muted = muted;
     371    if (RefPtr player = m_player.get())
     372        player->muteChanged(muted);
     373}
     374
     375void MediaPlayerPrivateRemote::timeChanged(RemoteMediaPlayerState&& state)
     376{
     377    m_seeking = false;
     378    updateCachedState(WTFMove(state));
     379    if (RefPtr player = m_player.get())
     380        player->timeChanged();
     381}
     382
     383void MediaPlayerPrivateRemote::durationChanged(RemoteMediaPlayerState&& state)
     384{
     385    updateCachedState(WTFMove(state));
     386    if (RefPtr player = m_player.get())
     387        player->durationChanged();
     388}
     389
     390void MediaPlayerPrivateRemote::rateChanged(double rate)
     391{
     392    m_rate = rate;
     393    if (RefPtr player = m_player.get())
     394        player->rateChanged();
     395}
     396
     397void MediaPlayerPrivateRemote::playbackStateChanged(bool paused, MediaTime&& mediaTime, MonotonicTime&& wallTime)
     398{
     399    m_cachedState.paused = paused;
     400    m_cachedMediaTime = mediaTime;
     401    m_cachedMediaTimeQueryTime = wallTime;
     402    if (RefPtr player = m_player.get())
     403        player->playbackStateChanged();
     404}
     405
     406void MediaPlayerPrivateRemote::engineFailedToLoad(long platformErrorCode)
     407{
     408    m_platformErrorCode = platformErrorCode;
     409    if (RefPtr player = m_player.get())
     410        player->remoteEngineFailedToLoad();
     411}
     412
     413void MediaPlayerPrivateRemote::characteristicChanged(RemoteMediaPlayerState&& state)
     414{
     415    updateCachedState(WTFMove(state));
     416    if (RefPtr player = m_player.get())
     417        player->characteristicChanged();
     418}
     419
     420void MediaPlayerPrivateRemote::sizeChanged(WebCore::FloatSize naturalSize)
     421{
     422    m_cachedState.naturalSize = naturalSize;
     423    if (RefPtr player = m_player.get())
     424        player->sizeChanged();
     425}
     426
     427void MediaPlayerPrivateRemote::currentTimeChanged(const MediaTime& mediaTime, const MonotonicTime& queryTime, bool timeIsProgressing)
     428{
     429    m_timeIsProgressing = timeIsProgressing;
     430    m_cachedMediaTime = mediaTime;
     431    m_cachedMediaTimeQueryTime = queryTime;
     432}
     433
     434void MediaPlayerPrivateRemote::firstVideoFrameAvailable()
     435{
     436    INFO_LOG(LOGIDENTIFIER);
     437    if (RefPtr player = m_player.get())
     438        player->firstVideoFrameAvailable();
     439}
     440
     441void MediaPlayerPrivateRemote::renderingModeChanged()
     442{
     443    INFO_LOG(LOGIDENTIFIER);
     444    if (RefPtr player = m_player.get())
     445        player->renderingModeChanged();
     446}
     447
     448String MediaPlayerPrivateRemote::engineDescription() const
     449{
     450    return m_configuration.engineDescription;
     451}
     452
     453bool MediaPlayerPrivateRemote::supportsScanning() const
     454{
     455    return m_configuration.supportsScanning;
     456}
     457
     458bool MediaPlayerPrivateRemote::supportsFullscreen() const
     459{
     460    return m_configuration.supportsFullscreen;
     461}
     462
     463bool MediaPlayerPrivateRemote::supportsPictureInPicture() const
     464{
     465    return m_configuration.supportsPictureInPicture;
     466}
     467
     468bool MediaPlayerPrivateRemote::supportsAcceleratedRendering() const
     469{
     470    return m_configuration.supportsAcceleratedRendering;
     471}
     472
     473void MediaPlayerPrivateRemote::acceleratedRenderingStateChanged()
     474{
     475    if (m_player) {
     476        m_renderingCanBeAccelerated = m_player->renderingCanBeAccelerated();
     477        connection().send(Messages::RemoteMediaPlayerProxy::AcceleratedRenderingStateChanged(m_renderingCanBeAccelerated), m_id);
     478    }
     479}
     480
     481void MediaPlayerPrivateRemote::checkAcceleratedRenderingState()
     482{
     483    if (m_player) {
     484        bool renderingCanBeAccelerated = m_player->renderingCanBeAccelerated();
    358485        if (m_renderingCanBeAccelerated != renderingCanBeAccelerated)
    359486            acceleratedRenderingStateChanged();
    … …  
    361488}
    362489
    363 void MediaPlayerPrivateRemote::volumeChanged(double volume)
    364 {
    365     m_volume = volume;
    366     if (RefPtr player = m_player.get())
    367         player->volumeChanged(volume);
    368 }
    369 
    370 void MediaPlayerPrivateRemote::muteChanged(bool muted)
    371 {
    372     m_muted = muted;
    373     if (RefPtr player = m_player.get())
    374         player->muteChanged(muted);
    375 }
    376 
    377 void MediaPlayerPrivateRemote::timeChanged(RemoteMediaPlayerState&& state)
    378 {
    379     m_seeking = false;
    380     updateCachedState(WTFMove(state));
    381     if (RefPtr player = m_player.get())
    382         player->timeChanged();
    383 }
    384 
    385 void MediaPlayerPrivateRemote::durationChanged(RemoteMediaPlayerState&& state)
    386 {
    387     updateCachedState(WTFMove(state));
    388     if (RefPtr player = m_player.get())
    389         player->durationChanged();
    390 }
    391 
    392 void MediaPlayerPrivateRemote::rateChanged(double rate)
    393 {
    394     m_rate = rate;
    395     if (RefPtr player = m_player.get())
    396         player->rateChanged();
    397 }
    398 
    399 void MediaPlayerPrivateRemote::playbackStateChanged(bool paused, MediaTime&& mediaTime, MonotonicTime&& wallTime)
    400 {
    401     m_cachedState.paused = paused;
    402     m_cachedMediaTime = mediaTime;
    403     m_cachedMediaTimeQueryTime = wallTime;
    404     if (RefPtr player = m_player.get())
    405         player->playbackStateChanged();
    406 }
    407 
    408 void MediaPlayerPrivateRemote::engineFailedToLoad(long platformErrorCode)
    409 {
    410     m_platformErrorCode = platformErrorCode;
    411     if (RefPtr player = m_player.get())
    412         player->remoteEngineFailedToLoad();
    413 }
    414 
    415 void MediaPlayerPrivateRemote::characteristicChanged(RemoteMediaPlayerState&& state)
    416 {
    417     updateCachedState(WTFMove(state));
    418     if (RefPtr player = m_player.get())
    419         player->characteristicChanged();
    420 }
    421 
    422 void MediaPlayerPrivateRemote::sizeChanged(WebCore::FloatSize naturalSize)
    423 {
    424     m_cachedState.naturalSize = naturalSize;
    425     if (RefPtr player = m_player.get())
    426         player->sizeChanged();
    427 }
    428 
    429 void MediaPlayerPrivateRemote::currentTimeChanged(const MediaTime& mediaTime, const MonotonicTime& queryTime, bool timeIsProgressing)
    430 {
    431     m_timeIsProgressing = timeIsProgressing;
    432     m_cachedMediaTime = mediaTime;
    433     m_cachedMediaTimeQueryTime = queryTime;
    434 }
    435 
    436 void MediaPlayerPrivateRemote::firstVideoFrameAvailable()
    437 {
    438     INFO_LOG(LOGIDENTIFIER);
    439     if (RefPtr player = m_player.get())
    440         player->firstVideoFrameAvailable();
    441 }
    442 
    443 void MediaPlayerPrivateRemote::renderingModeChanged()
    444 {
    445     INFO_LOG(LOGIDENTIFIER);
    446     if (RefPtr player = m_player.get())
    447         player->renderingModeChanged();
    448 }
    449 
    450 String MediaPlayerPrivateRemote::engineDescription() const
    451 {
    452     return m_configuration.engineDescription;
    453 }
    454 
    455 bool MediaPlayerPrivateRemote::supportsScanning() const
    456 {
    457     return m_configuration.supportsScanning;
    458 }
    459 
    460 bool MediaPlayerPrivateRemote::supportsFullscreen() const
    461 {
    462     return m_configuration.supportsFullscreen;
    463 }
    464 
    465 bool MediaPlayerPrivateRemote::supportsPictureInPicture() const
    466 {
    467     return m_configuration.supportsPictureInPicture;
    468 }
    469 
    470 bool MediaPlayerPrivateRemote::supportsAcceleratedRendering() const
    471 {
    472     return m_configuration.supportsAcceleratedRendering;
    473 }
    474 
    475 void MediaPlayerPrivateRemote::acceleratedRenderingStateChanged()
    476 {
    477     if (RefPtr player = m_player.get()) {
    478         m_renderingCanBeAccelerated = player->renderingCanBeAccelerated();
    479         connection().send(Messages::RemoteMediaPlayerProxy::AcceleratedRenderingStateChanged(m_renderingCanBeAccelerated), m_id);
    480     }
     490void MediaPlayerPrivateRemote::updateConfiguration(RemoteMediaPlayerConfiguration&& configuration)
     491{
     492    m_configuration = WTFMove(configuration);
     493    // player->renderingCanBeAccelerated() result is dependent on m_configuration.supportsAcceleratedRendering value.
     494    checkAcceleratedRenderingState();
    481495}
    482496
    … …  
    769783                return;
    770784
    771             m_configuration = WTFMove(configuration);
     785            updateConfiguration(WTFMove(configuration));
    772786            player->mediaEngineUpdated();
    773787        }, m_id);
    … …  
    824838    m_videoLayerManager->setVideoFullscreenLayer(videoFullscreenLayer, WTFMove(completionHandler), nullptr);
    825839#endif
     840    // A Fullscreen layer has been set, this could update the value returned by player->renderingCanBeAccelerated().
     841    checkAcceleratedRenderingState();
    826842}
    827843

  • trunk/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h

    r284439 r284741  
    315315    bool supportsAcceleratedRendering() const final;
    316316    void acceleratedRenderingStateChanged() final;
     317    void checkAcceleratedRenderingState();
    317318
    318319    void setShouldMaintainAspectRatio(bool) final;
    … …  
    398399    bool playAtHostTime(const MonotonicTime&) final;
    399400    bool pauseAtHostTime(const MonotonicTime&) final;
    400 
     401    void updateConfiguration(RemoteMediaPlayerConfiguration&&);
    401402
    402403    WeakPtr<WebCore::MediaPlayer> m_player;
Note: See TracChangeset for help on using the changeset viewer.