From 8737b9ca844d73907c49ec1c3c88d3a93f7d7137 Mon Sep 17 00:00:00 2001 From: Edward Hervey Date: Fri, 26 Apr 2024 10:12:23 +0200 Subject: [PATCH] playbin3: Handle combiner update in case of errors The assertion that was present before is a bit too harsh, since there is now a (understandable) use-case where this could happen. In gapless use-case, with two files containing the same type (ex:audio). The first one *does* expose a collection with an audio stream, but decoding fails (for whatever reason). That would cause us to have configured a audio combiner, which was never used (i.e. not active). Then the second file plays and we (wrongly) assume it should be activated ... whereas the combiner was indeed present. Demote the assertion to a warning and properly handle it Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/3389 Part-of: --- .../gst/playback/gstplaybin3.c | 47 ++++++++++++------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/subprojects/gst-plugins-base/gst/playback/gstplaybin3.c b/subprojects/gst-plugins-base/gst/playback/gstplaybin3.c index 1df2357e8a..3e735c3a35 100644 --- a/subprojects/gst-plugins-base/gst/playback/gstplaybin3.c +++ b/subprojects/gst-plugins-base/gst/playback/gstplaybin3.c @@ -2614,25 +2614,38 @@ reconfigure_output (GstPlayBin3 * playbin) GST_DEBUG_OBJECT (playbin, "Stream type '%s' is now requested", gst_stream_type_get_name (combine->stream_type)); - g_assert (combine->sinkpad == NULL); + if (combine->sinkpad) { + /* This was previously an assert but is now just a WARNING + * + * This *theoretically* should never happen, but there is the + * possibility where there was a failure within (uri)decodebin3 where + * the collection was posted (by demuxer for ex) but the decoding failed + * (no decoder, bad stream, etc...). + * + * in that case, we could have a combiner already prepared for that type + * but never got activated. + **/ + GST_WARNING_OBJECT (playbin, "Combiner already configured"); + } else { + + /* Request playsink sink pad */ + combine->sinkpad = + gst_play_sink_request_pad (playbin->playsink, + gst_play_sink_type_from_stream_type (combine->stream_type)); + gst_object_ref (combine->sinkpad); + /* Create combiner if needed and link it */ + create_combiner (playbin, combine); + if (combine->combiner) { + res = gst_pad_link (combine->srcpad, combine->sinkpad); + GST_DEBUG_OBJECT (playbin, "linked type %s, result: %d", + gst_stream_type_get_name (combine->stream_type), res); + if (res != GST_PAD_LINK_OK) { + GST_ELEMENT_ERROR (playbin, CORE, PAD, + ("Internal playbin error."), + ("Failed to link combiner to sink. Error %d", res)); + } - /* Request playsink sink pad */ - combine->sinkpad = - gst_play_sink_request_pad (playbin->playsink, - gst_play_sink_type_from_stream_type (combine->stream_type)); - gst_object_ref (combine->sinkpad); - /* Create combiner if needed and link it */ - create_combiner (playbin, combine); - if (combine->combiner) { - res = gst_pad_link (combine->srcpad, combine->sinkpad); - GST_DEBUG_OBJECT (playbin, "linked type %s, result: %d", - gst_stream_type_get_name (combine->stream_type), res); - if (res != GST_PAD_LINK_OK) { - GST_ELEMENT_ERROR (playbin, CORE, PAD, - ("Internal playbin error."), - ("Failed to link combiner to sink. Error %d", res)); } - } } }