[PATCH v3 0/7] MR7432: mp3dmod: Add future support for Float encoding and fix various discrepancies with Windows
This MR adds the ability to support float encoding (which will be used by the IMFTransform interface). It also adds tests and addresses a number of small discrepancies discovered between the Wine and native implementation. -- v3: mp3dmod: Ensure nChannels is greater than zero on input. mp3dmod/tests: Add test for nChannels = 0 on input. https://gitlab.winehq.org/wine/wine/-/merge_requests/7432
From: Brendan McGrath <bmcgrath(a)codeweavers.com> MS documentation states: "If the DMO does not support this method, the method returns S_OK" https://learn.microsoft.com/en-us/previous-versions/windows/desktop/api/medi... https://learn.microsoft.com/en-us/previous-versions/windows/desktop/api/medi... --- dlls/mp3dmod/mp3dmod.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/mp3dmod/mp3dmod.c b/dlls/mp3dmod/mp3dmod.c index 55a5b2f5933..5be79f9c1a4 100644 --- a/dlls/mp3dmod/mp3dmod.c +++ b/dlls/mp3dmod/mp3dmod.c @@ -421,16 +421,16 @@ static HRESULT WINAPI MediaObject_Discontinuity(IMediaObject *iface, DWORD index static HRESULT WINAPI MediaObject_AllocateStreamingResources(IMediaObject *iface) { - FIXME("(%p)->() stub!\n", iface); + TRACE("(%p)->()\n", iface); - return E_NOTIMPL; + return S_OK; } static HRESULT WINAPI MediaObject_FreeStreamingResources(IMediaObject *iface) { - FIXME("(%p)->() stub!\n", iface); + TRACE("(%p)->()\n", iface); - return E_NOTIMPL; + return S_OK; } static HRESULT WINAPI MediaObject_GetInputStatus(IMediaObject *iface, DWORD index, DWORD *flags) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7432
From: Brendan McGrath <bmcgrath(a)codeweavers.com> --- dlls/mp3dmod/mp3dmod.c | 101 ++++++++++++++++++++++------------------- 1 file changed, 54 insertions(+), 47 deletions(-) diff --git a/dlls/mp3dmod/mp3dmod.c b/dlls/mp3dmod/mp3dmod.c index 5be79f9c1a4..ce7193f4247 100644 --- a/dlls/mp3dmod/mp3dmod.c +++ b/dlls/mp3dmod/mp3dmod.c @@ -60,6 +60,59 @@ static inline struct mp3_decoder *impl_from_IUnknown(IUnknown *iface) return CONTAINING_RECORD(iface, struct mp3_decoder, IUnknown_inner); } +static HRESULT decoder_set_output_type(struct mp3_decoder *decoder, DWORD index, const DMO_MEDIA_TYPE *type, DWORD flags, BOOL allow_float) +{ + WAVEFORMATEX *format; + long enc; + int err; + + if (index) + return DMO_E_INVALIDSTREAMINDEX; + + if (!decoder->intype_set) + return DMO_E_TYPE_NOT_SET; + + if (flags & DMO_SET_TYPEF_CLEAR) + { + MoFreeMediaType(&decoder->outtype); + decoder->outtype_set = FALSE; + return S_OK; + } + + if (!IsEqualGUID(&type->formattype, &WMFORMAT_WaveFormatEx)) + return DMO_E_TYPE_NOT_ACCEPTED; + + format = (WAVEFORMATEX *)type->pbFormat; + + if (format->wBitsPerSample == 8) + enc = MPG123_ENC_UNSIGNED_8; + else if (format->wBitsPerSample == 16) + enc = MPG123_ENC_SIGNED_16; + else if (allow_float && format->wBitsPerSample == 32) + enc = MPG123_ENC_FLOAT_32; + else + { + ERR("Cannot decode to bit depth %u.\n", format->wBitsPerSample); + return DMO_E_TYPE_NOT_ACCEPTED; + } + + if (!(flags & DMO_SET_TYPEF_TEST_ONLY)) + { + err = mpg123_format(decoder->mh, format->nSamplesPerSec, format->nChannels, enc); + if (err != MPG123_OK) + { + ERR("Failed to set format: %u channels, %lu samples/sec, %u bits/sample.\n", + format->nChannels, format->nSamplesPerSec, format->wBitsPerSample); + return DMO_E_TYPE_NOT_ACCEPTED; + } + if (decoder->outtype_set) + MoFreeMediaType(&decoder->outtype); + MoCopyMediaType(&decoder->outtype, type); + decoder->outtype_set = TRUE; + } + + return S_OK; +} static HRESULT WINAPI Unknown_QueryInterface(IUnknown *iface, REFIID iid, void **obj) { struct mp3_decoder *This = impl_from_IUnknown(iface); @@ -261,56 +314,10 @@ static HRESULT WINAPI MediaObject_SetInputType(IMediaObject *iface, DWORD index, static HRESULT WINAPI MediaObject_SetOutputType(IMediaObject *iface, DWORD index, const DMO_MEDIA_TYPE *type, DWORD flags) { struct mp3_decoder *This = impl_from_IMediaObject(iface); - WAVEFORMATEX *format; - long enc; - int err; TRACE("(%p)->(%ld, %p, %#lx)\n", iface, index, type, flags); - if (index) - return DMO_E_INVALIDSTREAMINDEX; - - if (!This->intype_set) - return DMO_E_TYPE_NOT_SET; - - if (flags & DMO_SET_TYPEF_CLEAR) - { - MoFreeMediaType(&This->outtype); - This->outtype_set = FALSE; - return S_OK; - } - - if (!IsEqualGUID(&type->formattype, &WMFORMAT_WaveFormatEx)) - return DMO_E_TYPE_NOT_ACCEPTED; - - format = (WAVEFORMATEX *)type->pbFormat; - - if (format->wBitsPerSample == 8) - enc = MPG123_ENC_UNSIGNED_8; - else if (format->wBitsPerSample == 16) - enc = MPG123_ENC_SIGNED_16; - else - { - ERR("Cannot decode to bit depth %u.\n", format->wBitsPerSample); - return DMO_E_TYPE_NOT_ACCEPTED; - } - - if (!(flags & DMO_SET_TYPEF_TEST_ONLY)) - { - err = mpg123_format(This->mh, format->nSamplesPerSec, format->nChannels, enc); - if (err != MPG123_OK) - { - ERR("Failed to set format: %u channels, %lu samples/sec, %u bits/sample.\n", - format->nChannels, format->nSamplesPerSec, format->wBitsPerSample); - return DMO_E_TYPE_NOT_ACCEPTED; - } - if (This->outtype_set) - MoFreeMediaType(&This->outtype); - MoCopyMediaType(&This->outtype, type); - This->outtype_set = TRUE; - } - - return S_OK; + return decoder_set_output_type(This, index, type, flags, FALSE); } static HRESULT WINAPI MediaObject_GetInputCurrentType(IMediaObject *iface, DWORD index, DMO_MEDIA_TYPE *type) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7432
From: Brendan McGrath <brendan(a)redmandi.com> --- dlls/mp3dmod/tests/mp3dmod.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/dlls/mp3dmod/tests/mp3dmod.c b/dlls/mp3dmod/tests/mp3dmod.c index 107533e6316..f092f5846b1 100644 --- a/dlls/mp3dmod/tests/mp3dmod.c +++ b/dlls/mp3dmod/tests/mp3dmod.c @@ -746,6 +746,35 @@ static void test_media_types(void) check_dmo_media_type(&mt, &output_mt); MoFreeMediaType(&mt); + output_format.nSamplesPerSec = 24000; + output_format.nAvgBytesPerSec = 24000; + hr = IMediaObject_SetOutputType(dmo, 0, &output_mt, DMO_SET_TYPEF_TEST_ONLY); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + output_format.nSamplesPerSec = 12000; + output_format.nAvgBytesPerSec = 12000; + hr = IMediaObject_SetOutputType(dmo, 0, &output_mt, DMO_SET_TYPEF_TEST_ONLY); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + output_format.nSamplesPerSec = 6000; + output_format.nAvgBytesPerSec = 6000; + hr = IMediaObject_SetOutputType(dmo, 0, &output_mt, DMO_SET_TYPEF_TEST_ONLY); + todo_wine + ok(hr == DMO_E_TYPE_NOT_ACCEPTED, "Got hr %#lx.\n", hr); + + output_format.nSamplesPerSec = 12000; + output_format.nAvgBytesPerSec = 6000; + hr = IMediaObject_SetOutputType(dmo, 0, &output_mt, DMO_SET_TYPEF_TEST_ONLY); + todo_wine + ok(hr == E_INVALIDARG, "Got hr %#lx.\n", hr); + + output_format.nSamplesPerSec = 48000; + output_format.nAvgBytesPerSec = 48000; + output_format.nChannels = 2; + hr = IMediaObject_SetOutputType(dmo, 0, &output_mt, DMO_SET_TYPEF_TEST_ONLY); + todo_wine + ok(hr == E_INVALIDARG, "Got hr %#lx.\n", hr); + IMediaObject_Release(dmo); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7432
From: Brendan McGrath <brendan(a)redmandi.com> If the requested output type has a sample rate that doesn't equal the average bytes per second when multiplied by the block alignment, then Windows returns E_INVALIDARG. Block alignment must also be the number of channels multiplied by the bytes per sample (bytes per sample being the requested bits per sample divided by 8). --- dlls/mp3dmod/mp3dmod.c | 4 ++++ dlls/mp3dmod/tests/mp3dmod.c | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/mp3dmod/mp3dmod.c b/dlls/mp3dmod/mp3dmod.c index ce7193f4247..0a2cf4038a4 100644 --- a/dlls/mp3dmod/mp3dmod.c +++ b/dlls/mp3dmod/mp3dmod.c @@ -96,6 +96,10 @@ static HRESULT decoder_set_output_type(struct mp3_decoder *decoder, DWORD index, return DMO_E_TYPE_NOT_ACCEPTED; } + if (format->nChannels * format->wBitsPerSample/8 != format->nBlockAlign + || format->nSamplesPerSec * format->nBlockAlign != format->nAvgBytesPerSec) + return E_INVALIDARG; + if (!(flags & DMO_SET_TYPEF_TEST_ONLY)) { err = mpg123_format(decoder->mh, format->nSamplesPerSec, format->nChannels, enc); diff --git a/dlls/mp3dmod/tests/mp3dmod.c b/dlls/mp3dmod/tests/mp3dmod.c index f092f5846b1..5175df94f78 100644 --- a/dlls/mp3dmod/tests/mp3dmod.c +++ b/dlls/mp3dmod/tests/mp3dmod.c @@ -765,14 +765,12 @@ static void test_media_types(void) output_format.nSamplesPerSec = 12000; output_format.nAvgBytesPerSec = 6000; hr = IMediaObject_SetOutputType(dmo, 0, &output_mt, DMO_SET_TYPEF_TEST_ONLY); - todo_wine ok(hr == E_INVALIDARG, "Got hr %#lx.\n", hr); output_format.nSamplesPerSec = 48000; output_format.nAvgBytesPerSec = 48000; output_format.nChannels = 2; hr = IMediaObject_SetOutputType(dmo, 0, &output_mt, DMO_SET_TYPEF_TEST_ONLY); - todo_wine ok(hr == E_INVALIDARG, "Got hr %#lx.\n", hr); IMediaObject_Release(dmo); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7432
From: Brendan McGrath <bmcgrath(a)codeweavers.com> MS doc says: "The Windows Media MP3 decoder supports, but does not enumerate, the following output media types. - An output type that has half the sampling rate of the input type. - An output type that has one fourth the sampling rate of the input type" https://learn.microsoft.com/en-us/windows/win32/medfound/windows-media-mp3-d... --- dlls/mp3dmod/mp3dmod.c | 12 +++++++++++- dlls/mp3dmod/tests/mp3dmod.c | 1 - 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/dlls/mp3dmod/mp3dmod.c b/dlls/mp3dmod/mp3dmod.c index 0a2cf4038a4..4be2781f0d8 100644 --- a/dlls/mp3dmod/mp3dmod.c +++ b/dlls/mp3dmod/mp3dmod.c @@ -62,7 +62,7 @@ static inline struct mp3_decoder *impl_from_IUnknown(IUnknown *iface) static HRESULT decoder_set_output_type(struct mp3_decoder *decoder, DWORD index, const DMO_MEDIA_TYPE *type, DWORD flags, BOOL allow_float) { - WAVEFORMATEX *format; + WAVEFORMATEX *format, *in_format; long enc; int err; @@ -100,6 +100,16 @@ static HRESULT decoder_set_output_type(struct mp3_decoder *decoder, DWORD index, || format->nSamplesPerSec * format->nBlockAlign != format->nAvgBytesPerSec) return E_INVALIDARG; + in_format = (WAVEFORMATEX *)decoder->intype.pbFormat; + + if (format->nSamplesPerSec != in_format->nSamplesPerSec && + format->nSamplesPerSec*2 != in_format->nSamplesPerSec && + format->nSamplesPerSec*4 != in_format->nSamplesPerSec) + { + ERR("Cannot decode to %lu samples per second (input %lu).\n", format->nSamplesPerSec, in_format->nSamplesPerSec); + return DMO_E_TYPE_NOT_ACCEPTED; + } + if (!(flags & DMO_SET_TYPEF_TEST_ONLY)) { err = mpg123_format(decoder->mh, format->nSamplesPerSec, format->nChannels, enc); diff --git a/dlls/mp3dmod/tests/mp3dmod.c b/dlls/mp3dmod/tests/mp3dmod.c index 5175df94f78..4bfb9aef0c1 100644 --- a/dlls/mp3dmod/tests/mp3dmod.c +++ b/dlls/mp3dmod/tests/mp3dmod.c @@ -759,7 +759,6 @@ static void test_media_types(void) output_format.nSamplesPerSec = 6000; output_format.nAvgBytesPerSec = 6000; hr = IMediaObject_SetOutputType(dmo, 0, &output_mt, DMO_SET_TYPEF_TEST_ONLY); - todo_wine ok(hr == DMO_E_TYPE_NOT_ACCEPTED, "Got hr %#lx.\n", hr); output_format.nSamplesPerSec = 12000; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7432
From: Brendan McGrath <brendan(a)redmandi.com> --- dlls/mp3dmod/tests/mp3dmod.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dlls/mp3dmod/tests/mp3dmod.c b/dlls/mp3dmod/tests/mp3dmod.c index 4bfb9aef0c1..03f9a163bb7 100644 --- a/dlls/mp3dmod/tests/mp3dmod.c +++ b/dlls/mp3dmod/tests/mp3dmod.c @@ -772,6 +772,11 @@ static void test_media_types(void) hr = IMediaObject_SetOutputType(dmo, 0, &output_mt, DMO_SET_TYPEF_TEST_ONLY); ok(hr == E_INVALIDARG, "Got hr %#lx.\n", hr); + mp3fmt.wfx.nChannels = 0; + hr = IMediaObject_SetInputType(dmo, 0, &input_mt, DMO_SET_TYPEF_TEST_ONLY); + todo_wine + ok(hr == DMO_E_TYPE_NOT_ACCEPTED, "Got hr %#lx.\n", hr); + IMediaObject_Release(dmo); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7432
From: Brendan McGrath <brendan(a)redmandi.com> --- dlls/mp3dmod/mp3dmod.c | 6 ++++++ dlls/mp3dmod/tests/mp3dmod.c | 1 - 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/dlls/mp3dmod/mp3dmod.c b/dlls/mp3dmod/mp3dmod.c index 4be2781f0d8..664c87a3375 100644 --- a/dlls/mp3dmod/mp3dmod.c +++ b/dlls/mp3dmod/mp3dmod.c @@ -295,6 +295,7 @@ static HRESULT WINAPI MediaObject_GetOutputType(IMediaObject *iface, DWORD index static HRESULT WINAPI MediaObject_SetInputType(IMediaObject *iface, DWORD index, const DMO_MEDIA_TYPE *type, DWORD flags) { struct mp3_decoder *dmo = impl_from_IMediaObject(iface); + const WAVEFORMATEX *format; TRACE("iface %p, index %lu, type %p, flags %#lx.\n", iface, index, type, flags); @@ -314,6 +315,11 @@ static HRESULT WINAPI MediaObject_SetInputType(IMediaObject *iface, DWORD index, || !IsEqualGUID(&type->formattype, &WMFORMAT_WaveFormatEx)) return DMO_E_TYPE_NOT_ACCEPTED; + format = (WAVEFORMATEX *) type->pbFormat; + + if (!format->nChannels) + return DMO_E_TYPE_NOT_ACCEPTED; + if (!(flags & DMO_SET_TYPEF_TEST_ONLY)) { if (dmo->intype_set) diff --git a/dlls/mp3dmod/tests/mp3dmod.c b/dlls/mp3dmod/tests/mp3dmod.c index 03f9a163bb7..d8ddd94d4e5 100644 --- a/dlls/mp3dmod/tests/mp3dmod.c +++ b/dlls/mp3dmod/tests/mp3dmod.c @@ -774,7 +774,6 @@ static void test_media_types(void) mp3fmt.wfx.nChannels = 0; hr = IMediaObject_SetInputType(dmo, 0, &input_mt, DMO_SET_TYPEF_TEST_ONLY); - todo_wine ok(hr == DMO_E_TYPE_NOT_ACCEPTED, "Got hr %#lx.\n", hr); IMediaObject_Release(dmo); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7432
Manual testing shows that DMOs also interpret 32-bit depth as float, ignoring the format tag, so we don't need that "allow_float" parameter. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7432#note_96715
On Tue Mar 4 00:40:29 2025 +0000, Elizabeth Figura wrote:
Manual testing shows that DMOs also interpret 32-bit depth as float, ignoring the format tag, so we don't need that "allow_float" parameter. So it does; thanks for that. I'll capture that in a test and (I think) move that logic back to `MediaObject_SetOutputType` (I think I only moved it out to add that flag).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7432#note_96716
participants (3)
-
Brendan McGrath -
Brendan McGrath (＠redmcg)
-
Elizabeth Figura (＠zfigura)