videodecoder: fix segfault copying buffer metas

The current implementation copies metas without checking if the buffer
is writable.

The operation that needs to be done, replacing the input buffer and
copying the metas, is only part of that process. We create a new function
that does both.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5054>
This commit is contained in:
Andoni Morales Alastruey 2023-06-21 13:45:13 +02:00 committed by GStreamer Marge Bot
parent a1d2cea913
commit ee43b6421e
2 changed files with 44 additions and 11 deletions

View file

@ -542,6 +542,9 @@ static gboolean gst_video_decoder_transform_meta_default (GstVideoDecoder *
static gboolean gst_video_decoder_handle_missing_data_default (GstVideoDecoder *
decoder, GstClockTime timestamp, GstClockTime duration);
static void gst_video_decoder_replace_input_buffer (GstVideoDecoder * decoder,
GstVideoCodecFrame * frame, GstBuffer ** dest_buffer);
static void gst_video_decoder_copy_metas (GstVideoDecoder * decoder,
GstVideoCodecFrame * frame, GstBuffer * src_buffer,
GstBuffer * dest_buffer);
@ -2470,11 +2473,7 @@ gst_video_decoder_chain_forward (GstVideoDecoder * decoder,
GST_VIDEO_CODEC_FRAME_SET_SYNC_POINT (frame);
}
if (frame->input_buffer) {
gst_video_decoder_copy_metas (decoder, frame, frame->input_buffer, buf);
gst_buffer_unref (frame->input_buffer);
}
frame->input_buffer = buf;
gst_video_decoder_replace_input_buffer (decoder, frame, &buf);
if (decoder->input_segment.rate < 0.0) {
priv->parse_gather = g_list_prepend (priv->parse_gather, frame);
@ -3401,6 +3400,20 @@ gst_video_decoder_copy_metas (GstVideoDecoder * decoder,
}
}
static void
gst_video_decoder_replace_input_buffer (GstVideoDecoder * decoder,
GstVideoCodecFrame * frame, GstBuffer ** dest_buffer)
{
if (frame->input_buffer) {
*dest_buffer = gst_buffer_make_writable (*dest_buffer);
gst_video_decoder_copy_metas (decoder, frame, frame->input_buffer,
*dest_buffer);
gst_buffer_unref (frame->input_buffer);
}
frame->input_buffer = *dest_buffer;
}
/**
* gst_video_decoder_finish_frame:
* @decoder: a #GstVideoDecoder
@ -3841,12 +3854,8 @@ gst_video_decoder_have_frame (GstVideoDecoder * decoder)
buffer = gst_buffer_new_and_alloc (0);
}
if (priv->current_frame->input_buffer) {
gst_video_decoder_copy_metas (decoder, priv->current_frame,
priv->current_frame->input_buffer, buffer);
gst_buffer_unref (priv->current_frame->input_buffer);
}
priv->current_frame->input_buffer = buffer;
gst_video_decoder_replace_input_buffer (decoder, priv->current_frame,
&buffer);
gst_video_decoder_get_buffer_info_at_offset (decoder,
priv->frame_offset, &pts, &dts, &duration, &flags);

View file

@ -1315,12 +1315,18 @@ GST_START_TEST (videodecoder_playback_event_order)
GST_END_TEST;
/*
* MODE_META_COPY: takes an extra ref to the input buffer to check metas
* are copied to a writable buffer.
* see: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4912
*/
typedef enum
{
MODE_NONE = 0,
MODE_SUBFRAMES = 1,
MODE_PACKETIZED = 1 << 1,
MODE_META_ROI = 1 << 2,
MODE_META_COPY = 1 << 3,
} SubframeMode;
static void
@ -1379,10 +1385,19 @@ videodecoder_playback_subframe_mode (SubframeMode mode)
gst_buffer_add_video_region_of_interest_meta (buffer, "face", 0, 0, 10,
10);
/* Take an extra ref to check that we ensure buffer is writable when copying metas
* https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4912
*/
if (mode & MODE_META_COPY) {
gst_buffer_ref (buffer);
}
fail_unless (gst_pad_push (mysrcpad, buffer) == GST_FLOW_OK);
fail_unless (gst_pad_push_event (mysrcpad,
gst_event_new_custom (GST_EVENT_CUSTOM_DOWNSTREAM,
gst_structure_new_empty ("custom1"))));
if (mode & MODE_META_COPY) {
gst_buffer_unref (buffer);
}
}
/* Send EOS */
fail_unless (gst_pad_push_event (mysrcpad, gst_event_new_eos ()));
@ -1540,6 +1555,14 @@ GST_START_TEST (videodecoder_playback_packetized_subframes_metadata)
GST_END_TEST;
GST_START_TEST (videodecoder_playback_packetized_subframes_metadata_copy)
{
videodecoder_playback_subframe_mode (MODE_SUBFRAMES |
MODE_PACKETIZED | MODE_META_ROI | MODE_META_COPY);
}
GST_END_TEST;
GST_START_TEST (videodecoder_playback_invalid_ts_packetized)
{
videodecoder_playback_invalid_ts_subframe_mode (MODE_PACKETIZED);
@ -1589,6 +1612,7 @@ gst_videodecoder_suite (void)
tcase_add_test (tc, videodecoder_playback_parsed_subframes);
tcase_add_test (tc, videodecoder_playback_packetized_subframes);
tcase_add_test (tc, videodecoder_playback_packetized_subframes_metadata);
tcase_add_test (tc, videodecoder_playback_packetized_subframes_metadata_copy);
tcase_add_test (tc, videodecoder_playback_invalid_ts_packetized);
tcase_add_test (tc, videodecoder_playback_invalid_ts_packetized_subframes);