Merge r169200 - Video is resumed with old playback rate.

https://bugs.webkit.org/show_bug.cgi?id=132905 Patch by Piotr Grad <p.grad@samsung.com> on 2014-05-22 Reviewed by Philippe Normand. Source/WebCore: Setting '0' playback rate is causing pipeline to pause. GStreamer player impl. exposed this information to upper layers but it should not. Solution is to hidden such situation behind m_playbackRatePause flag. Test: media/video-paused-0-rate.html * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp: (WebCore::MediaPlayerPrivateGStreamer::MediaPlayerPrivateGStreamer): (WebCore::MediaPlayerPrivateGStreamer::play): (WebCore::MediaPlayerPrivateGStreamer::pause): (WebCore::MediaPlayerPrivateGStreamer::doSeek): (WebCore::MediaPlayerPrivateGStreamer::updatePlaybackRate): (WebCore::MediaPlayerPrivateGStreamer::paused): (WebCore::MediaPlayerPrivateGStreamer::setRate): (WebCore::MediaPlayerPrivateGStreamer::updateStates): * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h: LayoutTests: * media/video-paused-0-rate-expected.txt: Added. * media/video-paused-0-rate.html: Added.
WebKit · May 26, 2014 · 913401c · 913401c
1 parent 1772a19
commit 913401c
Show file tree

Hide file tree

Showing 6 changed files with 116 additions and 7 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2014-05-22  Piotr Grad  <p.grad@samsung.com>
+
+        Video is resumed with old playback rate.
+        https://bugs.webkit.org/show_bug.cgi?id=132905
+
+        Reviewed by Philippe Normand.
+
+        * media/video-paused-0-rate-expected.txt: Added.
+        * media/video-paused-0-rate.html: Added.
+
 2014-05-06  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Dragging text from one paragraph to another does not render as expected

diff --git a/LayoutTests/media/video-paused-0-rate-expected.txt b/LayoutTests/media/video-paused-0-rate-expected.txt
@@ -0,0 +1,8 @@
+
+Test that setting '0' playbackRate works correctly.
+
+EVENT(canplaythrough)
+EXPECTED (video.currentTime == '0') OK
+EXPECTED (video.paused == 'false') OK
+END OF TEST
+
diff --git a/LayoutTests/media/video-paused-0-rate.html b/LayoutTests/media/video-paused-0-rate.html
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <script src=media-file.js></script>
+        <script src=video-test.js></script>
+        <script>
+            var timeUpdateWasCalled = false;
+            function start()
+            {
+                findMediaElement();
+                video.src = findMediaFile("video", "content/test");
+                waitForEventAndFail('ended');
+                video.addEventListener('timeupdate', timeupdate);
+                waitForEventOnce('canplaythrough', canPlayThrough);
+            }
+
+            function canPlayThrough()
+            {
+                video.playbackRate = 0;
+                video.play();
+            }
+
+            function timeupdate()
+            {
+                testExpected('video.currentTime', 0);
+                testExpected('video.paused', false);
+                if(timeUpdateWasCalled)
+                    failTest("Time was updated, but should not change.");
+                timeUpdateWasCalled = true;
+                setTimeout("endTest()", 150);
+            }
+        </script>
+    </head>
+    <body onload="start()">
+        <video></video>
+        <p>Test that setting '0' playbackRate works correctly.</p>
+    </body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,27 @@
+2014-05-22  Piotr Grad  <p.grad@samsung.com>
+
+        Video is resumed with old playback rate.
+        https://bugs.webkit.org/show_bug.cgi?id=132905
+
+        Reviewed by Philippe Normand.
+
+        Setting '0' playback rate is causing pipeline to pause.
+        GStreamer player impl. exposed this information to upper layers but it should not.
+        Solution is to hidden such situation behind m_playbackRatePause flag.
+
+        Test: media/video-paused-0-rate.html
+
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
+        (WebCore::MediaPlayerPrivateGStreamer::MediaPlayerPrivateGStreamer):
+        (WebCore::MediaPlayerPrivateGStreamer::play):
+        (WebCore::MediaPlayerPrivateGStreamer::pause):
+        (WebCore::MediaPlayerPrivateGStreamer::doSeek):
+        (WebCore::MediaPlayerPrivateGStreamer::updatePlaybackRate):
+        (WebCore::MediaPlayerPrivateGStreamer::paused):
+        (WebCore::MediaPlayerPrivateGStreamer::setRate):
+        (WebCore::MediaPlayerPrivateGStreamer::updateStates):
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:
+
 2014-04-30  Víctor Manuel Jáquez Leal  <vjaquez@igalia.com>
 
         [GStreamer] Use GstMetaVideo

diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
@@ -245,6 +245,7 @@ MediaPlayerPrivateGStreamer::MediaPlayerPrivateGStreamer(MediaPlayer* player)
     , m_mediaLocationCurrentIndex(0)
     , m_resetPipeline(false)
     , m_paused(true)
