decodebin3: improve decoder selection

Currently the decoder selection is very naive: The type with the highest
rank that matches the current caps is used. This works well for software
decoders. The exact supported caps are always known and the static caps are
defined accordingly.
With hardware decoders, e.g. vaapi, the situation is different. The decoder
may reject the caps later during a caps query. At that point, a new decoder
is created. However, the same type is chosen an after several tries,
decodebin fails.

To avoid this, do the caps query while adding the decoder and try again
with other decoder types if the query fails:

1. create the decoder from the next matching type
2. add and link the decoder
3. change the decoder state to READY
4. do the caps query
   if it fails then remove the decoder again and go back to 1.
5. expose the source pad
6. sync the decoder state with the parent.

This way, the decoder is already part of the pipeline when the state change
to READY happens. So context handling should work as before.

Exposing the source pad after the query was successful is important:
Otherwise the thread from the decoder source pad may block in a blocked pad
downstream in the playsink waiting for other pads to be ready.
The thread now blocks trying to set the state back to NULL while holding
the SELECTION_LOCK. Other streams may block on the SELECTION_LOCK and the
playsink never unblocks the pad. The result is a deadlock.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/merge_requests/1201>
This commit is contained in:
Michael Olbrich 2021-06-11 09:02:29 +02:00 committed by Edward Hervey
parent 210cf85fdc
commit 5bfb78aa28

View file

