TextCombinerGStreamer uses the CAPS event to determine whether adding a webvttenc between the text track pad and the funnel element used to be able to display several subtitles at the same time. The way this was done previously had a race though: all text track pads were preemptively linked directly to the funnel, only adding the webvttenc element later in the middle when receiving the CAPS event. When two or more text tracks were present, it wasn't infrequent that one track had its CAPS event processed (causing the webvttenc element to be added) and propagated (fixating the funnel caps) before another track attempted caps negotiation. Because the pads were connected to the funnel preemptively, and because without the webvttenc element the caps of the text pad don't match the funnel's, this causes a caps mismatch error, stopping playback completely. The CAPS event is therefore never sent. To avoid this race, we must avoid linking elements until we get the CAPS events, when we actually know where we should link them to, therefore avoiding early caps negotiation errors.
Created attachment 391277 [details] Patch
Comment on attachment 391277 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391277&action=review > Source/WebCore/platform/graphics/gstreamer/TextCombinerGStreamer.cpp:69 > + GstPad* funnelPad; This seems to be never released. Please gst_object_unref during dispose. Actually I think it would be a wonderful idea to turn the Finalize into a Dispose (conceptually more correct, I think) and add the unref there. > Source/WebCore/platform/graphics/gstreamer/TextCombinerGStreamer.cpp:146 > + /* Caps are plain text, we want a WebVTT encoder between the ghostpad and the funnel. */ // > Source/WebCore/platform/graphics/gstreamer/TextCombinerGStreamer.cpp:148 > + /* Setup a WebVTT encoder. */ // > Source/WebCore/platform/graphics/gstreamer/TextCombinerGStreamer.cpp:158 > + /* Switch the ghostpad to target the WebVTT encoder. */ // > Source/WebCore/platform/graphics/gstreamer/TextCombinerGStreamer.cpp:165 > + /* Connect the WebVTT encoder to the funnel. */ // > Source/WebCore/platform/graphics/gstreamer/TextCombinerGStreamer.cpp:173 > + /* Caps are not plain text, we assume it's WebVTT. */ // > Source/WebCore/platform/graphics/gstreamer/TextCombinerGStreamer.cpp:175 > + /* Remove the WebVTT encoder if present. */ // > Source/WebCore/platform/graphics/gstreamer/TextCombinerGStreamer.cpp:184 > + /* Link the pad to the funnel. */ // > Source/WebCore/platform/graphics/gstreamer/TextCombinerGStreamer.cpp:188 > + } /* else: pipeline is already correct. */ //
Created attachment 391309 [details] Patch for landing
The commit-queue encountered the following flaky tests while processing attachment 391309 [details]: editing/spelling/spellcheck-input-search-crash.html bug 207995 (authors: arv@chromium.org, g.czajkowski@samsung.com, mark.lam@apple.com, and morrita@google.com) The commit-queue is continuing to process your patch.
The commit-queue encountered the following flaky tests while processing attachment 391309 [details]: fetch/fetch-worker-crash.html bug 187257 (author: youennf@gmail.com) The commit-queue is continuing to process your patch.
Comment on attachment 391309 [details] Patch for landing Clearing flags on attachment: 391309 Committed r257090: <https://trac.webkit.org/changeset/257090>
All reviewed patches have been landed. Closing bug.
(In reply to WebKit Commit Bot from comment #6) > Comment on attachment 391309 [details] > Patch for landing > > Clearing flags on attachment: 391309 > > Committed r257090: <https://trac.webkit.org/changeset/257090> This has broken the build on the GTK minimum dependencies bots: Debian 10: https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20Debian%20Stable%20%28Build%29/builds/30091 Ubuntu 18.04: https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20Ubuntu%20LTS%20%28Build%29/builds/29626 gst_clear_object and gst_clear_tag_list require GStreamer 1.16 Meanwhile both Debian 10 and Ubuntu 18.04 ship GStreamer 1.14
(In reply to Carlos Alberto Lopez Perez from comment #8) > (In reply to WebKit Commit Bot from comment #6) > > Comment on attachment 391309 [details] > > Patch for landing > > > > Clearing flags on attachment: 391309 > > > > Committed r257090: <https://trac.webkit.org/changeset/257090> > > This has broken the build on the GTK minimum dependencies bots: > > Debian 10: > https://build.webkit.org/builders/GTK%20Linux%2064- > bit%20Release%20Debian%20Stable%20%28Build%29/builds/30091 > Ubuntu 18.04: > https://build.webkit.org/builders/GTK%20Linux%2064- > bit%20Release%20Ubuntu%20LTS%20%28Build%29/builds/29626 > > gst_clear_object and gst_clear_tag_list require GStreamer 1.16 > > Meanwhile both Debian 10 and Ubuntu 18.04 ship GStreamer 1.14 See also https://bugs.webkit.org/show_bug.cgi?id=208041