[PATCH 0/1] 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). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2690
From: Ziqing Hui <zhui(a)codeweavers.com> 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). --- dlls/mfreadwrite/tests/mfplat.c | 133 +++++++++++++++++++++++++++++--- 1 file changed, 123 insertions(+), 10 deletions(-) diff --git a/dlls/mfreadwrite/tests/mfplat.c b/dlls/mfreadwrite/tests/mfplat.c index 41b2a033a59..42e4162b60a 100644 --- a/dlls/mfreadwrite/tests/mfplat.c +++ b/dlls/mfreadwrite/tests/mfplat.c @@ -82,14 +82,20 @@ static IDirect3DDevice9 *create_d3d9_device(IDirect3D9 *d3d9, HWND focus_window) } static HRESULT (WINAPI *pMFCreateMFByteStreamOnStream)(IStream *stream, IMFByteStream **bytestream); +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"); -#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(MFCreateWAVEMediaSink); +#undef X } enum source_state @@ -1415,19 +1421,120 @@ done: DestroyWindow(window); } -static void test_sink_writer(void) +static void test_sink_writer_create(const char *extension, BOOL todo) { + const GUID *container_type = &GUID_NULL; + WCHAR extension_w[MAX_PATH]; + WCHAR tmp_file[MAX_PATH]; IMFSinkWriter *writer; + IMFByteStream *stream; + IMFAttributes *attr; + IMFMediaSink *sink; + unsigned int i; HRESULT hr; - hr = MFCreateSinkWriterFromURL(NULL, NULL, NULL, NULL); - ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + static const struct extension_map + { + const char *extension; + const GUID *container_type; + } ext_map[] = + { + {"mp4", &MFTranscodeContainerType_MPEG4}, + {"mp3", &MFTranscodeContainerType_MP3}, + {"wav", &MFTranscodeContainerType_WAVE}, + {"avi", &MFTranscodeContainerType_AVI}, + {"asf", &MFTranscodeContainerType_ASF}, + }; + + 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}, + }; + + winetest_push_context("%s", extension); + + if (!pMFCreateWAVEMediaSink && !strcmp(extension, "wav")) + { + win_skip("WAVE is not supported.\n"); + winetest_pop_context(); + return; + } - 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); + MultiByteToWideChar(CP_ACP, 0, extension, -1, extension_w, ARRAY_SIZE(extension_w)); + wcscat(tmp_file, L"tmp."); + wcscat(tmp_file, extension_w); + + /* Create attribute and byte stream. */ + for (i = 0; i < ARRAY_SIZE(ext_map); ++i) + { + if (!strcmp(extension, ext_map[i].extension)) + { + container_type = ext_map[i].container_type; + break; + } + } + hr = MFCreateAttributes(&attr, 1); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFAttributes_SetGUID(attr, &MF_TRANSCODE_CONTAINERTYPE, container_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); + /* 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 && todo) + { + ok(hr == test->hr, "Unexpected hr %#lx, expected %#lx.\n", hr, test->hr); + if (test->writer) + ok((test->hr == S_OK && writer) || (test->hr != S_OK && !writer), "Unexpected writer pointer %p.\n", writer); + } + + if (hr == S_OK) + { + hr = IMFSinkWriter_GetServiceForStream(writer, MF_SINK_WRITER_MEDIASINK, + &GUID_NULL, &IID_IMFMediaSink, (void **)&sink); + if (IsEqualGUID(container_type, &MFTranscodeContainerType_ASF) + || IsEqualGUID(container_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); + } + + winetest_pop_context(); + } + + /* Test MFCreateSinkWriterFromMediaSink. */ hr = MFCreateSinkWriterFromMediaSink(NULL, NULL, NULL); ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); @@ -1435,6 +1542,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 +1560,11 @@ 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("wav", TRUE); + test_sink_writer_create("mp3", TRUE); + test_sink_writer_create("mp4", TRUE); + test_sink_writer_create("asf", TRUE); 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:
+ if (hr == S_OK) + { + hr = IMFSinkWriter_GetServiceForStream(writer, MF_SINK_WRITER_MEDIASINK, + &GUID_NULL, &IID_IMFMediaSink, (void **)&sink); + if (IsEqualGUID(container_type, &MFTranscodeContainerType_ASF) + || IsEqualGUID(container_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); + } I think this is the only part we need to keep for format specific tests. The rest could stay in test_sink_writer().
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2690#note_30943
Nikolay Sivov (@nsivov) commented about dlls/mfreadwrite/tests/mfplat.c:
+ {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}, + }; + + winetest_push_context("%s", extension); + + if (!pMFCreateWAVEMediaSink && !strcmp(extension, "wav")) + { + win_skip("WAVE is not supported.\n"); + winetest_pop_context(); + return; + } Is it ever unavailable?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2690#note_30944
Nikolay Sivov (@nsivov) commented about dlls/mfreadwrite/tests/mfplat.c:
+ if (!pMFCreateWAVEMediaSink && !strcmp(extension, "wav")) + { + win_skip("WAVE is not supported.\n"); + winetest_pop_context(); + return; + }
- 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); + MultiByteToWideChar(CP_ACP, 0, extension, -1, extension_w, ARRAY_SIZE(extension_w)); + wcscat(tmp_file, L"tmp."); + wcscat(tmp_file, extension_w); No need for conversion here.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2690#note_30945
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}, + }; Why do we need to run this for every format?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2690#note_30946
On Sat Apr 22 17:32:25 2023 +0000, Nikolay Sivov wrote:
Is it ever unavailable? It fails on Win7. See: https://testbot.winehq.org/JobDetails.pl?Key=132107&f201=exe32.report#k201
WAVE and AVI media sink is not supported before Win8. See MSDN here: https://learn.microsoft.com/en-us/windows/win32/medfound/mf-transcode-contai... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2690#note_30963
On Sat Apr 22 17:32:25 2023 +0000, Nikolay Sivov wrote:
I think this is the only part we need to keep for format specific tests. The rest could stay in test_sink_writer(). OK, so we create a new function which accept a format argument. Then move this part to the new function.
What should the 2 functions' name be? What abount naming the old function which contains creation tests to test_sink_writer_create(), and the new one to test_sink_writer()? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2690#note_30964
On Sat Apr 22 17:32:26 2023 +0000, Nikolay Sivov wrote:
Why do we need to run this for every format? OK, so we keep it in the old creation tests function.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2690#note_30965
On Sun Apr 23 07:12:34 2023 +0000, Ziqing Hui wrote:
OK, so we keep it in the old creation tests function. I think we need a single loop, creating writer for different extensions, and then checking GetServiceForStream().
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2690#note_31009
participants (3)
-
Nikolay Sivov (＠nsivov) -
Ziqing Hui
-
Ziqing Hui (＠zhui)