[PATCH 2/7] winegstreamer: Support MPEG-1 audio in wg_transform.

[Zebediah Figura (she/her)]

On 5/3/22 13:28, Anton Baskanov wrote:
> Signed-off-by: Anton Baskanov <baskanov at gmail.com>
> ---
>   dlls/winegstreamer/wg_format.c    | 17 ++++++++++++++++-
>   dlls/winegstreamer/wg_transform.c |  6 +++++-
>   2 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/dlls/winegstreamer/wg_format.c b/dlls/winegstreamer/wg_format.c
> index 608070b78e8..32d5a35e41a 100644
> --- a/dlls/winegstreamer/wg_format.c
> +++ b/dlls/winegstreamer/wg_format.c
> @@ -331,6 +331,21 @@ static void wg_channel_mask_to_gst(GstAudioChannelPosition *positions, uint32_t
>       }
>   }
>   
> +static GstCaps *wg_format_to_caps_mpeg1_audio(const struct wg_format *format)
> +{
> +    GstCaps *caps;
> +
> +    if (!(caps = gst_caps_new_empty_simple("audio/mpeg")))
> +        return NULL;
> +
> +    gst_caps_set_simple(caps, "mpegversion", G_TYPE_INT, 1, NULL);
> +    gst_caps_set_simple(caps, "layer", G_TYPE_INT, format->u.mpeg1_audio.layer, NULL);
> +    gst_caps_set_simple(caps, "rate", G_TYPE_INT, format->u.mpeg1_audio.rate, NULL);
> +    gst_caps_set_simple(caps, "channels", G_TYPE_INT, format->u.mpeg1_audio.channels, NULL);

Should we set "parsed: true" here?

> +
> +    return caps;
> +}
> +
>   static GstCaps *wg_format_to_caps_audio(const struct wg_format *format)
>   {
>       GstAudioChannelPosition positions[32];
> @@ -495,7 +510,7 @@ GstCaps *wg_format_to_caps(const struct wg_format *format)
>           case WG_MAJOR_TYPE_UNKNOWN:
>               return gst_caps_new_any();
>           case WG_MAJOR_TYPE_MPEG1_AUDIO:
> -            return NULL;
> +            return wg_format_to_caps_mpeg1_audio(format);
>           case WG_MAJOR_TYPE_WMA:
>               return wg_format_to_caps_wma(format);
>           case WG_MAJOR_TYPE_H264:
> diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c
> index dca85e7366e..3af9822b016 100644
> --- a/dlls/winegstreamer/wg_transform.c
> +++ b/dlls/winegstreamer/wg_transform.c
> @@ -114,6 +114,10 @@ static GstElement *transform_find_element(GstElementFactoryListType type, GstCap
>       for (tmp = transforms; tmp != NULL && element == NULL; tmp = tmp->next)
>       {
>           name = gst_plugin_feature_get_name(GST_PLUGIN_FEATURE(tmp->data));
> +        /* Ignore mpg123audiodec for now, as it has issues with frame delay,
> +         * e.g. it does not output any data until it receives 2 frames. */
> +        if (!strcmp(name, "mpg123audiodec"))
> +            continue;

I'd really rather not do this. Does this end up being a problem in 
practice, and if so, how? (E.g. if all it does is break tests, I'd 
rather just make the tests less restrictive, or mark things as todo_wine.)

Actually, I find this surprising anyway. IIRC mpg123 acted completely 
synchronously when I wrote the mp3dmod tests, and just looking at the 
GStreamer source I don't see any reason why mpg123audiodec wouldn't work 
synchronously. (Maybe it's different for layer 3, but I can't imagine 
why...)

>           if (!(element = gst_element_factory_create(GST_ELEMENT_FACTORY(tmp->data), NULL)))
>               GST_WARNING("Failed to create %s element.", name);
>       }
> @@ -224,6 +228,7 @@ NTSTATUS wg_transform_create(void *args)
>                */
>               transform->input_max_length = 16;
>               /* fallthrough */
> +        case WG_MAJOR_TYPE_MPEG1_AUDIO:
>           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))
> @@ -233,7 +238,6 @@ NTSTATUS wg_transform_create(void *args)
>               }
>               break;
>   
> -        case WG_MAJOR_TYPE_MPEG1_AUDIO:
>           case WG_MAJOR_TYPE_AUDIO:
>           case WG_MAJOR_TYPE_VIDEO:
>           case WG_MAJOR_TYPE_UNKNOWN: