[PATCH v4 1/5] winegstreamer: Remove \n at the end of GST log messages.

[Zebediah Figura]

From: Rémi Bernon

GStreamer already adds one.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51931
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52391
Signed-off-by: Rémi Bernon 
Signed-off-by: Zebediah Figura 
---
 dlls/winegstreamer/wg_format.c |  4 ++--
 dlls/winegstreamer/wg_parser.c | 16 ++++++++--------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/dlls/winegstreamer/wg_format.c b/dlls/winegstreamer/wg_format.c
index 40b9acfefff..fc50fad6d12 100644
--- a/dlls/winegstreamer/wg_format.c
+++ b/dlls/winegstreamer/wg_format.c
@@ -327,12 +327,12 @@ static void wg_channel_mask_to_gst(GstAudioChannelPosition *positions, uint32_t
             if (bit < ARRAY_SIZE(position_map))
                 positions[i] = position_map[bit];
             else
-                GST_WARNING("Invalid channel mask %#x.\n", orig_mask);
+                GST_WARNING("Invalid channel mask %#x.", orig_mask);
             mask &= ~(1 << bit);
         }
         else
         {
-            GST_WARNING("Incomplete channel mask %#x.\n", orig_mask);
+            GST_WARNING("Incomplete channel mask %#x.", orig_mask);
         }
     }
 }
diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c
index d0883c4c024..e5ed496387e 100644
--- a/dlls/winegstreamer/wg_parser.c
+++ b/dlls/winegstreamer/wg_parser.c
@@ -1436,7 +1436,7 @@ static BOOL decodebin_parser_init_gst(struct wg_parser *parser)

if ((ret = gst_pad_link(parser->my_src, parser->their_sink)) < 0)
     {
-        GST_ERROR("Failed to link pads, error %d.\n", ret);
+        GST_ERROR("Failed to link pads, error %d.", ret);
         return FALSE;
     }

@@ -1465,7 +1465,7 @@ static BOOL avi_parser_init_gst(struct wg_parser *parser)

if ((ret = gst_pad_link(parser->my_src, parser->their_sink)) < 0)
     {
-        GST_ERROR("Failed to link pads, error %d.\n", ret);
+        GST_ERROR("Failed to link pads, error %d.", ret);
         return FALSE;
     }

@@ -1486,7 +1486,7 @@ static BOOL mpeg_audio_parser_init_gst(struct wg_parser *parser)
     parser->their_sink = gst_element_get_static_pad(element, "sink");
     if ((ret = gst_pad_link(parser->my_src, parser->their_sink)) < 0)
     {
-        GST_ERROR("Failed to link sink pads, error %d.\n", ret);
+        GST_ERROR("Failed to link sink pads, error %d.", ret);
         return FALSE;
     }

@@ -1496,7 +1496,7 @@ static BOOL mpeg_audio_parser_init_gst(struct wg_parser *parser)
     gst_object_ref(stream->their_src = gst_element_get_static_pad(element, "src"));
     if ((ret = gst_pad_link(stream->their_src, stream->my_sink)) < 0)
     {
-        GST_ERROR("Failed to link source pads, error %d.\n", ret);
+        GST_ERROR("Failed to link source pads, error %d.", ret);
         return FALSE;
     }
     gst_pad_set_active(stream->my_sink, 1);
@@ -1520,7 +1520,7 @@ static BOOL wave_parser_init_gst(struct wg_parser *parser)
     parser->their_sink = gst_element_get_static_pad(element, "sink");
     if ((ret = gst_pad_link(parser->my_src, parser->their_sink)) < 0)
     {
-        GST_ERROR("Failed to link sink pads, error %d.\n", ret);
+        GST_ERROR("Failed to link sink pads, error %d.", ret);
         return FALSE;
     }

@@ -1531,7 +1531,7 @@ static BOOL wave_parser_init_gst(struct wg_parser *parser)
     gst_object_ref(stream->their_src);
     if ((ret = gst_pad_link(stream->their_src, stream->my_sink)) < 0)
     {
-        GST_ERROR("Failed to link source pads, error %d.\n", ret);
+        GST_ERROR("Failed to link source pads, error %d.", ret);
         return FALSE;
     }
     gst_pad_set_active(stream->my_sink, 1);
@@ -1559,7 +1559,7 @@ static void init_gstreamer_once(void)

