Bug 242156 – REGRESSION(243836@main): [GStreamer] Broke videos in yelp

• Component: Media
• Status: RESOLVED FIXED

[Michael Catanzaro]

243836@main "[iOS] AVAssetResourceLoadingRequest.request does not include a Range: header on iOS 15" broke video playback in yelp (the GNOME help viewer, not yelp.com).

The GStreamer debug log contains several errors:

0:00:00.146036288  4822 0x561c5fcb4600 DEBUG      webkitmediaplayer MediaPlayerPrivateGStreamer.cpp:1840:operator(): got missing plugin installation callback with result 203
0:00:00.146095073  4822 0x561c5fcb4600 WARN               structure gststructure.c:1863:priv_gst_structure_append_to_gstring: No value transform to serialize field 'gerror' of type 'GError'
0:00:00.146122003  4822 0x561c5fcb4600 ERROR           webkitcommon GStreamerCommon.cpp:466:operator():<media-player-0> Got message: error message: 0x561c60385cb0, time 99:99:99.999999999, seq-num 59, element 'uridecodebin0', GstMessageError, gerror=(GError)NULL, debug=(string)"gsturidecodebin.c\(1409\):\ gen_source_element\ \(\):\ /GstPlayBin:media-player-0/GstURIDecodeBin:uridecodebin0";
0:00:00.146147649  4822 0x561c5fcb4600 LOG        webkitmediaplayer MediaPlayerPrivateGStreamer.cpp:1708:handleMessage:<media-player-0> Message error received from element uridecodebin0
0:00:00.146180206  4822 0x561c5fcb4600 WARN               structure gststructure.c:1863:priv_gst_structure_append_to_gstring: No value transform to serialize field 'gerror' of type 'GError'
0:00:00.146202578  4822 0x561c5fcb4600 ERROR           webkitcommon GStreamerCommon.cpp:466:operator():<media-player-1> Got message: error message: 0x561c603a0a30, time 99:99:99.999999999, seq-num 139, element 'uridecodebin1', GstMessageError, gerror=(GError)NULL, debug=(string)"gsturidecodebin.c\(1409\):\ gen_source_element\ \(\):\ /GstPlayBin:media-player-1/GstURIDecodeBin:uridecodebin1";
0:00:00.146224398  4822 0x561c5fcb4600 LOG        webkitmediaplayer MediaPlayerPrivateGStreamer.cpp:1708:handleMessage:<media-player-1> Message error received from element uridecodebin1
0:00:00.146254924  4822 0x561c5fcb4600 ERROR      webkitmediaplayer MediaPlayerPrivateGStreamer.cpp:1714:handleMessage: Error 12: No URI handler implemented for "bogus-help". (url=bogus-help:/gnome-help/figures/gnome-task-switching.webm)
0:00:00.146349303  4822 0x561c5fcb4600 WARN               structure gststructure.c:1863:priv_gst_structure_append_to_gstring: No value transform to serialize field 'gerror' of type 'GError'
0:00:00.146389031  4822 0x561c5fcb4600 ERROR           webkitcommon GStreamerCommon.cpp:466:operator():<media-player-2> Got message: error message: 0x561c603b21c0, time 99:99:99.999999999, seq-num 219, element 'uridecodebin2', GstMessageError, gerror=(GError)NULL, debug=(string)"gsturidecodebin.c\(1409\):\ gen_source_element\ \(\):\ /GstPlayBin:media-player-2/GstURIDecodeBin:uridecodebin2";
0:00:00.146428686  4822 0x561c5fcb4600 LOG        webkitmediaplayer MediaPlayerPrivateGStreamer.cpp:1708:handleMessage:<media-player-2> Message error received from element uridecodebin2
0:00:00.146452332  4822 0x561c5fcb4600 ERROR      webkitmediaplayer MediaPlayerPrivateGStreamer.cpp:1714:handleMessage: Error 12: No URI handler implemented for "bogus-help". (url=bogus-help:/gnome-help/figures/gnome-windows-and-workspaces.webm)

I tried to throw together a minimal reproducer, but wound up hitting strange errors because I'm not familiar with mallard syntax. So instead my hacky test case is: clone gnome-user-doc, then hack up gnome-help/C/display-dual-monitors.page accordingly:

diff --git a/gnome-help/C/display-dual-monitors.page b/gnome-help/C/display-dual-monitors.page
index 6f74da61..b5d779db 100644
--- a/gnome-help/C/display-dual-monitors.page
+++ b/gnome-help/C/display-dual-monitors.page
@@ -29,10 +29,9 @@
 
 <title>Connect another monitor to your computer</title>
 
-<!-- TODO: update video
 <section id="video-demo">
   <title>Video Demo</title>
-  <media its:translate="no" type="video" width="500" mime="video/webm" src="figures/display-dual-monitors.webm">
+  <media its:translate="no" type="video" width="500" mime="video/webm" src="/home/mcatanzaro/big-buck-bunny_trailer.webm">
     <p its:translate="yes">Demo</p>
     <tt:tt its:translate="yes" xmlns:tt="http://www.w3.org/ns/ttml">
       <tt:body>
@@ -69,7 +68,6 @@
     </tt:tt>
   </media>
 </section>
--->
 
   <comment>
     <cite date="2018-07-29" href="mailto:[e-mail]">Michael Hill</cite>

