mirrors/gstreamer
mirrors/gstreamer
1
1
Fork 0
mirror of https://gitlab.freedesktop.org/gstreamer/gstreamer.git synced 2025-01-24 08:08:22 +00:00
Code Issues 1 Projects Releases Wiki Activity

videodecoder: Fix racy critical when pool negotiation occurs during flush

Browse source 
I found a rather reproducible race in a WebKit LayoutTest when a player
was intantiated and a VP8/9 video was loaded, then torn down after
getting the video dimensions from the caps.

The crash occurs during the handling of the first frame by gstvpxdec.
The following actions happen sequentially leading to a crash.

(MT=Main Thread, ST=Streaming Thread)

MT: Sets pipeline state to NULL, which deactivates vpxdec's srcpad,
    which in turn sets its FLUSHING flag.

ST: gst_vpx_dec_handle_frame() -- which is still running -- calls
    gst_video_decoder_allocate_output_frame(); this in turn calls
    gst_video_decoder_negotiate_unlocked() which fails because the
    srcpad is FLUSHING. As a direct consequence of the negotiation
    failure, a pool is NOT set.

    gst_video_decoder_negotiate_unlocked() still assumes there is a
    pool, crashing in a critical in gst_buffer_pool_acquire_buffer()
    a couple statements later.

This patch fixes the bug by returning != GST_FLOW_OK when the
negotiation fails. If the srcpad is FLUSHING, GST_FLOW_FLUSHING is
returned, otherwise GST_FLOW_ERROR is used.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/merge_requests/1031>
This commit is contained in:
Alicia Boya García 2021-02-13 00:27:04 +01:00 committed by GStreamer Merge Bot
parent 297a5f09b1
commit 29aeba639a
1 changed files with 16 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
gst-libs/gst/video/gstvideodecoder.c
View file

@ -4396,8 +4396,19 @@ gst_video_decoder_allocate_output_frame_with_params (GstVideoDecoder *
needs_reconfigure = gst_pad_check_reconfigure (decoder->srcpad);
if (G_UNLIKELY (decoder->priv->output_state_changed || needs_reconfigure)) {
if (!gst_video_decoder_negotiate_unlocked (decoder)) {
GST_DEBUG_OBJECT (decoder, "Failed to negotiate, fallback allocation");
gst_pad_mark_reconfigure (decoder->srcpad);
if (GST_PAD_IS_FLUSHING (decoder->srcpad)) {
GST_DEBUG_OBJECT (decoder,
"Failed to negotiate a pool: pad is flushing");
goto flushing;
} else if (!decoder->priv->pool || decoder->priv->output_state_changed) {
GST_DEBUG_OBJECT (decoder,
"Failed to negotiate a pool and no previous pool to reuse");
goto error;
} else {
GST_DEBUG_OBJECT (decoder,
"Failed to negotiate a pool, falling back to the previous pool");
}
}
}
@ -4410,6 +4421,10 @@ gst_video_decoder_allocate_output_frame_with_params (GstVideoDecoder *
return flow_ret;
flushing:
GST_VIDEO_DECODER_STREAM_UNLOCK (decoder);
return GST_FLOW_FLUSHING;
error:
GST_VIDEO_DECODER_STREAM_UNLOCK (decoder);
return GST_FLOW_ERROR;