[PATCH v2 0/2] MR5405: winegstreamer: Fallback to input caps only when no parser was found.

[Rémi Bernon (＠rbernon)]

Which is what !5255 was supposed to do.

--
  v2: winegstreamer: Fallback to input caps only when no parser was found.
      winegstreamer: Create the transform parsed caps from wg_format.

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

[PATCH v2 1/2] winegstreamer: Create the transform parsed caps from wg_format.

[Rémi Bernon]

From: Rémi Bernon

---
 dlls/winegstreamer/wg_transform.c | 55 ++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c
index 71ea8cd263c..b1a820c0dc9 100644
--- a/dlls/winegstreamer/wg_transform.c
+++ b/dlls/winegstreamer/wg_transform.c
@@ -332,6 +332,59 @@ static bool wg_format_video_is_flipped(const struct wg_format *format)
     return format->major_type == WG_MAJOR_TYPE_VIDEO && (format->u.video.height < 0);
 }

+static GstCaps *transform_get_parsed_caps(struct wg_format *format, const char *media_type)
+{
+    GstCaps *parsed_caps;
+
+    if (!(parsed_caps = gst_caps_new_empty_simple(media_type)))
+        return NULL;
+
+    switch (format->major_type)
+    {
+        case WG_MAJOR_TYPE_AUDIO_MPEG1:
+            gst_caps_set_simple(parsed_caps, "parsed", G_TYPE_BOOLEAN, true, "mpegversion", G_TYPE_INT, 1,
+                    "layer", G_TYPE_INT, format->u.audio_mpeg1.layer, NULL);
+            break;
+        case WG_MAJOR_TYPE_AUDIO_MPEG4:
+            gst_caps_set_simple(parsed_caps, "framed", G_TYPE_BOOLEAN, true, "mpegversion", G_TYPE_INT, 4, NULL);
+            break;
+        case WG_MAJOR_TYPE_AUDIO_WMA:
+            gst_caps_set_simple(parsed_caps, "wmaversion", G_TYPE_INT, format->u.audio_wma.version, NULL);
+            break;
+        case WG_MAJOR_TYPE_VIDEO_H264:
+            gst_caps_set_simple(parsed_caps, "parsed", G_TYPE_BOOLEAN, true, NULL);
+            break;
+        case WG_MAJOR_TYPE_VIDEO_MPEG1:
+            gst_caps_set_simple(parsed_caps, "parsed", G_TYPE_BOOLEAN, true, "mpegversion", G_TYPE_INT, 4, NULL);
+            break;
+        case WG_MAJOR_TYPE_VIDEO_WMV:
+            switch (format->u.video_wmv.format)
+            {
+                case WG_WMV_VIDEO_FORMAT_WMV1:
+                    gst_caps_set_simple(parsed_caps, "wmvversion", G_TYPE_INT, 1, NULL);
+                    break;
+                case WG_WMV_VIDEO_FORMAT_WMV2:
+                    gst_caps_set_simple(parsed_caps, "wmvversion", G_TYPE_INT, 2, NULL);
+                    break;
+                case WG_WMV_VIDEO_FORMAT_WMV3:
+                case WG_WMV_VIDEO_FORMAT_WMVA:
+                case WG_WMV_VIDEO_FORMAT_WVC1:
+                    gst_caps_set_simple(parsed_caps, "wmvversion", G_TYPE_INT, 3, NULL);
+                    break;
+                default:
+                    GST_WARNING("Unknown WMV format %u.", format->u.video_wmv.format);
+                    break;
+            }
+            break;
+        case WG_MAJOR_TYPE_AUDIO:
+        case WG_MAJOR_TYPE_VIDEO:
+        case WG_MAJOR_TYPE_UNKNOWN:
+            break;
+    }
+
+    return parsed_caps;
+}
+
 NTSTATUS wg_transform_create(void *args)
 {
     struct wg_transform_create_params *params = args;
@@ -391,7 +444,7 @@ NTSTATUS wg_transform_create(void *args)
     gst_pad_set_chain_function(transform->my_sink, transform_sink_chain_cb);

media_type = gst_structure_get_name(gst_caps_get_structure(src_caps, 0));
-    if (!(parsed_caps = gst_caps_new_empty_simple(media_type)))
+    if (!(parsed_caps = transform_get_parsed_caps(&input_format, media_type)))
         goto out;

/* Since we append conversion elements, we don't want to filter decoders

[PATCH v2 2/2] winegstreamer: Fallback to input caps only when no parser was found.

[Rémi Bernon]

From: Rémi Bernon

---
 dlls/winegstreamer/wg_transform.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c
index b1a820c0dc9..8e48b7a337f 100644
--- a/dlls/winegstreamer/wg_transform.c
+++ b/dlls/winegstreamer/wg_transform.c
@@ -468,7 +468,7 @@ NTSTATUS wg_transform_create(void *args)
             if ((element = find_element(GST_ELEMENT_FACTORY_TYPE_PARSER, src_caps, parsed_caps))
                     && !append_element(transform->container, element, &first, &last))
                 goto out;
-            else
+            else if (!element)
             {
                 gst_caps_unref(parsed_caps);
                 parsed_caps = gst_caps_ref(src_caps);

[PATCH v2 2/2] winegstreamer: Fallback to input caps only when no parser was found.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=144409

Your paranoid android.

=== debian11b (64 bit WoW report) ===

quartz:
mpegvideo.c:945: Test succeeded inside todo block: Got stop time 0, expected 0.
mpegvideo.c:945: Test succeeded inside todo block: Got stop time 0, expected 0.
mpegvideo.c:945: Test succeeded inside todo block: Got stop time 0, expected 0.
mpegvideo.c:945: Test succeeded inside todo block: Got stop time 0, expected 0.
mpegvideo.c:945: Test succeeded inside todo block: Got stop time 0, expected 0.
mpegvideo.c:945: Test succeeded inside todo block: Got stop time 0, expected 0.
mpegvideo.c:945: Test succeeded inside todo block: Got stop time 0, expected 0.

[Rémi Bernon (＠rbernon)]

v2: Create the parsed caps ourselves, some parsers such as mpegaudioparse0 output non-fixed caps which then won't let us find the correct decoder. We need to fix some properties such as mpegversion instead.

[Elizabeth Figura (＠zfigura)]

Do we really need to pass the parsed caps at all? Can't we just remove that restriction?

[Rémi Bernon (＠rbernon)]

This was v1 of this MR and it would pass `audio/x-mpeg` for MPEG3, which might find an AAC decoder and fail to connect it.

[Elizabeth Figura (＠zfigura)]

> This was v1 of this MR and it would pass `audio/x-mpeg` for MPEG3, which might find an AAC decoder and fail to connect it.

Aren't we passing audio/mpeg, mpegversion=1 as input caps in that case? How does that match aacparse?

[Rémi Bernon (＠rbernon)]

This is not what I said, `audio/mpeg, mpegversion=1` matches mpegaudioparse, which then outputs `audio/mpeg, mpegversion=1` but if we cut it to `audio/mpeg`, we might get an AAC decoder matching that caps enumerated first, which then won't connect. Could be the other way around for that matter, as long as there's parser and decoder with ambiguous mime types.

[Elizabeth Figura (＠zfigura)]

We could do it like 5090 then, and query caps from the parser's src.

[Rémi Bernon (＠rbernon)]

It doesn't work either because some parsers such as mpegaudioparse have non-fixed caps, which will not match, or match the wrong decoder.

[Elizabeth Figura (＠zfigura)]

Thanks, that makes sense. Unfortunate that it means more work for us, but it is what it is.

[Elizabeth Figura (＠zfigura)]

Unfortunately this is causing test failures for me:

mpegvideo.c:1220: Test failed: Got hr 0xc00d6d61.
mpegvideo.c:1321: Test failed: Got 0 calls to Receive().
mpegvideo.c:1220: Test succeeded inside todo block: Got hr 0.

(The above repeats.)