[PATCH 0/1] MR5090: winegstreamer: Append a parser when decoding H.264.

[Zebediah Figura (＠zfigura)]

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

[PATCH 1/1] winegstreamer: Append a parser when decoding H.264.

[Zebediah Figura]

From: Zebediah Figura <zfigura(a)codeweavers.com>

Windows always deals in Annex B, aka stream-format=byte-stream, but GStreamer
decoders may not be able to handle that. In particular, the AppleMedia decoder
(vtdec) only accepts AVC.

Add a parser before the decoder in order to convert between the two formats.
---
 dlls/winegstreamer/wg_transform.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c
index 516b28e82e2..a7b166224f3 100644
--- a/dlls/winegstreamer/wg_transform.c
+++ b/dlls/winegstreamer/wg_transform.c
@@ -343,6 +343,7 @@ NTSTATUS wg_transform_create(void *args)
     GstPadTemplate *template = NULL;
     struct wg_transform *transform;
     const gchar *media_type;
+    GstCaps *parsed_caps;
     GstEvent *event;
 
     if (!(transform = calloc(1, sizeof(*transform))))
@@ -398,9 +399,31 @@ NTSTATUS wg_transform_create(void *args)
     if (!(raw_caps = gst_caps_new_empty_simple(media_type)))
         goto out;
 
+    parsed_caps = gst_caps_ref(src_caps);
+
     switch (input_format.major_type)
     {
         case WG_MAJOR_TYPE_VIDEO_H264:
+            if ((element = find_element(GST_ELEMENT_FACTORY_TYPE_PARSER, src_caps, NULL)))
+            {
+                GstPad *parser_src;
+
+                if ((parser_src = gst_element_get_static_pad(element, "src")))
+                {
+                    if (append_element(transform->container, element, &first, &last))
+                        parsed_caps = gst_pad_query_caps(parser_src, NULL);
+                    else
+                        gst_object_unref(element);
+                    gst_object_unref(parser_src);
+                }
+                else
+                {
+                    GST_WARNING("Parser found, but has no src pad.");
+                    gst_object_unref(element);
+                }
+            }
+
+            /* fall through */
         case WG_MAJOR_TYPE_AUDIO_MPEG1:
         case WG_MAJOR_TYPE_AUDIO_MPEG4:
         case WG_MAJOR_TYPE_AUDIO_WMA:
@@ -408,9 +431,10 @@ NTSTATUS wg_transform_create(void *args)
         case WG_MAJOR_TYPE_VIDEO_INDEO:
         case WG_MAJOR_TYPE_VIDEO_WMV:
         case WG_MAJOR_TYPE_VIDEO_MPEG1:
-            if (!(element = find_element(GST_ELEMENT_FACTORY_TYPE_DECODER, src_caps, raw_caps))
+            if (!(element = find_element(GST_ELEMENT_FACTORY_TYPE_DECODER, parsed_caps, raw_caps))
                     || !append_element(transform->container, element, &first, &last))
             {
+                gst_caps_unref(parsed_caps);
                 gst_caps_unref(raw_caps);
                 goto out;
             }
@@ -425,6 +449,7 @@ NTSTATUS wg_transform_create(void *args)
             goto out;
     }
 
+    gst_caps_unref(parsed_caps);
     gst_caps_unref(raw_caps);
 
     switch (output_format.major_type)
-- 
GitLab

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

[Rémi Bernon]

This conflicts with https://gitlab.winehq.org/wine/wine/-/merge_requests/5055 and I would like to have that other one merged first.

Then what about doing something like https://gitlab.winehq.org/rbernon/wine/-/commit/7163bf25af53756e9d147d4810e1... and https://gitlab.winehq.org/rbernon/wine/-/commit/53d3c1b7e61448071750be501354... instead? I don't think we want to match *any* parsers, but only those which are parsing to the same media type, and then we also don't need to query any possibly missing pad.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5090#note_61441

[Rémi Bernon]

Also I don't see why restricting this to H264 while at the same time querying a parser in a generic fashion, IMO adding an optional parser before any decoder is better.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5090#note_61445

[Zebediah Figura (＠zfigura)]

> This conflicts with https://gitlab.winehq.org/wine/wine/-/merge_requests/5055 and I would like to have that other one merged first.

I don't think that has any bearing on whether or when this patch is accepted.
> I don't think we want to match parsers that output to *any* caps, but only those which are parsing to the same media type, and then we also don't need to query possibly missing pad.

What parser are you imagining would violate this?
> Also I don't see why restricting this to H264 while at the same time querying a parser in a generic fashion, IMO adding an optional parser before any decoder is better.

The point is to not depend on a specific parser name.

We probably should be adding a parser regardless of the codec, and I even originally had it that way when developing, but it broke MPEG-1 video. I do intend to look into that, but in the meantime I think it's reasonable to deal with one codec at a time.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5090#note_61497

[Rémi Bernon]

I opened https://gitlab.winehq.org/wine/wine/-/merge_requests/5255 with the changes I suggested, and some new attribute for clients to ask for no parser to be added to address the mpegvideo issue, though as far as I can see it's a GStreamer bug.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5090#note_63843

[PATCH 0/1] MR5090: winegstreamer: Append a parser when decoding H.264. - closed

[Elizabeth Figura (＠zfigura)]

This merge request was closed by Elizabeth Figura.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5090

[Elizabeth Figura (＠zfigura)]

Obviated by 5255.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5090#note_66454