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

[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 <rbernon(a)codeweavers.com>
---
 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 5a2e970a4dd..37da2d1e1c4 100644
--- a/dlls/winegstreamer/wg_parser.c
+++ b/dlls/winegstreamer/wg_parser.c
@@ -1443,7 +1443,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;
     }
 
@@ -1472,7 +1472,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;
     }
 
@@ -1493,7 +1493,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;
     }
 
@@ -1503,7 +1503,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);
@@ -1527,7 +1527,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;
     }
 
@@ -1538,7 +1538,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);
@@ -1566,7 +1566,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);
 }
 
@@ -1604,7 +1604,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;
 }
-- 
2.34.1

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

[Rémi Bernon]

And return STATUS_UNSUCCESSFUL on error.

Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
---
 dlls/winegstreamer/wg_transform.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c
index e4545774428..81badd0be67 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,15 +75,13 @@ 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;
 
@@ -124,8 +120,11 @@ done:
     if (status)
     {
         GST_ERROR("Failed to create winegstreamer transform.");
-        if (transform)
-            wg_transform_destroy(transform);
+        if (transform->my_sink)
+            g_object_unref(transform->my_sink);
+        if (transform->my_src)
+            g_object_unref(transform->my_src);
+        free(transform);
     }
     else
     {
-- 
2.34.1

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

[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
---

v2: * Refactor the error cleanup path so that the destroy function
       doesn't need to handle partially constructed transform.

     * Small tweaks to the code to make it terser and remove unnecessary
       variables.
-- 
Rémi Bernon <rbernon(a)codeweavers.com>

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

[Chip Davis]

February 22, 2022 2:37 AM, "Rémi Bernon" <rbernon(a)codeweavers.com> wrote:
> diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c
> index e4545774428..81badd0be67 100644
> --- a/dlls/winegstreamer/wg_transform.c
> +++ b/dlls/winegstreamer/wg_transform.c
> @@ -77,15 +75,13 @@ 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;

Wait, won't this...
> @@ -124,8 +120,11 @@ done:
>      if (status)
>      {
>          GST_ERROR("Failed to create winegstreamer transform.");
> -        if (transform)
> -            wg_transform_destroy(transform);
> +        if (transform->my_sink)
> +            g_object_unref(transform->my_sink);
> +        if (transform->my_src)
> +            g_object_unref(transform->my_src);

...result in a null dereference here?
> +        free(transform);
>      }
>      else
>      {
> -- 
> 2.34.1

Chip

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

[Rémi Bernon]

On 2/22/22 09:58, Chip Davis wrote:
> February 22, 2022 2:37 AM, "Rémi Bernon" <rbernon(a)codeweavers.com> wrote:
> > diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c
> > index e4545774428..81badd0be67 100644
> > --- a/dlls/winegstreamer/wg_transform.c
> > +++ b/dlls/winegstreamer/wg_transform.c
> > @@ -77,15 +75,13 @@ 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;
>
> Wait, won't this...
> > @@ -124,8 +120,11 @@ done:
> >       if (status)
> >       {
> >           GST_ERROR("Failed to create winegstreamer transform.");
> > -        if (transform)
> > -            wg_transform_destroy(transform);
> > +        if (transform->my_sink)
> > +            g_object_unref(transform->my_sink);
> > +        if (transform->my_src)
> > +            g_object_unref(transform->my_src);
>
> ...result in a null dereference here?
> > +        free(transform);
> >       }
> >       else
> >       {
> > -- 
> > 2.34.1
>
> Chip

Eh, indeed... thanks.
-- 
Rémi Bernon <rbernon(a)codeweavers.com>

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

[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 <rbernon(a)codeweavers.com>
---
 dlls/winegstreamer/wg_transform.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c
index 81badd0be67..ca4a5de9fa7 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,6 +87,8 @@ NTSTATUS wg_transform_create(void *args)
 
     if (!(transform = calloc(1, sizeof(*transform))))
         goto done;
+    if (!(transform->container = gst_bin_new("wg_transform")))
+        goto done;
 
     if (!(src_caps = wg_format_to_caps(&input_format)))
         goto done;
@@ -107,6 +112,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 done;
+
     status = STATUS_SUCCESS;
 
 done:
@@ -120,6 +129,11 @@ done:
     if (status)
     {
         GST_ERROR("Failed to create winegstreamer transform.");
+        if (transform->container)
+        {
+            gst_element_set_state(transform->container, GST_STATE_NULL);
+            g_object_unref(transform->container);
+        }
         if (transform->my_sink)
             g_object_unref(transform->my_sink);
         if (transform->my_src)
-- 
2.34.1

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

[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 <rbernon(a)codeweavers.com>
---
 dlls/winegstreamer/unix_private.h |  1 +
 dlls/winegstreamer/wg_parser.c    |  2 +-
 dlls/winegstreamer/wg_transform.c | 50 +++++++++++++++++++++++++++++++
 3 files changed, 52 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 37da2d1e1c4..d9bbc60964e 100644
--- a/dlls/winegstreamer/wg_parser.c
+++ b/dlls/winegstreamer/wg_parser.c
@@ -699,7 +699,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 ca4a5de9fa7..29fe4618f07 100644
--- a/dlls/winegstreamer/wg_transform.c
+++ b/dlls/winegstreamer/wg_transform.c
@@ -72,11 +72,34 @@ 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;
@@ -95,6 +118,33 @@ NTSTATUS wg_transform_create(void *args)
     if (!(sink_caps = wg_format_to_caps(&output_format)))
         goto done;
 
+    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, where 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,
+         * where GStreamer avdec_wmav2 output uses non-interleaved format.
+         *
+         * Adding an audioconvert and audioresample elements will make the required
+         * transformation to match the desired output format.
+         */
+        if (!(element = create_element("audioconvert", "base")) ||
+                !transform_append_element(transform, element, &first, &last))
+            goto done;
+        if (!(element = create_element("audioresample", "base")) ||
+                !transform_append_element(transform, element, &first, &last))
+            goto done;
+        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 done;
+    }
+
     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")))
-- 
2.34.1

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

[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 <rbernon(a)codeweavers.com>
---
 dlls/winegstreamer/wg_transform.c | 70 ++++++++++++++++++++++++++++++-
 1 file changed, 69 insertions(+), 1 deletion(-)

diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c
index 29fe4618f07..4a92ebfc9e4 100644
--- a/dlls/winegstreamer/wg_transform.c
+++ b/dlls/winegstreamer/wg_transform.c
@@ -72,6 +72,49 @@ NTSTATUS wg_transform_destroy(void *args)
     return STATUS_SUCCESS;
 }
 
+static GstElement *tranform_find_element(GstElementFactoryListType type, GstCaps *src_caps, GstCaps *sink_caps)
+{
+    GstElement *element = NULL;
+    GList *tmp, *transforms;
+    gchar *name;
+
+    transforms = gst_element_factory_list_get_elements(type, GST_RANK_MARGINAL);
+    if (!transforms)
+        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)
 {
@@ -97,13 +140,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;
@@ -118,6 +162,28 @@ NTSTATUS wg_transform_create(void *args)
     if (!(sink_caps = wg_format_to_caps(&output_format)))
         goto done;
 
+    /* As we append conversion elements we don't want to filter decoders
+     * based on the actual output format 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 done;
+
+    switch (input_format.major_type)
+    {
+    case WG_MAJOR_TYPE_WMA:
+        if (!(element = tranform_find_element(GST_ELEMENT_FACTORY_TYPE_DECODER, src_caps, raw_caps)) ||
+                !transform_append_element(transform, element, &first, &last))
+            goto done;
+        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);
+        goto done;
+    }
+
     switch (output_format.major_type)
     {
     case WG_MAJOR_TYPE_AUDIO:
@@ -171,6 +237,8 @@ NTSTATUS wg_transform_create(void *args)
 done:
     if (template)
         g_object_unref(template);
+    if (raw_caps)
+        gst_caps_unref(raw_caps);
     if (sink_caps)
         gst_caps_unref(sink_caps);
     if (src_caps)
-- 
2.34.1