GST_DEBUG_CATEGORY_INIT(wine, "WINE", GST_DEBUG_FG_RED, "Wine GStreamer support");

-    GST_INFO("GStreamer library version %s; wine built with %d.%d.%d.\n",
+    GST_INFO("GStreamer library version %s; wine built with %d.%d.%d.",
             gst_version_string(), GST_VERSION_MAJOR, GST_VERSION_MINOR, GST_VERSION_MICRO);
 }

@@ -1597,7 +1597,7 @@ static NTSTATUS wg_parser_create(void *args)
     parser->init_gst = init_funcs[params->type];
     parser->unlimited_buffering = params->unlimited_buffering;

-    GST_DEBUG("Created winegstreamer parser %p.\n", parser);
+    GST_DEBUG("Created winegstreamer parser %p.", parser);
     params->parser = parser;
     return S_OK;
 }

[PATCH v4 2/5] winegstreamer: Move the conditional cleanup to the create error path.

[Zebediah Figura]

From: Rémi Bernon

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51931
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52391
Signed-off-by: Rémi Bernon 
Signed-off-by: Zebediah Figura 
---
v4: Use multiple gotos instead of checking variables for NULL.

dlls/winegstreamer/wg_transform.c | 73 +++++++++++++------------------
 1 file changed, 31 insertions(+), 42 deletions(-)

diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c
index e4545774428..c309523dd36 100644
--- a/dlls/winegstreamer/wg_transform.c
+++ b/dlls/winegstreamer/wg_transform.c
@@ -62,12 +62,10 @@ NTSTATUS wg_transform_destroy(void *args)
 {
     struct wg_transform *transform = args;

-    if (transform->my_sink)
-        g_object_unref(transform->my_sink);
-    if (transform->my_src)
-        g_object_unref(transform->my_src);
-
+    g_object_unref(transform->my_sink);
+    g_object_unref(transform->my_src);
     free(transform);
+
     return STATUS_SUCCESS;
 }

@@ -77,61 +75,52 @@ NTSTATUS wg_transform_create(void *args)
     struct wg_format output_format = *params->output_format;
     struct wg_format input_format = *params->input_format;
     GstCaps *src_caps = NULL, *sink_caps = NULL;
+    NTSTATUS status = STATUS_UNSUCCESSFUL;
     GstPadTemplate *template = NULL;
     struct wg_transform *transform;
-    NTSTATUS status;

if (!init_gstreamer())
         return STATUS_UNSUCCESSFUL;

-    status = STATUS_NO_MEMORY;
-
     if (!(transform = calloc(1, sizeof(*transform))))
-        goto done;
+        return STATUS_NO_MEMORY;

if (!(src_caps = wg_format_to_caps(&input_format)))
-        goto done;
-    if (!(sink_caps = wg_format_to_caps(&output_format)))
-        goto done;
-
+        goto out_free_transform;
     if (!(template = gst_pad_template_new("src", GST_PAD_SRC, GST_PAD_ALWAYS, src_caps)))
-        goto done;
-    if (!(transform->my_src = gst_pad_new_from_template(template, "src")))
-        goto done;
+        goto out_free_src_caps;
+    transform->my_src = gst_pad_new_from_template(template, "src");
     g_object_unref(template);
-    template = NULL;
+    if (!transform->my_src)
+        goto out_free_src_caps;

+    if (!(sink_caps = wg_format_to_caps(&output_format)))
+        goto out_free_src_pad;
     if (!(template = gst_pad_template_new("sink", GST_PAD_SINK, GST_PAD_ALWAYS, sink_caps)))
-        goto done;
-    if (!(transform->my_sink = gst_pad_new_from_template(template, "sink")))
-        goto done;
+        goto out_free_sink_caps;
+    transform->my_sink = gst_pad_new_from_template(template, "sink");
     g_object_unref(template);
-    template = NULL;
+    if (!transform->my_sink)
+        goto out_free_sink_caps;

gst_pad_set_element_private(transform->my_sink, transform);
     gst_pad_set_chain_function(transform->my_sink, transform_sink_chain_cb);

