glupload: Don't skip all other methods than the currently selected one when transforming caps

This leads to spurious negotiation failures because the configured method can
change over time and caps queries (and thus transform_caps) are happening
independently from configuring caps. Instead prefer the transformed caps of the
current method, but always also return the transformed caps for all other
methods. Previously all other methods would've only been used if the current
method returned empty caps. If a different method is needed later when
configuring the caps, it will be and was selected regardless.

Later during caps fixation, prefer the caps of the current method too for the
fixated caps if possible in any way.

This should preserve the desired behaviour of preferring the current method if
possible but to change to a different method if nothing else is possible, while
also returning consistent (and not too narrow) caps every time.

The way how the current method was checked was also racy as the current method
might change at any moment during caps query handling, and apart from
inconsistent results also a NULL pointer dereference was possible here. Use the
GST_OBJECT_LOCK to protect access to the current method like in other places.

This part of the code was introduced in f349cdccf5e1538f3eb9caa31458b53fdd81671b
and tried to be fixed multiple times over the years without addressing the root
cause of caps queries and caps configuration happening independently from each
other, e.g. in !2687 and !2699.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/8431>
This commit is contained in:
Sebastian Dröge 2025-02-10 16:47:16 +01:00 committed by GStreamer Marge Bot
parent 4fb5784e53
commit fc026fcfbe

View File

@ -3086,43 +3086,23 @@ gst_gl_upload_transform_caps (GstGLUpload * upload, GstGLContext * context,
GstCaps *result, *tmp;
gint i;
GST_OBJECT_LOCK (upload);
// Prefer caps from the currently configured method, if any
if (upload->priv->method) {
tmp = upload->priv->method->transform_caps (upload->priv->method_impl,
context, direction, caps);
if (tmp) {
/* If we're generating sink pad caps, make sure to include raw caps if needed by
* the current method */
if (direction == GST_PAD_SRC
&& (upload->priv->method->flags & METHOD_FLAG_CAN_ACCEPT_RAW)) {
GstCapsFeatures *passthrough =
gst_caps_features_from_string
(GST_CAPS_FEATURE_META_GST_VIDEO_OVERLAY_COMPOSITION);
GstCaps *raw_tmp = _set_caps_features_with_passthrough (tmp,
GST_CAPS_FEATURE_MEMORY_SYSTEM_MEMORY, passthrough);
gst_caps_append (tmp, raw_tmp);
gst_caps_features_free (passthrough);
}
if (filter) {
result =
gst_caps_intersect_full (filter, tmp, GST_CAPS_INTERSECT_FIRST);
gst_caps_unref (tmp);
} else {
result = tmp;
}
if (!gst_caps_is_empty (result))
return result;
else
gst_caps_unref (result);
}
}
tmp = gst_caps_new_empty ();
}
for (i = 0; i < G_N_ELEMENTS (upload_methods); i++) {
GstCaps *tmp2;
// Checked above already and put to the front
if (upload->priv->method == upload_methods[i])
continue;
tmp2 =
upload_methods[i]->transform_caps (upload->priv->upload_impl[i],
context, direction, caps);
@ -3138,6 +3118,8 @@ gst_gl_upload_transform_caps (GstGLUpload * upload, GstGLContext * context,
result = tmp;
}
GST_OBJECT_UNLOCK (upload);
return result;
}
@ -3413,6 +3395,26 @@ gst_gl_upload_fixate_caps (GstGLUpload * upload, GstPadDirection direction,
goto out;
}
// If a method is currently configured then fixate to the caps of this method
// if possible.
GST_OBJECT_LOCK (upload);
if (upload->priv->method) {
GstCaps *current_method_caps =
upload->priv->method->transform_caps (upload->priv->method_impl,
upload->context, GST_PAD_SINK, caps);
GstCaps *tmp = gst_caps_intersect_full (current_method_caps, othercaps,
GST_CAPS_INTERSECT_FIRST);
if (!gst_caps_is_empty (tmp)) {
gst_caps_unref (othercaps);
othercaps = tmp;
} else {
gst_caps_unref (tmp);
}
gst_caps_unref (current_method_caps);
}
GST_OBJECT_UNLOCK (upload);
/* Prefer target 2D->rectangle->oes */
for (target = GST_GL_TEXTURE_TARGET_2D;
target <= GST_GL_TEXTURE_TARGET_EXTERNAL_OES; target++) {