[GStreamer] Streaming aac/mp3 audio doesn't always work

Midori (on Raspbian Buster) does not play some radio stations correctly when using the audio tag. The following HTML snippet can be used to reproduce the problem:

<p>The Breeze</p>
<audio controls src="https://stream.celador.co.uk/breeze-nglos-96.aac" preload="none"></audio>

<p>BBC Radio 4</p>
<audio controls src="http://bbcmedia.ic.llnwd.net/stream/bbcmedia_radio4fm_mf_p#.mp3" preload="none"></audio>

<p>Absolute 80s</p> 
<audio controls src="https://ais.absoluteradio.co.uk/absolute80smed.aac" preload="none"></audio>

These are audio tags for 3 different UK radio stations. The Breeze plays for 5s then stops. BBC Radio 4 plays for 10-30s and then stops. Absolute radio plays longer but sometines eventually stops.

This behaviour has been confirmed with Midori 8.0 on Raspbian Buster and Midori 9.0 on Linux Desktop. But it happens too with Epiphany, Surf and Ephemeral (all use Webkitgtk2 as backend). Streams work prefectly using gst123 or any other gstreamer frontend. So, it seems to be a Webkitgtk2 issue.

[Philippe Normand]

Which version of webkitgtk are you using?

I am using Midori v8.0 which uses webkit2gtk-4.0, according to what I can find in the build files.

[Philippe Normand]

(In reply to bugzilla from comment #2)
> webkit2gtk-4.0, according to what I can
> find in the build files.

That's not the version I'm looking for. What's the package version? 2.26.2 ?

2.16.6, I think, according to these lines in the make file:

pkg_check_modules(DEPS_GTK REQUIRED
                  gtk+-3.0>=3.12.0
                  webkit2gtk-4.0>=2.16.6
                  gcr-ui-3>=2.32
                  libpeas-gtk-1.0
                  json-glib-1.0>=0.12
                  libarchive
                  )

[Philippe Normand]

No, that's the minimum version required, not necessarily the detected version... Can you attach the midori build logs?

[Philippe Normand]

Or simply the output of: pkg-config --modversion webkit2gtk-4.0

pkg-config reports 2.26.2

[Philippe Normand]

I can reproduce the bug, the problem is that our httpsrc element quickly assumes those live stream have a content-length, hence EOS is pushed too early and playback stops :(

This works-around the bug, but might introduce other layout test failures, so it should be properly investigated:

diff --git a/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp
index 3dae81aa68d..6be6193ffd1 100644
--- a/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp
+++ b/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp
@@ -1171,10 +1171,10 @@ void CachedResourceStreamingClient::dataReceived(PlatformMediaResource&, const c
     if (priv->haveSize && (newPosition > priv->size)) {
         GST_DEBUG_OBJECT(src, "Got position previous estimated content size (%" G_GINT64_FORMAT " > %" G_GINT64_FORMAT ")", newPosition, priv->size);
         newSize = newPosition;
-    } else if (!priv->haveSize) {
-        GST_DEBUG_OBJECT(src, "Got initial response without Content-Length, assuming response size as duration.");
-        newSize = length;
-        priv->haveSize = true;
+    // } else if (!priv->haveSize) {
+    //     GST_DEBUG_OBJECT(src, "Got initial response without Content-Length, assuming response size as duration.");
+    //     newSize = length;
+    //     priv->haveSize = true;
     }
 
     if (newSize) {

[Enrique Ocaña]

Created attachment 390651 [details]
Patch

[Enrique Ocaña]

Created attachment 390656 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 390656 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=390656&action=review

> LayoutTests/http/tests/media/video-no-content-length.html:22
> +        testExpected("video.duration == Infinity", true);

shouldn't this be testExpected(video.duration, Infinity) ? That way the expectation would make more sense.

[Enrique Ocaña]

Created attachment 390660 [details]
Patch

[Philippe Normand]

Comment on attachment 390660 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=390660&action=review

Looks like the new test times out on mac-wk1 and mac-wk2

> LayoutTests/http/tests/media/resources/video-no-content-length.php:10
> +               header("Content-Type: video/mp4");
> +               $fileName = "test.mp4";
> +        } else if ($extension == 'ogv') {
> +               header("Content-Type: video/ogg");

4 spaces indentation please ;)

> LayoutTests/http/tests/media/video-no-content-length.html:8
> +<script src=../../media-resources/video-test.js></script>
> +<script src=../../media-resources/media-file.js></script>
> +<script>

i think script tags usually go in the head?

> LayoutTests/http/tests/media/video-no-content-length.html:17
> +        video.src="http://127.0.0.1:8000/media/resources/video-no-content-length.php?video="+movie;

missing spaces surrounding the =

[Eric Carlson]

(In reply to Philippe Normand from comment #13)
> Comment on attachment 390660 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=390660&action=review
> 
> Looks like the new test times out on mac-wk1 and mac-wk2
> 
Your new php won't work on macOS or iOS unless it supports range requests.

LayoutTests/http/tests/media/resources/serve-video.php already supports omitting the 'Content-Length' header, so you should be able to add a new option to LayoutTests/http/tests/media/resources/load-video.php to enable it for your test instead of adding range support to the new script.

[Enrique Ocaña]

(In reply to Eric Carlson from comment #14)

> Your new php won't work on macOS or iOS unless it supports range requests.
> 
> LayoutTests/http/tests/media/resources/serve-video.php already supports
> omitting the 'Content-Length' header, so you should be able to add a new
> option to LayoutTests/http/tests/media/resources/load-video.php to enable it
> for your test instead of adding range support to the new script.

Thanks for the suggestion, Eric. I've modified the test to use load-video.php here locally (patch not submitted yet) and now it "works" on macOS.

However, I'm getting a finite duration there instead of Infinite. It might be because the test.mp4 file used is a standalone file, while the ideal test case should be a stream like the one used in the Breeze radio stream (see original bug report). If I open a webpage with that stream in Safari, I get NaN before starting playback and then Infinite after playback start. I should find a way to replicate that behaviour in the test or just skip it completely on Mac.

[Enrique Ocaña]

Created attachment 392604 [details]
Patch

[Enrique Ocaña]

Looks like I was testing the wrong thing in the previous layout test. I was focusing too much on the duration, but that was only known after the video (now audio) had detected end-of-stream (EOS) and stopped.

I had to do a lot of empirical tests with lengths, chunk sizes, Icy headers and other stuff, but in the end I've finally managed to simulate a real live stream in this patch by stalling the server and not providing all the data at once. On the previous GStreamer code, this was detected as EOS and the audio stopped, showing a duration smaller than the real one (or Infinity on a real live stream).

I couldn't check for an exact currentTime because the Mac and GStreamer ports give different exact values. I just check if it's approximately higher than 6 sec and that fits all ports.

This time I've double checked that the new test fails without the patch (on GStreamer, in Mac it already passed), passes with the patch, and the modifications in serve-video.php don't create any regression in video-play-waiting.html and video-play-stall.html in Mac (the other tests using stallOffset/stallDuration).

[Philippe Normand]

Comment on attachment 392604 [details]
Patch

LGTM. Thanks Enrique

[WebKit Commit Bot]

Comment on attachment 392604 [details]
Patch

Clearing flags on attachment: 392604

Committed r257977: <https://trac.webkit.org/changeset/257977>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/60143165>