27 Oct
2021
27 Oct
'21
3:25 p.m.
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
27 Oct
27 Oct
3:25 p.m.
New subject: [PATCH 2/2] winegstreamer: Allow some audio resampling in the default pipeline.
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
8:34 p.m.
New subject: [PATCH 2/2] winegstreamer: Allow some audio resampling in the default pipeline.
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.
8 Nov
8 Nov
4:36 p.m.
New subject: [PATCH 2/2] winegstreamer: Allow some audio resampling in the default pipeline.
On 10/27/21 22:34, Zebediah Figura (she/her) wrote:
On 10/27/21 10:25, Rémi Bernon wrote:
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>
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.
5:41 p.m.
New subject: [PATCH 2/2] winegstreamer: Allow some audio resampling in the default pipeline.
On 11/8/21 17:36, Rémi Bernon wrote:
On 10/27/21 22:34, Zebediah Figura (she/her) wrote:
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-mfreadwri…
--
Rémi Bernon <rbernon(a)codeweavers.com>
On 10/27/21 10:25, Rémi Bernon wrote:
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. 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.
9:19 p.m.
New subject: [PATCH 2/2] winegstreamer: Allow some audio resampling in the default pipeline.
On 11/8/21 11:41 AM, Rémi Bernon wrote:
On 11/8/21 17:36, Rémi Bernon wrote:
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 22:34, Zebediah Figura (she/her)
wrote:
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-mfreadwri…
On 10/27/21 10:25, Rémi Bernon wrote:
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. 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.
27 Oct
27 Oct
8:30 p.m.
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.
8:45 p.m.
On 10/27/21 10:30 PM, Zebediah Figura (she/her) wrote:
On 10/27/21 10:25, Rémi Bernon wrote:
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>
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.
8:57 p.m.
On 10/27/21 10:45 PM, Rémi Bernon wrote:
On 10/27/21 10:30 PM, Zebediah Figura (she/her)
wrote:
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 10:25, Rémi Bernon wrote:
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? 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.
9:32 p.m.
On 10/27/21 15:57, Rémi Bernon wrote:
On 10/27/21 10:45 PM, Rémi Bernon wrote:
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 10:30 PM, Zebediah Figura (she/her)
wrote:
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.
On 10/27/21 10:25, Rémi Bernon wrote:
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? 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.
9:06 p.m.
On 10/27/21 15:45, Rémi Bernon wrote:
On 10/27/21 10:30 PM, Zebediah Figura (she/her)
wrote:
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.
On 10/27/21 10:25, Rémi Bernon wrote:
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? 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.
924
days inactive
936
days old
10 comments
3 participants
participants (3)
-
Rémi Bernon -
Zebediah Figura -
Zebediah Figura (she/her)