Bug 183362 - [GTK] [gstreamer] video won't unpause when built with -DUSE_GSTREAMER_GL=OFF
Summary: [GTK] [gstreamer] video won't unpause when built with -DUSE_GSTREAMER_GL=OFF
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Linux
: P3 Normal
Assignee: Miguel Gomez
Keywords: Gtk
Depends on:
Reported: 2018-03-06 03:53 PST by Jeremy Bicha
Modified: 2018-04-13 03:09 PDT (History)
9 users (show)

See Also:

webkit-no-gsteamer-gl-pause-log.txt (752.22 KB, text/plain)
2018-04-06 09:47 PDT, Jeremy Bicha
no flags Details
Patch (6.40 KB, patch)
2018-04-11 01:56 PDT, Hyunjun Ko
no flags Details | Formatted Diff | Diff
Patch (6.40 KB, patch)
2018-04-13 01:11 PDT, Hyunjun Ko
no flags Details | Formatted Diff | Diff
Patch (5.60 KB, patch)
2018-04-13 01:56 PDT, Miguel Gomez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Bicha 2018-03-06 03:53:15 PST
webkit2gtk 2.19.91
or webkit2gtk 2.18.6

Ubuntu has to use -DUSE_GSTREAMER_GL=OFF at least for Ubuntu 16.04 LTS since Ubuntu can't install the "bad" gstreamer plugin set by default. (By the way, Ubuntu 18.04 LTS needs graphene to be promoted to Ubuntu "main" but there is a backlog for those kind of requests.)

When webkit2gtk is built with this option, videos do not unpause correctly.

Test Case
From Ubuntu, Open the Help app (yelp)
Click Getting Started
Play one of the videos.
Press Pause
Press Play.
The video stays stuck on the paused frame, while the sound keeps playing. Also the timer 0:07/0:34 advances like normal.
Seek works (clicking a different point in the video progress bar, but the video still remains stuck on the paused frame.

This is also reproducible with Epiphany on YouTube.

Other Info
This was split off from https://bugs.webkit.org/show_bug.cgi?id=183085 which I guess is more about the seek feature not working at all, especially for webm videos.
Comment 1 Philippe Normand 2018-03-07 07:54:55 PST
Please attach GST_DEBUG="3,gl*:6,webkit*:6" logs.
Comment 2 Jeremy Bicha 2018-04-06 09:47:05 PDT
Created attachment 337372 [details]

Sorry for the late reply. Log attached.

This issue will be worked around soon (later today probably) in Ubuntu 18.04 LTS (Beta) by switching to gstreamergl.

Unfortunately, it's not possible for Ubuntu 16.04 LTS to use gstreamergl so it would be great to have this issue fixed there. Thanks!
Comment 3 Hyunjun Ko 2018-04-10 22:54:48 PDT
This isuue is introduced by the commit daa4c4991966 (https://bugs.webkit.org/show_bug.cgi?id=180978)

Once m_drawCancelled turned into TRUE, there's no way to get back to FALSE.
In this issue, when PAUSEing, sink calls unlock which makes m_drawCancelled TRUE and never get back.
Comment 4 Hyunjun Ko 2018-04-11 01:56:51 PDT
Created attachment 337686 [details]
Comment 5 Xabier Rodríguez Calvar 2018-04-11 04:02:27 PDT
Miguel, can you have a look at the graphics part?
Comment 6 Miguel Gomez 2018-04-12 07:47:58 PDT
(In reply to Hyunjun Ko from comment #4)
> Created attachment 337686 [details]
> Patch

Hyunjun, your patch is ok and works, but I'd like to upload a new one to improve some things on MediaPlayerPrivateGStreamerBase. I think there was an error to modify the drawCancelled flag inside cancelRepaint() and I'd like to remove it from there.

cancelRepaint() mission is to avoid a deadlock when pausing video at the same time that the gstreamer thread is waiting for the main thread to paint: the main thread will lock trying to change the state of the pipeline while gstreamer is locked waiting for the main thread to paint (see https://bugs.webkit.org/show_bug.cgi?id=170003). So we shouldn't do anything extra inside that function. BTW this can only happen in the non AC case, so no need to handle other cases there.

When AC is enabled, with gst-gl there's no condition to wait, and without gst-gl the condition is notified from the compositor thread, not the main thread, so there's no deadlock.

Eventually cancelRepaint() is used in the destructor as well to release the gstreamer thread from m_drawCondition, but setting the drawCancelled flag there is a mistake IMO.
Comment 7 Hyunjun Ko 2018-04-13 01:11:04 PDT
Created attachment 337873 [details]
Comment 8 Miguel Gomez 2018-04-13 01:56:09 PDT
Created attachment 337875 [details]
Comment 9 WebKit Commit Bot 2018-04-13 03:09:28 PDT
Comment on attachment 337875 [details]

Clearing flags on attachment: 337875

Committed r230627: <https://trac.webkit.org/changeset/230627>
Comment 10 WebKit Commit Bot 2018-04-13 03:09:29 PDT
All reviewed patches have been landed.  Closing bug.