[PATCH v2 0/2] MR2690: mfreadwrite/tests: Add more tests for MFCreateSinkWriterFromURL.
The tests shows that, for sink writer created by MFCreateSinkWriterFromURL, we are not able to grab the media sink object immediately after the creation, unless the media sink object can be created without setting media type(MP3, ASF). -- v2: mfreadwrite/tests: Test getting media sink immediately after MFCreateSinkWriterFromURL. mfreadwrite/tests: Add more tests for MFCreateSinkWriterFromURL. https://gitlab.winehq.org/wine/wine/-/merge_requests/2690
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/mfreadwrite/tests/mfplat.c | 70 +++++++++++++++++++++++++++++---- 1 file changed, 62 insertions(+), 8 deletions(-) diff --git a/dlls/mfreadwrite/tests/mfplat.c b/dlls/mfreadwrite/tests/mfplat.c index 41b2a033a59..87fdd917697 100644 --- a/dlls/mfreadwrite/tests/mfplat.c +++ b/dlls/mfreadwrite/tests/mfplat.c @@ -1415,19 +1415,71 @@ done: DestroyWindow(window); } -static void test_sink_writer(void) +static void test_sink_writer_create(void) { + WCHAR tmp_file[MAX_PATH]; IMFSinkWriter *writer; + IMFByteStream *stream; + IMFAttributes *attr; + unsigned int i; HRESULT hr; - hr = MFCreateSinkWriterFromURL(NULL, NULL, NULL, NULL); - ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + struct create_sink_writer_from_url_test + { + const WCHAR *url; + IMFByteStream **stream; + IMFAttributes **attr; + IMFSinkWriter **writer; + HRESULT hr; + } create_sink_writer_from_url_tests[] = + { + {NULL, NULL, NULL, NULL, E_INVALIDARG}, + {NULL, NULL, NULL, &writer, E_INVALIDARG}, + {NULL, NULL, &attr, &writer, E_INVALIDARG}, + {NULL, &stream, NULL, &writer, E_INVALIDARG}, + {NULL, &stream, &attr, &writer, S_OK}, + {tmp_file, NULL, NULL, &writer, S_OK}, + {tmp_file, NULL, &attr, &writer, S_OK}, + {tmp_file, &stream, NULL, &writer, S_OK}, + {tmp_file, &stream, &attr, &writer, S_OK}, + }; - writer = (void *)0xdeadbeef; - hr = MFCreateSinkWriterFromURL(NULL, NULL, NULL, &writer); - ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); - ok(!writer, "Unexpected pointer %p.\n", writer); + /* Get temp file name. */ + GetTempPathW(ARRAY_SIZE(tmp_file), tmp_file); + wcscat(tmp_file, L"tmp.mp4"); + + /* Create attribute and byte stream. */ + hr = MFCreateAttributes(&attr, 1); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFAttributes_SetGUID(attr, &MF_TRANSCODE_CONTAINERTYPE, &MFTranscodeContainerType_MPEG4); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = MFCreateTempFile(MF_ACCESSMODE_READWRITE, MF_OPENMODE_DELETE_IF_EXIST, MF_FILEFLAGS_NONE, &stream); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + /* Test MFCreateSinkWriterFromURL. */ + for (i = 0; i < ARRAY_SIZE(create_sink_writer_from_url_tests); ++i) + { + struct create_sink_writer_from_url_test *test = &create_sink_writer_from_url_tests[i]; + IMFByteStream *test_stream = test->stream ? *test->stream : NULL; + IMFAttributes *test_attr = test->attr ? *test->attr : NULL; + + winetest_push_context("Test %u", i); + + writer = (void *)0xdeadbeef; + hr = MFCreateSinkWriterFromURL(test->url, test_stream, test_attr, test->writer); + todo_wine_if(test->hr == S_OK) + ok(hr == test->hr, "Unexpected hr %#lx, expected %#lx.\n", hr, test->hr); + if (test->writer) + todo_wine_if(test->hr == S_OK) + ok((test->hr == S_OK && writer) || (test->hr != S_OK && !writer), "Unexpected writer pointer %p.\n", writer); + + if (hr == S_OK) + IMFSinkWriter_Release(writer); + + winetest_pop_context(); + } + + /* Test MFCreateSinkWriterFromMediaSink. */ hr = MFCreateSinkWriterFromMediaSink(NULL, NULL, NULL); ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); @@ -1435,6 +1487,8 @@ static void test_sink_writer(void) hr = MFCreateSinkWriterFromMediaSink(NULL, NULL, &writer); ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); ok(!writer, "Unexpected pointer %p.\n", writer); + + winetest_pop_context(); } START_TEST(mfplat) @@ -1451,7 +1505,7 @@ START_TEST(mfplat) test_source_reader("test.mp4", true); test_source_reader_from_media_source(); test_reader_d3d9(); - test_sink_writer(); + test_sink_writer_create(); hr = MFShutdown(); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2690
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/mfreadwrite/tests/mfplat.c | 116 +++++++++++++++++++++++++++++++- 1 file changed, 114 insertions(+), 2 deletions(-) diff --git a/dlls/mfreadwrite/tests/mfplat.c b/dlls/mfreadwrite/tests/mfplat.c index 87fdd917697..9c056754fac 100644 --- a/dlls/mfreadwrite/tests/mfplat.c +++ b/dlls/mfreadwrite/tests/mfplat.c @@ -82,14 +82,44 @@ static IDirect3DDevice9 *create_d3d9_device(IDirect3D9 *d3d9, HWND focus_window) } static HRESULT (WINAPI *pMFCreateMFByteStreamOnStream)(IStream *stream, IMFByteStream **bytestream); +static HRESULT (WINAPI *pMFCreate3GPMediaSink)(IMFByteStream *bytestream, + IMFMediaType *video_type, IMFMediaType *audio_type, IMFMediaSink **sink); +static HRESULT (WINAPI *pMFCreateADTSMediaSink)(IMFByteStream *bytestream, + IMFMediaType *media_type, IMFMediaSink **sink); +static HRESULT (WINAPI *pMFCreateASFMediaSink)(IMFByteStream *bytestream, IMFMediaSink **sink); +static HRESULT (WINAPI *pMFCreateAVIMediaSink)(IMFByteStream *bytestream, + IMFMediaType *video_type, IMFMediaType *audio_type, IMFMediaSink **sink); +static HRESULT (WINAPI *pMFCreateFMPEG4MediaSink)(IMFByteStream *bytestream, + IMFMediaType *video_type, IMFMediaType *audio_type, IMFMediaSink **sink); +static HRESULT (WINAPI *pMFCreateMP3MediaSink)(IMFByteStream *bytestream, IMFMediaSink **sink); +static HRESULT (WINAPI *pMFCreateMPEG4MediaSink)(IMFByteStream *bytestream, + IMFMediaType *video_type, IMFMediaType *audio_type, IMFMediaSink **sink); +static HRESULT (WINAPI *pMFCreateWAVEMediaSink)(IMFByteStream *bytestream, + IMFMediaType *media_type, IMFMediaSink **sink); static void init_functions(void) { - HMODULE mod = GetModuleHandleA("mfplat.dll"); + HMODULE mfplat = GetModuleHandleA("mfplat.dll"); + HMODULE mfsrcsnk = LoadLibraryA("mfsrcsnk.dll"); + HMODULE mf = LoadLibraryA("mf.dll"); -#define X(f) if (!(p##f = (void*)GetProcAddress(mod, #f))) return; +#define X(f) p##f = (void*)GetProcAddress(mfplat, #f) X(MFCreateMFByteStreamOnStream); #undef X + +#define X(f) p##f = (void*)GetProcAddress(mfsrcsnk, #f) + X(MFCreateAVIMediaSink); + X(MFCreateWAVEMediaSink); +#undef X + +#define X(f) p##f = (void*)GetProcAddress(mf, #f) + X(MFCreate3GPMediaSink); + X(MFCreateADTSMediaSink); + X(MFCreateASFMediaSink); + X(MFCreateFMPEG4MediaSink); + X(MFCreateMP3MediaSink); + X(MFCreateMPEG4MediaSink); +#undef X } enum source_state @@ -1491,6 +1521,87 @@ static void test_sink_writer_create(void) winetest_pop_context(); } +static void test_sink_writer_get_service(void) +{ + IMFSinkWriter *writer; + IMFByteStream *stream; + IMFAttributes *attr; + unsigned int i; + HRESULT hr; + + const struct container_type + { + const GUID *type; + void *create_sink; + BOOL todo; + } container_types[] = + { + {&MFTranscodeContainerType_3GP, pMFCreate3GPMediaSink, TRUE}, + {&MFTranscodeContainerType_ADTS, pMFCreateADTSMediaSink, TRUE}, + {&MFTranscodeContainerType_ASF, pMFCreateASFMediaSink, TRUE}, + {&MFTranscodeContainerType_AVI, pMFCreateAVIMediaSink, TRUE}, + {&MFTranscodeContainerType_FMPEG4, pMFCreateFMPEG4MediaSink, TRUE}, + {&MFTranscodeContainerType_MP3, pMFCreateMP3MediaSink, TRUE}, + {&MFTranscodeContainerType_MPEG4, pMFCreateMPEG4MediaSink, TRUE}, + {&MFTranscodeContainerType_WAVE, pMFCreateWAVEMediaSink, TRUE}, + }; + + hr = MFCreateAttributes(&attr, 1); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + for (i = 0; i < ARRAY_SIZE(container_types); ++i) + { + const GUID *type = container_types[i].type; + BOOL todo = container_types[i].todo; + IMFMediaSink *sink = NULL; + + winetest_push_context("Test %u", i); + + if (!container_types[i].create_sink) + { + todo_wine_if(IsEqualGUID(type, &MFTranscodeContainerType_AVI)) + win_skip("Unsupported container type.\n"); + winetest_pop_context(); + continue; + } + + hr = IMFAttributes_SetGUID(attr, &MF_TRANSCODE_CONTAINERTYPE, type); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = MFCreateTempFile(MF_ACCESSMODE_READWRITE, MF_OPENMODE_DELETE_IF_EXIST, MF_FILEFLAGS_NONE, &stream); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = MFCreateSinkWriterFromURL(NULL, stream, attr, &writer); + todo_wine_if(todo) + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + if (hr != S_OK) + { + IMFByteStream_Release(stream); + winetest_pop_context(); + continue; + } + + hr = IMFSinkWriter_GetServiceForStream(writer, MF_SINK_WRITER_MEDIASINK, + &GUID_NULL, &IID_IMFMediaSink, (void **)&sink); + if (IsEqualGUID(type, &MFTranscodeContainerType_ASF) + || IsEqualGUID(type, &MFTranscodeContainerType_MP3)) + { + ok(hr == S_OK, "Unexpected hr %#lx,\n", hr); + IMFMediaSink_Release(sink); + } + else + { + ok(hr == MF_E_UNSUPPORTED_SERVICE, "Unexpected hr %#lx.\n", hr); + } + + IMFSinkWriter_Release(writer); + IMFByteStream_Release(stream); + + winetest_pop_context(); + } + + IMFAttributes_Release(attr); +} + START_TEST(mfplat) { HRESULT hr; @@ -1506,6 +1617,7 @@ START_TEST(mfplat) test_source_reader_from_media_source(); test_reader_d3d9(); test_sink_writer_create(); + test_sink_writer_get_service(); hr = MFShutdown(); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2690
Nikolay Sivov (@nsivov) commented about dlls/mfreadwrite/tests/mfplat.c:
+ IMFByteStream **stream; + IMFAttributes **attr; + IMFSinkWriter **writer; + HRESULT hr; + } create_sink_writer_from_url_tests[] = + { + {NULL, NULL, NULL, NULL, E_INVALIDARG}, + {NULL, NULL, NULL, &writer, E_INVALIDARG}, + {NULL, NULL, &attr, &writer, E_INVALIDARG}, + {NULL, &stream, NULL, &writer, E_INVALIDARG}, + {NULL, &stream, &attr, &writer, S_OK}, + {tmp_file, NULL, NULL, &writer, S_OK}, + {tmp_file, NULL, &attr, &writer, S_OK}, + {tmp_file, &stream, NULL, &writer, S_OK}, + {tmp_file, &stream, &attr, &writer, S_OK}, + }; I already mentioned that before. This is not readable, just add a few cases without this loop.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2690#note_31094
Nikolay Sivov (@nsivov) commented about dlls/mfreadwrite/tests/mfplat.c:
+#define X(f) p##f = (void*)GetProcAddress(mfplat, #f) X(MFCreateMFByteStreamOnStream); #undef X + +#define X(f) p##f = (void*)GetProcAddress(mfsrcsnk, #f) + X(MFCreateAVIMediaSink); + X(MFCreateWAVEMediaSink); +#undef X + +#define X(f) p##f = (void*)GetProcAddress(mf, #f) + X(MFCreate3GPMediaSink); + X(MFCreateADTSMediaSink); + X(MFCreateASFMediaSink); + X(MFCreateFMPEG4MediaSink); + X(MFCreateMP3MediaSink); + X(MFCreateMPEG4MediaSink); Why do you need to check this explicitly? Doesn't it fail to create a writer?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2690#note_31095
On Tue Apr 25 10:34:58 2023 +0000, Nikolay Sivov wrote:
Why do you need to check this explicitly? Doesn't it fail to create a writer? According to my tests on testbot, Win8 and Win7 fail to create writers of some types, but they also succeed for some types.
What I found is, if MFCreateXXXMediaSink is missing, creating writer for this type of container fails. Otherwise, it succeeds. So I check MFCreateXXXMediaSink explicitly here to determine if writer creation for this type of container will succeed or not. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2690#note_31097
On Tue Apr 25 10:34:57 2023 +0000, Nikolay Sivov wrote:
I already mentioned that before. This is not readable, just add a few cases without this loop. OK
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2690#note_31098
On Tue Apr 25 10:49:02 2023 +0000, Ziqing Hui wrote:
According to my tests on testbot, Win8 and Win7 fail to create writers of some types, but they also succeed for some types. What I found is, if MFCreateXXXMediaSink is missing, creating writer for this type of container fails. Otherwise, it succeeds. So I check MFCreateXXXMediaSink explicitly here to determine if writer creation for this type of container will succeed or not. I think it's fine to have separate test functions for every type. I'd focus on types that we'll actually need. Is that MPEG4?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2690#note_31100
I think it's fine to have separate test functions for every type.
OK. Does it means that, if I want to test WAVE and MP4, we have 3 sink writer test functions: test_sink_writer_create(), test_sink_writer_mp4(), test_sink_writer_wave()?
Is that MPEG4?
Yep, it's mpeg4. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2690#note_31160
participants (3)
-
Nikolay Sivov (＠nsivov) -
Ziqing Hui
-
Ziqing Hui (＠zhui)