Bug 182936 - [GStreamer] Seek broken on YouTube
Summary: [GStreamer] Seek broken on YouTube
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: Other
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-02-19 11:34 PST by Michael Catanzaro
Modified: 2018-03-10 12:44 PST (History)
5 users (show)

See Also:


Attachments
Debug log (315.13 KB, text/plain)
2018-02-20 10:07 PST, Michael Catanzaro
no flags Details
Debug log (702.05 KB, text/plain)
2018-03-02 16:13 PST, Michael Catanzaro
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2018-02-19 11:34:41 PST
Seeking on YouTube videos no longer works. When I click around on the seek bar, the loading animation plays, but the video does not seek. It works fine in 2.18, so it's a regression.
Comment 1 Charlie Turner 2018-02-19 12:34:16 PST
Note, this is not the case on master, the job for this is to bisect what regressed between now and 2.19.90 (e.g. Ephy Tech Preview), in which apparently seeking on YouTube is broken.
Comment 2 Michael Catanzaro 2018-02-19 13:03:31 PST
My trunk checkout is on r228497, and it indeed seems fixed there.
Comment 3 Michael Catanzaro 2018-02-19 14:03:23 PST
(In reply to Michael Catanzaro from comment #2)
> My trunk checkout is on r228497, and it indeed seems fixed there.

I checked out r228089 and the issue does not occur there either, but that's actually 2.19.90. I've been testing in our JHBuild environment. Looks like this bug cannot be reproduced inside the JHBuild environment.

I can reproduce 100% of the time in Epiphany Technology Preview.
Comment 4 Michael Catanzaro 2018-02-19 14:04:28 PST
(In reply to Michael Catanzaro from comment #3)
> I can reproduce 100% of the time in Epiphany Technology Preview.

Nope, I immediately proved myself wrong. I'm not sure what the trigger for this is.
Comment 5 Michael Catanzaro 2018-02-19 14:15:51 PST
(In reply to Michael Catanzaro from comment #4)
> Nope, I immediately proved myself wrong. I'm not sure what the trigger for
> this is.

I still don't know what the trigger is, and it doesn't happen on every video, but it's extremely easy to reproduce in Epiphany Technology Preview. I've failed to reproduce in our JHBuild environment after several attempts with the same WebKit version (r228089).

I hope none of our GStreamer patches are important. ;)
Comment 6 Philippe Normand 2018-02-20 07:21:11 PST
I can't make it fail here. I use


flatpak info -c org.gnome.Epiphany/x86_64/master
e65b4310b18508709ab1736772962b2e6a480fbe4f412dbdefb7e50d987a62ea


Besides, how can set env vars in flatpak apps? a GST_DEBUG="webkit*:6" might be useful...
Comment 7 Michael Catanzaro 2018-02-20 10:06:38 PST
(In reply to Philippe Normand from comment #6)
> Besides, how can set env vars in flatpak apps?

$ flatpak run --command=/bin/bash org.gnome.Epiphany
bash-4.3$ GST_DEBUG="webkit*:6" epiphany -p

> a GST_DEBUG="webkit*:6" might be useful...

Coming right up... I'll attach a log where I was able to reproduce on my first attempt. I tried several seeks on one video, and all failed.

(In reply to Philippe Normand from comment #6)
> I can't make it fail here. I use
> 
> 
> flatpak info -c org.gnome.Epiphany/x86_64/master
> e65b4310b18508709ab1736772962b2e6a480fbe4f412dbdefb7e50d987a62ea
> 
> 
> Besides, how can set env vars in flatpak apps? a GST_DEBUG="webkit*:6" might
> be useful...

I get a30337eb2e6ee8f8a6a7df8dca16ac48764467b7a8f6d486928167fd77f1b680. Who knows which of us has a newer version. I assume your about dialog shows WebKitGTK+ 2.19.90.
Comment 8 Michael Catanzaro 2018-02-20 10:07:58 PST
Created attachment 334280 [details]
Debug log
Comment 9 Michael Catanzaro 2018-02-20 10:10:09 PST
(In reply to Michael Catanzaro from comment #7)
> I get a30337eb2e6ee8f8a6a7df8dca16ac48764467b7a8f6d486928167fd77f1b680. Who
> knows which of us has a newer version. I assume your about dialog shows
> WebKitGTK+ 2.19.90.

It does use git master versions of GStreamer. I wonder if there is any nice way to figure out exactly what commit is in use. I tried:

$ flatpak run --command=/bin/bash org.gnome.Epiphany
bash-4.3$ gst-launch-1.0 --version
gst-launch-1.0 version 1.13.0
GStreamer 1.13.0 (GIT)
Unknown package origin

But that's not very useful, all we know is that it's some git commit before 1.13.1.
Comment 10 Michael Catanzaro 2018-02-20 10:13:50 PST
Just now, I ran:

$ flatpak update
$ flatpak info -c org.gnome.Epiphany/x86_64/master
202cecbf9455e0b23e842dbd63be5d5ba1bd4ecafb29379757167b44a1fafca3

to make sure I have the absolute latest. Can still reproduce the issue.
Comment 11 Philippe Normand 2018-02-20 10:52:40 PST
I'll check tomorrow after updating my flatpak and also I'll check ToT with gst git master.
Comment 12 Philippe Normand 2018-02-20 11:37:25 PST
The log indicates MediaPlayerPrivateGStreamer::doSeek() returns too early. "[Seek] seek attempt" is logged but not "[Seek] seeking".

I suspect this is the faulty early return, https://trac.webkit.org/browser/webkit/releases/WebKitGTK/webkit-2.19.90/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp#L482
Comment 13 Michael Catanzaro 2018-02-20 12:31:57 PST
Oooh, logs are good!

I'm not testing live streams, so you think isLiveStream() is returning true improperly?

The check is also not at the right place in the function; it should be somewhere above the declaration of MediaTime time since the time variable isn't needed there.
Comment 14 Philippe Normand 2018-02-21 03:55:18 PST
(In reply to Michael Catanzaro from comment #13)
> Oooh, logs are good!
> 
> I'm not testing live streams, so you think isLiveStream() is returning true
> improperly?

Yes because in the log:
MediaPlayerPrivateGStreamer.cpp:1389:totalBytes: totalBytes 0

Can you add webkitweb*:6 in the GST_DEBUG please?

> 
> The check is also not at the right place in the function; it should be
> somewhere above the declaration of MediaTime time since the time variable
> isn't needed there.

Sure, why not :)
Comment 15 Philippe Normand 2018-02-21 03:59:42 PST
(In reply to Philippe Normand from comment #14)
> (In reply to Michael Catanzaro from comment #13)
> > Oooh, logs are good!
> > 
> > I'm not testing live streams, so you think isLiveStream() is returning true
> > improperly?
> 
> Yes because in the log:
> MediaPlayerPrivateGStreamer.cpp:1389:totalBytes: totalBytes 0
> 
> Can you add webkitweb*:6 in the GST_DEBUG please?

Ah no need, it's already there.
Comment 16 Philippe Normand 2018-02-21 05:21:08 PST
tl;dr: r228603 should be added in the 2.19 branch

I can reproduce the issue in ToT if:

- I revert r228603
- I force Youtube to send us a vp8 asset by removing libgstlibav.so from my JHBuild root.

But with ToT and libgstlibav.so removed, seek works.

So, Charlie fixed this issue :)
Comment 17 Philippe Normand 2018-02-21 06:11:18 PST
There's also the issue that our httpsrc element invalidates its total bytes duration cache when seeking, I don't think that makes any sense... I'll upload a separate patch for that...
Comment 18 Philippe Normand 2018-02-21 07:41:06 PST
See https://bugs.webkit.org/show_bug.cgi?id=183002

I think this can be closed then?
Comment 19 Michael Catanzaro 2018-02-21 11:53:53 PST
Yes, thanks!
Comment 20 Radar WebKit Bug Importer 2018-02-21 11:54:34 PST
<rdar://problem/37755510>
Comment 21 Michael Catanzaro 2018-02-21 11:54:39 PST
(In reply to Philippe Normand from comment #16)
> tl;dr: r228603 should be added in the 2.19 branch

It's already on the proposed backports list for 2.19.92. Unfortunately it just missed 2.19.91.
Comment 22 Philippe Normand 2018-02-21 13:21:01 PST
Right, I added it in the list after commenting here.
Comment 23 Michael Catanzaro 2018-03-02 10:52:43 PST
(In reply to Michael Catanzaro from comment #21)
> (In reply to Philippe Normand from comment #16)
> > tl;dr: r228603 should be added in the 2.19 branch
> 
> It's already on the proposed backports list for 2.19.92. Unfortunately it
> just missed 2.19.91.

This comment is wrong; r228603 really *did* get backported in time for 2.19.91. Unfortunately, seek on YouTube it's definitely still broken in Epiphany Tech Preview with 2.19.91. Of course, it doesn't use any local extra codecs (or any local anything). Reopening. :(
Comment 24 Michael Catanzaro 2018-03-02 10:53:23 PST
(In reply to Michael Catanzaro from comment #23)
> Unfortunately, seek on YouTube it's definitely still broken in
> Epiphany Tech Preview with 2.19.91.

Of course, I meant to say "seek on YouTube is definitely still broken"
Comment 25 Philippe Normand 2018-03-02 11:16:25 PST
Can you attach debug logs then?
Comment 26 Michael Catanzaro 2018-03-02 16:13:19 PST
Created attachment 334939 [details]
Debug log

I see "[Seek] seek attempt" is logged but not "[Seek] seeking", just like in comment #12. So r228603 definitely did not fix this.
Comment 27 Michael Catanzaro 2018-03-02 20:21:10 PST
Note: r228945 is not in 2.19.91 and has not been backported; is it possible you fixed it there?
Comment 28 Philippe Normand 2018-03-04 03:09:28 PST
(In reply to Michael Catanzaro from comment #27)
> Note: r228945 is not in 2.19.91 and has not been backported; is it possible
> you fixed it there?

Yes that's it. Added this patch in the 2.19.92 list.
Comment 29 Michael Catanzaro 2018-03-04 08:47:38 PST
OK, fingers crossed....
Comment 30 Michael Catanzaro 2018-03-10 12:44:27 PST
We finally got a new build... it's indeed fixed in 2.19.92. Thanks!