Bug 198376

Summary: [GStreamer] Cannot play Bert's Bytes radio stream from http://radio.dos.nl/
Product: WebKit Reporter: Alberto Garcia <berto>
Component: WebKitGTKAssignee: Philippe Normand <pnormand>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, aperez, ap, berto, bugs-noreply, calvaris, mailinglists, pnormand
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Alberto Garcia 2019-05-30 09:06:21 PDT
1. Go to http://radio.dos.nl/
2. Click on the menu button on the lower right corner
3. Select "Jazz"
4. Click on the "Bert's Bytes" icon.

This should play this radio stream, but it does nothing with WebKitGTK 2.24.2.

It reportedly works fine with 2.24.0 and fails with 2.24.1. It is likely related to bug 197410.

This bug was originally reported in Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=929749
Comment 1 Alberto Garcia 2019-05-30 09:07:34 PDT
(In reply to Alberto Garcia from comment #0)
> 2. Click on the menu button on the lower right corner

Sorry, I meant the lower *LEFT* corner.
Comment 2 Philippe Normand 2019-06-12 05:22:42 PDT
That's not a live stream though, http://www.bertvandenbrink.com/bertsbytes/concert.mp3
Comment 3 Richard Lucassen 2019-06-15 01:30:32 PDT
No, it's not a live stream, but 2.24.0 plays it and 2.24.1 does not.

Richard
Comment 4 Philippe Normand 2019-06-17 01:54:27 PDT
Actually it might if you wait long enough :D
It's an issue with the on-disk buffering, I started a patch.
Comment 5 Philippe Normand 2019-07-01 02:28:39 PDT
Created attachment 373217 [details]
Patch
Comment 6 Xabier Rodríguez Calvar 2019-07-01 03:48:55 PDT
Comment on attachment 373217 [details]
Patch

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

> Source/WebCore/ChangeLog:16
> +        No new tests, existing media tests cover this patch.

Why is no test turning green associated with the bug then?

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1474
> +    auto updateMaxTimeLoaded = [&]() {

Sometimes it is hard to choose when to use a lambda like this or just another regular object method. My preference in this case would be the latter.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1508
> +    case GST_BUFFERING_TIMESHIFT:
> +    case GST_BUFFERING_LIVE:

maybe just "default:" ?
Comment 7 Xabier Rodríguez Calvar 2019-07-01 04:15:09 PDT
Comment on attachment 373217 [details]
Patch

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

>> Source/WebCore/ChangeLog:16
>> +        No new tests, existing media tests cover this patch.
> 
> Why is no test turning green associated with the bug then?

There is no test and there should be. I know we are in a hurry for the release, so let's land it and open a bug for the test.
Comment 8 Aakash Jain 2019-07-01 05:57:02 PDT
I am testing gtk queue on new EWS and it seems to fail for this patch.

Can someone look if EWS caught a real issue or it's a false positive?

https://ews-build.webkit-uat.org/#/builders/7/builds/281
Retry: https://ews-build.webkit-uat.org/#/builders/7/builds/282
Comment 9 Philippe Normand 2019-07-01 06:15:35 PDT
(In reply to Aakash Jain from comment #8)
> I am testing gtk queue on new EWS and it seems to fail for this patch.
> 
> Can someone look if EWS caught a real issue or it's a false positive?
> 
> https://ews-build.webkit-uat.org/#/builders/7/builds/281
> Retry: https://ews-build.webkit-uat.org/#/builders/7/builds/282

I'll check it, thanks for the heads-up!
Comment 10 Philippe Normand 2019-07-01 06:16:55 PDT
It's a WPE issue, not GTK. I forgot an include:

GLibUtilities.h:38:15: error: ‘GUniquePtr’ does not name a type; did you mean ‘GUniquePtr_h’?
Comment 11 Aakash Jain 2019-07-01 06:20:14 PDT
(In reply to Philippe Normand from comment #10)
> It's a WPE issue, not GTK. 
Yeah, I confused that. Thanks for the correction.
Comment 12 Philippe Normand 2019-07-01 08:16:43 PDT
Created attachment 373236 [details]
Patch
Comment 13 Philippe Normand 2019-07-01 08:18:23 PDT
Comment on attachment 373236 [details]
Patch

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

> Source/WTF/wtf/glib/GUniquePtr.h:27
> +#include <memory.h>

That was the EWS issue I think. Let's see if EWS likes it now.
Comment 14 Philippe Normand 2019-07-01 08:29:53 PDT
Still red, I'll check it :)
Comment 15 Philippe Normand 2019-07-01 09:21:41 PDT
Created attachment 373239 [details]
Patch
Comment 16 Philippe Normand 2019-07-01 09:22:26 PDT
One of those weird issues with Forwarding headers and unified builds :(
Comment 17 Philippe Normand 2019-07-01 09:51:05 PDT
Created attachment 373242 [details]
Patch
Comment 18 Philippe Normand 2019-07-01 09:57:29 PDT
Committed r247010: <https://trac.webkit.org/changeset/247010>