-    status = STATUS_SUCCESS;
-
-done:
-    if (template)
-        g_object_unref(template);
-    if (sink_caps)
-        gst_caps_unref(sink_caps);
-    if (src_caps)
-        gst_caps_unref(src_caps);
-
-    if (status)
-    {
-        GST_ERROR("Failed to create winegstreamer transform.");
-        if (transform)
-            wg_transform_destroy(transform);
-    }
-    else
-    {
-        GST_INFO("Created winegstreamer transform %p.", transform);
-        params->transform = transform;
-    }
+    gst_caps_unref(sink_caps);
+    gst_caps_unref(src_caps);
+
+    GST_INFO("Created winegstreamer transform %p.", transform);
+    params->transform = transform;
+    return STATUS_SUCCESS;

+out_free_sink_caps:
+    gst_caps_unref(sink_caps);
+out_free_src_pad:
+    gst_object_unref(transform->my_src);
+out_free_src_caps:
+    gst_caps_unref(src_caps);
+out_free_transform:
+    free(transform);
+    GST_ERROR("Failed to create winegstreamer transform.");
     return status;
 }

[PATCH v4 2/5] winegstreamer: Move the conditional cleanup to the create error path.

[Rémi Bernon]

On 2/22/22 18:43, Zebediah Figura wrote:
> +out_free_sink_caps:
> - gst_caps_unref(sink_caps);
>
> +out_free_src_pad:
> - gst_object_unref(transform->my_src);
>
> +out_free_src_caps:
> - gst_caps_unref(src_caps);
>
> +out_free_transform:
> - free(transform);
> - GST_ERROR("Failed to create winegstreamer transform.");
>  return status;
> }

I hesitated with an approach like this. I haven't seen goto dipatch like 
this elsewhere so I thought it wasn't the preferred style.

[PATCH v4 2/5] winegstreamer: Move the conditional cleanup to the create error path.

[Zebediah Figura (she/her)]

On 2/22/22 12:03, Rémi Bernon wrote:
> On 2/22/22 18:43, Zebediah Figura wrote:
> > +out_free_sink_caps:
> > +    gst_caps_unref(sink_caps);
> > +out_free_src_pad:
> > +    gst_object_unref(transform->my_src);
> > +out_free_src_caps:
> > +    gst_caps_unref(src_caps);
> > +out_free_transform:
> > +    free(transform);
> > +    GST_ERROR("Failed to create winegstreamer transform.");
> >       return status;
> >   }
>
> I hesitated with an approach like this. I haven't seen goto dipatch like
> this elsewhere so I thought it wasn't the preferred style.

I haven't seen it used elsewhere in Wine, but it was adopted in a couple
of places in vkd3d, and I personally like it.

[PATCH v4 2/5] winegstreamer: Move the conditional cleanup to the create error path.

[Rémi Bernon]

On 2/22/22 19:05, Zebediah Figura (she/her) wrote:
> On 2/22/22 12:03, Rémi Bernon wrote:
> > On 2/22/22 18:43, Zebediah Figura wrote:
> > > +out_free_sink_caps:
> > > +    gst_caps_unref(sink_caps);
> > > +out_free_src_pad:
> > > +    gst_object_unref(transform->my_src);
> > > +out_free_src_caps:
> > > +    gst_caps_unref(src_caps);
> > > +out_free_transform:
> > > +    free(transform);
> > > +    GST_ERROR("Failed to create winegstreamer transform.");
> > >        return status;
> > >    }
> >
> > I hesitated with an approach like this. I haven't seen goto dipatch like
> > this elsewhere so I thought it wasn't the preferred style.
>
> I haven't seen it used elsewhere in Wine, but it was adopted in a couple
> of places in vkd3d, and I personally like it.

Tbh I'm not sure I do, I find it much harder to reason with, especially 
when there's many different error paths or stage like here, though I'm 
not the maintainer here.

[PATCH v4 2/5] winegstreamer: Move the conditional cleanup to the create error path.

[Rémi Bernon]

On 2/22/22 19:35, Rémi Bernon wrote:
> On 2/22/22 19:05, Zebediah Figura (she/her) wrote:
> > On 2/22/22 12:03, Rémi Bernon wrote:
> > > On 2/22/22 18:43, Zebediah Figura wrote:
> > > > +out_free_sink_caps:
> > > > +    gst_caps_unref(sink_caps);
> > > > +out_free_src_pad:
> > > > +    gst_object_unref(transform->my_src);
> > > > +out_free_src_caps:
> > > > +    gst_caps_unref(src_caps);
> > > > +out_free_transform:
> > > > +    free(transform);
> > > > +    GST_ERROR("Failed to create winegstreamer transform.");
> > > >        return status;
> > > >    }
> > >
> > > I hesitated with an approach like this. I haven't seen goto dipatch like
> > > this elsewhere so I thought it wasn't the preferred style.
> >
> > I haven't seen it used elsewhere in Wine, but it was adopted in a couple
> > of places in vkd3d, and I personally like it.
>
> Tbh I'm not sure I do, I find it much harder to reason with, especially 
> when there's many different error paths or stage like here, though I'm 
> not the maintainer here.