@ -2161,45 +2161,17 @@ have_factory (GstDecodebin3 * dbin, GstCaps * caps,
} }
#endif #endif
static GstElement * static GList *
create_element (GstDecodebin3 * dbin, GstStream * stream, create_decoder_factory_list (GstDecodebin3 * dbin, GstCaps * caps)
GstElementFactoryListType ftype)
{ {
GList *res; GList *res;
GstElement *element = NULL;
GstCaps *caps;
g_mutex_lock (&dbin->factories_lock); g_mutex_lock (&dbin->factories_lock);
gst_decode_bin_update_factories_list (dbin); gst_decode_bin_update_factories_list (dbin);
caps = gst_stream_get_caps (stream); res = gst_element_factory_list_filter (dbin->decoder_factories,
if (ftype == GST_ELEMENT_FACTORY_TYPE_DECODER) caps, GST_PAD_SINK, TRUE);
res =
gst_element_factory_list_filter (dbin->decoder_factories,
caps, GST_PAD_SINK, TRUE);
else
res =
gst_element_factory_list_filter (dbin->decodable_factories,
caps, GST_PAD_SINK, TRUE);
g_mutex_unlock (&dbin->factories_lock); g_mutex_unlock (&dbin->factories_lock);
return res;
if (res) {
element =
gst_element_factory_create ((GstElementFactory *) res->data, NULL);
GST_DEBUG ("Created element '%s'", GST_ELEMENT_NAME (element));
gst_plugin_feature_list_free (res);
} else {
GST_DEBUG ("Could not find an element for caps %" GST_PTR_FORMAT, caps);
}
gst_caps_unref (caps);
return element;
}
/* FIXME : VERY NAIVE. ASSUMING FIRST ONE WILL WORK */
static GstElement *
create_decoder (GstDecodebin3 * dbin, GstStream * stream)
{
return create_element (dbin, stream, GST_ELEMENT_FACTORY_TYPE_DECODER);
} }
static GstPadProbeReturn static GstPadProbeReturn
@ -2304,50 +2276,93 @@ reconfigure_output_stream (DecodebinOutputStream * output,
} }
} }
gst_caps_unref (new_caps);
gst_object_replace ((GstObject **) & output->decoder_sink, NULL); gst_object_replace ((GstObject **) & output->decoder_sink, NULL);
gst_object_replace ((GstObject **) & output->decoder_src, NULL); gst_object_replace ((GstObject **) & output->decoder_src, NULL);
/* If a decoder is required, create one */ /* If a decoder is required, create one */
if (needs_decoder) { if (needs_decoder) {
/* If we don't have a decoder yet, instantiate one */ GList *factories, *next_factory;
output->decoder = create_decoder (dbin, slot->active_stream);
if (output->decoder == NULL) {
GstCaps *caps;
SELECTION_UNLOCK (dbin); factories = next_factory = create_decoder_factory_list (dbin, new_caps);
/* FIXME : Should we be smarter if there's a missing decoder ? while (!output->decoder) {
* Should we deactivate that stream ? */ gboolean decoder_failed = FALSE;
caps = gst_stream_get_caps (slot->active_stream);
gst_element_post_message (GST_ELEMENT_CAST (dbin), /* If we don't have a decoder yet, instantiate one */
gst_missing_decoder_message_new (GST_ELEMENT_CAST (dbin), caps)); if (next_factory) {
gst_caps_unref (caps); output->decoder = gst_element_factory_create ((GstElementFactory *)
SELECTION_LOCK (dbin); next_factory->data, NULL);
goto cleanup; GST_DEBUG ("Created decoder '%s'", GST_ELEMENT_NAME (output->decoder));
} } else
if (!gst_bin_add ((GstBin *) dbin, output->decoder)) { GST_DEBUG ("Could not find an element for caps %" GST_PTR_FORMAT,
GST_ERROR_OBJECT (dbin, "could not add decoder to pipeline"); new_caps);
goto cleanup;
} if (output->decoder == NULL) {
output->decoder_sink = gst_element_get_static_pad (output->decoder, "sink"); GstCaps *caps;
output->decoder_src = gst_element_get_static_pad (output->decoder, "src");
if (output->type & GST_STREAM_TYPE_VIDEO) { SELECTION_UNLOCK (dbin);
GST_DEBUG_OBJECT (dbin, "Adding keyframe-waiter probe"); /* FIXME : Should we be smarter if there's a missing decoder ?
output->drop_probe_id = * Should we deactivate that stream ? */
gst_pad_add_probe (slot->src_pad, GST_PAD_PROBE_TYPE_BUFFER, caps = gst_stream_get_caps (slot->active_stream);
(GstPadProbeCallback) keyframe_waiter_probe, output, NULL); gst_element_post_message (GST_ELEMENT_CAST (dbin),
} gst_missing_decoder_message_new (GST_ELEMENT_CAST (dbin), caps));
if (gst_pad_link_full (slot->src_pad, output->decoder_sink, gst_caps_unref (caps);
GST_PAD_LINK_CHECK_NOTHING) != GST_PAD_LINK_OK) { SELECTION_LOCK (dbin);
GST_ERROR_OBJECT (dbin, "could not link to %s:%s", goto cleanup;
GST_DEBUG_PAD_NAME (output->decoder_sink)); }
goto cleanup; if (!gst_bin_add ((GstBin *) dbin, output->decoder)) {
GST_ERROR_OBJECT (dbin, "could not add decoder to pipeline");
goto cleanup;
}
output->decoder_sink =
gst_element_get_static_pad (output->decoder, "sink");
output->decoder_src = gst_element_get_static_pad (output->decoder, "src");
if (output->type & GST_STREAM_TYPE_VIDEO) {
GST_DEBUG_OBJECT (dbin, "Adding keyframe-waiter probe");
output->drop_probe_id =
gst_pad_add_probe (slot->src_pad, GST_PAD_PROBE_TYPE_BUFFER,
(GstPadProbeCallback) keyframe_waiter_probe, output, NULL);
}
if (gst_pad_link_full (slot->src_pad, output->decoder_sink,
GST_PAD_LINK_CHECK_NOTHING) != GST_PAD_LINK_OK) {
GST_ERROR_OBJECT (dbin, "could not link to %s:%s",
GST_DEBUG_PAD_NAME (output->decoder_sink));
goto cleanup;
}
if (gst_element_set_state (output->decoder,
GST_STATE_READY) == GST_STATE_CHANGE_FAILURE) {
GST_DEBUG_OBJECT (dbin,
"Decoder '%s' failed to reach READY state, trying the next type",
GST_ELEMENT_NAME (output->decoder));
decoder_failed = TRUE;
}
if (!gst_pad_query_accept_caps (output->decoder_sink, new_caps)) {
GST_DEBUG_OBJECT (dbin,
"Decoder '%s' did not accept the caps, trying the next type",
GST_ELEMENT_NAME (output->decoder));
decoder_failed = TRUE;
}
if (decoder_failed) {
gst_pad_unlink (slot->src_pad, output->decoder_sink);
if (output->drop_probe_id) {
gst_pad_remove_probe (slot->src_pad, output->drop_probe_id);
output->drop_probe_id = 0;
}
gst_element_set_locked_state (output->decoder, TRUE);
gst_element_set_state (output->decoder, GST_STATE_NULL);
gst_bin_remove ((GstBin *) dbin, output->decoder);
output->decoder = NULL;
}
next_factory = next_factory->next;
} }
gst_plugin_feature_list_free (factories);
} else { } else {
output->decoder_src = gst_object_ref (slot->src_pad); output->decoder_src = gst_object_ref (slot->src_pad);
output->decoder_sink = NULL; output->decoder_sink = NULL;
} }
gst_caps_unref (new_caps);
output->linked = TRUE; output->linked = TRUE;
if (!gst_ghost_pad_set_target ((GstGhostPad *) output->src_pad, if (!gst_ghost_pad_set_target ((GstGhostPad *) output->src_pad,
output->decoder_src)) { output->decoder_src)) {