Skip to content

Commit

Permalink
[GStreamer] Video goes blank on some platforms after playback ends
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=264739

Reviewed by Philippe Normand.

On some downstream platforms, the video gets black after playback finishes.

This happens because https://bugs.webkit.org/show_bug.cgi?id=89122 used to set
the pipeline to NULL, although later https://bugs.webkit.org/show_bug.cgi?id=117354
set it to READY. This change deinitializes a big part of the pipeline, which
should otherwise stay in a working state if the state was PAUSED as it should
be without those changes. While on desktop platforms the last video frame is
still kept and shown, on some downstream platforms using a custom multimedia
subsystem the deinitialization destroys internal resources and makes that last
video frame not to be visible anymore.

The original purpose of the mentioned bugs was to avoid having the audio device
opened after the video finished. This was relevant in 2012, when many
platforms still used ALSA as a sound system, but it's not so relevant nowadays,
when PulseAudio is the usual choice on desktop.

This patch reworks the readyTimerHandler and readyTimerFired into
pausedTimerHandler and pausedTimerFired, because now the pipeline stays in
PAUSED and having a timer to eventually releasing resources by transitioning
to NULL still makes sense. The timeout has been increased to 5 minutes and
maybe it would eventually need to be even larger.

Original author: suresh-khurdiya-epam <skhurdiya.contractor@libertyglobal.com>

See: WebPlatformForEmbedded/WPEWebKit#1205

* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::MediaPlayerPrivateGStreamer): Renamed m_readyTimerHandler to m_pausedTimerHandler.
(WebCore::MediaPlayerPrivateGStreamer::~MediaPlayerPrivateGStreamer): Ditto.
(WebCore::MediaPlayerPrivateGStreamer::changePipelineState): Ditto, and added extra conditions to only start the timer on PAUSE at end.
(WebCore::MediaPlayerPrivateGStreamer::loadingFailed): Ditto.
(WebCore::MediaPlayerPrivateGStreamer::didEnd): Set the pipeline to PAUSED on end, instead of setting it to READY.
(WebCore::MediaPlayerPrivateGStreamer::pausedTimerFired): Renamed from readyTimerFired.
(WebCore::MediaPlayerPrivateGStreamer::readyTimerFired): Deleted.
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h: Renamed m_readyTimerHandler to m_pausedTimerHandler and readyTimerFired() to pausedTimerFired().

Canonical link: https://commits.webkit.org/270766@main
  • Loading branch information
eocanha committed Nov 15, 2023
1 parent 80028d2 commit 0df3813
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ MediaPlayerPrivateGStreamer::MediaPlayerPrivateGStreamer(MediaPlayer* player)
, m_preload(player->preload())
, m_maxTimeLoadedAtLastDidLoadingProgress(MediaTime::zeroTime())
, m_drawTimer(RunLoop::main(), this, &MediaPlayerPrivateGStreamer::repaint)
, m_readyTimerHandler(RunLoop::main(), this, &MediaPlayerPrivateGStreamer::readyTimerFired)
, m_pausedTimerHandler(RunLoop::main(), this, &MediaPlayerPrivateGStreamer::pausedTimerFired)
#if USE(TEXTURE_MAPPER) && !USE(NICOSIA)
, m_platformLayerProxy(adoptRef(new TextureMapperPlatformLayerProxyGL))
#endif
Expand All @@ -164,7 +164,7 @@ MediaPlayerPrivateGStreamer::MediaPlayerPrivateGStreamer(MediaPlayer* player)
, m_loader(player->createResourceLoader())
{
#if USE(GLIB)
m_readyTimerHandler.setPriority(G_PRIORITY_DEFAULT_IDLE);
m_pausedTimerHandler.setPriority(G_PRIORITY_DEFAULT_IDLE);
#endif
m_isPlayerShuttingDown.store(false);

Expand Down Expand Up @@ -203,7 +203,7 @@ MediaPlayerPrivateGStreamer::~MediaPlayerPrivateGStreamer()
if (m_fillTimer.isActive())
m_fillTimer.stop();

m_readyTimerHandler.stop();
m_pausedTimerHandler.stop();

if (m_videoSink) {
GRefPtr<GstPad> videoSinkPad = adoptGRef(gst_element_get_static_pad(m_videoSink.get(), "sink"));
Expand Down Expand Up @@ -938,12 +938,13 @@ bool MediaPlayerPrivateGStreamer::changePipelineState(GstState newState)

// Create a timer when entering the READY state so that we can free resources if we stay for too long on READY.
// Also lets remove the timer if we request a state change for any state other than READY. See also https://bugs.webkit.org/show_bug.cgi?id=117354
if (newState == GST_STATE_READY && !m_readyTimerHandler.isActive()) {
// Max interval in seconds to stay in the READY state on manual state change requests.
static const Seconds readyStateTimerDelay { 1_min };
m_readyTimerHandler.startOneShot(readyStateTimerDelay);
} else if (newState != GST_STATE_READY)
m_readyTimerHandler.stop();
if (newState == GST_STATE_PAUSED && m_isEndReached && m_player.get() && !m_player.get()->isLooping()
&& !isMediaSource() && !m_pausedTimerHandler.isActive()) {
// Max interval in seconds to stay in the PAUSED state after video finished on manual state change requests.
static const Seconds readyStateTimerDelay { 5_min };
m_pausedTimerHandler.startOneShot(readyStateTimerDelay);
} else if (newState != GST_STATE_PAUSED)
m_pausedTimerHandler.stop();

return true;
}
Expand Down Expand Up @@ -1258,7 +1259,7 @@ void MediaPlayerPrivateGStreamer::loadingFailed(MediaPlayer::NetworkState networ
}

// Loading failed, remove ready timer.
m_readyTimerHandler.stop();
m_pausedTimerHandler.stop();
}

GstElement* MediaPlayerPrivateGStreamer::createAudioSink()
Expand Down Expand Up @@ -2783,7 +2784,7 @@ void MediaPlayerPrivateGStreamer::didEnd()

if (player && !player->isLooping() && !isMediaSource()) {
m_isPaused = true;
changePipelineState(GST_STATE_READY);
changePipelineState(GST_STATE_PAUSED);
m_didDownloadFinish = false;
configureMediaStreamAudioTracks();
}
Expand Down Expand Up @@ -3175,9 +3176,9 @@ bool MediaPlayerPrivateGStreamer::canSaveMediaData() const
return false;
}

void MediaPlayerPrivateGStreamer::readyTimerFired()
void MediaPlayerPrivateGStreamer::pausedTimerFired()
{
GST_DEBUG_OBJECT(pipeline(), "In READY for too long. Releasing pipeline resources.");
GST_DEBUG_OBJECT(pipeline(), "In PAUSED for too long. Releasing pipeline resources.");
changePipelineState(GST_STATE_NULL);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ class MediaPlayerPrivateGStreamer : public MediaPlayerPrivateInterface
static void volumeChangedCallback(MediaPlayerPrivateGStreamer*);
static void muteChangedCallback(MediaPlayerPrivateGStreamer*);

void readyTimerFired();
void pausedTimerFired();

template <typename TrackPrivateType> void notifyPlayerOfTrack();

Expand Down Expand Up @@ -548,7 +548,7 @@ class MediaPlayerPrivateGStreamer : public MediaPlayerPrivateInterface
Condition m_drawCondition;
Lock m_drawLock;
RunLoop::Timer m_drawTimer WTF_GUARDED_BY_LOCK(m_drawLock);
RunLoop::Timer m_readyTimerHandler;
RunLoop::Timer m_pausedTimerHandler;
#if USE(TEXTURE_MAPPER)
#if USE(NICOSIA)
RefPtr<Nicosia::ContentLayer> m_nicosiaLayer;
Expand Down

0 comments on commit 0df3813

Please sign in to comment.