[PATCH 0/2] MR7465: winegstreamer: Validate the value of cbSize in aac decoder.
This MR fixes a mf:transform test that started failing as a result of MR !7417. It also adds new tests to mf:transform to test different values of `MF_MT_USER_DATA` against the aac decoder. It adds validation of `cbSize` to the aac decoder, which, in addition to fixing the previous test, allows some of these new tests to pass. But it does not validate the content of the user data (as the new tests seem to indicate Windows does). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7465
From: Brendan McGrath <brendan(a)redmandi.com> --- dlls/mf/tests/transform.c | 131 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) diff --git a/dlls/mf/tests/transform.c b/dlls/mf/tests/transform.c index b8a78e949d0..1f9d1cd5632 100644 --- a/dlls/mf/tests/transform.c +++ b/dlls/mf/tests/transform.c @@ -728,6 +728,7 @@ static void check_mft_set_input_type_(int line, IMFTransform *transform, const s init_media_type(media_type, attributes, -1); hr = IMFTransform_SetInputType(transform, 0, media_type, MFT_SET_TYPE_TEST_ONLY); + todo_wine_if(todo) ok_(__FILE__, line)(hr == expect_hr, "SetInputType returned %#lx.\n", hr); hr = IMFTransform_SetInputType(transform, 0, media_type, 0); todo_wine_if(todo) @@ -2881,6 +2882,135 @@ failed: CoUninitialize(); } +static void test_aac_decoder_user_data(void) +{ + /* https://wiki.multimedia.cx/index.php/MPEG-4_Audio */ + static const BYTE aac_raw_codec_data[] = {0x12, 0x08}; /* short form of 1 channel 44.1 Khz */ + static const BYTE aac_raw_codec_data_long[] = {0x17, 0x80, 0x56, 0x22, 0x08}; /* long form of 1 channel 44.1 Khz */ + static const BYTE aac_raw_codec_data_48khz[] = {0x11, 0x90}; /* short form of 1 channel 48 Khz */ + static const struct attribute_desc raw_aac_input_type_desc[] = + { + ATTR_GUID(MF_MT_MAJOR_TYPE, MFMediaType_Audio, .required = TRUE), + ATTR_GUID(MF_MT_SUBTYPE, MFAudioFormat_RAW_AAC1, .required = TRUE), + ATTR_UINT32(MF_MT_AUDIO_SAMPLES_PER_SECOND, 44100, .required = TRUE), + ATTR_UINT32(MF_MT_AUDIO_NUM_CHANNELS, 1), + ATTR_BLOB(MF_MT_USER_DATA, aac_raw_codec_data, sizeof(aac_raw_codec_data), .required = TRUE), + {0}, + }; + static const struct attribute_desc raw_aac_input_type_desc_long[] = + { + ATTR_GUID(MF_MT_MAJOR_TYPE, MFMediaType_Audio, .required = TRUE), + ATTR_GUID(MF_MT_SUBTYPE, MFAudioFormat_RAW_AAC1, .required = TRUE), + ATTR_UINT32(MF_MT_AUDIO_SAMPLES_PER_SECOND, 44100, .required = TRUE), + ATTR_UINT32(MF_MT_AUDIO_NUM_CHANNELS, 1), + ATTR_BLOB(MF_MT_USER_DATA, aac_raw_codec_data_long, sizeof(aac_raw_codec_data_long), .required = TRUE), + {0}, + }; + static const struct attribute_desc raw_aac_input_type_desc_48khz[] = + { + ATTR_GUID(MF_MT_MAJOR_TYPE, MFMediaType_Audio, .required = TRUE), + ATTR_GUID(MF_MT_SUBTYPE, MFAudioFormat_RAW_AAC1, .required = TRUE), + ATTR_UINT32(MF_MT_AUDIO_SAMPLES_PER_SECOND, 48000, .required = TRUE), + ATTR_UINT32(MF_MT_AUDIO_NUM_CHANNELS, 1), + ATTR_BLOB(MF_MT_USER_DATA, aac_raw_codec_data_48khz, sizeof(aac_raw_codec_data_48khz), .required = TRUE), + {0}, + }; + static const struct attribute_desc raw_aac_input_type_desc_mismatch[] = + { + ATTR_GUID(MF_MT_MAJOR_TYPE, MFMediaType_Audio, .required = TRUE), + ATTR_GUID(MF_MT_SUBTYPE, MFAudioFormat_RAW_AAC1, .required = TRUE), + ATTR_UINT32(MF_MT_AUDIO_SAMPLES_PER_SECOND, 44100, .required = TRUE), + ATTR_UINT32(MF_MT_AUDIO_NUM_CHANNELS, 1), + ATTR_BLOB(MF_MT_USER_DATA, aac_raw_codec_data_48khz, sizeof(aac_raw_codec_data_48khz), .required = TRUE), + {0}, + }; + static const struct attribute_desc aac_input_type_desc[] = + { + ATTR_GUID(MF_MT_MAJOR_TYPE, MFMediaType_Audio, .required = TRUE), + ATTR_GUID(MF_MT_SUBTYPE, MFAudioFormat_AAC, .required = TRUE), + ATTR_UINT32(MF_MT_AUDIO_SAMPLES_PER_SECOND, 44100, .required = TRUE), + ATTR_BLOB(MF_MT_USER_DATA, test_aac_codec_data, sizeof(test_aac_codec_data), .required = TRUE), + ATTR_UINT32(MF_MT_AUDIO_BITS_PER_SAMPLE, 16), + ATTR_UINT32(MF_MT_AUDIO_NUM_CHANNELS, 1), + ATTR_UINT32(MF_MT_AUDIO_AVG_BYTES_PER_SECOND, 12000), + ATTR_UINT32(MF_MT_AAC_AUDIO_PROFILE_LEVEL_INDICATION, 41), + ATTR_UINT32(MF_MT_AAC_PAYLOAD_TYPE, 0), + {0}, + }; + + static struct { + const char *name; + const struct attribute_desc *desc; + HRESULT exp_result; + BOOL todo; + BOOL todo_short; + } tests[] = { + { "aac", aac_input_type_desc, S_OK, FALSE, TRUE }, + { "raw aac", raw_aac_input_type_desc, S_OK, FALSE, TRUE }, + { "raw aac long", raw_aac_input_type_desc_long, S_OK, FALSE, TRUE }, + { "raw aac 48Khz", raw_aac_input_type_desc_48khz, S_OK, FALSE, TRUE }, + { "raw aac mismatch", raw_aac_input_type_desc_mismatch, MF_E_INVALIDMEDIATYPE, TRUE }, + }; + + const struct attribute_desc *input_type_desc; + struct attribute_desc input_desc[64]; + unsigned int user_data_index = ~0u; + IMFTransform *transform; + ULONG ret, i, j; + HRESULT hr; + + winetest_push_context("aacdec user_data"); + hr = CoInitialize(NULL); + ok(hr == S_OK, "got %#lx.\n", hr); + + if (FAILED(hr = CoCreateInstance(&CLSID_MSAACDecMFT, NULL, CLSCTX_INPROC_SERVER, + &IID_IMFTransform, (void **)&transform))) + { + win_skip("AAC decoder transform is not available.\n"); + goto failed; + } + + for (i = 0; i < ARRAY_SIZE(tests); i++) + { + winetest_push_context("%s", tests[i].name); + user_data_index = ~0u; + input_type_desc = tests[i].desc; + for (j = 0; j < ARRAY_SIZE(input_desc); j++) + { + input_desc[j] = input_type_desc[j]; + if (!input_desc[j].key) + break; + if (IsEqualGUID(input_desc[j].key, &MF_MT_USER_DATA)) + user_data_index = j; + } + + ok(user_data_index != ~0u, "Could not find MF_MT_USER_DATA.\n"); + ok(i < ARRAY_SIZE(input_desc), "Too many attributes.\n"); + + /* confirm standard input result */ + check_mft_set_input_type_(__LINE__, transform, input_desc, tests[i].exp_result, tests[i].todo); + + if (tests[i].exp_result == S_OK) + { + /* confirm shorter fails */ + input_desc[user_data_index].value.blob.cbSize = input_type_desc[user_data_index].value.blob.cbSize - 1; + check_mft_set_input_type_(__LINE__, transform, input_desc, MF_E_INVALIDMEDIATYPE, tests[i].todo_short); + + /* confirm longer is OK */ + input_desc[user_data_index].value.blob.cbSize = input_type_desc[user_data_index].value.blob.cbSize + 1; + check_mft_set_input_type(transform, input_desc, S_OK); + } + winetest_pop_context(); + } + + ret = IMFTransform_Release(transform); + ok(!ret, "got %lu.\n", ret); + +failed: + winetest_pop_context(); + CoUninitialize(); +} + static void test_aac_decoder(void) { static const BYTE aac_raw_codec_data[] = {0x12, 0x08}; @@ -10303,6 +10433,7 @@ START_TEST(transform) test_sample_copier_output_processing(); test_aac_encoder(); test_aac_decoder(); + test_aac_decoder_user_data(); test_wma_encoder(); test_wma_decoder(); test_wma_decoder_dmo_input_type(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7465
From: Brendan McGrath <brendan(a)redmandi.com> --- dlls/mf/tests/transform.c | 6 +++--- dlls/winegstreamer/aac_decoder.c | 7 ++++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/dlls/mf/tests/transform.c b/dlls/mf/tests/transform.c index 1f9d1cd5632..dcc707b9722 100644 --- a/dlls/mf/tests/transform.c +++ b/dlls/mf/tests/transform.c @@ -2945,10 +2945,10 @@ static void test_aac_decoder_user_data(void) BOOL todo; BOOL todo_short; } tests[] = { - { "aac", aac_input_type_desc, S_OK, FALSE, TRUE }, - { "raw aac", raw_aac_input_type_desc, S_OK, FALSE, TRUE }, + { "aac", aac_input_type_desc, S_OK }, + { "raw aac", raw_aac_input_type_desc, S_OK }, { "raw aac long", raw_aac_input_type_desc_long, S_OK, FALSE, TRUE }, - { "raw aac 48Khz", raw_aac_input_type_desc_48khz, S_OK, FALSE, TRUE }, + { "raw aac 48Khz", raw_aac_input_type_desc_48khz, S_OK }, { "raw aac mismatch", raw_aac_input_type_desc_mismatch, MF_E_INVALIDMEDIATYPE, TRUE }, }; diff --git a/dlls/winegstreamer/aac_decoder.c b/dlls/winegstreamer/aac_decoder.c index d8e8246e1ca..f3b72932b05 100644 --- a/dlls/winegstreamer/aac_decoder.c +++ b/dlls/winegstreamer/aac_decoder.c @@ -32,6 +32,8 @@ WINE_DEFAULT_DEBUG_CHANNEL(mfplat); WINE_DECLARE_DEBUG_CHANNEL(winediag); +#define CBSIZE(x) (sizeof(x) - sizeof(WAVEFORMATEX)) + #define NEXT_WAVEFORMATEXTENSIBLE(format) (WAVEFORMATEXTENSIBLE *)((BYTE *)(&(format)->Format + 1) + (format)->Format.cbSize) static WAVEFORMATEXTENSIBLE const aac_decoder_output_types[] = @@ -356,7 +358,10 @@ static HRESULT WINAPI transform_SetInputType(IMFTransform *iface, DWORD id, IMFM if (!count) return MF_E_INVALIDMEDIATYPE; - if (wfx.Format.nChannels >= ARRAY_SIZE(default_channel_mask) || !wfx.Format.nSamplesPerSec || !wfx.Format.cbSize) + if (wfx.Format.nChannels >= ARRAY_SIZE(default_channel_mask) || !wfx.Format.nSamplesPerSec + /* 2 is the minimum size of AudioSpecificConfig() */ + || wfx.Format.cbSize < 2 + CBSIZE(WAVEFORMATEXTENSIBLE) || (IsEqualGUID(&wfx.SubFormat, &MFAudioFormat_AAC) + && wfx.Format.cbSize < 2 + CBSIZE(WAVEFORMATEXTENSIBLE) + CBSIZE(HEAACWAVEINFO))) return MF_E_INVALIDMEDIATYPE; if (flags & MFT_SET_TYPE_TEST_ONLY) return S_OK; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7465
I double checked the tests this time. Lots of `user32:input` failures, but the two `mf` related failures don't appear to be related to this change: ``` $ xmllint --xpath 'testsuites/testsuite/testcase[failure and starts-with(@name, "mf")]/@name' winetest.xml name="mfmediaengine:mfmediaengine mfmediaengine.c:2640 Test failed: Got unexpected refcount 1." name="mf:mf mf.c:6536 Test failed: Test 3: Unexpected hr 0xc00d36b2." ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7465#note_96496
I'm going to unassign myself since I don't know anything about decisions made for winegstr decoder modules. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7465#note_96545
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/aac_decoder.c:
if (!count) return MF_E_INVALIDMEDIATYPE;
- if (wfx.Format.nChannels >= ARRAY_SIZE(default_channel_mask) || !wfx.Format.nSamplesPerSec || !wfx.Format.cbSize) + if (wfx.Format.nChannels >= ARRAY_SIZE(default_channel_mask) || !wfx.Format.nSamplesPerSec + /* 2 is the minimum size of AudioSpecificConfig() */ + || wfx.Format.cbSize < 2 + CBSIZE(WAVEFORMATEXTENSIBLE) || (IsEqualGUID(&wfx.SubFormat, &MFAudioFormat_AAC) + && wfx.Format.cbSize < 2 + CBSIZE(WAVEFORMATEXTENSIBLE) + CBSIZE(HEAACWAVEINFO))) return MF_E_INVALIDMEDIATYPE;
I'm not sure it's worth adding more checks here, it was mostly just to pass the tests but I don't think anything should care about it failing with specific sizes. If some tests are getting in the way they can be changed to todo_wine / removed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7465#note_96549
On Mon Mar 3 12:43:12 2025 +0000, Rémi Bernon wrote:
I'm not sure it's worth adding more checks here, it was mostly just to pass the tests but I don't think anything should care about it failing with specific sizes. If some tests are getting in the way they can be changed to todo_wine / removed. I'm happy to mark the tests as `todo`. I don't want to remove any (as I think they're the most accurate documentation we have of the Windows API).
But to pass the existing failing test (`mf:transform transform.c:2587`), I think I need at least `wfx.Format.cbSize < 2 + CBSIZE(WAVEFORMATEXTENSIBLE)`. That will ensure `MF_MT_USER_DATA` is present and at least a size of 2. Alternatively I could call `IMFMediaType_GetBlobSize` on `type` and check it that way. That's assuming you didn't mean to mark the existing failing test as `todo` ? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7465#note_96625
I think I need at least `wfx.Format.cbSize < 2 + CBSIZE(WAVEFORMATEXTENSIBLE)`. That will ensure `MF_MT_USER_DATA` is present and at least a size of 2.
That sounds reasonable. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7465#note_96630
participants (4)
-
Brendan McGrath -
Brendan McGrath (＠redmcg)
-
Nikolay Sivov (＠nsivov)
-
Rémi Bernon