[PATCH v4 0/1] MR8719: winegstreamer: Only add the capsfilter for avdec_h264.
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
From: Brendan McGrath <bmcgrath(a)codeweavers.com> 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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8719
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8719#note_112650
participants (2)
-
Brendan McGrath -
Brendan McGrath (＠redmcg)