+    , m_playbackRatePause(false)
     , m_seeking(false)
     , m_seekIsPending(false)
     , m_timeOfOverlappingSeek(-1)
@@ -480,6 +481,11 @@ void MediaPlayerPrivateGStreamer::prepareToPlay()
 
 void MediaPlayerPrivateGStreamer::play()
 {
+    if (!m_playbackRate) {
+        m_playbackRatePause = true;
+        return;
+    }
+
     if (changePipelineState(GST_STATE_PLAYING)) {
         m_isEndReached = false;
         m_delayingLoad = false;
@@ -493,6 +499,7 @@ void MediaPlayerPrivateGStreamer::play()
 
 void MediaPlayerPrivateGStreamer::pause()
 {
+    m_playbackRatePause = false;
     GstState currentState, pendingState;
     gst_element_get_state(m_playBin.get(), &currentState, &pendingState, 0);
     if (currentState < GST_STATE_PAUSED && pendingState <= GST_STATE_PAUSED)
@@ -629,6 +636,9 @@ bool MediaPlayerPrivateGStreamer::doSeek(gint64 position, float rate, GstSeekFla
             endTime = position;
     }
 
+    if (!rate)
+        rate = 1.0;
+
     return gst_element_seek(m_playBin.get(), rate, GST_FORMAT_TIME, seekType,
         GST_SEEK_TYPE_SET, startTime, GST_SEEK_TYPE_SET, endTime);
 }
@@ -661,6 +671,17 @@ void MediaPlayerPrivateGStreamer::updatePlaybackRate()
         m_playbackRate = m_lastPlaybackRate;
         ERROR_MEDIA_MESSAGE("Set rate to %f failed", m_playbackRate);
     }
+
+    if (m_playbackRatePause) {
+        GstState state;
+        GstState pending;
+
+        gst_element_get_state(m_playBin.get(), &state, &pending, 0);
+        if (state != GST_STATE_PLAYING && pending != GST_STATE_PLAYING)
+            changePipelineState(GST_STATE_PLAYING);
+        m_playbackRatePause = false;
+    }
+
     m_changingRate = false;
     m_player->rateChanged();
 }
@@ -672,6 +693,9 @@ bool MediaPlayerPrivateGStreamer::paused() const
         return true;
     }
 
+    if (m_playbackRatePause)
+        return false;
+
     GstState state;
     gst_element_get_state(m_playBin.get(), &state, 0, 0);
     return state == GST_STATE_PAUSED;
@@ -888,15 +912,19 @@ void MediaPlayerPrivateGStreamer::setRate(float rate)
     m_changingRate = true;
 
     gst_element_get_state(m_playBin.get(), &state, &pending, 0);
-    if ((state != GST_STATE_PLAYING && state != GST_STATE_PAUSED)
-        || (pending == GST_STATE_PAUSED))
-        return;
 
     if (!rate) {
-        changePipelineState(GST_STATE_PAUSED);
+        m_changingRate = false;
+        m_playbackRatePause = true;
+        if (state != GST_STATE_PAUSED && pending != GST_STATE_PAUSED)
+            changePipelineState(GST_STATE_PAUSED);
         return;
     }
 
+    if ((state != GST_STATE_PLAYING && state != GST_STATE_PAUSED)
+        || (pending == GST_STATE_PAUSED))
+        return;
+
     updatePlaybackRate();
 }
 
@@ -1406,14 +1434,14 @@ void MediaPlayerPrivateGStreamer::updateStates()
                 m_volumeAndMuteInitialized = true;
             }
 
-            if (didBuffering && !m_buffering && !m_paused) {
+            if (didBuffering && !m_buffering && !m_paused && m_playbackRate) {
                 LOG_MEDIA_MESSAGE("[Buffering] Restarting playback.");
                 changePipelineState(GST_STATE_PLAYING);
             }
         } else if (state == GST_STATE_PLAYING) {
             m_paused = false;
 
-            if (m_buffering && !isLiveStream()) {
+            if ((m_buffering && !isLiveStream()) || !m_playbackRate) {
                 LOG_MEDIA_MESSAGE("[Buffering] Pausing stream for buffering.");
                 changePipelineState(GST_STATE_PAUSED);
             }
@@ -1450,7 +1478,7 @@ void MediaPlayerPrivateGStreamer::updateStates()
         } else if (state == GST_STATE_PLAYING)
             m_paused = false;
 
-        if (!m_paused)
+        if (!m_paused && m_playbackRate)
             changePipelineState(GST_STATE_PLAYING);
 
         m_networkState = MediaPlayer::Loading;

diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h
@@ -170,6 +170,7 @@ class MediaPlayerPrivateGStreamer : public MediaPlayerPrivateGStreamerBase {
     int m_mediaLocationCurrentIndex;
     bool m_resetPipeline;
     bool m_paused;
+    bool m_playbackRatePause;
     bool m_seeking;
     bool m_seekIsPending;
     float m_timeOfOverlappingSeek;