[GTK] [gstreamer] video won't unpause when built with -DUSE_GSTREAMER_GL=OFF

[Jeremy Bicha]

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.

[Philippe Normand]

Please attach GST_DEBUG="3,gl*:6,webkit*:6" logs.

[Jeremy Bicha]

Created attachment 337372 [details]
webkit-no-gsteamer-gl-pause-log.txt

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!

[Hyunjun Ko]

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.

[Hyunjun Ko]

Created attachment 337686 [details]
Patch

[Xabier Rodríguez Calvar]

Miguel, can you have a look at the graphics part?

[Miguel Gomez]

(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.

[Hyunjun Ko]

Created attachment 337873 [details]
Patch

[Miguel Gomez]

Created attachment 337875 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 337875 [details]
Patch

Clearing flags on attachment: 337875

Committed r230627: <https://trac.webkit.org/changeset/230627>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.