Bug 233230 – [GStreamer] MediaPlayerPrivateGStreamer mishandles failure to create WebKitTextCombiner

• Component: Media
• Status: RESOLVED FIXED

Webkit on ubuntu 21.10 is failing due to a missing gstreamer plugin called subenc, which provides WebVTT captioning. 

It is available in an package called: gstreamer1.0-plugins-bad 

The "fix" is to sudo apt install gstreamer1.0-plugins-bad

According to GStreamer, Bad Plug-ins is a set of plug-ins that aren't up to par compared to the rest. They might be close to being good quality, but they're missing something - be it a good code review, some documentation, a set of tests, a real live maintainer, or some actual wide use.

So when Ubuntu 21.10 doesn't include the bad libraries, and webkit assumes they are going to be available... there is a problem. I don't know whose problem exactly.

The code that fails is calling webkitTextCombinerNew() and asserting that the object returned is not null.
Error message is: "WebKit wasn't able to find a WebVTT encoder. Not continuing without platform support for subtitles"
And library quits on ASSERT failure.

A bug fix in webkit would be to not fail if the webVTT plugin isn't available.. or use a different plugin that is "not bad"? Or improve the error message to make it clear a GStreamer plugin that provides WebVTT is not installed. 


Code that fails is webkitTextCombinerNew() in Source/WebCore/platform/graphics/gstreamer/TextCombinerGStreamer.cpp
GstElement* webkitTextCombinerNew()
{
    // The combiner relies on webvttenc, fail early if it's not there.
    if (!isGStreamerPluginAvailable("subenc")) {
        WTFLogAlways("WebKit wasn't able to find a WebVTT encoder. Not continuing without platform support for subtitles.");
        return nullptr;
    }

    return GST_ELEMENT(g_object_new(WEBKIT_TYPE_TEXT_COMBINER, nullptr));
}

The plugin details for subenc are:
https://gstreamer.freedesktop.org/documentation/subenc/index.html?gi-language=c

Was googling and didn't find anyone discussing this issue or the simple apt install "fix". It was non-trivial to figure out. 
Reproduce with fresh ubuntu 21.10 follow along with https://github.com/openaudible/openaudible/issues/757 
I am mostly posting this bug report as a way to let the search engines guide users to the solution... So feel free to close as somewhat resolved..

[Michael Catanzaro]

