[PATCH v6 0/10] MR10645: mf/topology_loader: Insert optional nodes after each branch is connected.
New tests show each optional node is connected after the nodes on either side are connected. This is an issue in the game Psycho Sleuth (at least in the demo) because it uses a transform which must have its input types enumerated before it will work correctly. This occurs when a decoder is inserted before the transform, but an optional node lies immediately upstream of the transform, and if optional nodes are not handled correctly, the decoder is initially connected to the optional instead of being inserted later. A couple of other issues revealed while making new tests are fixed here too. -- v6: mf/topology_loader: Remove the fixme from topology_loader_Load(). mf/topology_loader: Try to insert optional nodes after resolving the surrounding branch. mf/topology_loader: Resolve topologies by walking down from each source node. mf/topology_loader: Introduce a helper to get the connection semantic flags. mf/tests: Test optional topology nodes. mf/tests: Fix comments for two topology loader tests. mf/topology_loader: Always call IsMediaTypeSupported() on downstream when connecting a branch. https://gitlab.winehq.org/wine/wine/-/merge_requests/10645
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/mf/tests/topology.c | 70 +++++++++++++++++++++++++++++----------- 1 file changed, 52 insertions(+), 18 deletions(-) diff --git a/dlls/mf/tests/topology.c b/dlls/mf/tests/topology.c index 6d433b3e1c2..a967fbe600d 100644 --- a/dlls/mf/tests/topology.c +++ b/dlls/mf/tests/topology.c @@ -2446,6 +2446,19 @@ static void test_topology_loader(void) ATTR_UINT32(MF_MT_FIXED_SIZE_SAMPLES, 1), ATTR_UINT32(MF_MT_INTERLACE_MODE, 7), }; + static const media_type_desc video_yuy2_1280 = + { + ATTR_GUID(MF_MT_MAJOR_TYPE, MFMediaType_Video), + ATTR_GUID(MF_MT_SUBTYPE, MFVideoFormat_YUY2), + ATTR_RATIO(MF_MT_FRAME_RATE, 30000, 1001), + ATTR_RATIO(MF_MT_FRAME_SIZE, 1280, 720), + ATTR_RATIO(MF_MT_PIXEL_ASPECT_RATIO, 1, 1), + ATTR_UINT32(MF_MT_SAMPLE_SIZE, 1280 * 720 * 3 / 2), + ATTR_UINT32(MF_MT_ALL_SAMPLES_INDEPENDENT, 1), + ATTR_UINT32(MF_MT_DEFAULT_STRIDE, 1280), + ATTR_UINT32(MF_MT_FIXED_SIZE_SAMPLES, 1), + ATTR_UINT32(MF_MT_INTERLACE_MODE, 7), + }; static const media_type_desc video_dummy = { ATTR_GUID(MF_MT_MAJOR_TYPE, MFMediaType_Video), @@ -2455,7 +2468,7 @@ static void test_topology_loader(void) { const media_type_desc *input_types[2]; const media_type_desc *output_types[2]; - const media_type_desc *mft_input_type; + const media_type_desc *mft_input_types[3]; const media_type_desc *mft_output_types[5]; const media_type_desc *current_input; const media_type_desc *mft_current_output; @@ -2464,6 +2477,7 @@ static void test_topology_loader(void) MF_CONNECT_METHOD source_method; MF_CONNECT_METHOD sink_method; HRESULT expected_result; + unsigned int mft_current_input_1based_index; unsigned int expected_output_index; unsigned int flags; GUID decoder_class; @@ -2683,7 +2697,7 @@ static void test_topology_loader(void) { /* MP3 -> PCM, different enumerated bps, add test MFT, configure MFT */ .input_types = {&audio_mp3_44100}, .output_types = {&audio_pcm_48000}, .sink_method = MF_CONNECT_DIRECT, .source_method = MF_CONNECT_DIRECT, - .mft_input_type = &audio_pcm_44100, .mft_output_types = {&audio_pcm_48000}, + .mft_input_types = {&audio_pcm_44100}, .mft_output_types = {&audio_pcm_48000}, .decoded_type = &audio_pcm_44100, .expected_result = S_OK, .decoder_class = CLSID_CMP3DecMediaObject, .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED, @@ -2691,7 +2705,7 @@ static void test_topology_loader(void) { /* MP3 -> PCM, different enumerated bps, add incompatible test MFT, configure MFT */ .input_types = {&audio_mp3_44100}, .output_types = {&audio_pcm_48000}, .sink_method = MF_CONNECT_DIRECT, .source_method = MF_CONNECT_DIRECT, - .mft_input_type = &audio_aac_44100, .mft_output_types = {&audio_pcm_48000}, + .mft_input_types = {&audio_aac_44100}, .mft_output_types = {&audio_pcm_48000}, .expected_result = MF_E_TOPO_CODEC_NOT_FOUND, .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED, }, @@ -2822,24 +2836,33 @@ static void test_topology_loader(void) { /* H264 -> RGB32, Color Convert media type, add test MFT */ .input_types = {&video_h264_1280}, .output_types = {&video_color_convert_1280_rgb32}, .sink_method = -1, .source_method = -1, - .mft_input_type = &video_color_convert_1280_rgb32, .mft_output_types = {&video_color_convert_1280_rgb32}, + .mft_input_types = {&video_color_convert_1280_rgb32}, .mft_output_types = {&video_color_convert_1280_rgb32}, .expected_result = MF_E_TOPO_CODEC_NOT_FOUND, .decoder_class = CLSID_CMSH264DecoderMFT, .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED, }, { /* H264 -> RGB32, Video Processor media type, add test MFT */ .input_types = {&video_h264_1280}, .output_types = {&video_video_processor_1280_rgb32}, .sink_method = -1, .source_method = -1, - .mft_input_type = &video_video_processor_1280_rgb32, .mft_output_types = {&video_video_processor_1280_rgb32}, + .mft_input_types = {&video_video_processor_1280_rgb32}, .mft_output_types = {&video_video_processor_1280_rgb32}, .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, .converter_class = CLSID_CColorConvertDMO, .flags = LOADER_ADD_TEST_MFT | LOADER_TEST_MFT_EXPECT_CONVERTER | LOADER_EXPECT_MFT_INPUT_ENUMERATED, }, { /* H264 -> NV12, add test MFT */ .input_types = {&video_h264_1280}, .output_types = {&video_nv12_1280}, .sink_method = -1, .source_method = -1, - .mft_input_type = &video_nv12_1280, .mft_output_types = {&video_nv12_1280}, + .mft_input_types = {&video_nv12_1280}, .mft_output_types = {&video_nv12_1280}, .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED, }, + + { + /* NV12 -> NV12, add test MFT, set unsupported MFT input */ + .input_types = {&video_nv12_1280}, .output_types = {&video_nv12_1280}, .sink_method = MF_CONNECT_DIRECT, .source_method = -1, + .mft_input_types = {&video_nv12_1280, &video_yuy2_1280}, .mft_output_types = {&video_nv12_1280}, + .mft_current_input_1based_index = 2, + .expected_result = S_OK, + .flags = LOADER_ADD_TEST_MFT, + }, }; IMFTopologyNode *src_node, *sink_node, *src_node2, *sink_node2, *mft_node; @@ -2987,7 +3010,7 @@ static void test_topology_loader(void) const struct loader_test *test = &loader_tests[i]; struct test_transform *test_transform = NULL; IMFMediaType *mft_output_types[4] = {0}; - IMFMediaType *mft_input_type = NULL; + IMFMediaType *mft_input_types[2] = {0}; winetest_push_context("%u", i); @@ -3042,21 +3065,25 @@ static void test_topology_loader(void) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); if (test->flags & LOADER_ADD_TEST_MFT) { - mft_input_type = input_types[0]; + UINT input_count, output_count; - if (test->mft_input_type) + for (input_count = 0; test->mft_input_types[input_count]; ++input_count) { - hr = MFCreateMediaType(&mft_input_type); + hr = MFCreateMediaType(&mft_input_types[input_count]); ok(hr == S_OK, "Failed to create media type, hr %#lx.\n", hr); - init_media_type(mft_input_type, *test->mft_input_type, -1); + init_media_type(mft_input_types[input_count], *test->mft_input_types[input_count], -1); } - for (j = 0; test->mft_output_types[j]; ++j) + if (!input_count) + IMFMediaType_AddRef(mft_input_types[input_count++] = input_types[0]); + + for (output_count = 0; test->mft_output_types[output_count]; ++output_count) { - hr = MFCreateMediaType(&mft_output_types[j]); + hr = MFCreateMediaType(&mft_output_types[output_count]); ok(hr == S_OK, "Failed to create media type, hr %#lx.\n", hr); - init_media_type(mft_output_types[j], *test->mft_output_types[j], -1); + init_media_type(mft_output_types[output_count], *test->mft_output_types[output_count], -1); } - test_transform_create(1, &mft_input_type, j, mft_output_types, FALSE, &transform); + + test_transform_create(input_count, mft_input_types, output_count, mft_output_types, FALSE, &transform); test_transform = test_transform_from_IMFTransform(transform); IMFTransform_AddRef(transform); } @@ -3082,7 +3109,13 @@ static void test_topology_loader(void) } else { - IMFTransform_SetInputType(transform, 0, NULL, 0); + if (test->mft_current_input_1based_index) + { + hr = IMFTransform_SetInputType(transform, 0, mft_input_types[test->mft_current_input_1based_index - 1], 0); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + } + else + IMFTransform_SetInputType(transform, 0, NULL, 0); IMFTransform_SetOutputType(transform, 0, NULL, 0); } IMFTransform_Release(transform); @@ -3176,6 +3209,7 @@ todo_wine { hr = IMFTopology_GetNodeCount(full_topology, &node_count); ok(hr == S_OK, "Failed to get node count, hr %#lx.\n", hr); + todo_wine_if(test->mft_current_input_1based_index) ok(node_count == count, "Unexpected node count %u.\n", node_count); hr = IMFTopologyNode_GetTopoNodeID(src_node, &node_id); @@ -3396,8 +3430,8 @@ todo_wine { ok(ref == 0, "Release returned %ld\n", ref); ref = IMFStreamDescriptor_Release(sd); ok(ref == 0, "Release returned %ld\n", ref); - if (mft_input_type && test->mft_input_type) - IMFMediaType_Release(mft_input_type); + for (j = 0; j < ARRAY_SIZE(mft_input_types) && mft_input_types[j]; ++j) + IMFMediaType_Release(mft_input_types[j]); for (j = 0; j < ARRAY_SIZE(mft_output_types) && mft_output_types[j]; ++j) IMFMediaType_Release(mft_output_types[j]); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10645
From: Conor McCarthy <cmccarthy@codeweavers.com> This may be the rule for indirect connection too, but it's not clear if we need it. --- dlls/mf/tests/topology.c | 1 - dlls/mf/topology_loader.c | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/mf/tests/topology.c b/dlls/mf/tests/topology.c index a967fbe600d..add77f3da79 100644 --- a/dlls/mf/tests/topology.c +++ b/dlls/mf/tests/topology.c @@ -3209,7 +3209,6 @@ todo_wine { hr = IMFTopology_GetNodeCount(full_topology, &node_count); ok(hr == S_OK, "Failed to get node count, hr %#lx.\n", hr); - todo_wine_if(test->mft_current_input_1based_index) ok(node_count == count, "Unexpected node count %u.\n", node_count); hr = IMFTopologyNode_GetTopoNodeID(src_node, &node_id); diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index 9cf4ee0b28c..c24f8bd72cb 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -599,7 +599,8 @@ static HRESULT topology_branch_connect_direct(IMFTopology *topology, struct topo if (SUCCEEDED(hr = IMFMediaTypeHandler_GetCurrentMediaType(branch->down.handler, &down_type))) { - if (IMFMediaType_IsEqual(up_type, down_type, &flags) == S_OK) + if (topology_node_get_type(branch->down.node) != MF_TOPOLOGY_OUTPUT_NODE + || IMFMediaType_IsEqual(up_type, down_type, &flags) == S_OK) hr = topology_branch_connect_with_type(topology, branch, up_type); else { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10645
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/mf/tests/topology.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dlls/mf/tests/topology.c b/dlls/mf/tests/topology.c index add77f3da79..e6a026aec6e 100644 --- a/dlls/mf/tests/topology.c +++ b/dlls/mf/tests/topology.c @@ -1739,6 +1739,7 @@ struct test_handler ULONG media_types_count; IMFMediaType **media_types; BOOL enum_complete; + BOOL is_supported_called; }; static struct test_handler *impl_from_IMFMediaTypeHandler(IMFMediaTypeHandler *iface) @@ -1776,6 +1777,8 @@ static HRESULT WINAPI test_handler_IsMediaTypeSupported(IMFMediaTypeHandler *ifa BOOL result; ULONG i; + impl->is_supported_called = TRUE; + if (out_type) *out_type = NULL; @@ -3157,6 +3160,8 @@ static void test_topology_loader(void) ok(hr == S_OK, "Failed to get attribute count, hr %#lx.\n", hr); ok(!count, "Unexpected count %u.\n", count); + handler.is_supported_called = FALSE; + if (test->flags & LOADER_SET_ENUMERATE_SOURCE_TYPES) IMFTopology_SetUINT32(topology, &MF_TOPOLOGY_ENUMERATE_SOURCE_TYPES, 1); if (test->flags & LOADER_SET_XVP_FOR_PLAYBACK) @@ -3234,6 +3239,9 @@ todo_wine { if (hr == S_OK) IMFMediaType_Release(media_type); + todo_wine_if(!handler.is_supported_called) + ok(handler.is_supported_called, "Sink input support not checked.\n"); + if (!IsEqualGUID(&test->decoder_class, &GUID_NULL)) { GUID class_id; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10645
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/mf/tests/topology.c | 4 +--- dlls/mf/topology_loader.c | 3 +++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/dlls/mf/tests/topology.c b/dlls/mf/tests/topology.c index e6a026aec6e..29b4ba7f467 100644 --- a/dlls/mf/tests/topology.c +++ b/dlls/mf/tests/topology.c @@ -2498,7 +2498,6 @@ static void test_topology_loader(void) .input_types = {&audio_pcm_44100}, .output_types = {&audio_pcm_44100}, .sink_method = MF_CONNECT_DIRECT, .source_method = -1, .current_input = &audio_pcm_44100_incomplete, .expected_result = MF_E_INVALIDMEDIATYPE, - .flags = LOADER_TODO, }, { /* PCM -> PCM, same enumerated bps, different current bps */ @@ -2525,7 +2524,7 @@ static void test_topology_loader(void) .input_types = {&audio_pcm_44100_incomplete}, .output_types = {&audio_pcm_44100}, .sink_method = MF_CONNECT_DIRECT, .source_method = -1, .current_input = &audio_pcm_44100, .expected_result = MF_E_NO_MORE_TYPES, - .flags = LOADER_SET_ENUMERATE_SOURCE_TYPES | LOADER_TODO, + .flags = LOADER_SET_ENUMERATE_SOURCE_TYPES, }, { @@ -3239,7 +3238,6 @@ todo_wine { if (hr == S_OK) IMFMediaType_Release(media_type); - todo_wine_if(!handler.is_supported_called) ok(handler.is_supported_called, "Sink input support not checked.\n"); if (!IsEqualGUID(&test->decoder_class, &GUID_NULL)) diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index c24f8bd72cb..def17ff85c3 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -435,6 +435,9 @@ static HRESULT topology_branch_connect_with_type(IMFTopology *topology, struct t { HRESULT hr; + if (FAILED(hr = IMFMediaTypeHandler_IsMediaTypeSupported(branch->down.handler, type, NULL))) + return hr; + if (FAILED(hr = IMFMediaTypeHandler_SetCurrentMediaType(branch->up.handler, type))) { WARN("Failed to set upstream node media type, hr %#lx\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10645
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/mf/tests/topology.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/mf/tests/topology.c b/dlls/mf/tests/topology.c index 29b4ba7f467..2270d7d6b0e 100644 --- a/dlls/mf/tests/topology.c +++ b/dlls/mf/tests/topology.c @@ -2697,7 +2697,7 @@ static void test_topology_loader(void) .flags = LOADER_ADD_TEST_MFT | LOADER_NO_CURRENT_OUTPUT, }, { - /* MP3 -> PCM, different enumerated bps, add test MFT, configure MFT */ + /* MP3 -> PCM, different enumerated bps, add test MFT */ .input_types = {&audio_mp3_44100}, .output_types = {&audio_pcm_48000}, .sink_method = MF_CONNECT_DIRECT, .source_method = MF_CONNECT_DIRECT, .mft_input_types = {&audio_pcm_44100}, .mft_output_types = {&audio_pcm_48000}, .decoded_type = &audio_pcm_44100, @@ -2705,7 +2705,7 @@ static void test_topology_loader(void) .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED, }, { - /* MP3 -> PCM, different enumerated bps, add incompatible test MFT, configure MFT */ + /* MP3 -> PCM, different enumerated bps, add incompatible test MFT */ .input_types = {&audio_mp3_44100}, .output_types = {&audio_pcm_48000}, .sink_method = MF_CONNECT_DIRECT, .source_method = MF_CONNECT_DIRECT, .mft_input_types = {&audio_aac_44100}, .mft_output_types = {&audio_pcm_48000}, .expected_result = MF_E_TOPO_CODEC_NOT_FOUND, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10645
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/mf/tests/topology.c | 310 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 297 insertions(+), 13 deletions(-) diff --git a/dlls/mf/tests/topology.c b/dlls/mf/tests/topology.c index 2270d7d6b0e..6f8b51b6e51 100644 --- a/dlls/mf/tests/topology.c +++ b/dlls/mf/tests/topology.c @@ -247,6 +247,8 @@ static void init_sink_node(IMFStreamSink *stream_sink, MF_CONNECT_METHOD method, } } +static LONG sequence_count; + struct test_transform { IMFTransform IMFTransform_iface; @@ -258,6 +260,8 @@ struct test_transform UINT input_count; IMFMediaType **input_types; IMFMediaType *input_type; + LONG set_input_sequence_id; + LONG set_output_sequence_id; UINT output_count; IMFMediaType **output_types; @@ -267,6 +271,7 @@ struct test_transform BOOL input_type_set; BOOL output_type_set; BOOL set_input_nulls_output; + BOOL validate_output_type; IDirect3DDeviceManager9 *expect_d3d9_device_manager; BOOL got_d3d9_device_manager; @@ -437,6 +442,9 @@ static HRESULT WINAPI test_transform_SetInputType(IMFTransform *iface, DWORD id, if (type) { + if (!transform->set_input_sequence_id) + transform->set_input_sequence_id = InterlockedIncrement(&sequence_count); + for (i = 0; i < transform->input_count; ++i) { if (IMFMediaType_Compare(transform->input_types[i], (IMFAttributes *)type, @@ -473,10 +481,25 @@ static HRESULT WINAPI test_transform_SetOutputType(IMFTransform *iface, DWORD id struct test_transform *transform = test_transform_from_IMFTransform(iface); if (flags & MFT_SET_TYPE_TEST_ONLY) return S_OK; + if (type && transform->validate_output_type) + { + BOOL result; + UINT i; + for (i = 0; i < transform->output_count; ++i) + { + if (IMFMediaType_Compare(transform->output_types[i], (IMFAttributes *)type, + MF_ATTRIBUTES_MATCH_OUR_ITEMS, &result) == S_OK && result) + break; + } + if (i == transform->output_count) + return MF_E_INVALIDMEDIATYPE; + } if (transform->output_type) IMFMediaType_Release(transform->output_type); if ((transform->output_type = type)) { + if (!transform->set_output_sequence_id) + transform->set_output_sequence_id = InterlockedIncrement(&sequence_count); transform->output_type_set = TRUE; IMFMediaType_AddRef(transform->output_type); } @@ -1739,7 +1762,7 @@ struct test_handler ULONG media_types_count; IMFMediaType **media_types; BOOL enum_complete; - BOOL is_supported_called; + LONG is_supported_sequence_id; }; static struct test_handler *impl_from_IMFMediaTypeHandler(IMFMediaTypeHandler *iface) @@ -1777,7 +1800,8 @@ static HRESULT WINAPI test_handler_IsMediaTypeSupported(IMFMediaTypeHandler *ifa BOOL result; ULONG i; - impl->is_supported_called = TRUE; + if (!impl->is_supported_sequence_id) + impl->is_supported_sequence_id = InterlockedIncrement(&sequence_count); if (out_type) *out_type = NULL; @@ -2265,6 +2289,8 @@ enum loader_test_flags LOADER_TEST_MFT_EXPECT_CONVERTER = 0x2000, LOADER_EXPECT_MFT_OUTPUT_ENUMERATED = 0x4000, LOADER_EXPECT_MFT_INPUT_ENUMERATED = 0x8000, + LOADER_ADD_OPTIONAL_TEST_MFT_DOWNSTREAM = 0x10000, + LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO = 0x20000, }; static void test_topology_loader(void) @@ -2473,6 +2499,8 @@ static void test_topology_loader(void) const media_type_desc *output_types[2]; const media_type_desc *mft_input_types[3]; const media_type_desc *mft_output_types[5]; + const media_type_desc *optional_mft_input_types[2]; /* one per node */ + const media_type_desc *optional_mft_output_type; const media_type_desc *current_input; const media_type_desc *mft_current_output; const media_type_desc *decoded_type; @@ -2482,6 +2510,8 @@ static void test_topology_loader(void) HRESULT expected_result; unsigned int mft_current_input_1based_index; unsigned int expected_output_index; + unsigned int optional_mft_count; + BOOL expect_optional_mft_rejected[2]; unsigned int flags; GUID decoder_class; GUID converter_class; @@ -2865,10 +2895,129 @@ static void test_topology_loader(void) .expected_result = S_OK, .flags = LOADER_ADD_TEST_MFT, }, + + { + /* H264 -> NV12, add optional test MFT and test MFT */ + .input_types = {&video_h264_1280}, .output_types = {&video_nv12_1280}, .sink_method = -1, .source_method = -1, + .mft_input_types = {&video_nv12_1280}, .mft_output_types = {&video_nv12_1280}, + .optional_mft_count = 1, + .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, + }, + { + /* H264 -> NV12, add two optional test MFTs and test MFT */ + .input_types = {&video_h264_1280}, .output_types = {&video_nv12_1280}, .sink_method = -1, .source_method = -1, + .mft_input_types = {&video_nv12_1280}, .mft_output_types = {&video_nv12_1280}, + .optional_mft_count = 2, + .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, + }, + { + /* H264 -> NV12, add unconnectable optional test MFT and test MFT */ + .input_types = {&video_h264_1280}, .output_types = {&video_nv12_1280}, .sink_method = -1, .source_method = -1, + .mft_input_types = {&video_nv12_1280}, .mft_output_types = {&video_nv12_1280}, + .optional_mft_input_types = {&video_yuy2_1280}, + .optional_mft_count = 1, .expect_optional_mft_rejected = {TRUE}, + .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, + }, + { + /* H264 -> NV12, add unconnectable optional test MFT, connectable optional test MFT and test MFT */ + .input_types = {&video_h264_1280}, .output_types = {&video_nv12_1280}, .sink_method = -1, .source_method = -1, + .mft_input_types = {&video_nv12_1280}, .mft_output_types = {&video_nv12_1280}, + .optional_mft_input_types = {&video_yuy2_1280, &video_nv12_1280}, + .optional_mft_count = 2, .expect_optional_mft_rejected = {TRUE, FALSE}, + .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, + }, + { + /* #60 H264 -> NV12, add connectable optional test MFT, unconnectable optional test MFT and test MFT */ + .input_types = {&video_h264_1280}, .output_types = {&video_nv12_1280}, .sink_method = -1, .source_method = -1, + .mft_input_types = {&video_nv12_1280}, .mft_output_types = {&video_nv12_1280}, + .optional_mft_input_types = {&video_nv12_1280, &video_yuy2_1280}, + .optional_mft_count = 2, .expect_optional_mft_rejected = {FALSE, TRUE}, + .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, + }, + { + /* H264 -> NV12, add two unconnectable optional test MFTs and test MFT */ + .input_types = {&video_h264_1280}, .output_types = {&video_nv12_1280}, .sink_method = -1, .source_method = -1, + .mft_input_types = {&video_nv12_1280}, .mft_output_types = {&video_nv12_1280}, + .optional_mft_input_types = {&video_yuy2_1280, &video_yuy2_1280}, + .optional_mft_count = 2, .expect_optional_mft_rejected = {TRUE, TRUE}, + .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, + }, + { + /* H264 -> NV12, add optional test MFT and test MFT, require test MFT input change for optional */ + .input_types = {&video_h264_1280}, .output_types = {&video_nv12_1280}, .sink_method = -1, .source_method = -1, + .mft_input_types = {&video_nv12_1280, &video_yuy2_1280}, .mft_output_types = {&video_nv12_1280}, + .optional_mft_output_type = &video_yuy2_1280, + .optional_mft_count = 1, + .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, + }, + { + /* H264 -> NV12, add test MFT and optional test MFT */ + .input_types = {&video_h264_1280}, .output_types = {&video_nv12_1280}, .sink_method = MF_CONNECT_DIRECT, .source_method = -1, + .mft_input_types = {&video_nv12_1280}, .mft_output_types = {&video_nv12_1280}, + .optional_mft_count = 1, + .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, + .flags = LOADER_ADD_TEST_MFT | LOADER_ADD_OPTIONAL_TEST_MFT_DOWNSTREAM | LOADER_EXPECT_MFT_INPUT_ENUMERATED, + }, + { + /* H264 -> NV12, add test MFT and optional test MFT, require sink input change for optional */ + .input_types = {&video_h264_1280}, .output_types = {&video_nv12_1280, &video_yuy2_1280}, .sink_method = MF_CONNECT_DIRECT, .source_method = -1, + .mft_input_types = {&video_nv12_1280}, .mft_output_types = {&video_nv12_1280}, + .optional_mft_output_type = &video_yuy2_1280, + .optional_mft_count = 1, + .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, + .flags = LOADER_ADD_TEST_MFT | LOADER_NO_CURRENT_OUTPUT | LOADER_ADD_OPTIONAL_TEST_MFT_DOWNSTREAM | LOADER_EXPECT_MFT_INPUT_ENUMERATED, + }, + { + /* H264 -> NV12, add test MFT and unconnectable optional test MFT, converter is not added for optional connection */ + .input_types = {&video_h264_1280}, .output_types = {&video_nv12_1280}, .sink_method = -1, .source_method = -1, + .mft_input_types = {&video_nv12_1280}, .mft_output_types = {&video_nv12_1280}, + .optional_mft_output_type = &video_yuy2_1280, + .optional_mft_count = 1, .expect_optional_mft_rejected = {TRUE}, + .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, + .flags = LOADER_ADD_TEST_MFT | LOADER_ADD_OPTIONAL_TEST_MFT_DOWNSTREAM | LOADER_EXPECT_MFT_INPUT_ENUMERATED, + }, + + { + /* PCM -> PCM, optional sink */ + .input_types = {&audio_pcm_44100}, .output_types = {&audio_pcm_44100}, .sink_method = MF_CONNECT_AS_OPTIONAL, .source_method = -1, + .expected_result = MF_E_TOPO_UNSUPPORTED, + .flags = LOADER_TODO, + }, + { + /* PCM -> PCM, optional source is ignored */ + .input_types = {&audio_pcm_44100}, .output_types = {&audio_pcm_44100}, .sink_method = -1, .source_method = MF_CONNECT_AS_OPTIONAL, + .expected_result = S_OK, + }, + { + /* H264 -> NV12, optional sink */ + .input_types = {&video_h264_1280}, .output_types = {&video_nv12_1280}, .sink_method = MF_CONNECT_AS_OPTIONAL | MF_CONNECT_ALLOW_DECODER, .source_method = -1, + .expected_result = MF_E_TOPO_UNSUPPORTED, .decoder_class = CLSID_CMSH264DecoderMFT, + .flags = LOADER_TODO, + }, + { + /* H264 -> NV12, optional source is ignored */ + .input_types = {&video_h264_1280}, .output_types = {&video_nv12_1280}, .sink_method = -1, .source_method = MF_CONNECT_AS_OPTIONAL, + .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, + }, + { + /* #70 H264 -> NV12, add test MFT, optional source is ignored */ + .input_types = {&video_h264_1280}, .output_types = {&video_nv12_1280}, .sink_method = -1, .source_method = MF_CONNECT_AS_OPTIONAL, + .mft_input_types = {&video_nv12_1280}, .mft_output_types = {&video_nv12_1280}, + .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED, + }, }; IMFTopologyNode *src_node, *sink_node, *src_node2, *sink_node2, *mft_node; IMFSampleGrabberSinkCallback *grabber_callback = create_test_grabber_callback(); + TOPOID node_id, oldtopoid, newtopoid, optional_mft_node_id[2]; IMFMediaType *media_type, *input_types[2], *output_types[2]; struct test_stream_sink stream_sink = test_stream_sink; IMFTopology *topology, *topology2, *full_topology; @@ -2883,7 +3032,6 @@ static void test_topology_loader(void) IMFTopoLoader *loader; IUnknown *node_object; WORD node_count; - TOPOID node_id, oldtopoid, newtopoid; DWORD index; HRESULT hr; BOOL ret; @@ -3009,8 +3157,10 @@ static void test_topology_loader(void) for (i = 0; i < ARRAY_SIZE(loader_tests); ++i) { + struct test_transform *test_transform = NULL, *optional_transforms[2] = {0}; const struct loader_test *test = &loader_tests[i]; - struct test_transform *test_transform = NULL; + IMFTopologyNode *optional_mft_nodes[2] = {0}; + IMFMediaType *optional_mft_types[2] = {0}; IMFMediaType *mft_output_types[4] = {0}; IMFMediaType *mft_input_types[2] = {0}; @@ -3088,6 +3238,40 @@ static void test_topology_loader(void) test_transform_create(input_count, mft_input_types, output_count, mft_output_types, FALSE, &transform); test_transform = test_transform_from_IMFTransform(transform); IMFTransform_AddRef(transform); + + if (test->optional_mft_count) + { + const media_type_desc *optional_desc; + IMFTransform *optional_mft; + + for (j = 0; j < test->optional_mft_count; j++) + { + IMFMediaType **optional_input_types = &mft_input_types[0], **optional_output_types = &mft_input_types[0]; + + optional_desc = test->optional_mft_input_types[j] ? test->optional_mft_input_types[j] : test->optional_mft_output_type; + if (optional_desc) + { + hr = MFCreateMediaType(&optional_mft_types[j]); + ok(hr == S_OK, "Failed to create media type, hr %#lx.\n", hr); + init_media_type(optional_mft_types[j], *optional_desc, -1); + if (test->optional_mft_input_types[j]) + optional_input_types = &optional_mft_types[j]; + else + optional_output_types = &optional_mft_types[j]; + } + + hr = MFCreateTopologyNode(MF_TOPOLOGY_TRANSFORM_NODE, &optional_mft_nodes[j]); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFTopologyNode_SetUINT32(optional_mft_nodes[j], &MF_TOPONODE_CONNECT_METHOD, MF_CONNECT_ALLOW_DECODER | MF_CONNECT_AS_OPTIONAL); + ok(hr == S_OK, "Failed to set connect method, hr %#lx.\n", hr); + + test_transform_create(1, optional_input_types, 1, optional_output_types, FALSE, &optional_mft); + hr = IMFTopologyNode_SetObject(optional_mft_nodes[j], (IUnknown *)optional_mft); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + optional_transforms[j] = test_transform_from_IMFTransform(optional_mft); + optional_transforms[j]->validate_output_type = !!test->optional_mft_output_type; + } + } } else { @@ -3126,14 +3310,56 @@ static void test_topology_loader(void) { test_transform->input_type_set = FALSE; test_transform->output_type_set = FALSE; + test_transform->set_input_sequence_id = 0; + test_transform->set_output_sequence_id = 0; } hr = IMFTopology_AddNode(topology, mft_node); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - hr = IMFTopologyNode_ConnectOutput(src_node, 0, mft_node, 0); - ok(hr == S_OK, "Failed to connect nodes, hr %#lx.\n", hr); - hr = IMFTopologyNode_ConnectOutput(mft_node, 0, sink_node, 0); - ok(hr == S_OK, "Failed to connect nodes, hr %#lx.\n", hr); + if (test->optional_mft_count) + { + for (j = 0; j < test->optional_mft_count; j++) + { + hr = IMFTopology_AddNode(topology, optional_mft_nodes[j]); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + } + if (test->flags & LOADER_ADD_OPTIONAL_TEST_MFT_DOWNSTREAM) + { + hr = IMFTopologyNode_ConnectOutput(src_node, 0, mft_node, 0); + ok(hr == S_OK, "Failed to connect nodes, hr %#lx.\n", hr); + hr = IMFTopologyNode_ConnectOutput(mft_node, 0, optional_mft_nodes[0], 0); + ok(hr == S_OK, "Failed to connect nodes, hr %#lx.\n", hr); + hr = IMFTopologyNode_ConnectOutput(optional_mft_nodes[test->optional_mft_count - 1], 0, sink_node, 0); + ok(hr == S_OK, "Failed to connect nodes, hr %#lx.\n", hr); + } + else + { + hr = IMFTopologyNode_ConnectOutput(src_node, 0, optional_mft_nodes[0], 0); + ok(hr == S_OK, "Failed to connect nodes, hr %#lx.\n", hr); + hr = IMFTopologyNode_ConnectOutput(optional_mft_nodes[test->optional_mft_count - 1], 0, mft_node, 0); + ok(hr == S_OK, "Failed to connect nodes, hr %#lx.\n", hr); + hr = IMFTopologyNode_ConnectOutput(mft_node, 0, sink_node, 0); + ok(hr == S_OK, "Failed to connect nodes, hr %#lx.\n", hr); + } + if (optional_mft_nodes[1]) + { + hr = IMFTopologyNode_ConnectOutput(optional_mft_nodes[0], 0, optional_mft_nodes[1], 0); + ok(hr == S_OK, "Failed to connect nodes, hr %#lx.\n", hr); + } + for (j = 0; j < test->optional_mft_count; j++) + { + hr = IMFTopologyNode_GetTopoNodeID(optional_mft_nodes[j], &optional_mft_node_id[j]); + ok(hr == S_OK, "Failed to get source node id, hr %#lx.\n", hr); + IMFTopologyNode_Release(optional_mft_nodes[j]); + } + } + else + { + hr = IMFTopologyNode_ConnectOutput(src_node, 0, mft_node, 0); + ok(hr == S_OK, "Failed to connect nodes, hr %#lx.\n", hr); + hr = IMFTopologyNode_ConnectOutput(mft_node, 0, sink_node, 0); + ok(hr == S_OK, "Failed to connect nodes, hr %#lx.\n", hr); + } IMFTopologyNode_Release(mft_node); } else @@ -3159,7 +3385,7 @@ static void test_topology_loader(void) ok(hr == S_OK, "Failed to get attribute count, hr %#lx.\n", hr); ok(!count, "Unexpected count %u.\n", count); - handler.is_supported_called = FALSE; + handler.is_supported_sequence_id = 0; if (test->flags & LOADER_SET_ENUMERATE_SOURCE_TYPES) IMFTopology_SetUINT32(topology, &MF_TOPOLOGY_ENUMERATE_SOURCE_TYPES, 1); @@ -3187,11 +3413,30 @@ static void test_topology_loader(void) } else if (test->expected_result == S_OK) { + BOOL expect_optional_rejected = FALSE; + IMFTopology_GetTopologyID(topology, &oldtopoid); IMFTopology_GetTopologyID(full_topology, &newtopoid); ok(oldtopoid == newtopoid, "Expected the same topology id. %I64u == %I64u\n", oldtopoid, newtopoid); ok(topology != full_topology, "Expected a different object for the resolved topology.\n"); + for (j = 0; j < test->optional_mft_count; ++j) + { + hr = IMFTopology_GetNodeByID(full_topology, optional_mft_node_id[j], &mft_node); + todo_wine_if(test->expect_optional_mft_rejected[j]) + ok(hr == (test->expect_optional_mft_rejected[j] ? MF_E_NOT_FOUND : S_OK), "Unexpected hr %#lx.\n", hr); + expect_optional_rejected |= test->expect_optional_mft_rejected[j]; + + if (hr == S_OK) + { + UINT32 method; + hr = IMFTopologyNode_GetUINT32(mft_node, &MF_TOPONODE_CONNECT_METHOD, &method); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(method == (MF_CONNECT_ALLOW_DECODER | MF_CONNECT_AS_OPTIONAL), "Unexpected method %#x\n", method); + IMFTopologyNode_Release(mft_node); + } + } + expected_count = 1 + !!(test->flags & LOADER_SET_ENUMERATE_SOURCE_TYPES) + !!(test->flags & LOADER_SET_XVP_FOR_PLAYBACK); hr = IMFTopology_GetCount(full_topology, &count); @@ -3203,9 +3448,10 @@ static void test_topology_loader(void) hr = IMFTopology_GetUINT32(full_topology, &MF_TOPOLOGY_RESOLUTION_STATUS, &value); todo_wine { ok(hr == S_OK, "Failed to get attribute, hr %#lx.\n", hr); - ok(value == MF_TOPOLOGY_RESOLUTION_SUCCEEDED, "Unexpected value %#x.\n", value); + ok(value == (expect_optional_rejected ? MF_OPTIONAL_NODE_REJECTED_MEDIA_TYPE : MF_TOPOLOGY_RESOLUTION_SUCCEEDED), + "Unexpected value %#x.\n", value); } - count = 2 + !!test_transform; + count = 2 + !!test_transform + test->optional_mft_count - test->expect_optional_mft_rejected[0] - test->expect_optional_mft_rejected[1]; if (!IsEqualGUID(&test->decoder_class, &GUID_NULL)) count++; if (!IsEqualGUID(&test->converter_class, &GUID_NULL)) @@ -3213,6 +3459,7 @@ todo_wine { hr = IMFTopology_GetNodeCount(full_topology, &node_count); ok(hr == S_OK, "Failed to get node count, hr %#lx.\n", hr); + todo_wine_if(expect_optional_rejected) ok(node_count == count, "Unexpected node count %u.\n", node_count); hr = IMFTopologyNode_GetTopoNodeID(src_node, &node_id); @@ -3238,7 +3485,37 @@ todo_wine { if (hr == S_OK) IMFMediaType_Release(media_type); - ok(handler.is_supported_called, "Sink input support not checked.\n"); + ok(!!handler.is_supported_sequence_id, "Sink input sequence id not set.\n"); + + if (optional_transforms[0]) + { + ok(!!optional_transforms[0]->set_input_sequence_id, "Optional transform input sequence id not set.\n"); + ok(!!test_transform->set_input_sequence_id, "Test transform input sequence id not set.\n"); + ok(!!test_transform->set_output_sequence_id, "Test transform output sequence id not set.\n"); + if (test->flags & LOADER_ADD_OPTIONAL_TEST_MFT_DOWNSTREAM) + { + todo_wine + ok(optional_transforms[0]->set_input_sequence_id > handler.is_supported_sequence_id, + "Optional transform input was not configured after the sink input.\n"); + } + else + { + todo_wine + ok(optional_transforms[0]->set_input_sequence_id > test_transform->set_input_sequence_id, + "Optional transform input was not configured after the non-optional one.\n"); + if (optional_transforms[0]->set_output_sequence_id) + { + /* Non-optional transform is connected to its downstream node after the optional + * node is inserted upstream, i.e. after each branch is connected, its optional + * nodes are inserted before the next branch is connected. */ + todo_wine + ok(optional_transforms[0]->set_output_sequence_id < test_transform->set_output_sequence_id, + "Optional transform output was not configured before the non-optional one.\n"); + } + } + for (j = 0; j < test->optional_mft_count; j++) + IMFTransform_Release(&optional_transforms[j]->IMFTransform_iface); + } if (!IsEqualGUID(&test->decoder_class, &GUID_NULL)) { @@ -3386,6 +3663,10 @@ todo_wine { ok(oldtopoid == newtopoid, "Expected the same topology id. %I64u == %I64u\n", oldtopoid, newtopoid); hr = IMFTopology_GetUINT32(topology2, &IID_IMFTopology, &value); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFTopology_GetNodeCount(topology2, &node_count); + ok(hr == S_OK, "Failed to get node count, hr %#lx.\n", hr); + todo_wine_if(expect_optional_rejected) + ok(node_count == count, "Unexpected node count %u.\n", node_count); ref = IMFTopology_Release(topology2); ok(ref == 0, "Release returned %ld\n", ref); @@ -3407,7 +3688,8 @@ todo_wine { if (test_transform) { - ok(test_transform->input_enum_complete == !!(test->flags & LOADER_EXPECT_MFT_INPUT_ENUMERATED), + todo_wine_if(test->flags & LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO) + ok(test_transform->input_enum_complete == !!(test->flags & (LOADER_EXPECT_MFT_INPUT_ENUMERATED | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO)), "got transform input_enum_complete %u\n", test_transform->input_enum_complete); todo_wine_if(test_transform->output_enum_complete && (test->flags & LOADER_TODO)) ok(test_transform->output_enum_complete == !!(test->flags & LOADER_EXPECT_MFT_OUTPUT_ENUMERATED), @@ -3437,6 +3719,8 @@ todo_wine { ok(ref == 0, "Release returned %ld\n", ref); for (j = 0; j < ARRAY_SIZE(mft_input_types) && mft_input_types[j]; ++j) IMFMediaType_Release(mft_input_types[j]); + for (j = 0; j < ARRAY_SIZE(optional_mft_types) && optional_mft_types[j]; ++j) + IMFMediaType_Release(optional_mft_types[j]); for (j = 0; j < ARRAY_SIZE(mft_output_types) && mft_output_types[j]; ++j) IMFMediaType_Release(mft_output_types[j]); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10645
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/mf/topology_loader.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index def17ff85c3..118fb1e9459 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -244,6 +244,17 @@ static HRESULT topology_branch_clone_nodes(struct topoloader_context *context, s return hr; } +static UINT32 topology_node_get_connect_method_semantics(IMFTopologyNode *node) +{ + UINT32 method; + + /* default to false for all semantic flags */ + if (FAILED(IMFTopologyNode_GetUINT32(node, &MF_TOPONODE_CONNECT_METHOD, &method))) + return 0; + + return method; +} + static HRESULT topology_node_list_branches(IMFTopologyNode *node, struct list *branches) { struct topology_branch *branch; @@ -734,9 +745,7 @@ static HRESULT topology_branch_connect(IMFTopology *topology, enum connect_metho down_method = MF_CONNECT_ALLOW_DECODER; down_method = connect_method_from_mf(down_method) & method_mask; - if (topology_node_get_type(branch->up.node) != MF_TOPOLOGY_SOURCESTREAM_NODE - || FAILED(IMFTopologyNode_GetUINT32(branch->up.node, &MF_TOPONODE_CONNECT_METHOD, &up_method))) - up_method = MF_CONNECT_DIRECT; + up_method = topology_node_get_connect_method_semantics(branch->up.node); if (up_method & MF_CONNECT_RESOLVE_INDEPENDENT_OUTPUTTYPES) hr = topology_branch_foreach_up_types(topology, down_method, branch, upstream); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10645
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/mf/tests/topology.c | 1 - dlls/mf/topology_loader.c | 114 ++++++++++++++++++++++---------------- 2 files changed, 67 insertions(+), 48 deletions(-) diff --git a/dlls/mf/tests/topology.c b/dlls/mf/tests/topology.c index 6f8b51b6e51..b02b858a7b7 100644 --- a/dlls/mf/tests/topology.c +++ b/dlls/mf/tests/topology.c @@ -3508,7 +3508,6 @@ todo_wine { /* Non-optional transform is connected to its downstream node after the optional * node is inserted upstream, i.e. after each branch is connected, its optional * nodes are inserted before the next branch is connected. */ - todo_wine ok(optional_transforms[0]->set_output_sequence_id < test_transform->set_output_sequence_id, "Optional transform output was not configured before the non-optional one.\n"); } diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index 118fb1e9459..e47e23d102e 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -105,6 +105,7 @@ struct topoloader_context { IMFTopology *input_topology; IMFTopology *output_topology; + UINT resolved_count; /* includes accepted and rejected optional nodes */ }; static HRESULT topology_loader_clone_node(struct topoloader_context *context, IMFTopologyNode *node, IMFTopologyNode **clone) @@ -146,8 +147,6 @@ struct topology_branch DWORD stream; IMFMediaTypeHandler *handler; } up, down; - - struct list entry; }; static const char *debugstr_topology_type(MF_TOPOLOGY_TYPE type) @@ -255,28 +254,6 @@ static UINT32 topology_node_get_connect_method_semantics(IMFTopologyNode *node) return method; } -static HRESULT topology_node_list_branches(IMFTopologyNode *node, struct list *branches) -{ - struct topology_branch *branch; - DWORD i, count, down_stream; - IMFTopologyNode *down_node; - HRESULT hr; - - hr = IMFTopologyNode_GetOutputCount(node, &count); - for (i = 0; SUCCEEDED(hr) && i < count; ++i) - { - if (FAILED(IMFTopologyNode_GetOutput(node, i, &down_node, &down_stream))) - continue; - - if (SUCCEEDED(hr = topology_branch_create(node, i, down_node, down_stream, &branch))) - list_add_tail(branches, &branch->entry); - - IMFTopologyNode_Release(down_node); - } - - return hr; -} - static void media_type_try_copy_attr(IMFMediaType *dst, IMFMediaType *src, const GUID *attr, HRESULT *hr) { PROPVARIANT value; @@ -763,24 +740,64 @@ static HRESULT topology_branch_connect(IMFTopology *topology, enum connect_metho return hr; } -static HRESULT topology_loader_resolve_branches(struct topoloader_context *context, struct list *branches) +static HRESULT topology_loader_resolve_subgraph(struct topoloader_context *context, IMFTopologyNode *node, DWORD stream) { enum connect_method method_mask = connect_method_from_mf(MF_CONNECT_ALLOW_DECODER); - struct topology_branch *branch, *next; + struct topology_branch *branch = NULL; + IMFTopologyNode *down_node = NULL; + DWORD down_stream; HRESULT hr = S_OK; + DWORD count; + + if (topology_node_get_connect_method_semantics(node) & MF_CONNECT_AS_OPTIONAL_BRANCH) + FIXME("Optional branches are not supported.\n"); + + if (FAILED(hr = IMFTopologyNode_GetOutput(node, stream, &down_node, &down_stream))) + return hr; + + context->resolved_count++; - LIST_FOR_EACH_ENTRY_SAFE(branch, next, branches, struct topology_branch, entry) + if (FAILED(hr = topology_branch_create(node, stream, down_node, down_stream, &branch))) + goto done; + + if (FAILED(hr = topology_branch_clone_nodes(context, branch))) { - list_remove(&branch->entry); + WARN("Failed to clone nodes for branch %s\n", debugstr_topology_branch(branch)); + goto done; + } - if (FAILED(hr = topology_branch_clone_nodes(context, branch))) - WARN("Failed to clone nodes for branch %s\n", debugstr_topology_branch(branch)); - else - hr = topology_branch_connect(context->output_topology, method_mask, branch, NULL); + if (FAILED(hr = topology_branch_connect(context->output_topology, method_mask, branch, NULL))) + goto done; + topology_branch_destroy(branch); + branch = NULL; + + if (SUCCEEDED(hr) && SUCCEEDED(hr = IMFTopologyNode_GetOutputCount(down_node, &count)) && count > down_stream) + hr = topology_loader_resolve_subgraph(context, down_node, down_stream); + +done: + if (branch) topology_branch_destroy(branch); - if (FAILED(hr)) - break; + IMFTopologyNode_Release(down_node); + + return hr; +} + +static HRESULT topology_loader_resolve_source_branch(struct topoloader_context *context, IMFTopologyNode *node) +{ + DWORD i, count; + HRESULT hr; + + if (FAILED(hr = IMFTopologyNode_GetOutputCount(node, &count))) + return hr; + + for (i = 0; i < count; ++i) + { + if (FAILED(hr = topology_loader_resolve_subgraph(context, node, i))) + { + WARN("Failed to resolve subgraph under source node %p, stream %lu.\n", node, i); + return hr; + } } return hr; @@ -1012,14 +1029,14 @@ static void topology_loader_resolve_complete(struct topoloader_context *context) static HRESULT WINAPI topology_loader_Load(IMFTopoLoader *iface, IMFTopology *input_topology, IMFTopology **ret_topology, IMFTopology *current_topology) { - struct list branches = LIST_INIT(branches); struct topoloader_context context = { 0 }; - struct topology_branch *branch, *next; IMFTopology *output_topology; + UINT source_count = 0; IMFTopologyNode *node; unsigned short i = 0; IMFStreamSink *sink; IUnknown *object; + WORD node_count; TOPOID topoid; HRESULT hr = E_FAIL; @@ -1072,23 +1089,26 @@ static HRESULT WINAPI topology_loader_Load(IMFTopoLoader *iface, IMFTopology *in for (i = 0; SUCCEEDED(IMFTopology_GetNode(input_topology, i, &node)); i++) { - hr = topology_node_list_branches(node, &branches); + if (topology_node_get_type(node) == MF_TOPOLOGY_SOURCESTREAM_NODE) + { + source_count++; + + if (topology_node_get_connect_method_semantics(node) & MF_CONNECT_AS_OPTIONAL_BRANCH) + FIXME("Optional branches are not supported.\n"); + hr = topology_loader_resolve_source_branch(&context, node); + } IMFTopologyNode_Release(node); if (FAILED(hr)) break; } - if (SUCCEEDED(hr) && list_empty(&branches)) - hr = MF_E_TOPO_UNSUPPORTED; + IMFTopology_GetNodeCount(input_topology, &node_count); - while (SUCCEEDED(hr) && !list_empty(&branches)) - hr = topology_loader_resolve_branches(&context, &branches); + if (SUCCEEDED(hr) && !context.resolved_count) + hr = MF_E_TOPO_UNSUPPORTED; - LIST_FOR_EACH_ENTRY_SAFE(branch, next, &branches, struct topology_branch, entry) - { - WARN("Failed to resolve branch %s\n", debugstr_topology_branch(branch)); - list_remove(&branch->entry); - topology_branch_destroy(branch); - } + context.resolved_count += source_count; + if (SUCCEEDED(hr) && context.resolved_count != node_count) + WARN("Failed to resolve %u nodes.\n", node_count - context.resolved_count); if (FAILED(hr)) IMFTopology_Release(output_topology); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10645
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/mf/tests/topology.c | 25 ++---- dlls/mf/topology_loader.c | 185 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 188 insertions(+), 22 deletions(-) diff --git a/dlls/mf/tests/topology.c b/dlls/mf/tests/topology.c index b02b858a7b7..7da20946e02 100644 --- a/dlls/mf/tests/topology.c +++ b/dlls/mf/tests/topology.c @@ -2290,7 +2290,6 @@ enum loader_test_flags LOADER_EXPECT_MFT_OUTPUT_ENUMERATED = 0x4000, LOADER_EXPECT_MFT_INPUT_ENUMERATED = 0x8000, LOADER_ADD_OPTIONAL_TEST_MFT_DOWNSTREAM = 0x10000, - LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO = 0x20000, }; static void test_topology_loader(void) @@ -2902,7 +2901,7 @@ static void test_topology_loader(void) .mft_input_types = {&video_nv12_1280}, .mft_output_types = {&video_nv12_1280}, .optional_mft_count = 1, .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, - .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED, }, { /* H264 -> NV12, add two optional test MFTs and test MFT */ @@ -2910,7 +2909,7 @@ static void test_topology_loader(void) .mft_input_types = {&video_nv12_1280}, .mft_output_types = {&video_nv12_1280}, .optional_mft_count = 2, .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, - .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED, }, { /* H264 -> NV12, add unconnectable optional test MFT and test MFT */ @@ -2919,7 +2918,7 @@ static void test_topology_loader(void) .optional_mft_input_types = {&video_yuy2_1280}, .optional_mft_count = 1, .expect_optional_mft_rejected = {TRUE}, .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, - .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED, }, { /* H264 -> NV12, add unconnectable optional test MFT, connectable optional test MFT and test MFT */ @@ -2928,7 +2927,7 @@ static void test_topology_loader(void) .optional_mft_input_types = {&video_yuy2_1280, &video_nv12_1280}, .optional_mft_count = 2, .expect_optional_mft_rejected = {TRUE, FALSE}, .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, - .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED, }, { /* #60 H264 -> NV12, add connectable optional test MFT, unconnectable optional test MFT and test MFT */ @@ -2937,7 +2936,7 @@ static void test_topology_loader(void) .optional_mft_input_types = {&video_nv12_1280, &video_yuy2_1280}, .optional_mft_count = 2, .expect_optional_mft_rejected = {FALSE, TRUE}, .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, - .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED, }, { /* H264 -> NV12, add two unconnectable optional test MFTs and test MFT */ @@ -2946,7 +2945,7 @@ static void test_topology_loader(void) .optional_mft_input_types = {&video_yuy2_1280, &video_yuy2_1280}, .optional_mft_count = 2, .expect_optional_mft_rejected = {TRUE, TRUE}, .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, - .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED, }, { /* H264 -> NV12, add optional test MFT and test MFT, require test MFT input change for optional */ @@ -2955,7 +2954,7 @@ static void test_topology_loader(void) .optional_mft_output_type = &video_yuy2_1280, .optional_mft_count = 1, .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, - .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED, }, { /* H264 -> NV12, add test MFT and optional test MFT */ @@ -2988,7 +2987,6 @@ static void test_topology_loader(void) /* PCM -> PCM, optional sink */ .input_types = {&audio_pcm_44100}, .output_types = {&audio_pcm_44100}, .sink_method = MF_CONNECT_AS_OPTIONAL, .source_method = -1, .expected_result = MF_E_TOPO_UNSUPPORTED, - .flags = LOADER_TODO, }, { /* PCM -> PCM, optional source is ignored */ @@ -2999,7 +2997,6 @@ static void test_topology_loader(void) /* H264 -> NV12, optional sink */ .input_types = {&video_h264_1280}, .output_types = {&video_nv12_1280}, .sink_method = MF_CONNECT_AS_OPTIONAL | MF_CONNECT_ALLOW_DECODER, .source_method = -1, .expected_result = MF_E_TOPO_UNSUPPORTED, .decoder_class = CLSID_CMSH264DecoderMFT, - .flags = LOADER_TODO, }, { /* H264 -> NV12, optional source is ignored */ @@ -3423,7 +3420,6 @@ static void test_topology_loader(void) for (j = 0; j < test->optional_mft_count; ++j) { hr = IMFTopology_GetNodeByID(full_topology, optional_mft_node_id[j], &mft_node); - todo_wine_if(test->expect_optional_mft_rejected[j]) ok(hr == (test->expect_optional_mft_rejected[j] ? MF_E_NOT_FOUND : S_OK), "Unexpected hr %#lx.\n", hr); expect_optional_rejected |= test->expect_optional_mft_rejected[j]; @@ -3459,7 +3455,6 @@ todo_wine { hr = IMFTopology_GetNodeCount(full_topology, &node_count); ok(hr == S_OK, "Failed to get node count, hr %#lx.\n", hr); - todo_wine_if(expect_optional_rejected) ok(node_count == count, "Unexpected node count %u.\n", node_count); hr = IMFTopologyNode_GetTopoNodeID(src_node, &node_id); @@ -3494,13 +3489,11 @@ todo_wine { ok(!!test_transform->set_output_sequence_id, "Test transform output sequence id not set.\n"); if (test->flags & LOADER_ADD_OPTIONAL_TEST_MFT_DOWNSTREAM) { - todo_wine ok(optional_transforms[0]->set_input_sequence_id > handler.is_supported_sequence_id, "Optional transform input was not configured after the sink input.\n"); } else { - todo_wine ok(optional_transforms[0]->set_input_sequence_id > test_transform->set_input_sequence_id, "Optional transform input was not configured after the non-optional one.\n"); if (optional_transforms[0]->set_output_sequence_id) @@ -3664,7 +3657,6 @@ todo_wine { ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IMFTopology_GetNodeCount(topology2, &node_count); ok(hr == S_OK, "Failed to get node count, hr %#lx.\n", hr); - todo_wine_if(expect_optional_rejected) ok(node_count == count, "Unexpected node count %u.\n", node_count); ref = IMFTopology_Release(topology2); @@ -3687,8 +3679,7 @@ todo_wine { if (test_transform) { - todo_wine_if(test->flags & LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO) - ok(test_transform->input_enum_complete == !!(test->flags & (LOADER_EXPECT_MFT_INPUT_ENUMERATED | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO)), + ok(test_transform->input_enum_complete == !!(test->flags & LOADER_EXPECT_MFT_INPUT_ENUMERATED), "got transform input_enum_complete %u\n", test_transform->input_enum_complete); todo_wine_if(test_transform->output_enum_complete && (test->flags & LOADER_TODO)) ok(test_transform->output_enum_complete == !!(test->flags & LOADER_EXPECT_MFT_OUTPUT_ENUMERATED), diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index e47e23d102e..d3021f17cf1 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -201,6 +201,25 @@ static HRESULT topology_branch_create(IMFTopologyNode *source, DWORD source_stre return S_OK; } +static HRESULT topology_branch_set_source_node(struct topology_branch *branch, + IMFTopologyNode *source, DWORD source_stream) +{ + IMFMediaTypeHandler *source_handler; + HRESULT hr; + + if (SUCCEEDED(hr = topology_node_get_type_handler(source, source_stream, TRUE, &source_handler))) + { + IMFTopologyNode_Release(branch->up.node); + IMFMediaTypeHandler_Release(branch->up.handler); + + IMFTopologyNode_AddRef((branch->up.node = source)); + branch->up.stream = source_stream; + branch->up.handler = source_handler; + } + + return hr; +} + static void topology_branch_destroy(struct topology_branch *branch) { IMFTopologyNode_Release(branch->up.node); @@ -709,6 +728,51 @@ static HRESULT topology_branch_foreach_up_types(IMFTopology *topology, enum conn return hr; } +static HRESULT topology_branch_connect_optional(struct topoloader_context *context, + IMFTopologyNode *up_node, DWORD up_stream, struct topology_branch *branch, BOOL *accepted) +{ + IMFTopologyNode *optional_node = branch->up.node; + IMFTransform *transform = NULL; + HRESULT hr, connect_hr = S_OK; + IMFMediaTypeHandler *handler; + IMFMediaType *type = NULL; + + TRACE("topology %p, branch %s.\n", context->output_topology, debugstr_topology_branch(branch)); + + if (topology_node_get_type(optional_node) != MF_TOPOLOGY_TRANSFORM_NODE) + { + WARN("Optional node is not a transform.\n"); + return S_OK; + } + + if (FAILED(hr = topology_node_get_type_handler(up_node, up_stream, TRUE, &handler))) + goto done; + + if (FAILED(hr = media_type_handler_get_current(handler, &type))) + hr = media_type_handler_get_type(handler, 0, &type); + + IMFMediaTypeHandler_Release(handler); + + if (FAILED(hr) || FAILED(hr = topology_node_get_object(optional_node, &IID_IMFTransform, (void **)&transform))) + goto done; + + /* Optional node connections are restricted to the current or first upstream type. + * The up node is not reconfigured to allow connection even if possible. */ + if (FAILED(connect_hr = IMFTransform_SetInputType(transform, branch->up.stream, type, 0))) + goto done; + + connect_hr = topology_branch_foreach_up_types(context->output_topology, CONNECT_DIRECT, branch, type); + if (!(*accepted = SUCCEEDED(connect_hr))) + WARN("Rejected optional node %p, hr %#lx.\n", optional_node, connect_hr); + +done: + if (transform) + IMFTransform_Release(transform); + if (type) + IMFMediaType_Release(type); + return hr; +} + static HRESULT topology_branch_connect(IMFTopology *topology, enum connect_method method_mask, struct topology_branch *branch, IMFMediaType *upstream) { @@ -740,23 +804,76 @@ static HRESULT topology_branch_connect(IMFTopology *topology, enum connect_metho return hr; } +static HRESULT topology_node_find_down_node(IMFTopologyNode *node, DWORD stream, IMFTopologyNode **optional_node, + DWORD *optional_stream, IMFTopologyNode **down_node, DWORD *down_stream, struct topoloader_context *context) +{ + IMFTopologyNode *optional = NULL; + DWORD opt_stream = 0; + HRESULT hr; + + *optional_node = NULL; + *optional_stream = 0; + *down_node = NULL; + *down_stream = 0; + + IMFTopologyNode_AddRef(node); + + for (;;) + { + IMFTopologyNode *next_node; + + if (FAILED(hr = IMFTopologyNode_GetOutput(node, stream, &next_node, &stream))) + break; + + context->resolved_count++; + + IMFTopologyNode_Release(node); + node = next_node; + + if (!(topology_node_get_connect_method_semantics(node) & MF_CONNECT_AS_OPTIONAL)) + { + *optional_node = optional; + *optional_stream = opt_stream; + *down_node = node; + *down_stream = stream; + return S_OK; + } + + if (topology_node_get_type(node) == MF_TOPOLOGY_OUTPUT_NODE) + { + hr = MF_E_TOPO_UNSUPPORTED; + break; + } + + if (!optional) + { + IMFTopologyNode_AddRef(optional = node); + opt_stream = stream; + } + } + + if (optional) + IMFTopologyNode_Release(optional); + IMFTopologyNode_Release(node); + return hr; +} + static HRESULT topology_loader_resolve_subgraph(struct topoloader_context *context, IMFTopologyNode *node, DWORD stream) { enum connect_method method_mask = connect_method_from_mf(MF_CONNECT_ALLOW_DECODER); + IMFTopologyNode *down_node = NULL, *optional_node = NULL; struct topology_branch *branch = NULL; - IMFTopologyNode *down_node = NULL; - DWORD down_stream; + DWORD down_stream, optional_stream; HRESULT hr = S_OK; DWORD count; if (topology_node_get_connect_method_semantics(node) & MF_CONNECT_AS_OPTIONAL_BRANCH) FIXME("Optional branches are not supported.\n"); - if (FAILED(hr = IMFTopologyNode_GetOutput(node, stream, &down_node, &down_stream))) + if (FAILED(hr = topology_node_find_down_node(node, stream, &optional_node, &optional_stream, + &down_node, &down_stream, context))) return hr; - context->resolved_count++; - if (FAILED(hr = topology_branch_create(node, stream, down_node, down_stream, &branch))) goto done; @@ -769,6 +886,62 @@ static HRESULT topology_loader_resolve_subgraph(struct topoloader_context *conte if (FAILED(hr = topology_branch_connect(context->output_topology, method_mask, branch, NULL))) goto done; + if (optional_node) + { + IMFTopologyNode *up_node; + DWORD up_stream; + + /* The loader may have inserted one or more transforms upstream. Only try inserting the optional node + * downstream of them. Support for upstream insertion is untested in native, but seems unlikely to work. */ + if (FAILED(hr = IMFTopologyNode_GetInput(branch->down.node, branch->down.stream, &up_node, &up_stream))) + { + ERR("Failed to get input.\n"); + goto done; + } + + do + { + IMFTopologyNode *ret_node, *optional_clone; + BOOL accepted = FALSE; + DWORD ret_stream; + + if (FAILED(hr = topology_loader_clone_node(context, optional_node, &optional_clone))) + break; + + if (SUCCEEDED(hr = topology_branch_set_source_node(branch, optional_clone, optional_stream))) + hr = topology_branch_connect_optional(context, up_node, up_stream, branch, &accepted); + + if (accepted) + { + hr = IMFTopologyNode_ConnectOutput(up_node, up_stream, optional_clone, optional_stream); + IMFTopologyNode_Release(up_node); + IMFTopologyNode_AddRef(up_node = optional_clone); + up_stream = optional_stream; + } + else + { + if (FAILED(IMFTopology_RemoveNode(context->output_topology, optional_clone))) + ERR("Failed to remove node.\n"); + } + + IMFTopologyNode_Release(optional_clone); + + if (SUCCEEDED(hr) + && SUCCEEDED(hr = IMFTopologyNode_GetOutput(optional_node, optional_stream, &ret_node, &ret_stream))) + { + IMFTopologyNode_Release(optional_node); + optional_node = ret_node; + optional_stream = ret_stream; + + if (!(topology_node_get_connect_method_semantics(optional_node) & MF_CONNECT_AS_OPTIONAL)) + break; + } + } + while (SUCCEEDED(hr)); + + IMFTopologyNode_Release(up_node); + } + topology_branch_destroy(branch); branch = NULL; @@ -778,6 +951,8 @@ static HRESULT topology_loader_resolve_subgraph(struct topoloader_context *conte done: if (branch) topology_branch_destroy(branch); + if (optional_node) + IMFTopologyNode_Release(optional_node); IMFTopologyNode_Release(down_node); return hr; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10645
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/mf/topology_loader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index d3021f17cf1..d5facc62f10 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -1215,7 +1215,7 @@ static HRESULT WINAPI topology_loader_Load(IMFTopoLoader *iface, IMFTopology *in TOPOID topoid; HRESULT hr = E_FAIL; - FIXME("iface %p, input_topology %p, ret_topology %p, current_topology %p stub!\n", + TRACE("iface %p, input_topology %p, ret_topology %p, current_topology %p.\n", iface, input_topology, ret_topology, current_topology); if (current_topology) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10645
On Thu Apr 16 09:29:39 2026 +0000, Rémi Bernon wrote:
We're about to connect with `up_type` here, shouldn't we rather call `IMFMediaTypeHandler_IsMediaTypeSupported` with it? Also I think it could instead be called in `topology_branch_connect_with_type`, before `IMFMediaTypeHandler_SetCurrentMediaType`, keeping the if a bit simpler here. Done. This fixes a couple of tests too.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10645#note_137306
Rebased, fixed the issue with IsMediaTypeSupported(), and replaced the branch list with a walk down from each source node output. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10645#note_137307
Fwiw I'm still wrapping my head around the remaining patches. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10645#note_137412
Rémi Bernon (@rbernon) commented about dlls/mf/topology_loader.c:
for (i = 0; SUCCEEDED(IMFTopology_GetNode(input_topology, i, &node)); i++) { - hr = topology_node_list_branches(node, &branches); + if (topology_node_get_type(node) == MF_TOPOLOGY_SOURCESTREAM_NODE) + { + source_count++; + + if (topology_node_get_connect_method_semantics(node) & MF_CONNECT_AS_OPTIONAL_BRANCH) + FIXME("Optional branches are not supported.\n"); + hr = topology_loader_resolve_source_branch(&context, node); + } IMFTopologyNode_Release(node); if (FAILED(hr)) break; }
I think we could use the IMFTopology_GetSourceNodeCollection if we want to iterate over source nodes only. It would make counting the node trivial as well. It would also allow us to return early with MF_E_TOPO_UNSUPPORTED when there's no source nodes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10645#note_137734
Rémi Bernon (@rbernon) commented about dlls/mf/topology_loader.c:
WARN("Failed to clone nodes for branch %s\n", debugstr_topology_branch(branch)); - else - hr = topology_branch_connect(context->output_topology, method_mask, branch, NULL); + goto done; + } + + if (FAILED(hr = topology_branch_connect(context->output_topology, method_mask, branch, NULL))) + goto done;
topology_branch_destroy(branch); - if (FAILED(hr)) - break; + branch = NULL; + + if (SUCCEEDED(hr) && SUCCEEDED(hr = IMFTopologyNode_GetOutputCount(down_node, &count)) && count > down_stream) + hr = topology_loader_resolve_subgraph(context, down_node, down_stream);
I don't understand what this does, down_stream is the index of the downstream node input stream? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10645#note_137735
Rémi Bernon (@rbernon) commented about dlls/mf/topology_loader.c:
+ *down_node = node; + *down_stream = stream; + return S_OK; + } + + if (topology_node_get_type(node) == MF_TOPOLOGY_OUTPUT_NODE) + { + hr = MF_E_TOPO_UNSUPPORTED; + break; + } + + if (!optional) + { + IMFTopologyNode_AddRef(optional = node); + opt_stream = stream; + } This looks wrong to me, stream gets updated when iterating but it's used once as output stream index and then gets updated from input stream index. Nodes input / output streams are mostly unrelated?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10645#note_137736
Rémi Bernon (@rbernon) commented about dlls/mf/topology_loader.c:
if (FAILED(hr = topology_branch_connect(context->output_topology, method_mask, branch, NULL))) goto done;
+ if (optional_node)
It seems to me that the control flow should better reflect the logic, here it looks like we are only considering one optional node (although it gets updated below in a loop). As far as I can tell from native behavior logic above, it seems to me that there's some sort of recursion happening after optional node is inserted. Roughly something like that (but it could be modeled differently perhaps): ``` connect_node( U: node, M: method, C: nodes ): - foreach U output stream US, leading to non-optional downstream node D with input stream DS and method DM: - return failure if connect_branch( U, US, D, DS, M | DM ) fails - foreach optional node O with input stream OS and method OM between U/US and D/DS: - break if connect_branch( U, US, O, OS, OM ) and connect_node( O, OM, C ) succeed - connect_branch( U, US, D, DS, M | DM ) # restore connection on failure - insert D in C if not already there - remove U from C ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10645#note_137737
On Mon Apr 27 09:35:08 2026 +0000, Rémi Bernon wrote:
Fwiw I'm still wrapping my head around the remaining patches. Some notes gathered about native behavior:
1) Connects up to down first, one stream at a time, by skipping optional nodes, creates decoders and converters as needed. If this fails, optional nodes connection aren't attempted, even if they would connect the graph. 2) Insertion of optional nodes happens one after each other, up to down, and if optional chain works as a whole but doesn't individually all the optional nodes are skipped. 3) Optional node is connected by iterating over each of its output stream, connecting the stream directly to the downstream node before attempting to insert extra optional nodes. 4) If one optional node connection fails, it is skipped and extra optional nodes down the main stream may be attempted, but other streams from the skipped node, and their nodes are also skipped. This seems fairly broken though. 5) If optional connection fails, original branch is often connected again before attempting other optional nodes. If one optional node connection succeeds, its successful output type is kept, and maybe restored if the next optional node fails to connect. 6) If optional node type matches source but a converter has been added, the connection is attempted with the converter anyway, even if connection with the source node would work directly. 7) If connection between optional and upstream converter fails, more types are attempted from the converter only if optional allows converter, and optional node is skipped otherwise. If there is already a converter it is reused, inserted otherwise if allowed. 8) If connection between up and down is direct, but optional requires a converter, they are inserted iff the optional node method allows them, both upstream and downstream. The downstream node connection method doesn't matter for this. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10645#note_137739
On Mon Apr 27 09:33:31 2026 +0000, Rémi Bernon wrote:
This looks wrong to me, stream gets updated when iterating but it's used once as output stream index and then gets updated from input stream index. Nodes input / output streams are mostly unrelated? Testing odd cases such as:
``` source 0 -> 1 optional 1 -> 0 sink source 0 -> 0 optional 1 -> 1 optional 0 -> 0 sink ``` seems complicated. Do you mind if we restrict all optional node streams to the downstream input for now, and emit a fixme where necessary? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10645#note_137917
On Mon Apr 27 09:35:08 2026 +0000, Rémi Bernon wrote:
Some notes gathered about native behavior: 1) Connects up to down first, one stream at a time, by skipping optional nodes, creates decoders and converters as needed. If this fails, optional nodes connection aren't attempted, even if they would connect the graph. 2) Insertion of optional nodes happens one after each other, up to down, and if optional chain works as a whole but doesn't individually all the optional nodes are skipped. 3) Optional node is connected by iterating over each of its output stream, connecting the stream directly to the downstream node before attempting to insert extra optional nodes. 4) If one optional node connection fails, it is skipped and extra optional nodes down the main stream may be attempted, but other streams from the skipped node, and their nodes are also skipped. This seems fairly broken though. 5) If optional connection fails, original branch is often connected again before attempting other optional nodes. If one optional node connection succeeds, its successful output type is kept, and maybe restored if the next optional node fails to connect. 6) If optional node type matches source but a converter has been added, the connection is attempted with the converter anyway, even if connection with the source node would work directly. 7) If connection between optional and upstream converter fails, more types are attempted from the converter only if optional allows converter, and optional node is skipped otherwise. If there is already a converter it is reused, inserted otherwise if allowed. 8) If connection between up and down is direct, but optional requires a converter, they are inserted iff the optional node method allows them, both upstream and downstream. The downstream node connection method doesn't matter for this. Optional nodes in the new tests are set to ALLOW_DECODER, and there's a test for point 8 above where the optional node is rejected. Do you have a test where it's accepted?
On point 7, the decoder output is not reconfigured to support the optional, but this is not tested for a converter. Does your testing show a difference between decoder and converter connection behaviour? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10645#note_137918
On Tue Apr 28 07:31:52 2026 +0000, Conor McCarthy wrote:
Testing odd cases such as: ``` source 0 -> 1 optional 1 -> 0 sink source 0 -> 0 optional 1 -> 1 optional 0 -> 0 sink ``` seems complicated. Do you mind if we restrict all optional node streams to the downstream input for now, and emit a fixme where necessary? Sure it may not be easy to adjust the tests to cover such cases, though perhaps it is because we are trying to squeeze every check within the same framework. I'm fine restricting the support to a single stream, but I still don't think input / output indexes are supposed to be related.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10645#note_137923
On Tue Apr 28 07:31:52 2026 +0000, Conor McCarthy wrote:
Optional nodes in the new tests are set to ALLOW_DECODER, and there's a test for point 8 above where the optional node is rejected. Do you have a test where it's accepted? On point 7, the decoder output is not reconfigured to support the optional, but this is not tested for a converter. Does your testing show a difference between decoder and converter connection behaviour? I actually mostly only tested the converter case, it indeed looks like that decoders won't be created even if method allow it, although as it includes the converter bit it still seem to try enumerating more types than when connection method is direct. I've been using the attached patch to check such things in a more flexible way than the current tests.
[0001-more-topo-loader-tests.patch](/uploads/c43d324934003e3b3366aa9a77588697/0001-more-topo-loader-tests.patch) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10645#note_137924
On Tue Apr 28 08:03:57 2026 +0000, Rémi Bernon wrote:
Sure it may not be easy to adjust the tests to cover such cases, though perhaps it is because we are trying to squeeze every check within the same framework. I'm fine restricting the support to a single stream, but I still don't think input / output indexes are supposed to be related. I think to find which output to use on the optional, we would need a recursive function which follows each available output stream, through any subsequent optionals, until it reaches the downstream node, and check if it connects to the stream which upstream is already connected to. Have you seen evidence Windows does that? It seems a bit elaborate.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10645#note_137991
On Tue Apr 28 13:19:57 2026 +0000, Conor McCarthy wrote:
I think to find which output to use on the optional, we would need a recursive function which follows each available output stream, through any subsequent optionals, until it reaches the downstream node, and check if it connects to the stream which upstream is already connected to. Have you seen evidence Windows does that? It seems a bit elaborate. I think native just connects through the first output of each optional node. Then once an optional node successfully connects, extra output streams (with index >0) from that node are explored, otherwise they are just ignored. Mostly what point 4) above is about, the "main" stream is the output stream #0 on each node, regardless to which input number it connects to on the next node.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10645#note_137993
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Rémi Bernon (＠rbernon)