[PATCH 1/2] winegstreamer: Support block_alignment attribute mapping.
Planet Coaster adds this attribute to its media type when loading user music in some of the supported formats, and missing it will make media type matching only partially succeed. This will also be useful for WMA/XMA compressed formats. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/mfplat.c | 10 ++++++++-- dlls/winegstreamer/unixlib.h | 1 + dlls/winegstreamer/wg_parser.c | 8 +++++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/dlls/winegstreamer/mfplat.c b/dlls/winegstreamer/mfplat.c index 58200dc409a..340cb92e87a 100644 --- a/dlls/winegstreamer/mfplat.c +++ b/dlls/winegstreamer/mfplat.c @@ -547,8 +547,8 @@ static inline UINT64 make_uint64(UINT32 high, UINT32 low) static IMFMediaType *mf_media_type_from_wg_format_audio(const struct wg_format *format) { + unsigned int i, block_alignment; IMFMediaType *type; - unsigned int i; for (i = 0; i < ARRAY_SIZE(audio_formats); ++i) { @@ -564,6 +564,8 @@ static IMFMediaType *mf_media_type_from_wg_format_audio(const struct wg_format * IMFMediaType_SetUINT32(type, &MF_MT_AUDIO_NUM_CHANNELS, format->u.audio.channels); IMFMediaType_SetUINT32(type, &MF_MT_AUDIO_CHANNEL_MASK, format->u.audio.channel_mask); IMFMediaType_SetUINT32(type, &MF_MT_ALL_SAMPLES_INDEPENDENT, TRUE); + block_alignment = format->u.audio.channels * audio_formats[i].depth / 8; + IMFMediaType_SetUINT32(type, &MF_MT_AUDIO_BLOCK_ALIGNMENT, block_alignment); return type; } @@ -621,7 +623,7 @@ IMFMediaType *mf_media_type_from_wg_format(const struct wg_format *format) static void mf_media_type_to_wg_format_audio(IMFMediaType *type, struct wg_format *format) { - UINT32 rate, channels, channel_mask, depth; + UINT32 rate, channels, channel_mask, depth, block_alignment; unsigned int i; GUID subtype; @@ -658,10 +660,14 @@ static void mf_media_type_to_wg_format_audio(IMFMediaType *type, struct wg_forma } } + if (FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_BLOCK_ALIGNMENT, &block_alignment))) + block_alignment = 0; + format->major_type = WG_MAJOR_TYPE_AUDIO; format->u.audio.channels = channels; format->u.audio.channel_mask = channel_mask; format->u.audio.rate = rate; + format->u.audio.block_alignment = block_alignment; for (i = 0; i < ARRAY_SIZE(audio_formats); ++i) { diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index 90101893541..9a98e97e7d7 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -87,6 +87,7 @@ struct wg_format uint32_t channels; uint32_t channel_mask; /* In WinMM format. */ uint32_t rate; + uint32_t block_alignment; } audio; } u; }; diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index c3c9051a174..3b341dad9eb 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -411,13 +411,19 @@ static GstCaps *wg_format_to_caps_audio(const struct wg_format *format) GstAudioChannelPosition positions[32]; GstAudioFormat audio_format; GstAudioInfo info; + GstCaps *caps; if ((audio_format = wg_audio_format_to_gst(format->u.audio.format)) == GST_AUDIO_FORMAT_UNKNOWN) return NULL; wg_channel_mask_to_gst(positions, format->u.audio.channel_mask, format->u.audio.channels); gst_audio_info_set_format(&info, audio_format, format->u.audio.rate, format->u.audio.channels, positions); - return gst_audio_info_to_caps(&info); + caps = gst_audio_info_to_caps(&info); + + if (format->u.audio.block_alignment) + gst_caps_set_simple(caps, "block_align", G_TYPE_INT, format->u.audio.block_alignment, NULL); + + return caps; } static GstVideoFormat wg_video_format_to_gst(enum wg_video_format format) -- 2.33.0
Planet Coaster requests an output format with 44100 rate for user provided music, which may not match what the files are decoded to. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/media_source.c | 21 +++++++++++++++------ dlls/winegstreamer/wg_parser.c | 10 ++++++++-- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index 6ecd345cb73..6a96ddae0bd 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -922,22 +922,31 @@ static HRESULT media_stream_init_desc(struct media_stream *stream) { /* Expose at least one PCM and one floating point type for the consumer to pick from. */ - static const enum wg_audio_format audio_types[] = + static const struct { - WG_AUDIO_FORMAT_S16LE, - WG_AUDIO_FORMAT_F32LE, + enum wg_audio_format format; + UINT32 rate; + } + audio_formats[] = + { + {WG_AUDIO_FORMAT_S16LE, 48000}, + {WG_AUDIO_FORMAT_S16LE, 44100}, + {WG_AUDIO_FORMAT_F32LE, 48000}, + {WG_AUDIO_FORMAT_F32LE, 44100}, }; stream_types[0] = mf_media_type_from_wg_format(&format); type_count = 1; - for (i = 0; i < ARRAY_SIZE(audio_types); i++) + for (i = 0; i < ARRAY_SIZE(audio_formats); i++) { struct wg_format new_format; - if (format.u.audio.format == audio_types[i]) + if (format.u.audio.format == audio_formats[i].format && + format.u.audio.rate == audio_formats[i].rate) continue; new_format = format; - new_format.u.audio.format = audio_types[i]; + new_format.u.audio.format = audio_formats[i].format; + new_format.u.audio.rate = audio_formats[i].rate; stream_types[type_count++] = mf_media_type_from_wg_format(&new_format); } } diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index 3b341dad9eb..b3fe25d70b1 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -1198,7 +1198,7 @@ static void pad_added_cb(GstElement *element, GstPad *pad, gpointer user) } else if (!strcmp(name, "audio/x-raw")) { - GstElement *convert; + GstElement *convert, *resample; /* Currently our dsound can't handle 64-bit formats or all * surround-sound configurations. Native dsound can't always handle @@ -1207,11 +1207,17 @@ static void pad_added_cb(GstElement *element, GstPad *pad, gpointer user) if (!(convert = create_element("audioconvert", "base"))) goto out; + if (!(resample = create_element("audioresample", "base"))) + goto out; + gst_bin_add(GST_BIN(parser->container), convert); gst_element_sync_state_with_parent(convert); + gst_bin_add(GST_BIN(parser->container), resample); + gst_element_sync_state_with_parent(resample); + gst_element_link(convert, resample); stream->post_sink = gst_element_get_static_pad(convert, "sink"); - stream->post_src = gst_element_get_static_pad(convert, "src"); + stream->post_src = gst_element_get_static_pad(resample, "src"); } if (stream->post_sink) -- 2.33.0
On 10/27/21 10:25, Rémi Bernon wrote:
Planet Coaster requests an output format with 44100 rate for user provided music, which may not match what the files are decoded to.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
This one looks good, although it'd be nice for this description to be in the code itself. It could also be split into front and backend parts.
On 10/27/21 22:34, Zebediah Figura (she/her) wrote:
On 10/27/21 10:25, Rémi Bernon wrote:
Planet Coaster requests an output format with 44100 rate for user provided music, which may not match what the files are decoded to.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
This one looks good, although it'd be nice for this description to be in the code itself.
It could also be split into front and backend parts.
So it ends up being a little bit more complicated than that, and I don't think hardcoding a list of supported audio format is the right way to do it. In order to support all possible user music formats, we would have to hardcode all possible variations of media types. As far as I could see (and with tests from https://source.winehq.org/patches/data/219024) native streams only enumerate their native media types. Then, more media types are supported by the IMFSourceReader, but probably using a dynamically allocated decoder / converter MF transforms, or using MF topology elements which I believe are doing more advanced logic than what src_reader_SetCurrentMediaType does. Doing it this way would mean instantiating an audio_converter MF transform for instance, and would then make the audioresampler element useless. This seems to be the direction existing code is generally going, but it kind of defeat the idea of using GStreamer and its dynamically created pipelines. Another way I can see to make this dynamic, using an always present audioresampler element, would be instead to change the way we match IMFSourceReader stream media types, by delegating the type matching to winegstreamer, and GStreamer through gst_pad_query_caps calls. I'm not sure how we can do that, maybe with a custom IMFMediaTypeHandler, but it's not supposed to do this. -- Rémi Bernon <rbernon(a)codeweavers.com>
On 11/8/21 17:36, Rémi Bernon wrote:
On 10/27/21 22:34, Zebediah Figura (she/her) wrote:
On 10/27/21 10:25, Rémi Bernon wrote:
Planet Coaster requests an output format with 44100 rate for user provided music, which may not match what the files are decoded to.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
This one looks good, although it'd be nice for this description to be in the code itself.
It could also be split into front and backend parts.
So it ends up being a little bit more complicated than that, and I don't think hardcoding a list of supported audio format is the right way to do it. In order to support all possible user music formats, we would have to hardcode all possible variations of media types.
As far as I could see (and with tests from https://source.winehq.org/patches/data/219024) native streams only enumerate their native media types. Then, more media types are supported by the IMFSourceReader, but probably using a dynamically allocated decoder / converter MF transforms, or using MF topology elements which I believe are doing more advanced logic than what src_reader_SetCurrentMediaType does.
Doing it this way would mean instantiating an audio_converter MF transform for instance, and would then make the audioresampler element useless. This seems to be the direction existing code is generally going, but it kind of defeat the idea of using GStreamer and its dynamically created pipelines.
Another way I can see to make this dynamic, using an always present audioresampler element, would be instead to change the way we match IMFSourceReader stream media types, by delegating the type matching to winegstreamer, and GStreamer through gst_pad_query_caps calls. I'm not sure how we can do that, maybe with a custom IMFMediaTypeHandler, but it's not supposed to do this.
Actually it should be possible to do something not too ugly, with an audioresampler element, allowing audio conversion by accepting media types directly in source_reader_set_compatible_media_type, as MSDN seems to suggest [1] is done since Win8 (and as the test confirm). [1] https://docs.microsoft.com/en-us/windows/win32/api/mfreadwrite/nf-mfreadwrit... -- Rémi Bernon <rbernon(a)codeweavers.com>
On 11/8/21 11:41 AM, Rémi Bernon wrote:
On 11/8/21 17:36, Rémi Bernon wrote:
On 10/27/21 22:34, Zebediah Figura (she/her) wrote:
On 10/27/21 10:25, Rémi Bernon wrote:
Planet Coaster requests an output format with 44100 rate for user provided music, which may not match what the files are decoded to.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
This one looks good, although it'd be nice for this description to be in the code itself.
It could also be split into front and backend parts.
So it ends up being a little bit more complicated than that, and I don't think hardcoding a list of supported audio format is the right way to do it. In order to support all possible user music formats, we would have to hardcode all possible variations of media types.
As far as I could see (and with tests from https://source.winehq.org/patches/data/219024) native streams only enumerate their native media types. Then, more media types are supported by the IMFSourceReader, but probably using a dynamically allocated decoder / converter MF transforms, or using MF topology elements which I believe are doing more advanced logic than what src_reader_SetCurrentMediaType does.
Doing it this way would mean instantiating an audio_converter MF transform for instance, and would then make the audioresampler element useless. This seems to be the direction existing code is generally going, but it kind of defeat the idea of using GStreamer and its dynamically created pipelines.
Another way I can see to make this dynamic, using an always present audioresampler element, would be instead to change the way we match IMFSourceReader stream media types, by delegating the type matching to winegstreamer, and GStreamer through gst_pad_query_caps calls. I'm not sure how we can do that, maybe with a custom IMFMediaTypeHandler, but it's not supposed to do this.
Actually it should be possible to do something not too ugly, with an audioresampler element, allowing audio conversion by accepting media types directly in source_reader_set_compatible_media_type, as MSDN seems to suggest [1] is done since Win8 (and as the test confirm).
[1] https://docs.microsoft.com/en-us/windows/win32/api/mfreadwrite/nf-mfreadwrit...
I'm not entirely sure I understand what you're proposing? The patch as-is looks fine and is perfectly in line with what we do already. We basically have the frontend (mfplat, quartz, wmvcore) request a specific media type from the backend, and give the whole responsibility of deciding *what* media type to the frontend. In terms of mfplat separating its demuxers and decoders (really quartz and wmvcore do this too, but the design of mfplat makes this fact harder to ignore) I've been thus far inclined to try to keep demuxing within the same GStreamer pipeline as much as we can. In particular I'm worried about the latency of marshalling buffers between two or three threads (after all, we already have latency problems even on high-end processors), plus the added code complexity is not insignificant.
On 10/27/21 10:25, Rémi Bernon wrote:
Planet Coaster adds this attribute to its media type when loading user music in some of the supported formats, and missing it will make media type matching only partially succeed.
This will also be useful for WMA/XMA compressed formats.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
Can you please describe what exactly it's doing, in terms of specific Media Foundation APIs? As it is I can't tell why you're trying to translate this to and from winegstreamer formats. Note also that "block_align" is not used by anything in GStreamer for PCM formats.
On 10/27/21 10:30 PM, Zebediah Figura (she/her) wrote:
On 10/27/21 10:25, Rémi Bernon wrote:
Planet Coaster adds this attribute to its media type when loading user music in some of the supported formats, and missing it will make media type matching only partially succeed.
This will also be useful for WMA/XMA compressed formats.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
Can you please describe what exactly it's doing, in terms of specific Media Foundation APIs? As it is I can't tell why you're trying to translate this to and from winegstreamer formats.
Note also that "block_align" is not used by anything in GStreamer for PCM formats.
WMA/XMA formats caps need a block_align attribute, applications specify it in the input media type of the MF transform, and it will need to be passed to gstreamer decoder plugin input caps. Yes, mapping it in this direction is not very useful right now but I thought it was better to add the new attribute mapping support all in the same change. I can split it if that matters. FWIW most other attributes will have no use either in GStreamer for non-PCM formats, audio/x-wma only has bitrate and block_align for instance, does it really matters if we map them nonetheless here and even maps them unconditionally to gst caps? -- Rémi Bernon <rbernon(a)codeweavers.com>
On 10/27/21 10:45 PM, Rémi Bernon wrote:
On 10/27/21 10:30 PM, Zebediah Figura (she/her) wrote:
On 10/27/21 10:25, Rémi Bernon wrote:
Planet Coaster adds this attribute to its media type when loading user music in some of the supported formats, and missing it will make media type matching only partially succeed.
This will also be useful for WMA/XMA compressed formats.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
Can you please describe what exactly it's doing, in terms of specific Media Foundation APIs? As it is I can't tell why you're trying to translate this to and from winegstreamer formats.
Note also that "block_align" is not used by anything in GStreamer for PCM formats.
WMA/XMA formats caps need a block_align attribute, applications specify it in the input media type of the MF transform, and it will need to be passed to gstreamer decoder plugin input caps.
Yes, mapping it in this direction is not very useful right now but I thought it was better to add the new attribute mapping support all in the same change. I can split it if that matters.
FWIW most other attributes will have no use either in GStreamer for non-PCM formats, audio/x-wma only has bitrate and block_align for instance, does it really matters if we map them nonetheless here and even maps them unconditionally to gst caps?
Regarding why we need to add the attribute when converting from winegstreamer format, well, like the commit message describes, the game is adding the attribute to its desired media format. If we don't add it to the source formats, media_type_is_equal will clear the MF_MEDIATYPE_EQUAL_FORMAT_DATA flag, and source_reader_set_compatible_media_type will never succeed finding an exact match. -- Rémi Bernon <rbernon(a)codeweavers.com>
On 10/27/21 15:57, Rémi Bernon wrote:
On 10/27/21 10:45 PM, Rémi Bernon wrote:
On 10/27/21 10:30 PM, Zebediah Figura (she/her) wrote:
On 10/27/21 10:25, Rémi Bernon wrote:
Planet Coaster adds this attribute to its media type when loading user music in some of the supported formats, and missing it will make media type matching only partially succeed.
This will also be useful for WMA/XMA compressed formats.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
Can you please describe what exactly it's doing, in terms of specific Media Foundation APIs? As it is I can't tell why you're trying to translate this to and from winegstreamer formats.
Note also that "block_align" is not used by anything in GStreamer for PCM formats.
WMA/XMA formats caps need a block_align attribute, applications specify it in the input media type of the MF transform, and it will need to be passed to gstreamer decoder plugin input caps.
Yes, mapping it in this direction is not very useful right now but I thought it was better to add the new attribute mapping support all in the same change. I can split it if that matters.
FWIW most other attributes will have no use either in GStreamer for non-PCM formats, audio/x-wma only has bitrate and block_align for instance, does it really matters if we map them nonetheless here and even maps them unconditionally to gst caps?
Regarding why we need to add the attribute when converting from winegstreamer format, well, like the commit message describes, the game is adding the attribute to its desired media format.
If we don't add it to the source formats, media_type_is_equal will clear the MF_MEDIATYPE_EQUAL_FORMAT_DATA flag, and source_reader_set_compatible_media_type will never succeed finding an exact match.
Part of the reason I ask is that I'm not so intimately familiar with mfplat as to know what "adding the attribute to its desired media format" means. I'm guessing that what happens is that the application either adds a MF_MT_AUDIO_BLOCK_ALIGNMENT attribute to the media type returned from IMFMediaTypeHandler::GetMediaTypeByIndex(), or constructs its own media type with that attribute, and then passes it to IMFMediaTypeHandler::SetCurrentMediaType(), and then that the mfreadwrite source reader requires this to exactly match one of the formats enumerated by the media source. In that case it seems like all we really need to do here is patch mf_media_type_from_wg_format(). I don't see any reason to do any extra translation or to pass this to the winegstreamer backend, since block alignment for PCM should be fully dependent on bit depth and channel count.
On 10/27/21 15:45, Rémi Bernon wrote:
On 10/27/21 10:30 PM, Zebediah Figura (she/her) wrote:
On 10/27/21 10:25, Rémi Bernon wrote:
Planet Coaster adds this attribute to its media type when loading user music in some of the supported formats, and missing it will make media type matching only partially succeed.
This will also be useful for WMA/XMA compressed formats.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
Can you please describe what exactly it's doing, in terms of specific Media Foundation APIs? As it is I can't tell why you're trying to translate this to and from winegstreamer formats.
Note also that "block_align" is not used by anything in GStreamer for PCM formats.
WMA/XMA formats caps need a block_align attribute, applications specify it in the input media type of the MF transform, and it will need to be passed to gstreamer decoder plugin input caps.
Yes, mapping it in this direction is not very useful right now but I thought it was better to add the new attribute mapping support all in the same change. I can split it if that matters.
FWIW most other attributes will have no use either in GStreamer for non-PCM formats, audio/x-wma only has bitrate and block_align for instance, does it really matters if we map them nonetheless here and even maps them unconditionally to gst caps?
Right, although for the purposes of this patch I'm more interested in what Planet Coaster is doing. If/when we need to translate WMA or other compressed formats directly to winegstreamer, we should deal with things like block_align then. Possibly we will want to add another struct to the union in wg_format instead of reusing the existing one.
participants (3)
-
Rémi Bernon -
Zebediah Figura -
Zebediah Figura (she/her)