Note the hardcoded path to my home directory, which would need to be changed. I used the video from https://dl8.webmfiles.org/big-buck-bunny_trailer.webm but I doubt that matters. Then run:

$ yelp gnome-help/C/display-dual-monitors.page

Before 243836@main, the big buck bunny video will play. With 243836@main it no longer plays. Note that all of the changes in this commit are platform-specific except for the change to HTMLMediaElement.cpp.

[Michael Catanzaro]

BTW this hack was needed because I failed to find any documentation with videos remaining so that I could give an easy reproducer. We only noticed because RHEL 8 still has some videos.

Anyway, the change that broke things is:

diff --git a/Source/WebCore/html/HTMLMediaElement.cpp b/Source/WebCore/html/HTMLMediaElement.cpp
index ff346f25f75e..345a7e310251 100644
--- a/Source/WebCore/html/HTMLMediaElement.cpp
+++ b/Source/WebCore/html/HTMLMediaElement.cpp
@@ -1472,10 +1472,12 @@ void HTMLMediaElement::loadResource(const URL& initialURL, ContentType& contentT
     }
 
     URL url = initialURL;
-    if (!url.isEmpty() && !frame->loader().willLoadMediaElementURL(url, *this)) {
+#if PLATFORM(COCOA)
+    if (url.isLocalFile() && !frame->loader().willLoadMediaElementURL(url, *this)) {
         mediaLoadingFailed(MediaPlayer::NetworkState::FormatError);
         return;
     }
+#endif
 
 #if ENABLE(CONTENT_EXTENSIONS)
     if (RefPtr documentLoader = frame->loader().documentLoader()) {

Jer's changelog contains reasoning for making this Cocoa-specific:

"""
Drive-by fix: to allow this change to be testable, we must revert a change which calls all the
network delegate callbacks with empty data. This was necessary at the time because (at least for
Cocoa ports) media loading happened outside WebCore's loader path. Currently, all http(s), data,
blob, and custom protocol schemes are loaded through WebCore, leaving file:// URLs as the
remaining protocol type that needs custom handling, and only on Cocoa ports.
"""

Notice the GStreamer log contains errors related to the URI scheme:

0:00:00.146452332  4822 0x561c5fcb4600 ERROR      webkitmediaplayer MediaPlayerPrivateGStreamer.cpp:1714:handleMessage: Error 12: No URI handler implemented for "bogus-help". (url=bogus-help:/gnome-help/figures/gnome-windows-and-workspaces.webm)

yelp seems to be using this bogus-help URI scheme to trick WebKit into... something something, I don't understand. It was added here:

https://gitlab.gnome.org/GNOME/yelp/-/commit/04683e89cef14f56e4c3d1595424e109f12f5d06

[Michael Catanzaro]

This fixes yelp:

diff --git a/Source/WebCore/html/HTMLMediaElement.cpp b/Source/WebCore/html/HTMLMediaElement.cpp
index 97f855b6b592..95c10e228b81 100644
--- a/Source/WebCore/html/HTMLMediaElement.cpp
+++ b/Source/WebCore/html/HTMLMediaElement.cpp
@@ -1503,12 +1503,10 @@ void HTMLMediaElement::loadResource(const URL& initialURL, ContentType& contentT
     }
 
     URL url = initialURL;
-#if PLATFORM(COCOA)
-    if (url.isLocalFile() && !frame->loader().willLoadMediaElementURL(url, *this)) {
+    if (!url.isEmpty() && !frame->loader().willLoadMediaElementURL(url, *this)) {
         mediaLoadingFailed(MediaPlayer::NetworkState::FormatError);
         return;
     }
-#endif
 
 #if ENABLE(CONTENT_EXTENSIONS)
     if (RefPtr documentLoader = frame->loader().documentLoader()) {

Phil, Jer, do you want a pull request for this?

[Philippe Normand]

"""
Drive-by fix: to allow this change to be testable, we must revert a change which calls all the
network delegate callbacks with empty data. This was necessary at the time because (at least for
Cocoa ports) media loading happened outside WebCore's loader path. Currently, all http(s), data,
blob, and custom protocol schemes are loaded through WebCore, leaving file:// URLs as the
remaining protocol type that needs custom handling, and only on Cocoa ports.
"""

Well, IIUC, GStreamer ports also apply here, file loading doesn't go through WebCore, but with a pure-GStreamer filesrc element.

Can you try this test?

if ((!url.isEmpty() || url.isLocalFile()) && !frame->loader().willLoadMediaElementURL(url, *this)) {

[Michael Catanzaro]

(In reply to Philippe Normand from comment #3)
> if ((!url.isEmpty() || url.isLocalFile()) &&
> !frame->loader().willLoadMediaElementURL(url, *this)) {

That works.

[Philippe Normand]

well actually an empty url can't be local I suppose, so my suggestion doesn't make much sense.

[Michael Catanzaro]

Created a yelp bug: https://gitlab.gnome.org/GNOME/yelp/-/issues/193

[Michael Catanzaro]

Pull request: https://github.com/WebKit/WebKit/pull/2635

[EWS]

Committed 252746@main (6cbee1cfa19b): <https://commits.webkit.org/252746@main>

Reviewed commits have been landed. Closing PR #2635 and removing active labels.

[Radar WebKit Bug Importer]

<rdar://problem/97459127>