[PATCH v4 0/8] 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. -- v4: mp3dmod: Ensure nChannels is greater than zero on input. mp3dmod/tests: Add test for nChannels = 0 on input. mp3dmod: Only allow 1/2 and 1/4 subsampling. mp3dmod: Return error if requested output format values don't agree. mp3dmod/tests: Test different output sample rates. mp3dmod: Add support for 32-bit sample size. mp3dmod/tests: Add test for 32-bit sample size. mp3dmod: Return S_OK in Allocate/Free Resources. 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/tests/mp3dmod.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/dlls/mp3dmod/tests/mp3dmod.c b/dlls/mp3dmod/tests/mp3dmod.c index 107533e6316..5eb888ad663 100644 --- a/dlls/mp3dmod/tests/mp3dmod.c +++ b/dlls/mp3dmod/tests/mp3dmod.c @@ -746,6 +746,16 @@ static void test_media_types(void) check_dmo_media_type(&mt, &output_mt); MoFreeMediaType(&mt); + /* Windows accepts 32 bits per sample but does not enumerate it */ + output_format.nChannels = 1; + output_format.wBitsPerSample = 32; + output_format.nBlockAlign = 4; + output_format.nSamplesPerSec = 48000; + output_format.nAvgBytesPerSec = 192000; + hr = IMediaObject_SetOutputType(dmo, 0, &output_mt, DMO_SET_TYPEF_TEST_ONLY); + todo_wine + ok(hr == S_OK, "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> --- dlls/mp3dmod/mp3dmod.c | 2 ++ dlls/mp3dmod/tests/mp3dmod.c | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/dlls/mp3dmod/mp3dmod.c b/dlls/mp3dmod/mp3dmod.c index 5be79f9c1a4..6e88c1905ae 100644 --- a/dlls/mp3dmod/mp3dmod.c +++ b/dlls/mp3dmod/mp3dmod.c @@ -289,6 +289,8 @@ static HRESULT WINAPI MediaObject_SetOutputType(IMediaObject *iface, DWORD index enc = MPG123_ENC_UNSIGNED_8; else if (format->wBitsPerSample == 16) enc = MPG123_ENC_SIGNED_16; + else if (format->wBitsPerSample == 32) + enc = MPG123_ENC_FLOAT_32; else { ERR("Cannot decode to bit depth %u.\n", format->wBitsPerSample); diff --git a/dlls/mp3dmod/tests/mp3dmod.c b/dlls/mp3dmod/tests/mp3dmod.c index 5eb888ad663..116da0e5bf7 100644 --- a/dlls/mp3dmod/tests/mp3dmod.c +++ b/dlls/mp3dmod/tests/mp3dmod.c @@ -753,7 +753,6 @@ static void test_media_types(void) output_format.nSamplesPerSec = 48000; output_format.nAvgBytesPerSec = 192000; hr = IMediaObject_SetOutputType(dmo, 0, &output_mt, DMO_SET_TYPEF_TEST_ONLY); - todo_wine ok(hr == S_OK, "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> --- 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 116da0e5bf7..b27d6f82a34 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); + /* Windows accepts 32 bits per sample but does not enumerate it */ output_format.nChannels = 1; output_format.wBitsPerSample = 32; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7432
From: Brendan McGrath <bmcgrath(a)codeweavers.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 6e88c1905ae..b69228865ad 100644 --- a/dlls/mp3dmod/mp3dmod.c +++ b/dlls/mp3dmod/mp3dmod.c @@ -297,6 +297,10 @@ static HRESULT WINAPI MediaObject_SetOutputType(IMediaObject *iface, 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(This->mh, format->nSamplesPerSec, format->nChannels, enc); diff --git a/dlls/mp3dmod/tests/mp3dmod.c b/dlls/mp3dmod/tests/mp3dmod.c index b27d6f82a34..7667fa112ee 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); /* Windows accepts 32 bits per sample but does not enumerate it */ -- 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 b69228865ad..9055f63cf67 100644 --- a/dlls/mp3dmod/mp3dmod.c +++ b/dlls/mp3dmod/mp3dmod.c @@ -261,7 +261,7 @@ 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; + WAVEFORMATEX *format, *in_format; long enc; int err; @@ -301,6 +301,16 @@ static HRESULT WINAPI MediaObject_SetOutputType(IMediaObject *iface, DWORD index || format->nSamplesPerSec * format->nBlockAlign != format->nAvgBytesPerSec) return E_INVALIDARG; + in_format = (WAVEFORMATEX *)This->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(This->mh, format->nSamplesPerSec, format->nChannels, enc); diff --git a/dlls/mp3dmod/tests/mp3dmod.c b/dlls/mp3dmod/tests/mp3dmod.c index 7667fa112ee..46ea79c3d58 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 <bmcgrath(a)codeweavers.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 46ea79c3d58..fc9e99d3833 100644 --- a/dlls/mp3dmod/tests/mp3dmod.c +++ b/dlls/mp3dmod/tests/mp3dmod.c @@ -781,6 +781,11 @@ static void test_media_types(void) hr = IMediaObject_SetOutputType(dmo, 0, &output_mt, DMO_SET_TYPEF_TEST_ONLY); ok(hr == S_OK, "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 <bmcgrath(a)codeweavers.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 9055f63cf67..211d4ac63a9 100644 --- a/dlls/mp3dmod/mp3dmod.c +++ b/dlls/mp3dmod/mp3dmod.c @@ -228,6 +228,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); @@ -247,6 +248,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 fc9e99d3833..655224da7ca 100644 --- a/dlls/mp3dmod/tests/mp3dmod.c +++ b/dlls/mp3dmod/tests/mp3dmod.c @@ -783,7 +783,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
On Tue Mar 4 00:40:29 2025 +0000, Brendan McGrath wrote:
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). OK - that's done. Makes the diff a lot smaller.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7432#note_96720
This merge request was approved by Elizabeth Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7432
participants (3)
-
Brendan McGrath -
Brendan McGrath (＠redmcg)
-
Elizabeth Figura (＠zfigura)