avviddec: Fix pool reallocation logic

Some check where incorect and also unsafe. The only reliable information
in get_buffer2 is the picture width/height really. The side effect is
that the width/height of the internal pool endup padded, so when we
switch we also need to switch to the a new width/height, hence we save
the pool info.

https://bugzilla.gnome.org/show_bug.cgi?id=753869

ext/libav/gstavviddec.c

@@ -669,38 +669,54 @@ gst_ffmpegvideodec_prepare_dr_pool (GstFFMpegVidDec * ffmpegdec,
 
 static void
 gst_ffmpegviddec_ensure_internal_pool (GstFFMpegVidDec * ffmpegdec,
-    AVFrame * picture, GstVideoInfo * info)
+    AVFrame * picture)
 {
-  AVCodecContext *context = ffmpegdec->context;
   GstAllocationParams params = DEFAULT_ALLOC_PARAM;
+  GstVideoInfo info;
   GstVideoFormat format;
   GstCaps *caps;
   GstStructure *config;
+  gint i;
+
+  if (ffmpegdec->internal_pool != NULL &&
+      ffmpegdec->pool_width == picture->width &&
+      ffmpegdec->pool_height == picture->height &&
+      ffmpegdec->pool_format == picture->format)
+    return;
+
+  GST_DEBUG_OBJECT (ffmpegdec, "Updating internal pool (%i, %i)",
+      picture->width, picture->height);
 
   format = gst_ffmpeg_pixfmt_to_videoformat (picture->format);
-  gst_video_info_set_format (info, format, context->width, context->height);
+  gst_video_info_set_format (&info, format, picture->width, picture->height);
 
-  if (ffmpegdec->internal_pool != NULL ||
+  for (i = 0; i < G_N_ELEMENTS (ffmpegdec->stride); i++)
-      (ffmpegdec->pic_width == context->width &&
+    ffmpegdec->stride[i] = -1;
-          ffmpegdec->pic_height == context->height &&
+
-          ffmpegdec->pic_pix_fmt == picture->format))
+  if (ffmpegdec->internal_pool)
-    return;
+    gst_object_unref (ffmpegdec->internal_pool);
 
   ffmpegdec->internal_pool = gst_video_buffer_pool_new ();
   config = gst_buffer_pool_get_config (ffmpegdec->internal_pool);
 
-  caps = gst_video_info_to_caps (info);
+  caps = gst_video_info_to_caps (&info);
-  gst_buffer_pool_config_set_params (config, caps, info->size, 2, 0);
+  gst_buffer_pool_config_set_params (config, caps, info.size, 2, 0);
   gst_buffer_pool_config_set_allocator (config, NULL, &params);
   gst_buffer_pool_config_add_option (config, GST_BUFFER_POOL_OPTION_VIDEO_META);
 
   gst_ffmpegvideodec_prepare_dr_pool (ffmpegdec,
-      ffmpegdec->internal_pool, info, config);
+      ffmpegdec->internal_pool, &info, config);
   /* generic video pool never fails */
   gst_buffer_pool_set_config (ffmpegdec->internal_pool, config);
   gst_caps_unref (caps);
 
   gst_buffer_pool_set_active (ffmpegdec->internal_pool, TRUE);
+
+  /* Remember pool size so we can detect changes */
+  ffmpegdec->pool_width = picture->width;
+  ffmpegdec->pool_height = picture->height;
+  ffmpegdec->pool_format = picture->format;
+  ffmpegdec->pool_info = info;
 }
 
 static gboolean
@@ -725,7 +741,6 @@ gst_ffmpegviddec_get_buffer2 (AVCodecContext * context, AVFrame * picture,
   GstFFMpegVidDecVideoFrame *dframe;
   GstFFMpegVidDec *ffmpegdec;
   gint c;
-  GstVideoInfo info;
   GstFlowReturn ret;
 
   ffmpegdec = (GstFFMpegVidDec *) context->opaque;
@@ -766,7 +781,7 @@ gst_ffmpegviddec_get_buffer2 (AVCodecContext * context, AVFrame * picture,
   if (!gst_ffmpegviddec_can_direct_render (ffmpegdec))
     goto no_dr;
 
-  gst_ffmpegviddec_ensure_internal_pool (ffmpegdec, picture, &info);
+  gst_ffmpegviddec_ensure_internal_pool (ffmpegdec, picture);
 
   ret = gst_buffer_pool_acquire_buffer (ffmpegdec->internal_pool,
       &frame->output_buffer, NULL);
@@ -781,13 +796,13 @@ gst_ffmpegviddec_get_buffer2 (AVCodecContext * context, AVFrame * picture,
   gst_buffer_replace (&frame->output_buffer, NULL);
 
   /* Fill avpicture */
-  if (!gst_video_frame_map (&dframe->vframe, &info, dframe->buffer,
+  if (!gst_video_frame_map (&dframe->vframe, &ffmpegdec->pool_info,
-          GST_MAP_READWRITE))
+          dframe->buffer, GST_MAP_READWRITE))
     goto invalid_frame;
   dframe->mapped = TRUE;
 
   for (c = 0; c < AV_NUM_DATA_POINTERS; c++) {
-    if (c < GST_VIDEO_INFO_N_PLANES (&info)) {
+    if (c < GST_VIDEO_INFO_N_PLANES (&ffmpegdec->pool_info)) {
       picture->data[c] = GST_VIDEO_FRAME_PLANE_DATA (&dframe->vframe, c);
       picture->linesize[c] = GST_VIDEO_FRAME_PLANE_STRIDE (&dframe->vframe, c);
 
@@ -1201,12 +1216,12 @@ get_output_buffer (GstFFMpegVidDec * ffmpegdec, GstVideoCodecFrame * frame)
     if (c < GST_VIDEO_INFO_N_PLANES (info)) {
       pic.data[c] = GST_VIDEO_FRAME_PLANE_DATA (&vframe, c);
       pic.linesize[c] = GST_VIDEO_FRAME_PLANE_STRIDE (&vframe, c);
+      GST_LOG_OBJECT (ffmpegdec, "[%i] linesize %d, data %p", c,
+          pic.linesize[c], pic.data[c]);
     } else {
       pic.data[c] = NULL;
       pic.linesize[c] = 0;
     }
-    GST_LOG_OBJECT (ffmpegdec, "linesize %d, data %p", pic.linesize[c],
-        pic.data[c]);
   }
 
   outpic = (AVPicture *) ffmpegdec->picture;
@@ -1712,6 +1727,10 @@ gst_ffmpegviddec_stop (GstVideoDecoder * decoder)
   ffmpegdec->ctx_time_n = 0;
   ffmpegdec->ctx_time_d = 0;
 
+  ffmpegdec->pool_width = 0;
+  ffmpegdec->pool_height = 0;
+  ffmpegdec->pool_format = 0;
+
   return TRUE;
 }
 
@@ -1826,6 +1845,7 @@ 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_info = state->info;
           goto done;
         }
       }

ext/libav/gstavviddec.h

@@ -70,6 +70,10 @@ struct _GstFFMpegVidDec
 
   /* Internally used for direct rendering */
   GstBufferPool *internal_pool;
+  gint pool_width;
+  gint pool_height;
+  enum PixelFormat pool_format;
+  GstVideoInfo pool_info;
 };
 
 typedef struct _GstFFMpegVidDecClass GstFFMpegVidDecClass;