(In reply to openaudible from comment #0)
> The code that fails is calling webkitTextCombinerNew() and asserting that
> the object returned is not null.
> Error message is: "WebKit wasn't able to find a WebVTT encoder. Not
> continuing without platform support for subtitles"

So far so good from WebKit's perspective. Printing an error message when the optional package is missing is the right thing to do.

CC: Berto because it's worth checking the Debian packaging to ensure it has a Recommends: to ensure this isn't broken by default. In Fedora I have Recommends: on both gst-plugins-good and gst-plugins-bad. I see you have a Requires: on gst-plugins-good in Debian (a little aggressive, but OK), and a Recommends: ${gst:Recommends}. I wonder what that ${gst:Recommends} evaluates to. gst-plugins-good and -bad are both optional in case users want to uninstall them, but important to have by default.

> And library quits on ASSERT failure.

That would be a WebKit bug: it shouldn't crash. But you didn't provide any info that would tell us where the problem is. Please provide a backtrace taken with gdb following the instructions at https://blogs.gnome.org/mcatanzaro/2021/09/18/creating-quality-backtraces-for-crash-reports/. Thanks.

> A bug fix in webkit would be to not fail if the webVTT plugin isn't
> available.. or use a different plugin that is "not bad"? Or improve the
> error message to make it clear a GStreamer plugin that provides WebVTT is
> not installed. 

From an upstream perspective, we just need to make it not crash. I think the error message is sufficiently detailed that somebody who has intentionally chosen not to install recommended packages should be able to figure out that a GStreamer element is missing.

I think webkitTextCombinerNew() is called just once in the code base.

Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2760

Current code is:

    GstElement* textCombiner = webkitTextCombinerNew();
    ASSERT(textCombiner);
    g_object_set(m_pipeline.get(), "text-stream-combiner", textCombiner, nullptr);

Crash would be in the ASSERT(textCombiner);

You might be able to change ASSERT(textCombiner); to if (textCombiner) 

The older 2.4.0 code is quite a bit different, but also just uses ASSERT... 

This does seem like a dependency issue-- when installing webkit, one of the dependences it now needs to have is the bad plugin.. I don't entirely know who made a change that ultimately broke things.

I think just having some google results when searching for the error message will be a big help.

Thanks and good luck.

[Michael Catanzaro]

You're not hitting that ASSERT because that's a debug assert. It's disabled in release builds.

> And library quits on ASSERT failure.

How do you know this is happening? Does WebKit print something indicating that an assertion failed?

(In reply to openaudible from comment #2)
> You might be able to change ASSERT(textCombiner); to if (textCombiner) 

Probably, yes. I looked up GstPlayBin3:text-stream-combiner and it is not nullable, so that is indeed a WebKit bug. We still want to see a backtrace for your crash, though, to figure out what goes wrong next. None of that should trigger a crash, and it's not obvious where the crash occurs.

I am not sure about much. I do know installing the missing library fixes it. I don’t know the exact code used on a new Ubuntu 21.10. 

If it wasn’t an assert, it would be a null pointer dereference, I assume. 

If you know how to get an html page to create an element that calls the newTextCombiner, it should fail on a fresh Ubuntu 21.10. You might also be able to reproduce by removing the bad gs library. Or temporarily return null and see if tests pass. It is possible it is already fixed in latest code but breaks in whatever source is used for 21.10. 

Apologize I don’t have better notes or skills. Was not able to build from source with debugging enabled. So do not have a full stack trace.  (A docker build would be nice for non c programmers!)

I just know it started to affect a few users of my app that uses eclipse swt browser that uses WebKit.

[Michael Catanzaro]

(In reply to openaudible from comment #4)
> If it wasn’t an assert, it would be a null pointer dereference, I assume. 

We'll find out for sure when you post a backtrace.

> If you know how to get an html page to create an element that calls the newTextCombiner,

I don't tbh, I have no idea what text combiner does either. Subtitles? Once you post a backtrace, I'll take a look again and we'll see what to do.

> Was not able to build from source with debugging enabled.

Oh no, don't try building it yourself! That's an unreasonable amount of effort to expect for a simple bug report. I do expect you to install the debug symbols provided by Ubuntu and generate a backtrace, though, since otherwise we'd rather go look at crash reports that do have backtraces instead. See the section "Debuginfo Installation" in my blog post. Thanks.

[Michael Catanzaro]

There are instructions for Ubuntu here: https://wiki.ubuntu.com/DebuggingProgramCrash

[Alberto Garcia]

(In reply to Michael Catanzaro from comment #1)
> CC: Berto because it's worth checking the Debian packaging to ensure it has
> a Recommends: to ensure this isn't broken by default. In Fedora I have
> Recommends: on both gst-plugins-good and gst-plugins-bad. I see you have a
> Requires: on gst-plugins-good in Debian (a little aggressive, but OK), and a
> Recommends: ${gst:Recommends}. I wonder what that ${gst:Recommends}
> evaluates to. gst-plugins-good and -bad are both optional in case users want
> to uninstall them, but important to have by default.

In Debian we depend on -base and -good, and recommend -gl, -bad and -libav

I don't remember the exact discussion at the moment, but I think I
depended on -good in order to have an audio sink available, otherwise
we were hitting this assertion:

https://salsa.debian.org/webkit-team/webkit/-/blob/debian/2.32.1-1_deb10u1/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp#L1322

The recommended packages are necessary in order to play videos
(YouTube etc).

Here is the stack trace. I've never used apport-retrace. 

#0  __pthread_kill_implementation (no_tid=0, signo=6, threadid=139724797575232) at pthread_kill.c:44
#1  __pthread_kill_internal (signo=6, threadid=139724797575232) at pthread_kill.c:80
#2  __GI___pthread_kill (threadid=139724797575232, signo=signo@entry=6) at pthread_kill.c:91
#3  0x00007f143d884476 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#4  0x00007f143d86a7b7 in __GI_abort () at abort.c:79
#5  0x00007f143e09d260 in CRASH_WITH_INFO(...) () at WTF/Headers/wtf/Assertions.h:750
#6  WebCore::makeGStreamerElement () at ../Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:508
#7  0x00007f14401e736e in WebCore::MediaPlayerPrivateGStreamer::createVideoSink () at ../Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:3428
#8  0x00007f14401efef1 in WebCore::MediaPlayerPrivateGStreamer::createGSTPlayBin () at ../Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2741
#9  0x00007f14401f0aeb in WebCore::MediaPlayerPrivateGStreamer::load () at ../Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:313
#10 0x00007f143fb3c91e in WebCore::MediaPlayerPrivateInterface::load () at ../Source/WebCore/platform/graphics/MediaPlayerPrivate.h:44
#11 WebCore::MediaPlayer::loadWithNextMediaEngine () at ../Source/WebCore/platform/graphics/MediaPlayer.cpp:595
#12 0x00007f143fb3cf11 in WebCore::MediaPlayer::load () at ../Source/WebCore/platform/graphics/MediaPlayer.cpp:474
#13 0x00007f143f5ffafb in WebCore::HTMLMediaElement::loadResource () at ../Source/WebCore/html/HTMLMediaElement.cpp:1576
#14 0x00007f143f600ab5 in WebCore::HTMLMediaElement::loadNextSourceChild () at ../Source/WebCore/html/HTMLMediaElement.cpp:1449
#15 0x00007f143f3ff72a in WTF::Function<void ()>::operator()() const () at WTF/Headers/wtf/Function.h:82
#16 WebCore::EventLoopFunctionDispatchTask::execute () at ../Source/WebCore/dom/EventLoop.cpp:159
#17 WebCore::EventLoop::run () at ../Source/WebCore/dom/EventLoop.cpp:123
#18 0x00007f143f4897d1 in WebCore::WindowEventLoop::didReachTimeToRun () at ../Source/WebCore/dom/WindowEventLoop.cpp:120
#19 0x00007f143fa96de9 in WebCore::ThreadTimers::sharedTimerFiredInternal () at ../Source/WebCore/platform/ThreadTimers.cpp:127
#20 WebCore::ThreadTimers::sharedTimerFiredInternal () at ../Source/WebCore/platform/ThreadTimers.cpp:99
#21 0x00007f143d076415 in operator() () at ../Source/WTF/wtf/glib/RunLoopGLib.cpp:177

If I'm reading the right source, I should get a friendly message saying "GStreamer element %s not found. Please install it"... ? But don't see it.

[Xabier Rodríguez Calvar]

(In reply to openaudible from comment #8)
> Here is the stack trace. I've never used apport-retrace. 
> 
> #0  __pthread_kill_implementation (no_tid=0, signo=6,
> threadid=139724797575232) at pthread_kill.c:44
> #1  __pthread_kill_internal (signo=6, threadid=139724797575232) at
> pthread_kill.c:80
> #2  __GI___pthread_kill (threadid=139724797575232, signo=signo@entry=6) at
> pthread_kill.c:91
> #3  0x00007f143d884476 in __GI_raise (sig=sig@entry=6) at
> ../sysdeps/posix/raise.c:26
> #4  0x00007f143d86a7b7 in __GI_abort () at abort.c:79
> #5  0x00007f143e09d260 in CRASH_WITH_INFO(...) () at
> WTF/Headers/wtf/Assertions.h:750
> #6  WebCore::makeGStreamerElement () at
> ../Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:508
> #7  0x00007f14401e736e in
> WebCore::MediaPlayerPrivateGStreamer::createVideoSink () at
> ../Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.
> cpp:3428
> #8  0x00007f14401efef1 in
> WebCore::MediaPlayerPrivateGStreamer::createGSTPlayBin () at
> ../Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.
> cpp:2741
> #9  0x00007f14401f0aeb in WebCore::MediaPlayerPrivateGStreamer::load () at
> ../Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.
> cpp:313
> #10 0x00007f143fb3c91e in WebCore::MediaPlayerPrivateInterface::load () at
> ../Source/WebCore/platform/graphics/MediaPlayerPrivate.h:44
> #11 WebCore::MediaPlayer::loadWithNextMediaEngine () at
> ../Source/WebCore/platform/graphics/MediaPlayer.cpp:595
> #12 0x00007f143fb3cf11 in WebCore::MediaPlayer::load () at
> ../Source/WebCore/platform/graphics/MediaPlayer.cpp:474
> #13 0x00007f143f5ffafb in WebCore::HTMLMediaElement::loadResource () at
> ../Source/WebCore/html/HTMLMediaElement.cpp:1576
> #14 0x00007f143f600ab5 in WebCore::HTMLMediaElement::loadNextSourceChild ()
> at ../Source/WebCore/html/HTMLMediaElement.cpp:1449
> #15 0x00007f143f3ff72a in WTF::Function<void ()>::operator()() const () at
> WTF/Headers/wtf/Function.h:82
> #16 WebCore::EventLoopFunctionDispatchTask::execute () at
> ../Source/WebCore/dom/EventLoop.cpp:159
> #17 WebCore::EventLoop::run () at ../Source/WebCore/dom/EventLoop.cpp:123
> #18 0x00007f143f4897d1 in WebCore::WindowEventLoop::didReachTimeToRun () at
> ../Source/WebCore/dom/WindowEventLoop.cpp:120
> #19 0x00007f143fa96de9 in WebCore::ThreadTimers::sharedTimerFiredInternal ()
> at ../Source/WebCore/platform/ThreadTimers.cpp:127
> #20 WebCore::ThreadTimers::sharedTimerFiredInternal () at
> ../Source/WebCore/platform/ThreadTimers.cpp:99
> #21 0x00007f143d076415 in operator() () at
> ../Source/WTF/wtf/glib/RunLoopGLib.cpp:177
> 
> If I'm reading the right source, I should get a friendly message saying
> "GStreamer element %s not found. Please install it"... ? But don't see it.

This should be stored in the journal or wherever logs go, but it's crashing in a RELEASE_ASSERT because WebKit can't function for that operation without the required GStreamer plugin.

[Adrian Perez]

(In reply to openaudible from comment #8)
> Here is the stack trace. I've never used apport-retrace. 
> 
> #0  __pthread_kill_implementation (no_tid=0, signo=6,
> threadid=139724797575232) at pthread_kill.c:44
> #1  __pthread_kill_internal (signo=6, threadid=139724797575232) at
> pthread_kill.c:80
> #2  __GI___pthread_kill (threadid=139724797575232, signo=signo@entry=6) at
> pthread_kill.c:91
> #3  0x00007f143d884476 in __GI_raise (sig=sig@entry=6) at
> ../sysdeps/posix/raise.c:26
> #4  0x00007f143d86a7b7 in __GI_abort () at abort.c:79
> #5  0x00007f143e09d260 in CRASH_WITH_INFO(...) () at
> WTF/Headers/wtf/Assertions.h:750
> #6  WebCore::makeGStreamerElement () at
> ../Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:508
> #7  0x00007f14401e736e in
> WebCore::MediaPlayerPrivateGStreamer::createVideoSink () at
> ../Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.
> cpp:3428
> #8  0x00007f14401efef1 in
> WebCore::MediaPlayerPrivateGStreamer::createGSTPlayBin () at
> ../Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.
> cpp:2741
> #9  0x00007f14401f0aeb in WebCore::MediaPlayerPrivateGStreamer::load () at
> ../Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.
> cpp:313
> #10 0x00007f143fb3c91e in WebCore::MediaPlayerPrivateInterface::load () at
> ../Source/WebCore/platform/graphics/MediaPlayerPrivate.h:44
> #11 WebCore::MediaPlayer::loadWithNextMediaEngine () at
> ../Source/WebCore/platform/graphics/MediaPlayer.cpp:595
> #12 0x00007f143fb3cf11 in WebCore::MediaPlayer::load () at
> ../Source/WebCore/platform/graphics/MediaPlayer.cpp:474
> #13 0x00007f143f5ffafb in WebCore::HTMLMediaElement::loadResource () at
> ../Source/WebCore/html/HTMLMediaElement.cpp:1576
> #14 0x00007f143f600ab5 in WebCore::HTMLMediaElement::loadNextSourceChild ()
> at ../Source/WebCore/html/HTMLMediaElement.cpp:1449
> #15 0x00007f143f3ff72a in WTF::Function<void ()>::operator()() const () at
> WTF/Headers/wtf/Function.h:82
> #16 WebCore::EventLoopFunctionDispatchTask::execute () at
> ../Source/WebCore/dom/EventLoop.cpp:159
> #17 WebCore::EventLoop::run () at ../Source/WebCore/dom/EventLoop.cpp:123
> #18 0x00007f143f4897d1 in WebCore::WindowEventLoop::didReachTimeToRun () at
> ../Source/WebCore/dom/WindowEventLoop.cpp:120
> #19 0x00007f143fa96de9 in WebCore::ThreadTimers::sharedTimerFiredInternal ()
> at ../Source/WebCore/platform/ThreadTimers.cpp:127
> #20 WebCore::ThreadTimers::sharedTimerFiredInternal () at
> ../Source/WebCore/platform/ThreadTimers.cpp:99
> #21 0x00007f143d076415 in operator() () at
> ../Source/WTF/wtf/glib/RunLoopGLib.cpp:177
> 
> If I'm reading the right source, I should get a friendly message saying
> "GStreamer element %s not found. Please install it"... ? But don't see it.

That comes from the makeGStreamerElement() helper function (source in
Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp) indeed,
and as Michael mentioned it typically goes to the journal. As

From your backtrace and knowing that you are running Ubuntu 21.10, that
means it's WebKitGTK 2.34.0 or 2.34.1, which means frame #7 is doing this:

    if (!m_player->isVideoPlayer()) {
        m_videoSink = makeGStreamerElement("fakevideosink", nullptr);
        return m_videoSink.get();
    }

This element comes from libgstdebugutilsbad.so, which is part of the
gstreamer1.0-plugins-bad Ubuntu package. It looks to me like a packaging
issue, and that distro packages should depend on -bad, not only recommend
it.

Great. I don't know what/where the journal is.. but a message stderr/stdout would be nice in WTFCrashWithInfo.

I do think there is just a dependency issue going on. Make bad plugins required? That missing plug-in "might" be causing other bugs I didn't uncover. 

Thanks for looking into this and your hard work. I don't envy all those bugs in your tracker. 

Cheers

[Michael Catanzaro]

(In reply to Adrian Perez from comment #10)
> This element comes from libgstdebugutilsbad.so, which is part of the
> gstreamer1.0-plugins-bad Ubuntu package. It looks to me like a packaging
> issue, and that distro packages should depend on -bad, not only recommend
> it.

Well that's for Calvaris and the other WebKit/GStreamer developers to decide, certainly not for me. I have no strong opinion either way. I'm fine with changing Fedora's packaging to use Requires instead of Recommends if crashing is the desired behavior when the plugins are missing. But look at this:

    // The combiner relies on webvttenc, fail early if it's not there.
    if (!isGStreamerPluginAvailable("subenc")) {
        WTFLogAlways("WebKit wasn't able to find a WebVTT encoder. Not continuing without platform support for subtitles.");
        return nullptr;
    }

The existence of this check sort of implies the plugin is optional, and expected behavior if missing is to fail gracefully without crashing, right? Whichever strategy is adopted to handle missing GStreamer elements, the code should be consistent. I could be wrong, but I thought the strategy was that we can crash if base elements are missing, since WebKit cannot build without those, but -good or -bad elements use runtime checks since those could be easily forgotten by mistake. If the strategy is to always crash, that's fine too and I'll adjust our packaging.

[Michael Catanzaro]

(In reply to openaudible from comment #8)
> Here is the stack trace.

That helps, thanks. That's a good stacktrace.

[Adrian Perez]

(In reply to Michael Catanzaro from comment #12)
> (In reply to Adrian Perez from comment #10)
> > This element comes from libgstdebugutilsbad.so, which is part of the
> > gstreamer1.0-plugins-bad Ubuntu package. It looks to me like a packaging
> > issue, and that distro packages should depend on -bad, not only recommend
> > it.
> 
> Well that's for Calvaris and the other WebKit/GStreamer developers to
> decide, certainly not for me. I have no strong opinion either way. I'm fine
> with changing Fedora's packaging to use Requires instead of Recommends if
> crashing is the desired behavior when the plugins are missing. But look at
> this:
> 
>     // The combiner relies on webvttenc, fail early if it's not there.
>     if (!isGStreamerPluginAvailable("subenc")) {
>         WTFLogAlways("WebKit wasn't able to find a WebVTT encoder. Not
> continuing without platform support for subtitles.");
>         return nullptr;
>     }
> 
> The existence of this check sort of implies the plugin is optional, and
> expected behavior if missing is to fail gracefully without crashing, right?
> Whichever strategy is adopted to handle missing GStreamer elements, the code
> should be consistent. I could be wrong, but I thought the strategy was that
> we can crash if base elements are missing, since WebKit cannot build without
> those, but -good or -bad elements use runtime checks since those could be
> easily forgotten by mistake. If the strategy is to always crash, that's fine
> too and I'll adjust our packaging.

I think that I did not manage to make my point clear earlier..

 * WebVTT (subtitles) support is optional, and as you correctly point out
   there is a runtime check on the “subenc“ element.

 * OTOH gst-plugins-bad is *NOT* optional because media elements which do
   not have video (I guess they are audio-only) will use “fakevideosink“
   unconditionally. Line 3428 in MediaPlayerPrivateGStreamer.cpp (frame #7)
   from the Very Good Indeed™ stacktrace is using makeGStreamerElement(),
   which will assert that the element was created.

[Michael Catanzaro]

(In reply to Adrian Perez from comment #14)
>  * WebVTT (subtitles) support is optional, and as you correctly point out
>    there is a runtime check on the “subenc“ element.
> 
>  * OTOH gst-plugins-bad is *NOT* optional because media elements which do
>    not have video (I guess they are audio-only) will use “fakevideosink“
>    unconditionally. Line 3428 in MediaPlayerPrivateGStreamer.cpp (frame #7)
>    from the Very Good Indeed™ stacktrace is using makeGStreamerElement(),
>    which will assert that the element was created.

What I don't understand is whether this crash is the desired behavior or not. Clearly both Berto and myself assumed otherwise, or we would have had Requires in our packaging. If this is the desired end result, then we can say I was wrong about it being optional, close the WebKit bug here, and go add our downstream Requires. But it seems inconsistent with how we handle other missing GStreamer elements, yes?

Anyway... yeah, it's a good stacktrace. :P The place where the crash happens is not even close to where OP thought it was happening. That's why we need stacktraces.

[Philippe Normand]

Created attachment 447881 [details]
Patch

[Philippe Normand]

Created attachment 447882 [details]
Patch

[Michael Catanzaro]

Can you confirm that we should add Requires for gst-plugins-bad to downstream packaging, in addition to both -good and -base?

[Philippe Normand]

(In reply to Michael Catanzaro from comment #18)
> Can you confirm that we should add Requires for gst-plugins-bad to
> downstream packaging, in addition to both -good and -base?

Yes, due mostly to fakevideosink, I'm afraid (and some day for GstWebRTC, unless it moves elsewhere).

[EWS]

Committed r287410 (245545@main): <https://commits.webkit.org/245545@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 447882 [details].

[Radar WebKit Bug Importer]

<rdar://problem/86863068>

[Michael Catanzaro]

Due to some downstream complaints about increased install media size, I'll ask one more time just to be perfectly sure we have this correct:

Option 1: if GStreamer elements are missing, WebKit will print a warning, fail to play media, and not crash. This corresponds to a Recommends dependency downstream.

Option 2: if GStreamer elements are missing, WebKit will gracefully crash. This corresponds to a Requires dependency downstream.

I've implemented Option 2 downstream per feedback in this bug, but that doesn't seem entirely consistent with WebKit's actual behavior. Do you still agree that Option 2 (Requires) is what we are aiming for?

In retrospect, Option 1 seemed nicer to me, but of course that's up to GStreamer port maintainers to decide, not up to me.

[Adam Williamson]

Just to chime in with a few more notes from the downstream issue here - https://bugzilla.redhat.com/show_bug.cgi?id=2031213 : the actual use case here is the Fedora (and RHEL) installer. In this environment yelp (GNOME's help browser) is installed so that we can display the installer's own help pages to the user. yelp uses webkit for rendering.

There is no general-purpose web browser in the environment. There are no video or audio files for webkit to play. There are only help pages to be displayed in yelp.

So it's a bit sad that we now have a dozen entirely useless libraries related to audio and video playback pulled into the installer environment, which needs to be as streamlined as possible.

We do have a mechanism by which we can override the dependencies and rip the packages back out when build the installer, but this is somewhat fragile if the package names change. We'll have to do that if webkit isn't going to fail more gracefully in this kind of case, I guess, but it'd be nice not to have to.

[Xabier Rodríguez Calvar]

(In reply to Adam Williamson from comment #23)
> Just to chime in with a few more notes from the downstream issue here -
> https://bugzilla.redhat.com/show_bug.cgi?id=2031213 : the actual use case
> here is the Fedora (and RHEL) installer. In this environment yelp (GNOME's
> help browser) is installed so that we can display the installer's own help
> pages to the user. yelp uses webkit for rendering.
> 
> There is no general-purpose web browser in the environment. There are no
> video or audio files for webkit to play. There are only help pages to be
> displayed in yelp.
> 
> So it's a bit sad that we now have a dozen entirely useless libraries
> related to audio and video playback pulled into the installer environment,
> which needs to be as streamlined as possible.
> 
> We do have a mechanism by which we can override the dependencies and rip the
> packages back out when build the installer, but this is somewhat fragile if
> the package names change. We'll have to do that if webkit isn't going to
> fail more gracefully in this kind of case, I guess, but it'd be nice not to
> have to.

If it's in the installer, I guess you can build WebKit without the VIDEO option active maybe even as a static library.

[Adam Williamson]

Unfortunately that's not an option - we don't build the installer components separately, the image is built out of the regular distro packages.

If there's nothing else that can be done we can strip the packages out in the 'cleanup' phase of the installer build, it's just slightly sucky :/

[Philippe Normand]

incoming patch to gracefully handle missing fakevideosink

[Philippe Normand]

Created attachment 451364 [details]
Patch

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #22)
> Option 1: if GStreamer elements are missing, WebKit will print a warning,
> fail to play media, and not crash. This corresponds to a Recommends
> dependency downstream.

I'll let Calvaris or another multimedia reviewer approve this. But it looks good to me.

For avoidance of doubt, this implements Option 1.

[Adam Williamson]

That's awesome, thanks very much!

[EWS]

Committed r289480 (247022@main): <https://commits.webkit.org/247022@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 451364 [details].

[Michael Catanzaro]

*** Bug 237383 has been marked as a duplicate of this bug. ***