Experiencing it a bit more after rebasing my local patches I think it 
also makes re-odering of code much more difficult as you need to be very 
careful to update the gotos correctly as well as with potential 
reordering of the cleanup labels and steps.

[PATCH v4 2/5] winegstreamer: Move the conditional cleanup to the create error path.

[Zebediah Figura (she/her)]

On 2/22/22 13:37, Rémi Bernon wrote:
> On 2/22/22 19:35, Rémi Bernon wrote:
> > On 2/22/22 19:05, Zebediah Figura (she/her) wrote:
> > > On 2/22/22 12:03, Rémi Bernon wrote:
> > > > On 2/22/22 18:43, Zebediah Figura wrote:
> > > > > +out_free_sink_caps:
> > > > > +    gst_caps_unref(sink_caps);
> > > > > +out_free_src_pad:
> > > > > +    gst_object_unref(transform->my_src);
> > > > > +out_free_src_caps:
> > > > > +    gst_caps_unref(src_caps);
> > > > > +out_free_transform:
> > > > > +    free(transform);
> > > > > +    GST_ERROR("Failed to create winegstreamer transform.");
> > > > >        return status;
> > > > >    }
> > > >
> > > > I hesitated with an approach like this. I haven't seen goto dipatch
> > > > like
> > > > this elsewhere so I thought it wasn't the preferred style.
> > >
> > > I haven't seen it used elsewhere in Wine, but it was adopted in a couple
> > > of places in vkd3d, and I personally like it.
> >
> > Tbh I'm not sure I do, I find it much harder to reason with,
> > especially when there's many different error paths or stage like here,
> > though I'm not the maintainer here.
>
> Experiencing it a bit more after rebasing my local patches I think it
> also makes re-odering of code much more difficult as you need to be very
> careful to update the gotos correctly as well as with potential
> reordering of the cleanup labels and steps.

I guess the advantage is that it makes it easier to conditionally clean
up in cases that can't be simply expressed as "if (x) free(x)". That
doesn't apply here (yet), but there is something to be said for consistency.

Still, if it becomes too much of a hassle, I'm open to reverting this
commit, or reverting to your v3; I guess I don't care quite that much.

[PATCH v4 2/5] winegstreamer: Move the conditional cleanup to the create error path.

[Alexandre Julliard]

Rémi Bernon  writes:
> On 2/22/22 19:35, Rémi Bernon wrote:
> > On 2/22/22 19:05, Zebediah Figura (she/her) wrote:
> > > On 2/22/22 12:03, Rémi Bernon wrote:
> > > > On 2/22/22 18:43, Zebediah Figura wrote:
> > > > > +out_free_sink_caps:
> > > > > +    gst_caps_unref(sink_caps);
> > > > > +out_free_src_pad:
> > > > > +    gst_object_unref(transform->my_src);
> > > > > +out_free_src_caps:
> > > > > +    gst_caps_unref(src_caps);
> > > > > +out_free_transform:
> > > > > +    free(transform);
> > > > > +    GST_ERROR("Failed to create winegstreamer transform.");
> > > > >        return status;
> > > > >    }
> > > >
> > > > I hesitated with an approach like this. I haven't seen goto dipatch like
> > > > this elsewhere so I thought it wasn't the preferred style.
> > >
> > > I haven't seen it used elsewhere in Wine, but it was adopted in a couple
> > > of places in vkd3d, and I personally like it.
> >
> > Tbh I'm not sure I do, I find it much harder to reason with,
> > especially when there's many different error paths or stage like
> > here, though I'm not the maintainer here.
>
> Experiencing it a bit more after rebasing my local patches I think it
> also makes re-odering of code much more difficult as you need to be
> very careful to update the gotos correctly as well as with potential 
> reordering of the cleanup labels and steps.

