Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 6b895b68961a0128baba0961a788c44db71d535c
https://github.com/WebKit/WebKit/commit/6b895b68961a0128baba0961a788c44db71d535c
Author: Alicia Boya Garcia <ab...@igalia.com>
Date: 2025-03-03 (Mon, 03 Mar 2025)
Changed paths:
M Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
M
Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp
Log Message:
-----------
[MSE][GStreamer] Fix crash in seek when video is torn down
https://bugs.webkit.org/show_bug.cgi?id=260455
Reviewed by Philippe Normand.
This patch fixes a crash in webKitMediaSrcFlushStream() after the
pipeline has been torn down as consequence of the render proxy of the
player becoming inactive.
# Background
The root cause of the crash is this performance fix:
https://github.com/WebKit/WebKit/pull/30549 [GStreamer] Paused pipelines
accumulate when scrolling on Reddit pages with many videos
Since that patch, the player is torn down every time its associated
render proxy becomes inactive.
That is particularly helpful in pages with lots of videos the user
scrolls past, but it's a flawed solution, as tearDown() is
irrecoverable, but the render proxy can become inactive only
temporarily.
# How the crash happens
This can be observed in the nbcnews.com pages linked in the bug report,
where as you scroll, the video is transplanted from the big widget at
the top to a smaller floating miniplayer.
With the pipeline torn down, WebKitMediaSrc was set to NULL state,
cleared its streams list, and therefore failed to find the stream for
which the flush (consequence of a seek) was requested. WebKit is not
supposed to ever request WebKitMediaSrc to operate on a stream it
doesn't have, so this causes a crash.
# Remaining problems
Note however that, even if this patch prevents the crash, the video
still becomes blank afterwards as the pipeline is still (wrongly) torn
down.
Reverting the tearing down logic of the player would unfortunately
regress performance in affected websites such as Reddit, so I'm limiting
the scope of this patch to just preventing the crash.
Going forward, a better mechanism for keeping only a reasonable number
of GStreamer threads (and by extension pipelines) in a page is needed.
Such a mechanism should avoid false positives like the one observed in
nbcnews.com and be able to handle resuming playback after suspension
(the current one is irrecoverable).
# Other bugs in the same Bugzilla entry
Some of the crashes in the bug report are unrelated to this fix. In
general, flushing and seeking has many moving parts, so most multimedia
bugs involve them in some way, and it can be non-obvious to tell two
bugs apart involving flushing.
Here is a summary of the different bugs I'm aware of from reading and
testing the pages in that Bugzilla entry:
* (Fixed, this patch, appears in the report)
Crash in webKitMediaSrcFlushStream() after the render proxy is inactive.
* (Not fixed, already mentioned, does not appear in the report)
Need better logic for player suspension.
* (Already fixed, appears in the report)
The crashes related to scrolling on the Apple MacBook Air pages were
fixed by aa09557dfddc2a32fe6c10f9c5e756b9cea48f61.
In this case the bug related to a different suspension logic that is
not meant to tear down the pipeline, but due to a bug, it accidentally
set the pipeline to READY, which causes the WebKitMediaSrc element to
be torn down -- hence the very similar traceback.
* (Not fixed, does not appear in the report)
Sometimes video is blank in the Mac Book Air page.
This is caused by an upstream bug in basetransform, of which vp9parse
is derived from:
https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/4193
baseparse: loses (doesn't propagate) sticky events after a flush
* (Not fixed, appears in one of the backtraces: gdb.txt 2024-08-09 by Kdwk)
gst_alpha_combine_unlock_stop: assertion failed: (self->flushing)
This crash is reproduced sometimes in the Mac Book Air page.
It's caused by an upstream bug in alphacombine:
https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/4174
alphacombine expects FLUSH_START count to equal FLUSH_STOP count
* (Not reproduced at my end) gst_caps_remove_structure: assertion
'IS_WRITABLE (caps)' failed.
https://bugs.webkit.org/show_bug.cgi?id=260455#c17
Reported by Philippe, happens in vp9parse. Could be related to another
bug or be its own.
At this point I think it makes sense to close this Bugzilla entry. New
entries can be created for each of the remaining bugs that at this point
we know are different if needed, and for any other ones found thereafter.
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::setRate):
*
Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp:
(WebCore::MediaPlayerPrivateGStreamerMSE::seekToTarget):
Canonical link: https://commits.webkit.org/291490@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
