viddec: don't configure and incorrect buffer pool when receiving a gap event

Scenario is this:
1. libav receives enough data to want a buffer with get_buffer2()
   which wants a buffer pool with a certain format, say Y42B but does
   not negotiate and therefore GstVideoDecoder does not have any output
   state configured
2. A gap event is received which GstVideoDecoder wants to forward.  It
   needs caps to forward the gap event so attempts to negotiate with some
   default information which chooses e.g. I420 and overwrites the
   previously configured bufferpool decided on by get_buffer2()
3. There is a mismatch between what ensure_internal_pool() check for
   consistency and what decide_allocation() sets when overriding the
   internal pool with the downstream pool.
4. FFMpeg then requests a Y42B buffer from an I420 pool and predictably
   crashes writing past the contents of the buffer

This is fixed by keeping track of the internal pool states correctly.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-libav/-/merge_requests/116>
This commit is contained in:
Matthew Waters 2021-01-28 06:11:14 +00:00 committed by GStreamer Merge Bot
parent 96dd596e54
commit f166c9d234

View file

@ -90,6 +90,8 @@ static gboolean gst_ffmpegviddec_negotiate (GstFFMpegVidDec * ffmpegdec,
/* some sort of bufferpool handling, but different */
static int gst_ffmpegviddec_get_buffer2 (AVCodecContext * context,
AVFrame * picture, int flags);
static gboolean gst_ffmpegviddec_can_direct_render (GstFFMpegVidDec *
ffmpegdec);
static GstFlowReturn gst_ffmpegviddec_finish (GstVideoDecoder * decoder);
static GstFlowReturn gst_ffmpegviddec_drain (GstVideoDecoder * decoder);
@ -735,7 +737,10 @@ gst_ffmpegviddec_ensure_internal_pool (GstFFMpegVidDec * ffmpegdec,
GstStructure *config;
guint i;
format = gst_ffmpeg_pixfmt_to_videoformat (picture->format);
if (ffmpegdec->internal_pool != NULL &&
GST_VIDEO_INFO_FORMAT (&ffmpegdec->pool_info) == format &&
ffmpegdec->pool_width == picture->width &&
ffmpegdec->pool_height == picture->height &&
ffmpegdec->pool_format == picture->format)
@ -744,6 +749,11 @@ gst_ffmpegviddec_ensure_internal_pool (GstFFMpegVidDec * ffmpegdec,
GST_DEBUG_OBJECT (ffmpegdec, "Updating internal pool (%i, %i)",
picture->width, picture->height);
/* if we are negotiating from get_buffer, then renegotiate later in order
* to potentially use a downstream pool */
if (gst_ffmpegviddec_can_direct_render (ffmpegdec))
gst_pad_mark_reconfigure (GST_VIDEO_DECODER_SRC_PAD (ffmpegdec));
format = gst_ffmpeg_pixfmt_to_videoformat (picture->format);
if (interlace_mode == GST_VIDEO_INTERLACE_MODE_ALTERNATE) {
@ -2195,6 +2205,8 @@ gst_ffmpegviddec_decide_allocation (GstVideoDecoder * decoder, GstQuery * query)
if (ffmpegdec->internal_pool)
gst_object_unref (ffmpegdec->internal_pool);
ffmpegdec->internal_pool = gst_object_ref (pool);
ffmpegdec->pool_width = GST_VIDEO_INFO_WIDTH (&state->info);
ffmpegdec->pool_height = GST_VIDEO_INFO_HEIGHT (&state->info);
ffmpegdec->pool_info = state->info;
gst_structure_free (config);
goto done;
@ -2204,6 +2216,8 @@ gst_ffmpegviddec_decide_allocation (GstVideoDecoder * decoder, GstQuery * query)
}
if (have_videometa && ffmpegdec->internal_pool
&& gst_ffmpeg_pixfmt_to_videoformat (ffmpegdec->pool_format) ==
GST_VIDEO_INFO_FORMAT (&state->info)
&& ffmpegdec->pool_width == state->info.width
&& ffmpegdec->pool_height == state->info.height) {
update_pool = TRUE;