I have to agree, with more than a couple of goto targets it becomes
painful to check for correctness, and looks awful to maintain. IMHO even
nested if() blocks would be more readable. I'd recommend finding some
other way.

[PATCH v4 3/5] winegstreamer: Add a bin container element to wg_transform.

[Zebediah Figura]

From: Rémi Bernon

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51931
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52391
Signed-off-by: Rémi Bernon 
Signed-off-by: Zebediah Figura 
---
 dlls/winegstreamer/wg_transform.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c
index c309523dd36..ad30653c147 100644
--- a/dlls/winegstreamer/wg_transform.c
+++ b/dlls/winegstreamer/wg_transform.c
@@ -44,6 +44,7 @@ GST_DEBUG_CATEGORY_EXTERN(wine);

struct wg_transform
 {
+    GstElement *container;
     GstPad *my_src, *my_sink;
 };

@@ -62,6 +63,8 @@ NTSTATUS wg_transform_destroy(void *args)
 {
     struct wg_transform *transform = args;

+    gst_element_set_state(transform->container, GST_STATE_NULL);
+    g_object_unref(transform->container);
     g_object_unref(transform->my_sink);
     g_object_unref(transform->my_src);
     free(transform);
@@ -84,9 +87,11 @@ NTSTATUS wg_transform_create(void *args)

if (!(transform = calloc(1, sizeof(*transform))))
         return STATUS_NO_MEMORY;
+    if (!(transform->container = gst_bin_new("wg_transform")))
+        goto out_free_transform;

if (!(src_caps = wg_format_to_caps(&input_format)))
-        goto out_free_transform;
+        goto out_free_container;
     if (!(template = gst_pad_template_new("src", GST_PAD_SRC, GST_PAD_ALWAYS, src_caps)))
         goto out_free_src_caps;
     transform->my_src = gst_pad_new_from_template(template, "src");
@@ -106,6 +111,10 @@ NTSTATUS wg_transform_create(void *args)
     gst_pad_set_element_private(transform->my_sink, transform);
     gst_pad_set_chain_function(transform->my_sink, transform_sink_chain_cb);

+    gst_element_set_state(transform->container, GST_STATE_PAUSED);
+    if (!gst_element_get_state(transform->container, NULL, NULL, -1))
+        goto out_free_sink_pad;
+
     gst_caps_unref(sink_caps);
     gst_caps_unref(src_caps);

@@ -113,12 +122,16 @@ NTSTATUS wg_transform_create(void *args)
     params->transform = transform;
     return STATUS_SUCCESS;

+out_free_sink_pad:
+    gst_object_unref(transform->my_sink);
 out_free_sink_caps:
     gst_caps_unref(sink_caps);
 out_free_src_pad:
     gst_object_unref(transform->my_src);
 out_free_src_caps:
     gst_caps_unref(src_caps);
+out_free_container:
+    gst_object_unref(transform->container);
 out_free_transform:
     free(transform);
     GST_ERROR("Failed to create winegstreamer transform.");

[PATCH v4 4/5] winegstreamer: Append audioconvert and audioresample elements.

[Zebediah Figura]

From: Rémi Bernon

So that we can decode WMA to something else than the default avdec_wmav2
F32LE / non-interleaved format.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51931
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52391
Signed-off-by: Rémi Bernon 
Signed-off-by: Zebediah Figura 
---
 dlls/winegstreamer/unix_private.h |  1 +
 dlls/winegstreamer/wg_parser.c    |  2 +-
 dlls/winegstreamer/wg_transform.c | 54 +++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/dlls/winegstreamer/unix_private.h b/dlls/winegstreamer/unix_private.h
index f9c4da2f6ea..d3f32484ee6 100644
--- a/dlls/winegstreamer/unix_private.h
+++ b/dlls/winegstreamer/unix_private.h
@@ -26,6 +26,7 @@
 #include <gst/gst.h>

extern bool init_gstreamer(void) DECLSPEC_HIDDEN;
+extern GstElement *create_element(const char *name, const char *plugin_set) DECLSPEC_HIDDEN;

extern void wg_format_from_caps(struct wg_format *format, const GstCaps *caps) DECLSPEC_HIDDEN;
 extern bool wg_format_compare(const struct wg_format *a, const struct wg_format *b) DECLSPEC_HIDDEN;
diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c
index e5ed496387e..85c28895159 100644
--- a/dlls/winegstreamer/wg_parser.c
+++ b/dlls/winegstreamer/wg_parser.c
@@ -692,7 +692,7 @@ static gboolean sink_query_cb(GstPad *pad, GstObject *parent, GstQuery *query)
     }
 }

