theoradec: make sure the selected pool accepts the new config

If gst_buffer_pool_set_config() fails then the pool will use its old
config. This may include different width or height when
pic_width/pic_height != frame_width/frame_height.
As a result, the assertions in theora_handle_image() will fail.

So check the result of gst_buffer_pool_set_config() and only use the pool
if it succeeds. Otherwise let the parrent decide_allocation() create a new
pool.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4600>

4 files changed, 185 insertions, 33 deletions

diff --git a/subprojects/gst-plugins-base/ext/theora/gsttheoradec.c b/subprojects/gst-plugins-base/ext/theora/gsttheoradec.c
index 827a7c85a5..351e6f92da 100644
--- a/subprojects/gst-plugins-base/ext/theora/gsttheoradec.c
+++ b/subprojects/gst-plugins-base/ext/theora/gsttheoradec.c
@@ -926,46 +926,59 @@ theora_dec_decide_allocation (GstVideoDecoder * decoder, GstQuery * query)
   guint size, min, max;
   GstStructure *config;
 
-  if (!GST_VIDEO_DECODER_CLASS (parent_class)->decide_allocation (decoder,
-          query))
-    return FALSE;
-
-  gst_query_parse_nth_allocation_pool (query, 0, &pool, &size, &min, &max);
+  if ((dec->info.pic_width != dec->info.frame_width ||
+          dec->info.pic_height != dec->info.frame_height) &&
+      (gst_query_get_n_allocation_pools (query) > 0)) {
+    gst_query_parse_nth_allocation_pool (query, 0, &pool, &size, &min, &max);
+
+    if (pool) {
+      dec->can_crop = FALSE;
+      config = gst_buffer_pool_get_config (pool);
+      if (gst_query_find_allocation_meta (query, GST_VIDEO_META_API_TYPE, NULL)) {
+        gst_buffer_pool_config_add_option (config,
+            GST_BUFFER_POOL_OPTION_VIDEO_META);
+        dec->can_crop =
+            gst_query_find_allocation_meta (query, GST_VIDEO_CROP_META_API_TYPE,
+            NULL);
+      }
+
+      if (dec->can_crop) {
+        GstVideoInfo *info = &dec->uncropped_info;
+        GstCaps *caps;
+
+        GST_LOG_OBJECT (decoder,
+            "Using GstVideoCropMeta, uncropped wxh = %dx%d", info->width,
+            info->height);
+
+        gst_video_info_set_format (info, info->finfo->format,
+            dec->info.frame_width, dec->info.frame_height);
 
-  dec->can_crop = FALSE;
-  config = gst_buffer_pool_get_config (pool);
-  if (gst_query_find_allocation_meta (query, GST_VIDEO_META_API_TYPE, NULL)) {
-    gst_buffer_pool_config_add_option (config,
-        GST_BUFFER_POOL_OPTION_VIDEO_META);
-    dec->can_crop =
-        gst_query_find_allocation_meta (query, GST_VIDEO_CROP_META_API_TYPE,
-        NULL);
-  }
+        /* Calculate uncropped size */
+        size = MAX (size, info->size);
+        caps = gst_video_info_to_caps (info);
+        gst_buffer_pool_config_set_params (config, caps, size, min, max);
+        gst_caps_unref (caps);
+      }
 
-  if (dec->can_crop) {
-    GstVideoInfo *info = &dec->uncropped_info;
-    GstCaps *caps;
+      if (gst_buffer_pool_set_config (pool, config)) {
+        gst_query_set_nth_allocation_pool (query, 0, pool, size, min, max);
+      } else {
+        GstVideoInfo *info = &dec->uncropped_info;
 
-    GST_LOG_OBJECT (decoder, "Using GstVideoCropMeta, uncropped wxh = %dx%d",
-        info->width, info->height);
+        GST_DEBUG_OBJECT (dec, "ignoring unusable pool");
 
-    gst_video_info_set_format (info, info->finfo->format, dec->info.frame_width,
-        dec->info.frame_height);
+        gst_query_remove_nth_allocation_pool (query, 0);
+        gst_video_info_set_format (info, info->finfo->format,
+            dec->info.pic_width, dec->info.pic_height);
+        dec->can_crop = FALSE;
+      }
 
-    /* Calculate uncropped size */
-    size = MAX (size, info->size);
-    caps = gst_video_info_to_caps (info);
-    gst_buffer_pool_config_set_params (config, caps, size, min, max);
-    gst_caps_unref (caps);
+      gst_object_unref (pool);
+    }
   }
 
-  gst_buffer_pool_set_config (pool, config);
-
-  gst_query_set_nth_allocation_pool (query, 0, pool, size, min, max);
-
-  gst_object_unref (pool);
-
-  return TRUE;
+  return GST_VIDEO_DECODER_CLASS (parent_class)->decide_allocation (decoder,
+      query);
 }
 
 static void
