Bug 162860

Summary: [GStreamer] Audio glitches
Product: WebKit Reporter: jeremy9856
Component: MediaAssignee: Nobody <webkit-unassigned>
Status: RESOLVED WORKSFORME    
Severity: Normal CC: bugs-noreply, calvaris, cturner, mcatanzaro, pnormand, webkit-bug-importer, webkit
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: PC   
OS: Linux   
See Also: https://bugzilla.gnome.org/show_bug.cgi?id=772294
https://bugs.webkit.org/show_bug.cgi?id=182829
Attachments:
Description Flags
patch none

Description jeremy9856 2016-10-02 10:29:46 PDT
Hello,

I just tried Epiphany and I noticed audio glitches when I played radio paradise (webradio - https://www.radioparadise.com). The stream work well in Firefox. I'm on Fedora 24, WebKitGTK 2.12.5. 
I already reported the bug against Epiphany but they asked me to report here (https://bugzilla.gnome.org/show_bug.cgi?id=772294)

If you need some particular infos to fix this, I will be pleased to provide them :)

Thanks !
Comment 1 Philippe Normand 2016-10-06 01:38:42 PDT
Seems like this website uses Flash?
Comment 2 jeremy9856 2016-10-06 02:24:05 PDT
No it don't. I don't have flash installed.
Comment 3 Michael Catanzaro 2016-10-06 06:42:33 PDT
I see JavaScript errors:

[Error] TypeError: undefined is not an object (evaluating 'soundManager.getSoundById('rp').readyState')
	reStart (rp2_player_1.php:100)
	Global Code (Script Element 1:1)

Generally, I would say the presence of JS errors indicates the website is broken unless the website developers tell us they think otherwise.
Comment 4 Michael Catanzaro 2016-10-06 06:43:10 PDT
(In reply to comment #1)
> Seems like this website uses Flash?

Why do you say that?
Comment 5 Michael Catanzaro 2016-10-06 06:44:05 PDT
Wait, you say "audio glitches." Were you able to get sound at all? I hear nothing. Maybe they're using an encumbered codec like MP3?
Comment 6 jeremy9856 2016-10-06 08:58:45 PDT
(In reply to comment #5)
> Wait, you say "audio glitches." Were you able to get sound at all? I hear
> nothing. Maybe they're using an encumbered codec like MP3?

I think they use MP3. I tried some other stream in MP3 and the glitches are here too. I tried some Ogg vorbis stream and there was no glitch !

I use RPMfusion repo for MP3 support. Can this be a problem with the repo ?
Comment 7 Philippe Normand 2016-10-06 09:10:51 PDT
This plays without glitches here on Debian Testing (after having removed the Flash plugin, which is used by this website if installed).

Do you also hear glitches with this command?

gst-play-1.0 http://stream-uk1.radioparadise.com/mp3-192?1475769631


In this case WebKit doesn't do anything fancy with the audio rendering, so if the gst-play-1.0 command above also shows glitches, you should report the bug in GNOME's Bugzilla against the GStreamer product. Sorry for the various bugzilla redirections :)
Comment 8 jeremy9856 2016-10-06 09:21:12 PDT
No glitch with gst-play-1.0. On the other hand there is glitches even with the stream URL in Epiphany.
Comment 9 Philippe Normand 2016-10-06 09:27:47 PDT
(In reply to comment #8)
> No glitch with gst-play-1.0. On the other hand there is glitches even with
> the stream URL in Epiphany.

This doesn't make much sense.
Can you add this option to gst-play-1.0 ? --audiosink="scaletempo ! audioconvert ! audioresample ! autoaudiosink"

This should replicate the same audio rendering chain as used in WebKit.
Comment 10 jeremy9856 2016-10-06 11:07:45 PDT
The "?" seem to pose problem 

Lecture en cours de /home/jeremy/?
ERROR Ressource introuvable. for file:///home/jeremy/%3F
ERROR debug information: gstfilesrc.c(530): gst_file_src_start (): /GstPlayBin:playbin/GstURIDecodeBin:uridecodebin0/GstFileSrc:source:
No such file "/home/jeremy/?"

So I used this command

gst-play-1.0 --audiosink="scaletempo ! audioconvert ! audioresample ! autoaudiosink" http://stream-uk1.radioparadise.com/mp3-192?1475769631

No glitch too
Comment 11 jeremy9856 2016-10-06 11:09:24 PDT
Oh I re-read your comment and that was a question :D
Comment 12 jeremy9856 2016-12-10 22:56:16 PST
The problem is still here :(
Comment 13 jeremy9856 2017-03-19 01:58:00 PDT
Same problem on Fedora 25
Comment 14 Charlie Turner 2017-07-03 04:00:24 PDT
This played fine using MiniBrowser from a nightly build.

I had the same issue as Michael using Epiphany from f25 linked again 2.16.3.
Comment 15 Charlie Turner 2017-07-12 04:47:38 PDT
I don't hear glitches, and I suspect the issue heard was fixed by r218703, which could non-deterministically cause horrible audio glitches in Icecast streams.
Comment 16 jeremy9856 2017-07-14 22:48:01 PDT
Thanks. In which webkitgtk version I will be able to test that ?
Comment 17 Charlie Turner 2017-07-15 05:23:33 PDT
Jeremy, 2.16.6
Comment 18 jeremy9856 2017-07-31 22:36:29 PDT
Fedora 25 just had the 2.16.6 version and the problem is still here :(
Comment 19 Charlie Turner 2017-08-04 02:53:40 PDT
Sorry Jeremy, it looks like r218703 wasn't merged into the 2.16.6 branch. I'm going to propose it for merging and I'll update you again.
Comment 20 Charlie Turner 2017-08-08 03:03:50 PDT
This is now proposed for merging into 2.16.7: https://trac.webkit.org/wiki/WebKitGTK/2.16.x
Comment 21 jeremy9856 2017-09-29 04:32:49 PDT
I can confirm it's fixed !
Thanks !
Comment 22 Radar WebKit Bug Importer 2017-09-29 04:34:05 PDT
<rdar://problem/34736614>
Comment 23 webkit 2017-11-01 12:29:30 PDT
With webkitgtk4-2.18.2 (and epiphany-3.26.1), this seems to be mostly fixed, but not entirely.

With WebKit 2.16.x, I experienced regular glitches (slight audio discontinuity every half-second or so, which continued the entire time the stream played.

With 2.18.2, audio streams start with the same half-second glitches, but they reliably go away after 4-5 seconds of playback.

On e.g. Firefox or various stream-capable media players, there are no glitches at any point.
Comment 24 Michael Catanzaro 2017-11-01 14:39:04 PDT
Is there a specific website you can reproduce this problem on?
Comment 25 webkit 2017-11-01 14:49:31 PDT
Several radio streams at least.

E.g.:

http://bbcmedia.ic.llnwd.net/stream/bbcmedia_radio1_mf_p
http://media-ice.musicradio.com/ChillMP3
http://icy-e-ba-07-boh.sharp-stream.com:8000/kissnational.mp3
https://ais.absoluteradio.co.uk/absoluteradiomed.aac (needs to be contained in an <audio> element to be played instead of downloaded, at least in Epiphany/Web)
http://radio.virginradio.co.uk/stream
Comment 26 Michael Catanzaro 2017-11-01 18:38:13 PDT
OK, I can hear it sometimes, but not always. It seems to happen more often on the first stream, at least for me.
Comment 27 webkit 2017-11-02 06:26:34 PDT
Interesting, happens every time on each one for me, though it's difficult to hear if it's spoken word (e.g. advertising, which seems to be sometimes pre-rolled on some of those streams).

The Absolute Radio AAC stream (https://ais.absoluteradio.co.uk/absoluteradiomed.aac) is especially obvious - plays back with errors at 150-200% speed for the first 4 or so seconds.

The Chill stream should also be useful to reproduce since it uses an identical pre-roll at each stream start.
Comment 28 Philippe Normand 2018-01-04 05:08:25 PST
(In reply to webkit from comment #27)
> Interesting, happens every time on each one for me, though it's difficult to
> hear if it's spoken word (e.g. advertising, which seems to be sometimes
> pre-rolled on some of those streams).
> 
> The Absolute Radio AAC stream
> (https://ais.absoluteradio.co.uk/absoluteradiomed.aac) is especially obvious
> - plays back with errors at 150-200% speed for the first 4 or so seconds.
> 
> The Chill stream should also be useful to reproduce since it uses an
> identical pre-roll at each stream start.

I can reproduce the issue on Absolute Radio AAC but it's not a playback speed issue. Seems more like the initial ICY metadata isn't correctly parsed.
Comment 29 webkit 2018-01-04 05:24:10 PST
I don't think it's a playback speed issue with the Absolute stream it sounds like it's just not successfully getting ~ every second packet for the first few seconds, and then playing the remaining (buffered) ones without any gaps for the missing packets. So it's normal pitch, but sounds "double-speed" (i.e. like a "time stretch" high-speed playback, rather than a "chipmunk" high speed playback).

Are you hearing something different from that? I just re-tested and it's still doing the same thing on that stream.

I suspect that the behaviour (no silence inserted for missing packets) is the same on the other streams, and it' just there is a lower ratio of missing packets for the glitchy few seconds on the other streams. So something like:

Absolute: 13579 etc. (10-packet section played back in 5-packet time)
Others: 12456790 etc. (10-packet section played back in 8-packet time)

Instead of:

Absolute: 1.3.5.7.9. etc.
Others: 12.4567.90 etc.

Where "." represents a one packet-duration silence.
Comment 30 Philippe Normand 2018-01-04 06:07:04 PST
Created attachment 330461 [details]
patch

Can you test this patch please?
Comment 31 webkit 2018-01-05 08:37:50 PST
I tried to build from the latest Fedora 27 SRPM for webkitgtk4 (2.18.4-1), and though the patch applies fine, there's an eventual build error unfortunately. I haven't tried building without the patch to see if it still fails.
Comment 32 Philippe Normand 2018-01-05 09:02:34 PST
(In reply to webkit from comment #31)
> I tried to build from the latest Fedora 27 SRPM for webkitgtk4 (2.18.4-1),
> and though the patch applies fine, there's an eventual build error
> unfortunately. I haven't tried building without the patch to see if it still
> fails.

Ah well, don't bother. It seems to be yet another bug in our httpsrc element because if I disable it, there is no glitch.
Comment 33 Charlie Turner 2018-02-15 10:08:42 PST
Bug 182829 contains a patch that helps this issue quite a bit. Instead of ~4s of bad audio, it's about 1/2 s. That's still not good enough though.

I'm testing the absolute radio stream.
Comment 34 Xabier Rodríguez Calvar 2018-02-15 23:25:18 PST
Comment on attachment 330461 [details]
patch

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

It would be awesome if you could explain why this improves the situation as I see several things but I fail to understand why they work together.

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:873
> -    // Emit a GST_EVENT_CUSTOM_DOWNSTREAM_STICKY event to let GStreamer know about the HTTP headers sent and received.
> +    // Emit a GST_EVENT_CUSTOM_DOWNSTREAM_STICKY event and message to let
> +    // GStreamer know about the HTTP headers sent and received.
>      GstStructure* httpHeaders = gst_structure_new_empty("http-headers");
> -    gst_structure_set(httpHeaders, "uri", G_TYPE_STRING, priv->originalURI.data(), nullptr);
> +    gst_structure_set(httpHeaders, "uri", G_TYPE_STRING, priv->originalURI.data(),
> +                      "http-status-code", G_TYPE_UINT, response.httpStatusCode(), nullptr);

You can keep this in a single line.

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:888
> +    gst_element_post_message(GST_ELEMENT_CAST(src), gst_message_new_element(GST_OBJECT_CAST(src),
> +        gst_structure_copy(httpHeaders)));

ditto
Comment 35 Philippe Normand 2018-02-16 01:09:46 PST
(In reply to Xabier Rodríguez Calvar from comment #34)
> Comment on attachment 330461 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=330461&action=review
> 
> It would be awesome if you could explain why this improves the situation as
> I see several things but I fail to understand why they work together.
> 
> > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:873
> > -    // Emit a GST_EVENT_CUSTOM_DOWNSTREAM_STICKY event to let GStreamer know about the HTTP headers sent and received.
> > +    // Emit a GST_EVENT_CUSTOM_DOWNSTREAM_STICKY event and message to let
> > +    // GStreamer know about the HTTP headers sent and received.
> >      GstStructure* httpHeaders = gst_structure_new_empty("http-headers");
> > -    gst_structure_set(httpHeaders, "uri", G_TYPE_STRING, priv->originalURI.data(), nullptr);
> > +    gst_structure_set(httpHeaders, "uri", G_TYPE_STRING, priv->originalURI.data(),
> > +                      "http-status-code", G_TYPE_UINT, response.httpStatusCode(), nullptr);
> 
> You can keep this in a single line.
> 
> > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:888
> > +    gst_element_post_message(GST_ELEMENT_CAST(src), gst_message_new_element(GST_OBJECT_CAST(src),
> > +        gst_structure_copy(httpHeaders)));
> 
> ditto

I wasn't expecting a review of this patch. Some of its changes were merged some weeks ago though, as part of other bugs.
Comment 36 Xabier Rodríguez Calvar 2018-02-16 02:09:31 PST
(In reply to Philippe Normand from comment #35)
> I wasn't expecting a review of this patch. Some of its changes were merged
> some weeks ago though, as part of other bugs.

I assumed it was WIP and you probably wanted feedback.
Comment 37 Philippe Normand 2018-02-21 07:48:16 PST
I think this can be closed, the behavior in gst-play and webkit are now the same, at least for the absolute AAC stream which had glitches in the past for us.

Please re-open again otherwise :)