-static GstElement *create_element(const char *name, const char *plugin_set)
+GstElement *create_element(const char *name, const char *plugin_set)
 {
     GstElement *element;

diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c
index ad30653c147..a7f249b15b4 100644
--- a/dlls/winegstreamer/wg_transform.c
+++ b/dlls/winegstreamer/wg_transform.c
@@ -72,11 +72,36 @@ NTSTATUS wg_transform_destroy(void *args)
     return STATUS_SUCCESS;
 }

+static bool transform_append_element(struct wg_transform *transform, GstElement *element,
+        GstElement **first, GstElement **last)
+{
+    gchar *name = gst_element_get_name(element);
+    bool success = false;
+
+    if (!gst_bin_add(GST_BIN(transform->container), element) ||
+            (*last && !gst_element_link(*last, element)))
+    {
+        GST_ERROR("Failed to link %s element.", name);
+    }
+    else
+    {
+        GST_DEBUG("Linked %s element %p.", name, element);
+        if (!*first)
+            *first = element;
+        *last = element;
+        success = true;
+    }
+
+    g_free(name);
+    return success;
+}
+
 NTSTATUS wg_transform_create(void *args)
 {
     struct wg_transform_create_params *params = args;
     struct wg_format output_format = *params->output_format;
     struct wg_format input_format = *params->input_format;
+    GstElement *first = NULL, *last = NULL, *element;
     GstCaps *src_caps = NULL, *sink_caps = NULL;
     NTSTATUS status = STATUS_UNSUCCESSFUL;
     GstPadTemplate *template = NULL;
@@ -115,6 +140,33 @@ NTSTATUS wg_transform_create(void *args)
     if (!gst_element_get_state(transform->container, NULL, NULL, -1))
         goto out_free_sink_pad;

+    switch (output_format.major_type)
+    {
+        case WG_MAJOR_TYPE_AUDIO:
+            /* The MF audio decoder transforms allow decoding to various formats
+             * as well as resampling the audio at the same time, whereas
+             * GStreamer decoder plugins usually only support decoding to a
+             * single format and at the original rate.
+             *
+             * The WMA decoder transform also has output samples interleaved on
+             * Windows, whereas GStreamer avdec_wmav2 output uses
+             * non-interleaved format.
+             */
+            if (!(element = create_element("audioconvert", "base"))
+                    || !transform_append_element(transform, element, &first, &last))
+                goto out_stop_container;
+            if (!(element = create_element("audioresample", "base"))
+                    || !transform_append_element(transform, element, &first, &last))
+                goto out_stop_container;
+            break;
+
+        case WG_MAJOR_TYPE_VIDEO:
+        case WG_MAJOR_TYPE_WMA:
+        case WG_MAJOR_TYPE_UNKNOWN:
+            GST_FIXME("Format %u not implemented!", output_format.major_type);
+            goto out_stop_container;
+    }
+
     gst_caps_unref(sink_caps);
     gst_caps_unref(src_caps);

@@ -122,6 +174,8 @@ NTSTATUS wg_transform_create(void *args)
     params->transform = transform;
     return STATUS_SUCCESS;

+out_stop_container:
+    gst_element_set_state(transform->container, GST_STATE_NULL);
 out_free_sink_pad:
     gst_object_unref(transform->my_sink);
 out_free_sink_caps:

[PATCH v4 4/5] winegstreamer: Append audioconvert and audioresample elements.

[Rémi Bernon]

On 2/22/22 18:43, Zebediah Figura wrote:
> - switch (output_format.major_type)
> - {
> -
>    case WG_MAJOR_TYPE_AUDIO:
> -
>        /* The MF audio decoder transforms allow decoding to various formats
> -
>         * as well as resampling the audio at the same time, whereas
> -
>         * GStreamer decoder plugins usually only support decoding to a
> -
>         * single format and at the original rate.
> -
>         *
> -
>         * The WMA decoder transform also has output samples interleaved on
> -
>         * Windows, whereas GStreamer avdec_wmav2 output uses
> -
>         * non-interleaved format.
> -
>         */
> -
>        if (!(element = create_element("audioconvert", "base"))
> -
>                || !transform_append_element(transform, element, &first, &last))
> -
>            goto out_stop_container;
> -
>        if (!(element = create_element("audioresample", "base"))
> -
>                || !transform_append_element(transform, element, &first, &last))
> -
>            goto out_stop_container;
> -
>        break;
> -
> -
>    case WG_MAJOR_TYPE_VIDEO:
> -
>    case WG_MAJOR_TYPE_WMA:
> -
>    case WG_MAJOR_TYPE_UNKNOWN:
> -
>        GST_FIXME("Format %u not implemented!", output_format.major_type);
> -
>        goto out_stop_container;
> - }
> -
> gst_caps_unref(sink_caps);
> gst_caps_unref(src_caps);

I'm not sure which one is right, but starting the bin before appending 
the elements, and in a later patch getting the elements pads, linking 
them with ours and activating them, causes the pipeline to fail to run.

Starting the bin only after everything is setup, like it was done in the 
previous version worked.

Also, in a later patch and for H264 we will want to modify sink_caps to 
remove frame size information, I was doing it in this switch but that 
won't work if it's below my_sink creation.

[PATCH v4 4/5] winegstreamer: Append audioconvert and audioresample elements.

[Zebediah Figura (she/her)]

On 2/22/22 12:07, Rémi Bernon wrote:
> I'm not sure which one is right, but starting the bin before appending
> the elements, and in a later patch getting the elements pads, linking
> them with ours and activating them, causes the pipeline to fail to run.
>
> Starting the bin only after everything is setup, like it was done in the
> previous version worked.

Hmm, I was under the impression it's legal to add an element to an
already running bin; GStreamer certainly has code to handle that case.
What does "fail to run" mean in this case?
> Also, in a later patch and for H264 we will want to modify sink_caps to
> remove frame size information, I was doing it in this switch but that
> won't work if it's below my_sink creation.

Since it doesn't really matter I suppose I'll send a v5 to rearrange
things...

[PATCH v4 4/5] winegstreamer: Append audioconvert and audioresample elements.

[Rémi Bernon]

On 2/22/22 19:15, Zebediah Figura (she/her) wrote:
> On 2/22/22 12:07, Rémi Bernon wrote:
> > I'm not sure which one is right, but starting the bin before appending
> > the elements, and in a later patch getting the elements pads, linking
> > them with ours and activating them, causes the pipeline to fail to run.
> >
> > Starting the bin only after everything is setup, like it was done in the
> > previous version worked.
>
> Hmm, I was under the impression it's legal to add an element to an
> already running bin; GStreamer certainly has code to handle that case.
> What does "fail to run" mean in this case?

The initial gst_pad_push_event events and later gst_push_buffer fail, 
and the pads are in a flushing state.

[PATCH v4 4/5] winegstreamer: Append audioconvert and audioresample elements.

[Zebediah Figura (she/her)]

On 2/22/22 12:17, Rémi Bernon wrote:
> On 2/22/22 19:15, Zebediah Figura (she/her) wrote:
> > On 2/22/22 12:07, Rémi Bernon wrote:
> > > I'm not sure which one is right, but starting the bin before appending
> > > the elements, and in a later patch getting the elements pads, linking
> > > them with ours and activating them, causes the pipeline to fail to run.
> > >
> > > Starting the bin only after everything is setup, like it was done in the
> > > previous version worked.
> >
> > Hmm, I was under the impression it's legal to add an element to an
> > already running bin; GStreamer certainly has code to handle that case.
> > What does "fail to run" mean in this case?
>
> The initial gst_pad_push_event events and later gst_push_buffer fail,
> and the pads are in a flushing state.

Eh, never mind, I forgot you had to manually call
gst_element_sync_state_with_parent()...

[PATCH v4 5/5] winegstreamer: Lookup and instantiate a WMA decoder element.

[Zebediah Figura]

From: Rémi Bernon

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51931
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52391
Signed-off-by: Rémi Bernon 
Signed-off-by: Zebediah Figura 
---
 dlls/winegstreamer/wg_transform.c | 76 ++++++++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 1 deletion(-)

diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c
index a7f249b15b4..0d44292db6c 100644
--- a/dlls/winegstreamer/wg_transform.c
+++ b/dlls/winegstreamer/wg_transform.c
@@ -72,6 +72,50 @@ NTSTATUS wg_transform_destroy(void *args)
     return STATUS_SUCCESS;
 }

