[PATCH 1/7] winegstreamer: Convert MPEG-1 audio to a major type.

[Anton Baskanov]

Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
---
 dlls/winegstreamer/mfplat.c        |   1 +
 dlls/winegstreamer/quartz_parser.c | 151 ++++++++++++++++-------------
 dlls/winegstreamer/unixlib.h       |  11 ++-
 dlls/winegstreamer/wg_format.c     |  22 ++---
 dlls/winegstreamer/wg_transform.c  |   2 +
 dlls/winegstreamer/wm_reader.c     |   4 +
 6 files changed, 106 insertions(+), 85 deletions(-)

diff --git a/dlls/winegstreamer/mfplat.c b/dlls/winegstreamer/mfplat.c
index 97e27bb7301..4d8476d7b49 100644
--- a/dlls/winegstreamer/mfplat.c
+++ b/dlls/winegstreamer/mfplat.c
@@ -661,6 +661,7 @@ IMFMediaType *mf_media_type_from_wg_format(const struct wg_format *format)
     {
         case WG_MAJOR_TYPE_H264:
         case WG_MAJOR_TYPE_WMA:
+        case WG_MAJOR_TYPE_MPEG1_AUDIO:
             FIXME("Format %u not implemented!\n", format->major_type);
             /* fallthrough */
         case WG_MAJOR_TYPE_UNKNOWN:
diff --git a/dlls/winegstreamer/quartz_parser.c b/dlls/winegstreamer/quartz_parser.c
index 7b82f3efbc9..3e00813ae75 100644
--- a/dlls/winegstreamer/quartz_parser.c
+++ b/dlls/winegstreamer/quartz_parser.c
@@ -111,50 +111,6 @@ static bool amt_from_wg_format_audio(AM_MEDIA_TYPE *mt, const struct wg_format *
     case WG_AUDIO_FORMAT_UNKNOWN:
         return false;
 
-    case WG_AUDIO_FORMAT_MPEG1_LAYER1:
-    case WG_AUDIO_FORMAT_MPEG1_LAYER2:
-    {
-        MPEG1WAVEFORMAT *wave_format;
-
-        if (!(wave_format = CoTaskMemAlloc(sizeof(*wave_format))))
-            return false;
-        memset(wave_format, 0, sizeof(*wave_format));
-
-        mt->subtype = MEDIASUBTYPE_MPEG1AudioPayload;
-        mt->cbFormat = sizeof(*wave_format);
-        mt->pbFormat = (BYTE *)wave_format;
-        wave_format->wfx.wFormatTag = WAVE_FORMAT_MPEG;
-        wave_format->wfx.nChannels = format->u.audio.channels;
-        wave_format->wfx.nSamplesPerSec = format->u.audio.rate;
-        wave_format->wfx.cbSize = sizeof(*wave_format) - sizeof(WAVEFORMATEX);
-        wave_format->fwHeadLayer = (format->u.audio.format == WG_AUDIO_FORMAT_MPEG1_LAYER1 ? 1 : 2);
-        return true;
-    }
-
-    case WG_AUDIO_FORMAT_MPEG1_LAYER3:
-    {
-        MPEGLAYER3WAVEFORMAT *wave_format;
-
-        if (!(wave_format = CoTaskMemAlloc(sizeof(*wave_format))))
-            return false;
-        memset(wave_format, 0, sizeof(*wave_format));
-
-        mt->subtype = MEDIASUBTYPE_MP3;
-        mt->cbFormat = sizeof(*wave_format);
-        mt->pbFormat = (BYTE *)wave_format;
-        wave_format->wfx.wFormatTag = WAVE_FORMAT_MPEGLAYER3;
-        wave_format->wfx.nChannels = format->u.audio.channels;
-        wave_format->wfx.nSamplesPerSec = format->u.audio.rate;
-        wave_format->wfx.cbSize = sizeof(*wave_format) - sizeof(WAVEFORMATEX);
-        /* FIXME: We can't get most of the MPEG data from the caps. We may have
-         * to manually parse the header. */
-        wave_format->wID = MPEGLAYER3_ID_MPEG;
-        wave_format->fdwFlags = MPEGLAYER3_FLAG_PADDING_ON;
-        wave_format->nFramesPerBlock = 1;
-        wave_format->nCodecDelay = 1393;
-        return true;
-    }
-
     case WG_AUDIO_FORMAT_U8:
     case WG_AUDIO_FORMAT_S16LE:
     case WG_AUDIO_FORMAT_S24LE:
@@ -238,6 +194,62 @@ static bool amt_from_wg_format_audio(AM_MEDIA_TYPE *mt, const struct wg_format *
     return false;
 }
 
+static bool amt_from_wg_format_mpeg1_audio(AM_MEDIA_TYPE *mt, const struct wg_format *format)
+{
+    mt->majortype = MEDIATYPE_Audio;
+    mt->formattype = FORMAT_WaveFormatEx;
+
+    switch (format->u.mpeg1_audio.layer)
+    {
+    case 1:
+    case 2:
+    {
+        MPEG1WAVEFORMAT *wave_format;
+
+        if (!(wave_format = CoTaskMemAlloc(sizeof(*wave_format))))
+            return false;
+        memset(wave_format, 0, sizeof(*wave_format));
+
+        mt->subtype = MEDIASUBTYPE_MPEG1AudioPayload;
+        mt->cbFormat = sizeof(*wave_format);
+        mt->pbFormat = (BYTE *)wave_format;
+        wave_format->wfx.wFormatTag = WAVE_FORMAT_MPEG;
+        wave_format->wfx.nChannels = format->u.mpeg1_audio.channels;
+        wave_format->wfx.nSamplesPerSec = format->u.mpeg1_audio.rate;
+        wave_format->wfx.cbSize = sizeof(*wave_format) - sizeof(WAVEFORMATEX);
+        wave_format->fwHeadLayer = format->u.mpeg1_audio.layer;
+        return true;
+    }
+
+    case 3:
+    {
+        MPEGLAYER3WAVEFORMAT *wave_format;
+
+        if (!(wave_format = CoTaskMemAlloc(sizeof(*wave_format))))
+            return false;
+        memset(wave_format, 0, sizeof(*wave_format));
+
+        mt->subtype = MEDIASUBTYPE_MP3;
+        mt->cbFormat = sizeof(*wave_format);
+        mt->pbFormat = (BYTE *)wave_format;
+        wave_format->wfx.wFormatTag = WAVE_FORMAT_MPEGLAYER3;
+        wave_format->wfx.nChannels = format->u.mpeg1_audio.channels;
+        wave_format->wfx.nSamplesPerSec = format->u.mpeg1_audio.rate;
+        wave_format->wfx.cbSize = sizeof(*wave_format) - sizeof(WAVEFORMATEX);
+        /* FIXME: We can't get most of the MPEG data from the caps. We may have
+         * to manually parse the header. */
+        wave_format->wID = MPEGLAYER3_ID_MPEG;
+        wave_format->fdwFlags = MPEGLAYER3_FLAG_PADDING_ON;
+        wave_format->nFramesPerBlock = 1;
+        wave_format->nCodecDelay = 1393;
+        return true;
+    }
+    }
+
+    assert(0);
+    return false;
+}
+
 #define ALIGN(n, alignment) (((n) + (alignment) - 1) & ~((alignment) - 1))
 
 unsigned int wg_format_get_max_size(const struct wg_format *format)
@@ -312,15 +324,6 @@ unsigned int wg_format_get_max_size(const struct wg_format *format)
                 case WG_AUDIO_FORMAT_F64LE:
                     return rate * channels * 8;
 
-                case WG_AUDIO_FORMAT_MPEG1_LAYER1:
-                    return 56000;
-
-                case WG_AUDIO_FORMAT_MPEG1_LAYER2:
-                    return 48000;
-
-                case WG_AUDIO_FORMAT_MPEG1_LAYER3:
-                    return 40000;
-
                 case WG_AUDIO_FORMAT_UNKNOWN:
                     FIXME("Cannot guess maximum sample size for unknown audio format.\n");
                     return 0;
@@ -328,6 +331,22 @@ unsigned int wg_format_get_max_size(const struct wg_format *format)
             break;
         }
 
+        case WG_MAJOR_TYPE_MPEG1_AUDIO:
+            switch (format->u.mpeg1_audio.layer)
+            {
+            case 1:
+                return 56000;
+
+            case 2:
+                return 48000;
+
+            case 3:
+                return 40000;
+            }
+            assert(0);
+            return 0;
+
+
         case WG_MAJOR_TYPE_H264:
         case WG_MAJOR_TYPE_WMA:
             FIXME("Format %u not implemented!\n", format->major_type);
@@ -431,6 +450,9 @@ bool amt_from_wg_format(AM_MEDIA_TYPE *mt, const struct wg_format *format, bool
     case WG_MAJOR_TYPE_UNKNOWN:
         return false;
 
+    case WG_MAJOR_TYPE_MPEG1_AUDIO:
+        return amt_from_wg_format_mpeg1_audio(mt, format);
+
     case WG_MAJOR_TYPE_AUDIO:
         return amt_from_wg_format_audio(mt, format);
 
@@ -526,17 +548,10 @@ static bool amt_to_wg_format_audio_mpeg1(const AM_MEDIA_TYPE *mt, struct wg_form
         return false;
     }
 
-    format->major_type = WG_MAJOR_TYPE_AUDIO;
-    format->u.audio.channels = audio_format->wfx.nChannels;
-    format->u.audio.rate = audio_format->wfx.nSamplesPerSec;
-    if (audio_format->fwHeadLayer == 1)
-        format->u.audio.format = WG_AUDIO_FORMAT_MPEG1_LAYER1;
-    else if (audio_format->fwHeadLayer == 2)
-        format->u.audio.format = WG_AUDIO_FORMAT_MPEG1_LAYER2;
-    else if (audio_format->fwHeadLayer == 3)
-        format->u.audio.format = WG_AUDIO_FORMAT_MPEG1_LAYER3;
-    else
-        return false;
+    format->major_type = WG_MAJOR_TYPE_MPEG1_AUDIO;
+    format->u.mpeg1_audio.channels = audio_format->wfx.nChannels;
+    format->u.mpeg1_audio.rate = audio_format->wfx.nSamplesPerSec;
+    format->u.mpeg1_audio.layer = audio_format->fwHeadLayer;
     return true;
 }
 
@@ -555,10 +570,10 @@ static bool amt_to_wg_format_audio_mpeg1_layer3(const AM_MEDIA_TYPE *mt, struct
         return false;
     }
 
-    format->major_type = WG_MAJOR_TYPE_AUDIO;
-    format->u.audio.channels = audio_format->wfx.nChannels;
-    format->u.audio.rate = audio_format->wfx.nSamplesPerSec;
-    format->u.audio.format = WG_AUDIO_FORMAT_MPEG1_LAYER3;
+    format->major_type = WG_MAJOR_TYPE_MPEG1_AUDIO;
+    format->u.mpeg1_audio.channels = audio_format->wfx.nChannels;
+    format->u.mpeg1_audio.rate = audio_format->wfx.nSamplesPerSec;
+    format->u.mpeg1_audio.layer = 3;
     return true;
 }
 
diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h
index f4e2ea4966b..881eb720b6c 100644
--- a/dlls/winegstreamer/unixlib.h
+++ b/dlls/winegstreamer/unixlib.h
@@ -37,6 +37,7 @@ struct wg_format
         WG_MAJOR_TYPE_UNKNOWN,
         WG_MAJOR_TYPE_VIDEO,
         WG_MAJOR_TYPE_AUDIO,
+        WG_MAJOR_TYPE_MPEG1_AUDIO,
         WG_MAJOR_TYPE_WMA,
         WG_MAJOR_TYPE_H264,
     } major_type;
@@ -80,10 +81,6 @@ struct wg_format
                 WG_AUDIO_FORMAT_S32LE,
                 WG_AUDIO_FORMAT_F32LE,
                 WG_AUDIO_FORMAT_F64LE,
-
-                WG_AUDIO_FORMAT_MPEG1_LAYER1,
-                WG_AUDIO_FORMAT_MPEG1_LAYER2,
-                WG_AUDIO_FORMAT_MPEG1_LAYER3,
             } format;
 
             uint32_t channels;
@@ -91,6 +88,12 @@ struct wg_format
             uint32_t rate;
         } audio;
         struct
+        {
+            uint32_t layer;
+            uint32_t rate;
+            uint32_t channels;
+        } mpeg1_audio;
+        struct
         {
             uint32_t version;
             uint32_t bitrate;
diff --git a/dlls/winegstreamer/wg_format.c b/dlls/winegstreamer/wg_format.c
index ff9238a6a69..608070b78e8 100644
--- a/dlls/winegstreamer/wg_format.c
+++ b/dlls/winegstreamer/wg_format.c
@@ -177,7 +177,7 @@ static void wg_format_from_video_info(struct wg_format *format, const GstVideoIn
     format->u.video.fps_d = GST_VIDEO_INFO_FPS_D(info);
 }
 
-static void wg_format_from_caps_audio_mpeg(struct wg_format *format, const GstCaps *caps)
+static void wg_format_from_caps_audio_mpeg1(struct wg_format *format, const GstCaps *caps)
 {
     const GstStructure *structure = gst_caps_get_structure(caps, 0);
     gint layer, channels, rate;
@@ -198,17 +198,10 @@ static void wg_format_from_caps_audio_mpeg(struct wg_format *format, const GstCa
         return;
     }
 
-    format->major_type = WG_MAJOR_TYPE_AUDIO;
-
-    if (layer == 1)
-        format->u.audio.format = WG_AUDIO_FORMAT_MPEG1_LAYER1;
-    else if (layer == 2)
-        format->u.audio.format = WG_AUDIO_FORMAT_MPEG1_LAYER2;
-    else if (layer == 3)
-        format->u.audio.format = WG_AUDIO_FORMAT_MPEG1_LAYER3;
-
-    format->u.audio.channels = channels;
-    format->u.audio.rate = rate;
+    format->major_type = WG_MAJOR_TYPE_MPEG1_AUDIO;
+    format->u.mpeg1_audio.layer = layer;
+    format->u.mpeg1_audio.channels = channels;
+    format->u.mpeg1_audio.rate = rate;
 }
 
 static void wg_format_from_caps_video_cinepak(struct wg_format *format, const GstCaps *caps)
@@ -263,7 +256,7 @@ void wg_format_from_caps(struct wg_format *format, const GstCaps *caps)
     }
     else if (!strcmp(name, "audio/mpeg"))
     {
-        wg_format_from_caps_audio_mpeg(format, caps);
+        wg_format_from_caps_audio_mpeg1(format, caps);
     }
     else if (!strcmp(name, "video/x-cinepak"))
     {
@@ -501,6 +494,8 @@ 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;
         case WG_MAJOR_TYPE_WMA:
             return wg_format_to_caps_wma(format);
         case WG_MAJOR_TYPE_H264:
@@ -521,6 +516,7 @@ bool wg_format_compare(const struct wg_format *a, const struct wg_format *b)
 
     switch (a->major_type)
     {
+        case WG_MAJOR_TYPE_MPEG1_AUDIO:
         case WG_MAJOR_TYPE_WMA:
         case WG_MAJOR_TYPE_H264:
             GST_FIXME("Format %u not implemented!", a->major_type);
diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c
index 49c7bfaa927..dca85e7366e 100644
--- a/dlls/winegstreamer/wg_transform.c
+++ b/dlls/winegstreamer/wg_transform.c
@@ -233,6 +233,7 @@ 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:
@@ -266,6 +267,7 @@ NTSTATUS wg_transform_create(void *args)
         case WG_MAJOR_TYPE_VIDEO:
             break;
 
+        case WG_MAJOR_TYPE_MPEG1_AUDIO:
         case WG_MAJOR_TYPE_H264:
         case WG_MAJOR_TYPE_WMA:
         case WG_MAJOR_TYPE_UNKNOWN:
diff --git a/dlls/winegstreamer/wm_reader.c b/dlls/winegstreamer/wm_reader.c
index 57ba8633a84..03adea8a318 100644
--- a/dlls/winegstreamer/wm_reader.c
+++ b/dlls/winegstreamer/wm_reader.c
@@ -1686,6 +1686,7 @@ HRESULT wm_reader_get_output_format_count(struct wm_reader *reader, DWORD output
             *count = ARRAY_SIZE(video_formats);
             break;
 
+        case WG_MAJOR_TYPE_MPEG1_AUDIO:
         case WG_MAJOR_TYPE_WMA:
         case WG_MAJOR_TYPE_H264:
             FIXME("Format %u not implemented!\n", format.major_type);
@@ -1736,6 +1737,7 @@ HRESULT wm_reader_get_output_format(struct wm_reader *reader, DWORD output,
             format.u.audio.format = WG_AUDIO_FORMAT_S16LE;
             break;
 
+        case WG_MAJOR_TYPE_MPEG1_AUDIO:
         case WG_MAJOR_TYPE_WMA:
         case WG_MAJOR_TYPE_H264:
             FIXME("Format %u not implemented!\n", format.major_type);
@@ -1815,6 +1817,8 @@ static const char *get_major_type_string(enum wg_major_type type)
             return "video";
         case WG_MAJOR_TYPE_UNKNOWN:
             return "unknown";
+        case WG_MAJOR_TYPE_MPEG1_AUDIO:
+            return "mpeg1-audio";
         case WG_MAJOR_TYPE_WMA:
             return "wma";
         case WG_MAJOR_TYPE_H264:
-- 
2.34.1

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

[Anton Baskanov]

Signed-off-by: Anton Baskanov <baskanov(a)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);
+
+    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;
         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:
-- 
2.34.1

[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(a)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:

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

[Anton Baskanov]

On 04.05.2022 04:18, Zebediah Figura (she/her) wrote:
> On 5/3/22 13:28, Anton Baskanov wrote:
> > Signed-off-by: Anton Baskanov <baskanov(a)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?

Fixed in v2.
> > +
> > +    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...)

There are two issues, actually:
* mpg123 itself doesn't produce any output until it gets 2 frames of
   input data. Then it decodes both of them, though.
* mpg123audiodec calls mpg123_decode_frame() only once per input frame,
   so the call sequence looks like this:

   mpg123_feed(frame 0)
   mpg123_decode_frame() -> MPG123_NEED_MORE
   mpg123_feed(frame 1)
   mpg123_decode_frame() -> MPG123_NEW_FORMAT
   mpg123_feed(frame 2)
   mpg123_decode_frame() -> MPG123_OK (frame 0)
   mpg123_feed(frame 3)
   mpg123_decode_frame() -> MPG123_OK (frame 1)

This causes the output to always be 2 frames behind the input.

This should not be a problem in practice, as we get all the data 
eventually, except for the last 2 frames (to fix these we'd need to send 
a buffer with the DISCONT flag set in EndOfStream()).

I've fixed the tests and removed this check in v2.
> > 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:

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

[Rémi Bernon]

On 5/3/22 23:18, Zebediah Figura (she/her) wrote:
> On 5/3/22 13:28, Anton Baskanov wrote:
> > Signed-off-by: Anton Baskanov <baskanov(a)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...)

FWIW I intend to do a similar thing to disallow vaapi codecs for H264. 
They are prioritized over avdec_h264, when both are available, but they 
have issues and won't let us implement correct plane alignment easily.

It's a bit pointless to use them anyway as we don't support GPU buffers 
yet, and they could be re-enabled later when we do and for that case 
only, assuming the alignments are correct, or when they get fixed.

-- 
Rémi Bernon <rbernon(a)codeweavers.com>

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

[Zebediah Figura (she/her)]

On 5/4/22 03:32, Rémi Bernon wrote:
> > > 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...)
>
> FWIW I intend to do a similar thing to disallow vaapi codecs for H264. 
> They are prioritized over avdec_h264, when both are available, but they 
> have issues and won't let us implement correct plane alignment easily.
>
> It's a bit pointless to use them anyway as we don't support GPU buffers 
> yet, and they could be re-enabled later when we do and for that case 
> only, assuming the alignments are correct, or when they get fixed.

Disabling vaapi is a bit more reasonable, although it'd also be nice if 
we can find a way to work around vaapi's bugs that doesn't require 
blacklisting the filters entirely. Better yet if we can find a way to 
avoid using vaapi if and only if it's broken.

[PATCH 3/7] quartz/tests: Don't fail tests when MPEG audio decoder creation fails.

[Anton Baskanov]

Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
---
 dlls/quartz/tests/mpegaudio.c | 35 +++++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/dlls/quartz/tests/mpegaudio.c b/dlls/quartz/tests/mpegaudio.c
index 296f5f479a0..def5dc90238 100644
--- a/dlls/quartz/tests/mpegaudio.c
+++ b/dlls/quartz/tests/mpegaudio.c
@@ -160,9 +160,8 @@ static const AM_MEDIA_TYPE pcm16ex_mt =
 static IBaseFilter *create_mpeg_audio_codec(void)
 {
     IBaseFilter *filter = NULL;
-    HRESULT hr = CoCreateInstance(&CLSID_CMpegAudioCodec, NULL, CLSCTX_INPROC_SERVER,
+    CoCreateInstance(&CLSID_CMpegAudioCodec, NULL, CLSCTX_INPROC_SERVER,
             &IID_IBaseFilter, (void **)&filter);
-    ok(hr == S_OK, "Got hr %#lx.\n", hr);
     return filter;
 }
 
@@ -321,7 +320,6 @@ static void test_aggregation(void)
 
     hr = CoCreateInstance(&CLSID_CMpegAudioCodec, &test_outer, CLSCTX_INPROC_SERVER,
             &IID_IUnknown, (void **)&unk);
-    ok(hr == S_OK, "Got hr %#lx.\n", hr);
     if (FAILED(hr))
     {
         skip("Failed to create MPEG audio decoder instance, skipping tests.\n");
@@ -436,8 +434,8 @@ static void test_unconnected_filter_state(void)
 
 static void test_enum_pins(void)
 {
-    IBaseFilter *filter;
     IEnumPins *enum1, *enum2;
+    IBaseFilter *filter;
     ULONG count, ref;
     IPin *pins[3];
     HRESULT hr;
@@ -564,8 +562,8 @@ static void test_enum_pins(void)
 
 static void test_find_pin(void)
 {
-    IBaseFilter *filter;
     IEnumPins *enum_pins;
+    IBaseFilter *filter;
     IPin *pin, *pin2;
     HRESULT hr;
     ULONG ref;
@@ -688,13 +686,20 @@ static void test_pin_info(void)
 
 static void test_enum_media_types(void)
 {
-    IBaseFilter *filter = create_mpeg_audio_codec();
     IEnumMediaTypes *enum1, *enum2;
     AM_MEDIA_TYPE *mts[1];
+    IBaseFilter *filter;
     ULONG ref, count;
     HRESULT hr;
     IPin *pin;
 
+    filter = create_mpeg_audio_codec();
+    if (!filter)
+    {
+        skip("Failed to create MPEG audio decoder instance, skipping tests.\n");
+        return;
+    }
+
     IBaseFilter_FindPin(filter, L"In", &pin);
 
     hr = IPin_EnumMediaTypes(pin, &enum1);
@@ -763,13 +768,20 @@ static void test_enum_media_types(void)
 
 static void test_media_types(void)
 {
-    IBaseFilter *filter = create_mpeg_audio_codec();
+    IBaseFilter *filter;
     WAVEFORMATEX format;
     AM_MEDIA_TYPE mt;
     HRESULT hr;
     ULONG ref;
     IPin *pin;
 
+    filter = create_mpeg_audio_codec();
+    if (!filter)
+    {
+        skip("Failed to create MPEG audio decoder instance, skipping tests.\n");
+        return;
+    }
+
     IBaseFilter_FindPin(filter, L"In", &pin);
 
     hr = IPin_QueryAccept(pin, &mp1_mt);
@@ -1082,7 +1094,6 @@ static void test_source_allocator(IFilterGraph2 *graph, IMediaControl *control,
 
 static void test_connect_pin(void)
 {
-    IBaseFilter *filter = create_mpeg_audio_codec();
     struct testfilter testsource, testsink;
     AM_MEDIA_TYPE mt, source_mt, *pmt;
     WAVEFORMATEX source_format;
@@ -1093,10 +1104,18 @@ static void test_connect_pin(void)
     IMemInputPin *meminput;
     AM_MEDIA_TYPE req_mt;
     IFilterGraph2 *graph;
+    IBaseFilter *filter;
     unsigned int i;
     HRESULT hr;
     ULONG ref;
 
+    filter = create_mpeg_audio_codec();
+    if (!filter)
+    {
+        skip("Failed to create MPEG audio decoder instance, skipping tests.\n");
+        return;
+    }
+
     CoCreateInstance(&CLSID_FilterGraph, NULL, CLSCTX_INPROC_SERVER,
             &IID_IFilterGraph2, (void **)&graph);
     testfilter_init(&testsource);
-- 
2.34.1

[PATCH 3/7] quartz/tests: Don't fail tests when MPEG audio decoder creation fails.

[Zebediah Figura (she/her)]

On 5/3/22 13:28, Anton Baskanov wrote:
> Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
> ---
>   dlls/quartz/tests/mpegaudio.c | 35 +++++++++++++++++++++++++++--------
>   1 file changed, 27 insertions(+), 8 deletions(-)

I'd recommend checking once at the beginning, like is done in e.g. 
mpegsplit.c.

[PATCH 4/7] winegstreamer: Check whether transform is supported when creating MPEG audio decoder.

[Anton Baskanov]

Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
---
 dlls/winegstreamer/quartz_transform.c | 28 +++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/dlls/winegstreamer/quartz_transform.c b/dlls/winegstreamer/quartz_transform.c
index 375c549aad5..98953a5b47f 100644
--- a/dlls/winegstreamer/quartz_transform.c
+++ b/dlls/winegstreamer/quartz_transform.c
@@ -296,9 +296,37 @@ static const struct transform_ops mpeg_audio_codec_transform_ops =
 
 HRESULT mpeg_audio_codec_create(IUnknown *outer, IUnknown **out)
 {
+    struct wg_format output_format =
+    {
+        .major_type = WG_MAJOR_TYPE_AUDIO,
+        .u.audio = {
+            .format = WG_AUDIO_FORMAT_S16LE,
+            .channel_mask = 1,
+            .channels = 1,
+            .rate = 44100,
+        },
+    };
+    struct wg_format input_format =
+    {
+        .major_type = WG_MAJOR_TYPE_MPEG1_AUDIO,
+        .u.mpeg1_audio = {
+            .layer = 2,
+            .rate = 44100,
+            .channels = 1,
+        },
+    };
+    struct wg_transform *transform;
     struct transform *object;
     HRESULT hr;
 
+    transform = wg_transform_create(&input_format, &output_format);
+    if (!transform)
+    {
+        FIXME("GStreamer doesn't support MPEG-1 audio decoding, please install appropriate plugins.\n");
+        return E_FAIL;
+    }
+    wg_transform_destroy(transform);
+
     hr = transform_create(outer, &CLSID_CMpegAudioCodec, &mpeg_audio_codec_transform_ops, &object);
     if (FAILED(hr))
         return hr;
-- 
2.34.1

[PATCH 4/7] winegstreamer: Check whether transform is supported when creating MPEG audio decoder.

[Zebediah Figura (she/her)]

On 5/3/22 13:28, Anton Baskanov wrote:
> Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
> ---
>   dlls/winegstreamer/quartz_transform.c | 28 +++++++++++++++++++++++++++
>   1 file changed, 28 insertions(+)
>
> diff --git a/dlls/winegstreamer/quartz_transform.c b/dlls/winegstreamer/quartz_transform.c
> index 375c549aad5..98953a5b47f 100644
> --- a/dlls/winegstreamer/quartz_transform.c
> +++ b/dlls/winegstreamer/quartz_transform.c
> @@ -296,9 +296,37 @@ static const struct transform_ops mpeg_audio_codec_transform_ops =
>
> HRESULT mpeg_audio_codec_create(IUnknown *outer, IUnknown **out)
>   {
> +    struct wg_format output_format =
> +    {
> +        .major_type = WG_MAJOR_TYPE_AUDIO,
> +        .u.audio = {
> +            .format = WG_AUDIO_FORMAT_S16LE,
> +            .channel_mask = 1,
> +            .channels = 1,
> +            .rate = 44100,
> +        },
> +    };
> +    struct wg_format input_format =
> +    {
> +        .major_type = WG_MAJOR_TYPE_MPEG1_AUDIO,
> +        .u.mpeg1_audio = {
> +            .layer = 2,
> +            .rate = 44100,
> +            .channels = 1,
> +        },
> +    };
> +    struct wg_transform *transform;
>       struct transform *object;
>       HRESULT hr;
>
> +    transform = wg_transform_create(&input_format, &output_format);
> +    if (!transform)
> +    {
> +        FIXME("GStreamer doesn't support MPEG-1 audio decoding, please install appropriate plugins.\n");

I didn't notice this when reviewing Rémi's patches, but this should be a 
winediag error, not a FIXME. I'll send patches to address that for the 
WMA and H.264 decoders as well.
> +        return E_FAIL;
> +    }
> +    wg_transform_destroy(transform);
> +
>       hr = transform_create(outer, &CLSID_CMpegAudioCodec, &mpeg_audio_codec_transform_ops, &object);
>       if (FAILED(hr))
>           return hr;

[PATCH 5/7] winegstreamer: Create wg_transform in MPEG audio decoder.

[Anton Baskanov]

Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
---
 dlls/winegstreamer/quartz_transform.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/dlls/winegstreamer/quartz_transform.c b/dlls/winegstreamer/quartz_transform.c
index 98953a5b47f..f4d27613480 100644
--- a/dlls/winegstreamer/quartz_transform.c
+++ b/dlls/winegstreamer/quartz_transform.c
@@ -29,6 +29,8 @@ struct transform
     struct strmbase_sink sink;
     struct strmbase_source source;
 
+    struct wg_transform *transform;
+
     const struct transform_ops *ops;
 };
 
@@ -69,10 +71,22 @@ static void transform_destroy(struct strmbase_filter *iface)
 static HRESULT transform_init_stream(struct strmbase_filter *iface)
 {
     struct transform *filter = impl_from_strmbase_filter(iface);
+    struct wg_format input_format;
+    struct wg_format output_format;
     HRESULT hr;
 
     if (filter->source.pin.peer)
     {
+        if (!amt_to_wg_format(&filter->sink.pin.mt, &input_format))
+            return E_FAIL;
+
+        if (!amt_to_wg_format(&filter->source.pin.mt, &output_format))
+            return E_FAIL;
+
+        filter->transform = wg_transform_create(&input_format, &output_format);
+        if (!filter->transform)
+            return E_FAIL;
+
         hr = IMemAllocator_Commit(filter->source.pAllocator);
         if (FAILED(hr))
             ERR("Failed to commit allocator, hr %#lx.\n", hr);
@@ -86,8 +100,12 @@ static HRESULT transform_cleanup_stream(struct strmbase_filter *iface)
     struct transform *filter = impl_from_strmbase_filter(iface);
 
     if (filter->source.pin.peer)
+    {
         IMemAllocator_Decommit(filter->source.pAllocator);
 
+        wg_transform_destroy(filter->transform);
+    }
+
     return S_OK;
 }
 
-- 
2.34.1

[PATCH 6/7] quartz/tests: Add tests for MPEG audio decoder sample processing.

[Anton Baskanov]

Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
---
 dlls/quartz/tests/mpegaudio.c | 83 +++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/dlls/quartz/tests/mpegaudio.c b/dlls/quartz/tests/mpegaudio.c
index def5dc90238..0044d3bef6f 100644
--- a/dlls/quartz/tests/mpegaudio.c
+++ b/dlls/quartz/tests/mpegaudio.c
@@ -846,6 +846,7 @@ struct testfilter
     struct strmbase_source source;
     struct strmbase_sink sink;
     const AM_MEDIA_TYPE *mt;
+    unsigned int got_sample;
 };
 
 static inline struct testfilter *impl_from_strmbase_filter(struct strmbase_filter *iface)
@@ -919,11 +920,39 @@ static HRESULT testsink_connect(struct strmbase_sink *iface, IPin *peer, const A
     return S_OK;
 }
 
+static HRESULT WINAPI testsink_Receive(struct strmbase_sink *iface, IMediaSample *sample)
+{
+    struct testfilter *filter = impl_from_strmbase_filter(iface->pin.filter);
+    unsigned int i;
+    BYTE *data;
+    HRESULT hr;
+    LONG size;
+
+    ++filter->got_sample;
+
+    size = IMediaSample_GetSize(sample);
+    ok(size == 3072, "Got size %lu.\n", size);
+    size = IMediaSample_GetActualDataLength(sample);
+    ok(size == 768, "Got valid size %lu.\n", size);
+
+    hr = IMediaSample_GetPointer(sample, &data);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+    for (i = 0; i < size / 2; ++i)
+    {
+        short value;
+        memcpy(&value, &data[i * 2], 2);
+        ok(abs(value) <= 1, "Got value %d.\n", value);
+    }
+
+    return S_OK;
+}
+
 static const struct strmbase_sink_ops testsink_ops =
 {
     .base.pin_query_interface = testsink_query_interface,
     .base.pin_get_media_type = testsink_get_media_type,
     .sink_connect = testsink_connect,
+    .pfnReceive = testsink_Receive,
 };
 
 static void testfilter_init(struct testfilter *filter)
@@ -1092,6 +1121,58 @@ static void test_source_allocator(IFilterGraph2 *graph, IMediaControl *control,
     IFilterGraph2_Disconnect(graph, &testsource->source.pin.IPin_iface);
 }
 
+static void test_sample_processing(IMediaControl *control, IMemInputPin *input, struct testfilter *sink)
+{
+    IMemAllocator *allocator;
+    IMediaSample *sample;
+    HRESULT hr;
+    BYTE *data;
+    LONG size;
+
+    hr = IMemInputPin_ReceiveCanBlock(input);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+
+    hr = IMemInputPin_GetAllocator(input, &allocator);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+
+    hr = IMediaControl_Pause(control);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+
+    hr = IMemAllocator_GetBuffer(allocator, &sample, NULL, NULL, 0);
+    ok(hr == VFW_E_NOT_COMMITTED, "Got hr %#lx.\n", hr);
+
+    hr = IMemAllocator_Commit(allocator);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+    hr = IMemAllocator_GetBuffer(allocator, &sample, NULL, NULL, 0);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+
+    hr = IMediaSample_GetPointer(sample, &data);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+    size = IMediaSample_GetSize(sample);
+    ok(size == 256, "Got size %ld.\n", size);
+    memset(data, 0, 48);
+    data[0] = 0xff;
+    data[1] = 0xff;
+    data[2] = 0x18;
+    data[3] = 0xc4;
+    hr = IMediaSample_SetActualDataLength(sample, 48);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+
+    hr = IMemInputPin_Receive(input, sample);
+    todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr);
+    todo_wine ok(sink->got_sample == 1, "Got %u calls to Receive().\n", sink->got_sample);
+    sink->got_sample = 0;
+
+    hr = IMediaControl_Stop(control);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+
+    hr = IMemInputPin_Receive(input, sample);
+    todo_wine ok(hr == VFW_E_WRONG_STATE, "Got hr %#lx.\n", hr);
+
+    IMediaSample_Release(sample);
+    IMemAllocator_Release(allocator);
+}
+
 static void test_connect_pin(void)
 {
     struct testfilter testsource, testsink;
@@ -1292,6 +1373,8 @@ static void test_connect_pin(void)
     hr = IMediaControl_Stop(control);
     ok(hr == S_OK, "Got hr %#lx.\n", hr);
 
+    test_sample_processing(control, meminput, &testsink);
+
     hr = IFilterGraph2_Disconnect(graph, source);
     ok(hr == S_OK, "Got hr %#lx.\n", hr);
     hr = IFilterGraph2_Disconnect(graph, source);
-- 
2.34.1

[PATCH 6/7] quartz/tests: Add tests for MPEG audio decoder sample processing.

[Zebediah Figura (she/her)]

On 5/3/22 13:28, Anton Baskanov wrote:
> Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
> ---
>   dlls/quartz/tests/mpegaudio.c | 83 +++++++++++++++++++++++++++++++++++
>   1 file changed, 83 insertions(+)
>
> diff --git a/dlls/quartz/tests/mpegaudio.c b/dlls/quartz/tests/mpegaudio.c
> index def5dc90238..0044d3bef6f 100644
> --- a/dlls/quartz/tests/mpegaudio.c
> +++ b/dlls/quartz/tests/mpegaudio.c
> @@ -846,6 +846,7 @@ struct testfilter
>       struct strmbase_source source;
>       struct strmbase_sink sink;
>       const AM_MEDIA_TYPE *mt;
> +    unsigned int got_sample;
>   };
>
> static inline struct testfilter *impl_from_strmbase_filter(struct strmbase_filter *iface)
> @@ -919,11 +920,39 @@ static HRESULT testsink_connect(struct strmbase_sink *iface, IPin *peer, const A
>       return S_OK;
>   }
>
> +static HRESULT WINAPI testsink_Receive(struct strmbase_sink *iface, IMediaSample *sample)
> +{
> +    struct testfilter *filter = impl_from_strmbase_filter(iface->pin.filter);
> +    unsigned int i;
> +    BYTE *data;
> +    HRESULT hr;
> +    LONG size;
> +
> +    ++filter->got_sample;
> +
> +    size = IMediaSample_GetSize(sample);
> +    ok(size == 3072, "Got size %lu.\n", size);
> +    size = IMediaSample_GetActualDataLength(sample);
> +    ok(size == 768, "Got valid size %lu.\n", size);
> +
> +    hr = IMediaSample_GetPointer(sample, &data);
> +    ok(hr == S_OK, "Got hr %#lx.\n", hr);
> +    for (i = 0; i < size / 2; ++i)
> +    {
> +        short value;
> +        memcpy(&value, &data[i * 2], 2);
> +        ok(abs(value) <= 1, "Got value %d.\n", value);

I don't know if we really need to bother with validating the output data.

(If we do, though, it seems a bit more idiomatic to just alias "data" 
with a "const short *" pointer.)

[PATCH 6/7] quartz/tests: Add tests for MPEG audio decoder sample processing.

[Anton Baskanov]

On 04.05.2022 04:18, Zebediah Figura (she/her) wrote:
> On 5/3/22 13:28, Anton Baskanov wrote:
> > Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
> > ---
> >    dlls/quartz/tests/mpegaudio.c | 83 +++++++++++++++++++++++++++++++++++
> >    1 file changed, 83 insertions(+)
> >
> > diff --git a/dlls/quartz/tests/mpegaudio.c b/dlls/quartz/tests/mpegaudio.c
> > index def5dc90238..0044d3bef6f 100644
> > --- a/dlls/quartz/tests/mpegaudio.c
> > +++ b/dlls/quartz/tests/mpegaudio.c
> > @@ -846,6 +846,7 @@ struct testfilter
> >        struct strmbase_source source;
> >        struct strmbase_sink sink;
> >        const AM_MEDIA_TYPE *mt;
> > +    unsigned int got_sample;
> >    };
> >
> > static inline struct testfilter *impl_from_strmbase_filter(struct strmbase_filter *iface)
> > @@ -919,11 +920,39 @@ static HRESULT testsink_connect(struct strmbase_sink *iface, IPin *peer, const A
> >        return S_OK;
> >    }
> >
> > +static HRESULT WINAPI testsink_Receive(struct strmbase_sink *iface, IMediaSample *sample)
> > +{
> > +    struct testfilter *filter = impl_from_strmbase_filter(iface->pin.filter);
> > +    unsigned int i;
> > +    BYTE *data;
> > +    HRESULT hr;
> > +    LONG size;
> > +
> > +    ++filter->got_sample;
> > +
> > +    size = IMediaSample_GetSize(sample);
> > +    ok(size == 3072, "Got size %lu.\n", size);
> > +    size = IMediaSample_GetActualDataLength(sample);
> > +    ok(size == 768, "Got valid size %lu.\n", size);
> > +
> > +    hr = IMediaSample_GetPointer(sample, &data);
> > +    ok(hr == S_OK, "Got hr %#lx.\n", hr);
> > +    for (i = 0; i < size / 2; ++i)
> > +    {
> > +        short value;
> > +        memcpy(&value, &data[i * 2], 2);
> > +        ok(abs(value) <= 1, "Got value %d.\n", value);
>
> I don't know if we really need to bother with validating the output data.
>
> (If we do, though, it seems a bit more idiomatic to just alias "data"
> with a "const short *" pointer.)

Oh, I didn't realize that we are compiling with -fno-strict-aliasing, so 
I thought that this would break the aliasing rules.

Anyway, I've removed the data check in v2.

[PATCH 6/7] quartz/tests: Add tests for MPEG audio decoder sample processing.

[Zebediah Figura (she/her)]

On 5/4/22 02:58, Anton Baskanov wrote:
> On 04.05.2022 04:18, Zebediah Figura (she/her) wrote:
> > On 5/3/22 13:28, Anton Baskanov wrote:
> > > Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
> > > ---
> > >    dlls/quartz/tests/mpegaudio.c | 83 
> > > +++++++++++++++++++++++++++++++++++
> > >    1 file changed, 83 insertions(+)
> > >
> > > diff --git a/dlls/quartz/tests/mpegaudio.c 
> > > b/dlls/quartz/tests/mpegaudio.c
> > > index def5dc90238..0044d3bef6f 100644
> > > --- a/dlls/quartz/tests/mpegaudio.c
> > > +++ b/dlls/quartz/tests/mpegaudio.c
> > > @@ -846,6 +846,7 @@ struct testfilter
> > >        struct strmbase_source source;
> > >        struct strmbase_sink sink;
> > >        const AM_MEDIA_TYPE *mt;
> > > +    unsigned int got_sample;
> > >    };
> > >    static inline struct testfilter *impl_from_strmbase_filter(struct 
> > > strmbase_filter *iface)
> > > @@ -919,11 +920,39 @@ static HRESULT testsink_connect(struct 
> > > strmbase_sink *iface, IPin *peer, const A
> > >        return S_OK;
> > >    }
> > > +static HRESULT WINAPI testsink_Receive(struct strmbase_sink *iface, 
> > > IMediaSample *sample)
> > > +{
> > > +    struct testfilter *filter = 
> > > impl_from_strmbase_filter(iface->pin.filter);
> > > +    unsigned int i;
> > > +    BYTE *data;
> > > +    HRESULT hr;
> > > +    LONG size;
> > > +
> > > +    ++filter->got_sample;
> > > +
> > > +    size = IMediaSample_GetSize(sample);
> > > +    ok(size == 3072, "Got size %lu.\n", size);
> > > +    size = IMediaSample_GetActualDataLength(sample);
> > > +    ok(size == 768, "Got valid size %lu.\n", size);
> > > +
> > > +    hr = IMediaSample_GetPointer(sample, &data);
> > > +    ok(hr == S_OK, "Got hr %#lx.\n", hr);
> > > +    for (i = 0; i < size / 2; ++i)
> > > +    {
> > > +        short value;
> > > +        memcpy(&value, &data[i * 2], 2);
> > > +        ok(abs(value) <= 1, "Got value %d.\n", value);
> >
> > I don't know if we really need to bother with validating the output data.
> >
> > (If we do, though, it seems a bit more idiomatic to just alias "data"
> > with a "const short *" pointer.)
>
> Oh, I didn't realize that we are compiling with -fno-strict-aliasing, so 
> I thought that this would break the aliasing rules.

Indeed, it'd be hard to do any win32 programming at all with strict 
aliasing :-)

[PATCH 7/7] winegstreamer: Implement Receive() for MPEG audio decoder sink.

[Anton Baskanov]

Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
---
 dlls/quartz/tests/mpegaudio.c         |  6 +-
 dlls/winegstreamer/quartz_transform.c | 86 +++++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 3 deletions(-)

diff --git a/dlls/quartz/tests/mpegaudio.c b/dlls/quartz/tests/mpegaudio.c
index 0044d3bef6f..a7622b07f77 100644
--- a/dlls/quartz/tests/mpegaudio.c
+++ b/dlls/quartz/tests/mpegaudio.c
@@ -1159,15 +1159,15 @@ static void test_sample_processing(IMediaControl *control, IMemInputPin *input,
     ok(hr == S_OK, "Got hr %#lx.\n", hr);
 
     hr = IMemInputPin_Receive(input, sample);
-    todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr);
-    todo_wine ok(sink->got_sample == 1, "Got %u calls to Receive().\n", sink->got_sample);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+    ok(sink->got_sample == 1, "Got %u calls to Receive().\n", sink->got_sample);
     sink->got_sample = 0;
 
     hr = IMediaControl_Stop(control);
     ok(hr == S_OK, "Got hr %#lx.\n", hr);
 
     hr = IMemInputPin_Receive(input, sample);
-    todo_wine ok(hr == VFW_E_WRONG_STATE, "Got hr %#lx.\n", hr);
+    ok(hr == VFW_E_WRONG_STATE, "Got hr %#lx.\n", hr);
 
     IMediaSample_Release(sample);
     IMemAllocator_Release(allocator);
diff --git a/dlls/winegstreamer/quartz_transform.c b/dlls/winegstreamer/quartz_transform.c
index f4d27613480..ad33ca98311 100644
--- a/dlls/winegstreamer/quartz_transform.c
+++ b/dlls/winegstreamer/quartz_transform.c
@@ -20,6 +20,8 @@
 
 #include "gst_private.h"
 
+#include "mferror.h"
+
 WINE_DEFAULT_DEBUG_CHANNEL(quartz);
 
 struct transform
@@ -137,10 +139,94 @@ static HRESULT transform_sink_query_interface(struct strmbase_pin *pin, REFIID i
     return S_OK;
 }
 
+static HRESULT WINAPI transform_sink_receive(struct strmbase_sink *pin, IMediaSample *sample)
+{
+    struct transform *filter = impl_from_strmbase_filter(pin->pin.filter);
+    struct wg_sample input_wg_sample = {0};
+    HRESULT hr;
+
+    /* We do not expect pin connection state to change while the filter is
+     * running. This guarantee is necessary, since otherwise we would have to
+     * take the filter lock, and we can't take the filter lock from a streaming
+     * thread. */
+    if (!filter->source.pMemInputPin)
+    {
+        WARN("Source is not connected, returning VFW_E_NOT_CONNECTED.\n");
+        return VFW_E_NOT_CONNECTED;
+    }
+
+    if (filter->filter.state == State_Stopped)
+        return VFW_E_WRONG_STATE;
+
+    if (filter->sink.flushing)
+        return S_FALSE;
+
+    input_wg_sample.max_size = IMediaSample_GetSize(sample);
+    input_wg_sample.size = IMediaSample_GetActualDataLength(sample);
+
+    hr = IMediaSample_GetPointer(sample, &input_wg_sample.data);
+    if (FAILED(hr))
+        return hr;
+
+    hr = wg_transform_push_data(filter->transform, &input_wg_sample);
+    if (FAILED(hr))
+        return hr;
+
+    for (;;)
+    {
+        struct wg_sample output_wg_sample = {0};
+        IMediaSample *output_sample;
+
+        hr = IMemAllocator_GetBuffer(filter->source.pAllocator, &output_sample, NULL, NULL, 0);
+        if (FAILED(hr))
+            return hr;
+
+        output_wg_sample.max_size = IMediaSample_GetSize(output_sample);
+
+        hr = IMediaSample_GetPointer(output_sample, &output_wg_sample.data);
+        if (FAILED(hr))
+        {
+            IMediaSample_Release(output_sample);
+            return hr;
+        }
+
+        hr = wg_transform_read_data(filter->transform, &output_wg_sample);
+        if (hr == MF_E_TRANSFORM_NEED_MORE_INPUT)
+        {
+            IMediaSample_Release(output_sample);
+            break;
+        }
+        if (FAILED(hr))
+        {
+            IMediaSample_Release(output_sample);
+            return hr;
+        }
+
+        hr = IMediaSample_SetActualDataLength(output_sample, output_wg_sample.size);
+        if (FAILED(hr))
+        {
+            IMediaSample_Release(output_sample);
+            return hr;
+        }
+
+        hr = IMemInputPin_Receive(filter->source.pMemInputPin, output_sample);
+        if (FAILED(hr))
+        {
+            IMediaSample_Release(output_sample);
+            return hr;
+        }
+
+        IMediaSample_Release(output_sample);
+    }
+
+    return S_OK;
+}
+
 static const struct strmbase_sink_ops sink_ops =
 {
     .base.pin_query_accept = transform_sink_query_accept,
     .base.pin_query_interface = transform_sink_query_interface,
+    .pfnReceive = transform_sink_receive,
 };
 
 static HRESULT transform_source_query_accept(struct strmbase_pin *pin, const AM_MEDIA_TYPE *mt)
-- 
2.34.1

[Zebediah Figura (she/her)]

On 5/3/22 13:27, Anton Baskanov wrote:
> Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
> ---
>   dlls/winegstreamer/mfplat.c        |   1 +
>   dlls/winegstreamer/quartz_parser.c | 151 ++++++++++++++++-------------
>   dlls/winegstreamer/unixlib.h       |  11 ++-
>   dlls/winegstreamer/wg_format.c     |  22 ++---
>   dlls/winegstreamer/wg_transform.c  |   2 +
>   dlls/winegstreamer/wm_reader.c     |   4 +
>   6 files changed, 106 insertions(+), 85 deletions(-)

I'm not necessarily opposed, but it also doesn't seem clear that this is 
an improvement. The audio fields used for MPEG-1 audio is a proper 
subset of those used for raw audio, and one can see from the diffstat 
that the code doesn't exactly get simpler this way.

Is this patch necessary or helpful for others later in the series?

[Anton Baskanov]

On 04.05.2022 04:18, Zebediah Figura (she/her) wrote:
> On 5/3/22 13:27, Anton Baskanov wrote:
> > Signed-off-by: Anton Baskanov <baskanov(a)gmail.com>
> > ---
> >    dlls/winegstreamer/mfplat.c        |   1 +
> >    dlls/winegstreamer/quartz_parser.c | 151 ++++++++++++++++-------------
> >    dlls/winegstreamer/unixlib.h       |  11 ++-
> >    dlls/winegstreamer/wg_format.c     |  22 ++---
> >    dlls/winegstreamer/wg_transform.c  |   2 +
> >    dlls/winegstreamer/wm_reader.c     |   4 +
> >    6 files changed, 106 insertions(+), 85 deletions(-)
>
> I'm not necessarily opposed, but it also doesn't seem clear that this is
> an improvement. The audio fields used for MPEG-1 audio is a proper
> subset of those used for raw audio, and one can see from the diffstat
> that the code doesn't exactly get simpler this way.
>
> Is this patch necessary or helpful for others later in the series?

It was supposed to make the next patch a bit less awkward, but you are 
right, given all the extra changes, it's probably not worth it.

Removed it in v2.