From: commit-queue@webkit.org Date: Thu, 5 Sep 2013 07:33:38 +0000 (+0000) Subject: [Qt][WK1] REGRESSION(r154988): compositing/video/video-with-invalid-source.html X-Git-Url: https://git.webkit.org/?p=WebKit-https.git;a=commitdiff_plain;h=49bc046750e1695559a22c638818a93a21a8b43e [Qt][WK1] REGRESSION(r154988): compositing/video/video-with-invalid-source.html https://bugs.webkit.org/show_bug.cgi?id=120683 Patch by Andre Moreira Magalhaes on 2013-09-05 Reviewed by Philippe Normand. Do not set pipeline state to NULL on MediaPlayerPrivateGStreamer::loadingFailed() otherwise the bus is flushed and we never get a GST_MESSAGE_ERROR when failing to load uris. Also restore previous behaviour (before r154988) of not invoking loadingFailed() for all failed manual state change attempts. * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp: (WebCore::MediaPlayerPrivateGStreamer::changePipelineState): Do not call loadingFailed() if state change fails as all manual state changes are now done with changePipelineState(). (WebCore::MediaPlayerPrivateGStreamer::play): (WebCore::MediaPlayerPrivateGStreamer::pause): (WebCore::MediaPlayerPrivateGStreamer::seek): (WebCore::MediaPlayerPrivateGStreamer::handleMessage): Restore previous behaviour (before changeset r154988) when calling changePipelineState(). (WebCore::MediaPlayerPrivateGStreamer::updateStates): Do nothing if changing to READY on EOS (same behaviour as setting to NULL as it was before changeset r154988). (WebCore::MediaPlayerPrivateGStreamer::loadingFailed): Do not set pipeline state to NULL so we properly get GST_MESSAGE_ERROR on loading failures. git-svn-id: https://svn.webkit.org/repository/webkit/trunk@155104 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 38837c3e5ef7..c631f4df9234 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,31 @@ +2013-09-05 Andre Moreira Magalhaes + + [Qt][WK1] REGRESSION(r154988): compositing/video/video-with-invalid-source.html + https://bugs.webkit.org/show_bug.cgi?id=120683 + + Reviewed by Philippe Normand. + + Do not set pipeline state to NULL on MediaPlayerPrivateGStreamer::loadingFailed() + otherwise the bus is flushed and we never get a GST_MESSAGE_ERROR when failing to + load uris. + Also restore previous behaviour (before r154988) of not invoking loadingFailed() for + all failed manual state change attempts. + + * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp: + (WebCore::MediaPlayerPrivateGStreamer::changePipelineState): + Do not call loadingFailed() if state change fails as all manual state changes are + now done with changePipelineState(). + (WebCore::MediaPlayerPrivateGStreamer::play): + (WebCore::MediaPlayerPrivateGStreamer::pause): + (WebCore::MediaPlayerPrivateGStreamer::seek): + (WebCore::MediaPlayerPrivateGStreamer::handleMessage): + Restore previous behaviour (before changeset r154988) when calling changePipelineState(). + (WebCore::MediaPlayerPrivateGStreamer::updateStates): + Do nothing if changing to READY on EOS (same behaviour as setting to NULL as it was before + changeset r154988). + (WebCore::MediaPlayerPrivateGStreamer::loadingFailed): + Do not set pipeline state to NULL so we properly get GST_MESSAGE_ERROR on loading failures. + 2013-09-05 Alberto Garcia [GTK] MediaControlsGtk: fix warning in constructor due to incorrect order of attributes diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp index 5d94f69bf65b..1b7c57925d59 100644 --- a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp +++ b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp @@ -441,7 +441,6 @@ bool MediaPlayerPrivateGStreamer::changePipelineState(GstState newState) GstStateChangeReturn setStateResult = gst_element_set_state(m_playBin.get(), newState); GstState pausedOrPlaying = newState == GST_STATE_PLAYING ? GST_STATE_PAUSED : GST_STATE_PLAYING; if (currentState != pausedOrPlaying && setStateResult == GST_STATE_CHANGE_FAILURE) { - loadingFailed(MediaPlayer::Empty); return false; } @@ -476,6 +475,8 @@ void MediaPlayerPrivateGStreamer::play() m_preload = MediaPlayer::Auto; setDownloadBuffering(); LOG_MEDIA_MESSAGE("Play"); + } else { + loadingFailed(MediaPlayer::Empty); } } @@ -488,6 +489,8 @@ void MediaPlayerPrivateGStreamer::pause() if (changePipelineState(GST_STATE_PAUSED)) INFO_MEDIA_MESSAGE("Pause"); + else + loadingFailed(MediaPlayer::Empty); } float MediaPlayerPrivateGStreamer::duration() const @@ -586,7 +589,8 @@ void MediaPlayerPrivateGStreamer::seek(float time) if (m_isEndReached) { LOG_MEDIA_MESSAGE("[Seek] reset pipeline"); m_resetPipeline = true; - changePipelineState(GST_STATE_PAUSED); + if (!changePipelineState(GST_STATE_PAUSED)) + loadingFailed(MediaPlayer::Empty); } } else { // We can seek now. @@ -941,7 +945,8 @@ gboolean MediaPlayerPrivateGStreamer::handleMessage(GstMessage* message) INFO_MEDIA_MESSAGE("Element %s requested state change to %s", elementName.get(), gst_element_state_get_name(requestedState)); m_requestedState = requestedState; - changePipelineState(requestedState); + if (!changePipelineState(requestedState)) + loadingFailed(MediaPlayer::Empty); } break; case GST_MESSAGE_ELEMENT: @@ -1221,6 +1226,11 @@ void MediaPlayerPrivateGStreamer::updateStates() case GST_STATE_CHANGE_SUCCESS: { LOG_MEDIA_MESSAGE("State: %s, pending: %s", gst_element_state_get_name(state), gst_element_state_get_name(pending)); + // Do nothing if on EOS and state changed to READY to avoid recreating the player + // on HTMLMediaElement and properly generate the video 'ended' event. + if (m_isEndReached && state == GST_STATE_READY) + break; + if (state <= GST_STATE_READY) { m_resetPipeline = true; m_mediaDuration = 0; @@ -1238,12 +1248,8 @@ void MediaPlayerPrivateGStreamer::updateStates() m_networkState = MediaPlayer::Empty; break; case GST_STATE_READY: - // Do not change network/ready states if on EOS and state changed to READY to avoid - // recreating the player on HTMLMediaElement. - if (!m_isEndReached) { - m_readyState = MediaPlayer::HaveMetadata; - m_networkState = MediaPlayer::Empty; - } + m_readyState = MediaPlayer::HaveMetadata; + m_networkState = MediaPlayer::Empty; break; case GST_STATE_PAUSED: case GST_STATE_PLAYING: @@ -1529,8 +1535,7 @@ void MediaPlayerPrivateGStreamer::loadingFailed(MediaPlayer::NetworkState error) m_player->readyStateChanged(); } - // Loading failed, force reset pipeline and remove ready timer. - gst_element_set_state(m_playBin.get(), GST_STATE_NULL); + // Loading failed, remove ready timer. if (m_readyTimerHandler) { g_source_remove(m_readyTimerHandler); m_readyTimerHandler = 0;