+static GstElement *transform_find_element(GstElementFactoryListType type, GstCaps *src_caps, GstCaps *sink_caps)
+{
+    GstElement *element = NULL;
+    GList *tmp, *transforms;
+    const gchar *name;
+
+    if (!(transforms = gst_element_factory_list_get_elements(type, GST_RANK_MARGINAL)))
+        goto done;
+
+    tmp = gst_element_factory_list_filter(transforms, src_caps, GST_PAD_SINK, FALSE);
+    gst_plugin_feature_list_free(transforms);
+    if (!(transforms = tmp))
+        goto done;
+
+    tmp = gst_element_factory_list_filter(transforms, sink_caps, GST_PAD_SRC, FALSE);
+    gst_plugin_feature_list_free(transforms);
+    if (!(transforms = tmp))
+        goto done;
+
+    transforms = g_list_sort(transforms, gst_plugin_feature_rank_compare_func);
+    for (tmp = transforms; tmp != NULL && element == NULL; tmp = tmp->next)
+    {
+        name = gst_plugin_feature_get_name(GST_PLUGIN_FEATURE(tmp->data));
+        if (!(element = gst_element_factory_create(GST_ELEMENT_FACTORY(tmp->data), NULL)))
+            GST_WARNING("Failed to create %s element.", name);
+    }
+    gst_plugin_feature_list_free(transforms);
+
+done:
+    if (element)
+    {
+        GST_DEBUG("Created %s element %p.", name, element);
+    }
+    else
+    {
+        gchar *src_str = gst_caps_to_string(src_caps), *sink_str = gst_caps_to_string(sink_caps);
+        GST_WARNING("Failed to create transform matching caps %s / %s.", src_str, sink_str);
+        g_free(sink_str);
+        g_free(src_str);
+    }
+
+    return element;
+}
+
 static bool transform_append_element(struct wg_transform *transform, GstElement *element,
         GstElement **first, GstElement **last)
 {
@@ -99,13 +143,14 @@ static bool transform_append_element(struct wg_transform *transform, GstElement
 NTSTATUS wg_transform_create(void *args)
 {
     struct wg_transform_create_params *params = args;
+    GstCaps *raw_caps = NULL, *src_caps = NULL, *sink_caps = NULL;
     struct wg_format output_format = *params->output_format;
     struct wg_format input_format = *params->input_format;
     GstElement *first = NULL, *last = NULL, *element;
-    GstCaps *src_caps = NULL, *sink_caps = NULL;
     NTSTATUS status = STATUS_UNSUCCESSFUL;
     GstPadTemplate *template = NULL;
     struct wg_transform *transform;
+    const gchar *media_type;

if (!init_gstreamer())
         return STATUS_UNSUCCESSFUL;
@@ -140,6 +185,35 @@ NTSTATUS wg_transform_create(void *args)
     if (!gst_element_get_state(transform->container, NULL, NULL, -1))
         goto out_free_sink_pad;

+    /* Since we append conversion elements, we don't want to filter decoders
+     * based on the actual output caps now. Matching decoders with the
+     * raw output media type should be enough.
+     */
+    media_type = gst_structure_get_name(gst_caps_get_structure(sink_caps, 0));
+    if (!(raw_caps = gst_caps_new_empty_simple(media_type)))
+        goto out_stop_container;
+
+    switch (input_format.major_type)
+    {
+        case WG_MAJOR_TYPE_WMA:
+            if (!(element = transform_find_element(GST_ELEMENT_FACTORY_TYPE_DECODER, src_caps, raw_caps))
+                    || !transform_append_element(transform, element, &first, &last))
+            {
+                gst_caps_unref(raw_caps);
+                goto out_stop_container;
+            }
+            break;
+
+        case WG_MAJOR_TYPE_AUDIO:
+        case WG_MAJOR_TYPE_VIDEO:
+        case WG_MAJOR_TYPE_UNKNOWN:
+            GST_FIXME("Format %u not implemented!", input_format.major_type);
+            gst_caps_unref(raw_caps);
+            goto out_stop_container;
+    }
+
+    gst_caps_unref(raw_caps);
+
     switch (output_format.major_type)
     {
         case WG_MAJOR_TYPE_AUDIO: