[GStreamer] Seeking fails on media content provided by servers not supporting Range...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Nov 2013 14:03:25 +0000 (14:03 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Nov 2013 14:03:25 +0000 (14:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=85994

Patch by Andres Gomez <agomez@igalia.com> on 2013-11-25
Reviewed by Philippe Normand.

Source/WebCore:

Based on the patch written by Andre Moreira Magalhaes.

When the GStreamer web source was doing a "Range" request we were
expecting to receive a 206 status reply with the "Content-Range"
header and just the requested data. Supporting "Range" requests is
not mandatory so, for the servers not supporting it they will be
replying with a 200 status and the whole content of the media
element. Now, we are properly handling these replies too.

Test: http/tests/media/media-seeking-no-ranges-server.html

* platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
(StreamingClient::handleResponseReceived): Do not fail when
receiving 200 as response for HTTP range requests.
(StreamingClient::handleDataReceived): Manually seek on stream
when receiving the full data after a seek.

LayoutTests:

Added test to check that seeking media files on http servers not
supporting "Range" requests doesn't trigger an error.

* http/tests/media/media-seeking-no-ranges-server-expected.txt: Added.
* http/tests/media/media-seeking-no-ranges-server.html: Added.
* http/tests/media/resources/load-video.php: Added support for new
"ranges" paramenter.
* http/tests/media/resources/serve-video.php: Added support for
new "ranges" paramenter. When "ranges" is "no" we always answer
the HTTP status "200 OK" and send the whole file.
* platform/mac/TestExpectations: New test skipped in Mac port as
media playback download control is passed to AVFoundation that
doesn't like fancy PHP URLs like the one used in the test and, in
addition, they won't care about HTTP servers not supporting
"Range" requests.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@159746 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/media/media-seeking-no-ranges-server-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/media/media-seeking-no-ranges-server.html [new file with mode: 0644]
LayoutTests/http/tests/media/resources/load-video.php
LayoutTests/http/tests/media/resources/serve-video.php
LayoutTests/platform/mac/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp

index 2dcf2aa..1645d3e 100644 (file)
@@ -1,3 +1,26 @@
+2013-11-25  Andres Gomez  <agomez@igalia.com>
+
+        [GStreamer] Seeking fails on media content provided by servers not supporting Range requests
+        https://bugs.webkit.org/show_bug.cgi?id=85994
+
+        Reviewed by Philippe Normand.
+
+        Added test to check that seeking media files on http servers not
+        supporting "Range" requests doesn't trigger an error.
+
+        * http/tests/media/media-seeking-no-ranges-server-expected.txt: Added.
+        * http/tests/media/media-seeking-no-ranges-server.html: Added.
+        * http/tests/media/resources/load-video.php: Added support for new
+        "ranges" paramenter.
+        * http/tests/media/resources/serve-video.php: Added support for
+        new "ranges" paramenter. When "ranges" is "no" we always answer
+        the HTTP status "200 OK" and send the whole file.
+        * platform/mac/TestExpectations: New test skipped in Mac port as
+        media playback download control is passed to AVFoundation that
+        doesn't like fancy PHP URLs like the one used in the test and, in
+        addition, they won't care about HTTP servers not supporting
+        "Range" requests.
+
 2013-11-25  Mario Sanchez Prada  <mario.prada@samsung.com>
 
         Unreviewed GTK gardening. Removed expectations for test that is no
diff --git a/LayoutTests/http/tests/media/media-seeking-no-ranges-server-expected.txt b/LayoutTests/http/tests/media/media-seeking-no-ranges-server-expected.txt
new file mode 100644 (file)
index 0000000..b22654a
--- /dev/null
@@ -0,0 +1,12 @@
+
+Test ended by:
+
+Seek media file served by a server not supporting "Range" requests.
+Verify that the 'ended' event fires upon finishing the playback.
+
+EVENT(playing)
+RUN(audio.currentTime = 0.5)
+
+EVENT(ended)
+END OF TEST
+
diff --git a/LayoutTests/http/tests/media/media-seeking-no-ranges-server.html b/LayoutTests/http/tests/media/media-seeking-no-ranges-server.html
new file mode 100644 (file)
index 0000000..3eadfc4
--- /dev/null
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <script src=../../media-resources/video-test.js></script>
+        <script src=../../media-resources/media-file.js></script>
+
+        <script>
+            var media = findMediaFile('audio', '../../../../media/content/silence');
+            var type = mimeTypeForExtension(media.split('.').pop());
+            var audio;
+
+            function start()
+            {
+                audio = document.querySelector("audio");
+                audio.src = 'http://127.0.0.1:8000/media/resources/load-video.php?name=' + media + '&type=' + type + '&ranges=no';
+
+                waitForEventAndFail('error');
+                waitForEventAndEnd('ended');
+                waitForEventOnce('playing', canPlay);
+            }
+
+            function canPlay()
+            {
+                run("audio.currentTime = 0.5");
+                consoleWrite("");
+            }
+        </script>
+    </head>
+    <body onload="start()">
+        <audio controls autoplay></audio>
+        <p><b>Test ended by:</b>
+        <ol>
+            <li>Seek media file served by a server not supporting "Range" requests.</li>
+            <li>Verify that the 'ended' event fires upon finishing the playback.</li>
+        </ol>
+        </p>
+        <br/>
+    </body>
+</html>
index 00a39f5..26eb2c4 100644 (file)
@@ -2,10 +2,14 @@
 
     $fileName = $_GET["name"];
     $type = $_GET["type"];
+    if (array_key_exists("ranges", $_GET))
+        $ranges = $_GET["ranges"];
 
     $_GET = array();
     $_GET['name'] = $fileName;
     $_GET['type'] = $type;
-    @include("./serve-video.php"); 
+    if (isset($ranges))
+        $_GET["ranges"] = $ranges;
+    @include("./serve-video.php");
 
 ?>
index 1c732bd..5d4cb76 100644 (file)
@@ -2,14 +2,17 @@
 
     $fileName = $_GET["name"];
     $type = $_GET["type"];
+    if (array_key_exists("ranges", $_GET))
+        $ranges = $_GET["ranges"];
 
     $fileSize = filesize($fileName);
     $start = 0;
     $end = $fileSize - 1;
-    $contentRange = $_SERVER["HTTP_RANGE"];
+    if ($ranges != "no")
+        $contentRange = $_SERVER["HTTP_RANGE"];
     if (isset($contentRange)) {
-        $range = explode("-", substr($contentRange, strlen("bytes="))); 
-        $start = intval($range[0]); 
+        $range = explode("-", substr($contentRange, strlen("bytes=")));
+        $start = intval($range[0]);
         if (!empty($range[1]))
             $end = intval($range[1]);
         $httpStatus = "206 Partial Content";
     header("Pragma: no-cache");
     header("Etag: " . '"' . $fileSize . "-" . filemtime($fileName) . '"');
     header("Content-Type: " . $type);
-    header("Accept-Ranges: bytes");
+    if ($ranges != "no")
+        header("Accept-Ranges: bytes");
     header("Content-Length: " . ($end - $start + 1));
-    if ($contentRange)
-               header("Content-Range: bytes " . $start . "-" . $end . "/" . $fileSize); 
+    if (isset($contentRange))
+        header("Content-Range: bytes " . $start . "-" . $end . "/" . $fileSize);
     header("Connection: close");
 
     $chunkSize = 1024 * 256;
index 8da6540..8541ea2 100644 (file)
@@ -590,6 +590,9 @@ http/tests/media/video-play-stall-seek.html
 http/tests/media/video-play-stall.html
 media/media-fullscreen-not-in-document.html
 
+# Media playback control is passed to AVFoundation that doesn't like fancy PHP URLs like the one used in the test and, in addition, they won't care about HTTP servers not supporting "Range" requests
+http/tests/media/media-seeking-no-ranges-server.html
+
 # <rdar://problem/11358748> http/tests/multipart/multipart-wait-before-boundary.html fails on ML as of r115745
 http/tests/multipart/multipart-wait-before-boundary.html
 
index 6e3b426..0b7455a 100644 (file)
@@ -1,3 +1,27 @@
+2013-11-25  Andres Gomez  <agomez@igalia.com>
+
+        [GStreamer] Seeking fails on media content provided by servers not supporting Range requests
+        https://bugs.webkit.org/show_bug.cgi?id=85994
+
+        Reviewed by Philippe Normand.
+
+        Based on the patch written by Andre Moreira Magalhaes.
+
+        When the GStreamer web source was doing a "Range" request we were
+        expecting to receive a 206 status reply with the "Content-Range"
+        header and just the requested data. Supporting "Range" requests is
+        not mandatory so, for the servers not supporting it they will be
+        replying with a 200 status and the whole content of the media
+        element. Now, we are properly handling these replies too.
+
+        Test: http/tests/media/media-seeking-no-ranges-server.html
+
+        * platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
+        (StreamingClient::handleResponseReceived): Do not fail when
+        receiving 200 as response for HTTP range requests.
+        (StreamingClient::handleDataReceived): Manually seek on stream
+        when receiving the full data after a seek.
+
 2013-11-25  Radu Stavila  <stavila@adobe.com>
 
         Removed obsolete FIXME after the landing of visual overflow patch (https://bugs.webkit.org/show_bug.cgi?id=118665).
index ff1889a..a944933 100644 (file)
@@ -839,17 +839,28 @@ void StreamingClient::handleResponseReceived(const ResourceResponse& response)
 
     GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
 
-    // If we seeked we need 206 == PARTIAL_CONTENT
-    if (priv->requestedOffset && response.httpStatusCode() != 206) {
-        locker.unlock();
-        GST_ELEMENT_ERROR(src, RESOURCE, READ, (0), (0));
-        gst_app_src_end_of_stream(priv->appsrc);
-        webKitWebSrcStop(src);
+    if (priv->seekID) {
+        GST_DEBUG_OBJECT(src, "Seek in progress, ignoring response");
         return;
     }
 
+    if (priv->requestedOffset) {
+        // Seeking ... we expect a 206 == PARTIAL_CONTENT
+        if (response.httpStatusCode() == 200) {
+            // Range request didn't have a ranged response; resetting offset.
+            priv->offset = 0;
+        } else if (response.httpStatusCode() != 206) {
+            // Range request completely failed.
+            locker.unlock();
+            GST_ELEMENT_ERROR(src, RESOURCE, READ, ("Received unexpected %d HTTP status code", response.httpStatusCode()), (0));
+            gst_app_src_end_of_stream(priv->appsrc);
+            webKitWebSrcStop(src);
+            return;
+        }
+    }
+
     long long length = response.expectedContentLength();
-    if (length > 0)
+    if (length > 0 && priv->requestedOffset && response.httpStatusCode() == 206)
         length += priv->requestedOffset;
 
     priv->size = length >= 0 ? length : 0;
@@ -934,6 +945,27 @@ void StreamingClient::handleDataReceived(const char* data, int length)
         return;
     }
 
+    if (priv->offset < priv->requestedOffset) {
+        // Range request failed; seeking manually.
+        if (priv->offset + length <= priv->requestedOffset) {
+            // Discard all the buffers coming before the requested seek position.
+            priv->offset += length;
+            priv->buffer.clear();
+            return;
+        }
+
+        if (priv->offset + length > priv->requestedOffset) {
+            guint64 offset = priv->requestedOffset - priv->offset;
+            data += offset;
+            length -= offset;
+            if (priv->buffer)
+                gst_buffer_resize(priv->buffer.get(), offset, -1);
+            priv->offset = priv->requestedOffset;
+        }
+
+        priv->requestedOffset = 0;
+    }
+
     // Ports using the GStreamer backend but not the soup implementation of ResourceHandle
     // won't be using buffers provided by this client, the buffer is created here in that case.
     if (!priv->buffer)