diff --git a/subprojects/gst-plugins-base/tests/check/elements/theoradec.c b/subprojects/gst-plugins-base/tests/check/elements/theoradec.c
new file mode 100644
index 0000000000..909dd1d2dd
--- /dev/null
+++ b/subprojects/gst-plugins-base/tests/check/elements/theoradec.c
@@ -0,0 +1,138 @@
+/* GStreamer
+ *
+ * Copyright (C) <2021> Michael Olbrich <m.olbrich@pengutronix.de>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ */
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <gst/video/video.h>
+#include <gst/check/gstcheck.h>
+
+static GstPadProbeReturn
+query_handler (GstPad * pad, GstPadProbeInfo * info, gpointer user_data)
+{
+  GstQuery *query = GST_PAD_PROBE_INFO_QUERY (info);
+  GstBufferPool *pool;
+  GstStructure *config;
+  GstVideoInfo vinfo;
+  GstCaps *caps;
+
+  if (GST_QUERY_TYPE (query) != GST_QUERY_ALLOCATION)
+    return GST_PAD_PROBE_OK;
+
+  gst_query_parse_allocation (query, &caps, NULL);
+  fail_unless (caps != NULL);
+
+  gst_video_info_init (&vinfo);
+  gst_video_info_from_caps (&vinfo, caps);
+
+  pool = gst_video_buffer_pool_new ();
+  config = gst_buffer_pool_get_config (pool);
+  gst_buffer_pool_config_set_params (config, caps, vinfo.size, 0, 1);
+  gst_buffer_pool_set_config (pool, config);
+  /* activate the pool to ensure that gst_buffer_pool_set_config() will
+   * fail later */
+  gst_buffer_pool_set_active (pool, TRUE);
+  gst_query_add_allocation_pool (query, pool, vinfo.size, 0, 1);
+  gst_object_unref (pool);
+  gst_query_add_allocation_meta (query, GST_VIDEO_META_API_TYPE, NULL);
+  gst_query_add_allocation_meta (query, GST_VIDEO_CROP_META_API_TYPE, NULL);
+
+  return GST_PAD_PROBE_OK;
+}
+
+static void
+demux_pad_added_cb (GstElement * dec, GstPad * pad, gpointer user_data)
+{
+  GstElement *sink = user_data;
+  GstPad *sinkpad;
+
+  sinkpad = gst_element_get_static_pad (sink, "sink");
+  gst_pad_link (pad, sinkpad);
+  gst_object_unref (sinkpad);
+}
+
+GST_START_TEST (test_decide_allocation)
+{
+  GstElement *pipe, *src, *demux, *decode, *sink;
+  GstStateChangeReturn sret;
+  GstMessage *msg;
+  GstPad *pad;
+  gchar *path;
+
+  pipe = gst_pipeline_new (NULL);
+
+  src = gst_element_factory_make ("filesrc", NULL);
+  fail_unless (src != NULL, "Failed to create filesrc element");
+
+  demux = gst_element_factory_make ("oggdemux", NULL);
+  fail_unless (demux != NULL, "Failed to create oggdemux element");
+
+  decode = gst_element_factory_make ("theoradec", NULL);
+  fail_unless (decode != NULL, "Failed to create theoradec element");
+
+  sink = gst_element_factory_make ("fakesink", NULL);
+  fail_unless (sink != NULL, "Failed to create fakesink element");
+
+  gst_bin_add_many (GST_BIN (pipe), src, demux, decode, sink, NULL);
+  gst_element_link (src, demux);
+  gst_element_link (decode, sink);
+
+  path = g_build_filename (GST_TEST_FILES_PATH, "theora.ogg", NULL);
+  g_object_set (src, "location", path, NULL);
+  g_free (path);
+
+  g_signal_connect (demux, "pad-added",
+      G_CALLBACK (demux_pad_added_cb), decode);
+
+  pad = gst_element_get_static_pad (decode, "src");
+  gst_pad_add_probe (pad, GST_PAD_PROBE_TYPE_QUERY_DOWNSTREAM,
+      query_handler, NULL, NULL);
+  gst_object_unref (pad);
+
+  sret = gst_element_set_state (pipe, GST_STATE_PLAYING);
+  fail_unless_equals_int (sret, GST_STATE_CHANGE_ASYNC);
+
+  /* wait for EOS or error */
+  msg = gst_bus_timed_pop_filtered (GST_ELEMENT_BUS (pipe),
+      GST_CLOCK_TIME_NONE, GST_MESSAGE_ERROR | GST_MESSAGE_EOS);
+  fail_unless (msg != NULL);
+  fail_unless (GST_MESSAGE_TYPE (msg) == GST_MESSAGE_EOS);
+  gst_message_unref (msg);
+
+  gst_element_set_state (pipe, GST_STATE_NULL);
+  gst_object_unref (pipe);
+}
+
+GST_END_TEST;
+
+
+static Suite *
+theoradec_suite (void)
+{
+  Suite *s = suite_create ("theoradec");
+  TCase *tc_chain = tcase_create ("general");
+
+  suite_add_tcase (s, tc_chain);
+  tcase_add_test (tc_chain, test_decide_allocation);
+
+  return s;
+}
+
+GST_CHECK_MAIN (theoradec);
diff --git a/subprojects/gst-plugins-base/tests/check/meson.build b/subprojects/gst-plugins-base/tests/check/meson.build
index b5a749429c..dfcb92b547 100644
--- a/subprojects/gst-plugins-base/tests/check/meson.build
+++ b/subprojects/gst-plugins-base/tests/check/meson.build
@@ -73,6 +73,7 @@ if host_machine.system() != 'windows'
     [ 'elements/multisocketsink.c', not core_conf.has('HAVE_SYS_SOCKET_H') or not core_conf.has('HAVE_UNISTD_H') ],
     [ 'elements/playbin-complex.c', not ogg_dep.found() ],
     [ 'elements/textoverlay.c', not pango_dep.found() ],
+    [ 'elements/theoradec.c', not ogg_dep.found() and theoradec_dep.found() ],
     [ 'elements/vorbisdec.c', not vorbis_dep.found(), [ vorbis_dep, vorbisenc_dep ] ],
     [ 'elements/vorbistag.c', not vorbisenc_dep.found(), [ vorbis_dep, vorbisenc_dep ] ],
     [ 'pipelines/oggmux.c', not ogg_dep.found(), [ ogg_dep, ] ],
diff --git a/subprojects/gst-plugins-base/tests/files/theora.ogg b/subprojects/gst-plugins-base/tests/files/theora.ogg
new file mode 100644
index 0000000000..7f2a7789c3
--- /dev/null
+++ b/subprojects/gst-plugins-base/tests/files/theora.ogg