[PATCH 1/2] mfreadwrite/tests: Test setting a non-native bit depth on the reader.
--- dlls/mfreadwrite/tests/mfplat.c | 52 +++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/dlls/mfreadwrite/tests/mfplat.c b/dlls/mfreadwrite/tests/mfplat.c index 8ea4e5038f6..8381e535d26 100644 --- a/dlls/mfreadwrite/tests/mfplat.c +++ b/dlls/mfreadwrite/tests/mfplat.c @@ -362,6 +362,8 @@ static HRESULT WINAPI test_source_CreatePresentationDescriptor(IMFMediaSource *i ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); hr = IMFMediaType_SetGUID(media_type, &MF_MT_SUBTYPE, &MFAudioFormat_PCM); ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); + hr = IMFMediaType_SetUINT32(media_type, &MF_MT_AUDIO_BITS_PER_SAMPLE, 32); + ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); hr = MFCreateStreamDescriptor(i, 1, &media_type, &sds[i]); ok(hr == S_OK, "Failed to create stream descriptor, hr %#x.\n", hr); @@ -897,6 +899,7 @@ static void test_source_reader_from_media_source(void) struct async_callback *callback; IMFSourceReader *reader; IMFMediaSource *source; + IMFMediaType *media_type; HRESULT hr; DWORD actual_index, stream_flags; IMFSample *sample; @@ -1042,6 +1045,55 @@ static void test_source_reader_from_media_source(void) IMFSourceReader_Release(reader); IMFMediaSource_Release(source); + /* Request a non-native bit depth. */ + source = create_test_source(1); + ok(!!source, "Failed to create test source.\n"); + + hr = MFCreateSourceReaderFromMediaSource(source, NULL, &reader); + ok(hr == S_OK, "Failed to create source reader, hr %#x.\n", hr); + + hr = MFCreateMediaType(&media_type); + ok(hr == S_OK, "Failed to create media type, hr %#x.\n", hr); + hr = IMFMediaType_SetGUID(media_type, &MF_MT_MAJOR_TYPE, &MFMediaType_Audio); + ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); + hr = IMFMediaType_SetGUID(media_type, &MF_MT_SUBTYPE, &MFAudioFormat_PCM); + ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); + hr = IMFMediaType_SetUINT32(media_type, &MF_MT_AUDIO_BITS_PER_SAMPLE, 16); + ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); + + hr = IMFSourceReader_SetCurrentMediaType(reader, 0, NULL, media_type); +todo_wine + ok(hr == MF_E_TOPO_CODEC_NOT_FOUND, "Unexpected success setting current media type, hr %#x.\n", hr); + + IMFMediaType_Release(media_type); + + hr = MFCreateMediaType(&media_type); + ok(hr == S_OK, "Failed to create media type, hr %#x.\n", hr); + hr = IMFMediaType_SetGUID(media_type, &MF_MT_MAJOR_TYPE, &MFMediaType_Audio); + ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); + hr = IMFMediaType_SetGUID(media_type, &MF_MT_SUBTYPE, &MFAudioFormat_PCM); + ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); + hr = IMFMediaType_SetUINT32(media_type, &MF_MT_AUDIO_BITS_PER_SAMPLE, 32); + ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); + + hr = IMFSourceReader_SetCurrentMediaType(reader, 0, NULL, media_type); + ok(hr == S_OK, "Failed to set current media type, hr %#x.\n", hr); + + hr = IMFSourceReader_SetStreamSelection(reader, 0, TRUE); + ok(hr == S_OK, "Failed to select a stream, hr %#x.\n", hr); + + hr = IMFSourceReader_ReadSample(reader, 0, 0, &actual_index, &stream_flags, ×tamp, &sample); + ok(hr == S_OK, "Failed to get a sample, hr %#x.\n", hr); + ok(actual_index == 0, "Unexpected stream index %u\n", actual_index); + ok(!stream_flags, "Unexpected stream flags %#x.\n", stream_flags); + ok(timestamp == 123, "Unexpected timestamp.\n"); + ok(!!sample, "Expected sample object.\n"); + IMFSample_Release(sample); + + IMFMediaType_Release(media_type); + IMFSourceReader_Release(reader); + IMFMediaSource_Release(source); + /* Async mode. */ source = create_test_source(3); ok(!!source, "Failed to create test source.\n"); -- 2.31.0
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/mfreadwrite/reader.c | 4 ++-- dlls/mfreadwrite/tests/mfplat.c | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/dlls/mfreadwrite/reader.c b/dlls/mfreadwrite/reader.c index d3df7abd9fc..6a8606bcbc6 100644 --- a/dlls/mfreadwrite/reader.c +++ b/dlls/mfreadwrite/reader.c @@ -1606,7 +1606,7 @@ static HRESULT source_reader_set_compatible_media_type(struct source_reader *rea return MF_E_INVALIDMEDIATYPE; /* No need for a decoder or type change. */ - if (flags & MF_MEDIATYPE_EQUAL_FORMAT_TYPES) + if (flags & MF_MEDIATYPE_EQUAL_FORMAT_DATA) return S_OK; if (FAILED(hr = source_reader_get_source_type_handler(reader, index, &type_handler))) @@ -1614,7 +1614,7 @@ static HRESULT source_reader_set_compatible_media_type(struct source_reader *rea while (!type_set && IMFMediaTypeHandler_GetMediaTypeByIndex(type_handler, i++, &native_type) == S_OK) { - static const DWORD compare_flags = MF_MEDIATYPE_EQUAL_MAJOR_TYPES | MF_MEDIATYPE_EQUAL_FORMAT_TYPES; + static const DWORD compare_flags = MF_MEDIATYPE_EQUAL_MAJOR_TYPES | MF_MEDIATYPE_EQUAL_FORMAT_DATA; if (SUCCEEDED(IMFMediaType_IsEqual(native_type, type, &flags)) && (flags & compare_flags) == compare_flags) { diff --git a/dlls/mfreadwrite/tests/mfplat.c b/dlls/mfreadwrite/tests/mfplat.c index 8381e535d26..ded45afe771 100644 --- a/dlls/mfreadwrite/tests/mfplat.c +++ b/dlls/mfreadwrite/tests/mfplat.c @@ -1062,7 +1062,6 @@ static void test_source_reader_from_media_source(void) ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); hr = IMFSourceReader_SetCurrentMediaType(reader, 0, NULL, media_type); -todo_wine ok(hr == MF_E_TOPO_CODEC_NOT_FOUND, "Unexpected success setting current media type, hr %#x.\n", hr); IMFMediaType_Release(media_type); -- 2.31.0
On 3/31/21 4:30 PM, Giovanni Mascellani wrote:
@@ -1606,7 +1606,7 @@ static HRESULT source_reader_set_compatible_media_type(struct source_reader *rea return MF_E_INVALIDMEDIATYPE;
/* No need for a decoder or type change. */ - if (flags & MF_MEDIATYPE_EQUAL_FORMAT_TYPES) + if (flags & MF_MEDIATYPE_EQUAL_FORMAT_DATA) return S_OK;
if (FAILED(hr = source_reader_get_source_type_handler(reader, index, &type_handler))) @@ -1614,7 +1614,7 @@ static HRESULT source_reader_set_compatible_media_type(struct source_reader *rea
while (!type_set && IMFMediaTypeHandler_GetMediaTypeByIndex(type_handler, i++, &native_type) == S_OK) { - static const DWORD compare_flags = MF_MEDIATYPE_EQUAL_MAJOR_TYPES | MF_MEDIATYPE_EQUAL_FORMAT_TYPES; + static const DWORD compare_flags = MF_MEDIATYPE_EQUAL_MAJOR_TYPES | MF_MEDIATYPE_EQUAL_FORMAT_DATA;
if (SUCCEEDED(IMFMediaType_IsEqual(native_type, type, &flags)) && (flags & compare_flags) == compare_flags) { I doesn't seem like a right thing to do. This helper is meant to check if decoder is necessary, for that you only need a subtype test. Admittedly existing naming does not make it obvious.
What should happen in general case: - check major/subtype to see if decoder is necessary at all; - if it's necessary, look for the decoder; - unconditionally compare for format data to see if converter is necessary, and append one. According to docs it should support local transforms, so registering local dummy converter (that is only able to negotiate types) for 16 -> 32bit resampling, and trying again could potentially succeed instead where it returns MF_E_TOPO_CODEC_NOT_FOUND now. It's possible that on Windows reader is either confused by raw types source returns and skips that part, or fails to find a converter in first place (or conversion is disabled by default despite what docs say).
Hi, Il 07/05/21 10:30, Nikolay Sivov ha scritto:
I doesn't seem like a right thing to do. This helper is meant to check if decoder is necessary, for that you only need a subtype test. Admittedly existing naming does not make it obvious.
What should happen in general case:
- check major/subtype to see if decoder is necessary at all; - if it's necessary, look for the decoder; - unconditionally compare for format data to see if converter is necessary, and append one.
According to docs it should support local transforms, so registering local dummy converter (that is only able to negotiate types) for 16 -> 32bit resampling, and trying again could potentially succeed instead where it returns MF_E_TOPO_CODEC_NOT_FOUND now. It's possible that on Windows reader is either confused by raw types source returns and skips that part, or fails to find a converter in first place (or conversion is disabled by default despite what docs say).
I admit my patch is not perfect, but still it seems better than what happens now. At some point the data in the requested media type must be compared with the native media types provided by the source. Currently this does not happen and if the caller requests a 16 bit format while the source provides a 32 bit format, the source reader will acknowledge the request with 16 bit but return 32 bit audio, which will result in garbled audio. Also, suppose that a media source exposes both a 32 bit and a 16 bit media type. I expect (and my tests on Windows seem to confirm it) that if you request 32 bit you get the first media type and if you request 16 bit you get the second, even if they both match at the major/subtype level. The current implementation can only match the first one. Since Windows matches the correct one, it seems that Windows does at least one round of matching taking also media type data into account. According to [1], the search for a decoder only happens if the native type is compressed, otherwise only native media types are allowed. I understand that MSDN can be incorrect, but this seems to align with my (admittedly limited) testing. Do you have evidence of the contrary? [1] https://docs.microsoft.com/en-us/windows/win32/api/mfreadwrite/nf-mfreadwrit... Thanks, Giovanni.
On 5/7/21 1:56 PM, Giovanni Mascellani wrote:
Hi,
Il 07/05/21 10:30, Nikolay Sivov ha scritto:
I doesn't seem like a right thing to do. This helper is meant to check if decoder is necessary, for that you only need a subtype test. Admittedly existing naming does not make it obvious.
What should happen in general case:
- check major/subtype to see if decoder is necessary at all; - if it's necessary, look for the decoder; - unconditionally compare for format data to see if converter is necessary, and append one.
According to docs it should support local transforms, so registering local dummy converter (that is only able to negotiate types) for 16 -> 32bit resampling, and trying again could potentially succeed instead where it returns MF_E_TOPO_CODEC_NOT_FOUND now. It's possible that on Windows reader is either confused by raw types source returns and skips that part, or fails to find a converter in first place (or conversion is disabled by default despite what docs say).
I admit my patch is not perfect, but still it seems better than what happens now.
At some point the data in the requested media type must be compared with the native media types provided by the source. Currently this does not happen and if the caller requests a 16 bit format while the source provides a 32 bit format, the source reader will acknowledge the request with 16 bit but return 32 bit audio, which will result in garbled audio.
Also, suppose that a media source exposes both a 32 bit and a 16 bit media type. I expect (and my tests on Windows seem to confirm it) that if you request 32 bit you get the first media type and if you request 16 bit you get the second, even if they both match at the major/subtype level. The current implementation can only match the first one. Since Windows matches the correct one, it seems that Windows does at least one round of matching taking also media type data into account.
Ok, I checked with video type too, type with different frame size is also rejected, so patches are probably safe. Please resend both of them.
According to [1], the search for a decoder only happens if the native type is compressed, otherwise only native media types are allowed. I understand that MSDN can be incorrect, but this seems to align with my (admittedly limited) testing. Do you have evidence of the contrary?
[1] https://docs.microsoft.com/en-us/windows/win32/api/mfreadwrite/nf-mfreadwrit...
Thanks, Giovanni.
Signed-off-by: Giovanni Mascellani<gmascellani(a)codeweavers.com> Il 31/03/21 15:30, Giovanni Mascellani ha scritto:
--- dlls/mfreadwrite/tests/mfplat.c | 52 +++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)
diff --git a/dlls/mfreadwrite/tests/mfplat.c b/dlls/mfreadwrite/tests/mfplat.c index 8ea4e5038f6..8381e535d26 100644 --- a/dlls/mfreadwrite/tests/mfplat.c +++ b/dlls/mfreadwrite/tests/mfplat.c @@ -362,6 +362,8 @@ static HRESULT WINAPI test_source_CreatePresentationDescriptor(IMFMediaSource *i ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); hr = IMFMediaType_SetGUID(media_type, &MF_MT_SUBTYPE, &MFAudioFormat_PCM); ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); + hr = IMFMediaType_SetUINT32(media_type, &MF_MT_AUDIO_BITS_PER_SAMPLE, 32); + ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr);
hr = MFCreateStreamDescriptor(i, 1, &media_type, &sds[i]); ok(hr == S_OK, "Failed to create stream descriptor, hr %#x.\n", hr); @@ -897,6 +899,7 @@ static void test_source_reader_from_media_source(void) struct async_callback *callback; IMFSourceReader *reader; IMFMediaSource *source; + IMFMediaType *media_type; HRESULT hr; DWORD actual_index, stream_flags; IMFSample *sample; @@ -1042,6 +1045,55 @@ static void test_source_reader_from_media_source(void) IMFSourceReader_Release(reader); IMFMediaSource_Release(source);
+ /* Request a non-native bit depth. */ + source = create_test_source(1); + ok(!!source, "Failed to create test source.\n"); + + hr = MFCreateSourceReaderFromMediaSource(source, NULL, &reader); + ok(hr == S_OK, "Failed to create source reader, hr %#x.\n", hr); + + hr = MFCreateMediaType(&media_type); + ok(hr == S_OK, "Failed to create media type, hr %#x.\n", hr); + hr = IMFMediaType_SetGUID(media_type, &MF_MT_MAJOR_TYPE, &MFMediaType_Audio); + ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); + hr = IMFMediaType_SetGUID(media_type, &MF_MT_SUBTYPE, &MFAudioFormat_PCM); + ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); + hr = IMFMediaType_SetUINT32(media_type, &MF_MT_AUDIO_BITS_PER_SAMPLE, 16); + ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); + + hr = IMFSourceReader_SetCurrentMediaType(reader, 0, NULL, media_type); +todo_wine + ok(hr == MF_E_TOPO_CODEC_NOT_FOUND, "Unexpected success setting current media type, hr %#x.\n", hr); + + IMFMediaType_Release(media_type); + + hr = MFCreateMediaType(&media_type); + ok(hr == S_OK, "Failed to create media type, hr %#x.\n", hr); + hr = IMFMediaType_SetGUID(media_type, &MF_MT_MAJOR_TYPE, &MFMediaType_Audio); + ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); + hr = IMFMediaType_SetGUID(media_type, &MF_MT_SUBTYPE, &MFAudioFormat_PCM); + ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); + hr = IMFMediaType_SetUINT32(media_type, &MF_MT_AUDIO_BITS_PER_SAMPLE, 32); + ok(hr == S_OK, "Failed to set attribute, hr %#x.\n", hr); + + hr = IMFSourceReader_SetCurrentMediaType(reader, 0, NULL, media_type); + ok(hr == S_OK, "Failed to set current media type, hr %#x.\n", hr); + + hr = IMFSourceReader_SetStreamSelection(reader, 0, TRUE); + ok(hr == S_OK, "Failed to select a stream, hr %#x.\n", hr); + + hr = IMFSourceReader_ReadSample(reader, 0, 0, &actual_index, &stream_flags, ×tamp, &sample); + ok(hr == S_OK, "Failed to get a sample, hr %#x.\n", hr); + ok(actual_index == 0, "Unexpected stream index %u\n", actual_index); + ok(!stream_flags, "Unexpected stream flags %#x.\n", stream_flags); + ok(timestamp == 123, "Unexpected timestamp.\n"); + ok(!!sample, "Expected sample object.\n"); + IMFSample_Release(sample); + + IMFMediaType_Release(media_type); + IMFSourceReader_Release(reader); + IMFMediaSource_Release(source); + /* Async mode. */ source = create_test_source(3); ok(!!source, "Failed to create test source.\n");
participants (2)
-
Giovanni Mascellani -
Nikolay Sivov