[PATCH v4 0/1] MR8719: winegstreamer: Only add the capsfilter for avdec_h264.

[Brendan McGrath (＠redmcg)]

h264parse currently has a bug where it will send an avc caps without
codec_data when it has received an SPS but not PPS.

This bug currently causes avdec_h264 to fail when a drain request
is made during caps neg.

As a workaround, a capsfilter was added to force a stream-format of
byte-stream for all h.264 decoders. However, this introduced a new
issue with the vtdec element, as it only supports the avc stream-format.

Therefore, this patch looks to address both problems by only introducing
the capsfilter when the avdec_h264 element is used.

It also removes the stream-format and alignment requirements from
the parsed caps, applying them only to the capsfilter.

--
  v4: winegstreamer: Only add the capsfilter for avdec_h264.

https://gitlab.winehq.org/wine/wine/-/merge_requests/8719

[PATCH v4 1/1] winegstreamer: Only add the capsfilter for avdec_h264.

[Brendan McGrath]

From: Brendan McGrath

h264parse currently has a bug where it will send an avc caps without
codec_data when it has received an SPS but not PPS.

This bug currently causes avdec_h264 to fail when a drain request
is made during caps neg.

As a workaround, a capsfilter was added to force a stream-format of
byte-stream for all h.264 decoders. However, this introduced a new
issue with the vtdec element, as it only supports the avc stream-format.

Therefore, this patch looks to address both problems by only introducing
the capsfilter when the avdec_h264 element is used.

It also removes the stream-format and alignment requirements from
the parsed caps, applying them only to the capsfilter.
---
 dlls/winegstreamer/wg_transform.c | 55 ++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 20 deletions(-)

diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c
index 9ca3368ecb9..3a56d0c4cfc 100644
--- a/dlls/winegstreamer/wg_transform.c
+++ b/dlls/winegstreamer/wg_transform.c
@@ -495,15 +495,7 @@ static GstCaps *transform_get_parsed_caps(GstCaps *caps, const char *media_type)
     else if (gst_structure_get_int(structure, "wmvversion", &value))
         gst_caps_set_simple(parsed_caps, "parsed", G_TYPE_BOOLEAN, true, "wmvversion", G_TYPE_INT, value, NULL);
     else
-    {
-        if (!strcmp(media_type, "video/x-h264"))
-        {
-            gst_caps_set_simple(parsed_caps, "stream-format", G_TYPE_STRING, "byte-stream", NULL);
-            gst_caps_set_simple(parsed_caps, "alignment", G_TYPE_STRING, "au", NULL);
-        }
-
         gst_caps_set_simple(parsed_caps, "parsed", G_TYPE_BOOLEAN, true, NULL);
-    }

return parsed_caps;
 }
@@ -512,7 +504,9 @@ static bool transform_create_decoder_elements(struct wg_transform *transform,
         const gchar *input_mime, const gchar *output_mime, GstElement **first, GstElement **last)
 {
     GstCaps *parsed_caps = NULL, *sink_caps = NULL;
-    GstElement *element;
+    GstElement *element, *capsfilter;
+    const char *shortname = NULL;
+    GstElementFactory *factory;
     bool ret = false;
     char *str;

@@ -535,15 +529,6 @@ static bool transform_create_decoder_elements(struct wg_transform *transform,

if (element)
     {
-        if (!(element = create_element("capsfilter", "good")) ||
-                !append_element(transform->container, element, first, last))
-            goto done;
-        if ((str = gst_caps_to_string(parsed_caps)))
-        {
-            gst_util_set_object_arg(G_OBJECT(element), "caps", str);
-            free(str);
-        }
-
         /* We try to intercept buffers produced by the parser, so if we push a large buffer into the
          * parser, it won't push everything into the decoder all in one go.
          */
@@ -558,8 +543,38 @@ static bool transform_create_decoder_elements(struct wg_transform *transform,
         parsed_caps = gst_caps_ref(transform->input_caps);
     }

-    if (!(element = find_element(GST_ELEMENT_FACTORY_TYPE_DECODER, parsed_caps, sink_caps))
-            || !append_element(transform->container, element, first, last))
+    if (!(element = find_element(GST_ELEMENT_FACTORY_TYPE_DECODER, parsed_caps, sink_caps)))
+        goto done;
+
+    /* h264parse currently has a bug where it will send an avc caps without codec_data when it
+     * has received an SPS but not PPS. As a result, when a drain request is made, the caps is fixated
+     * to avc but no codec_data is provided to the decoder. This results in libav rejecting every
+     * packet it receives.
+     * As a workaround, we need to insert a capsfilter for avdec_h264 in order for it to use
+     * the byte-stream stream-format.
+     */
+    if ((factory = gst_element_get_factory(element)) &&
+            (shortname = gst_plugin_feature_get_name(GST_PLUGIN_FEATURE(factory))) &&
+            !strcmp(shortname, "avdec_h264"))
+    {
+        if (!(capsfilter = create_element("capsfilter", "good")) ||
+                !append_element(transform->container, capsfilter, first, last))
+        {
+            g_object_unref(element);
+            goto done;
+        }
+
+        gst_caps_set_simple(parsed_caps, "stream-format", G_TYPE_STRING, "byte-stream", NULL);
+        gst_caps_set_simple(parsed_caps, "alignment", G_TYPE_STRING, "au", NULL);
+
+        if ((str = gst_caps_to_string(parsed_caps)))
+        {
+            gst_util_set_object_arg(G_OBJECT(capsfilter), "caps", str);
+            free(str);
+        }
+    }
+
+    if (!append_element(transform->container, element, first, last))
         goto done;

set_max_threads(element);

[Brendan McGrath (＠redmcg)]

On Mon Aug 11 21:28:54 2025 +0000, Elizabeth Figura wrote:
> > Do you think it's worth changing to opt-out rather than opt-in (i.e.
>
> explicitly exclude `vtdec` as opposed to explicitly include `avdec_h264`)?
> > Although, I'll point out that this bug seems to affect only this test
>
> case. We've had `h264parse` in Proton for almost 12 months and not
> needed this workaround.
> Yeah, that makes me more nervous actually. If just targeting avdec_h264
> seems to be enough in practice then let's just limit it to that for now.
> > > Anyway, the workaround seems reasonable to me, but we should alter
>
> the comment to make clear that it's a workaround for a GStreamer bug.
> > Done.
>
> Thanks, could you please also edit the code comment?

Oops. Yep. Missed that one, sorry. And done.