[PATCH v3 0/7] MR10009: mf/topology_loader: Fix some topology loader test failures.
A followup MR will move converter connection behaviour closer to native, and maybe revise decoder connection a bit. -- v3: mf/topology_loader: Load the down connection method in topology_branch_connect(). mf/topology_loader: Enumerate branch up types if the node has no current type. mf/topology: Actually set types for transforms in IsMediaTypeSupported(). mf/topology_loader: Load the branch up connection method only for source nodes. mf/topology_loader: Delete unused struct connect_context. mf/topology_loader: Do not create a new branch list containing downstream nodes. mf: Trace additional topology details. https://gitlab.winehq.org/wine/wine/-/merge_requests/10009
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/mf/main.c | 183 ++++++++++++++++++++++++++++++++++++++ dlls/mf/mf_private.h | 2 + dlls/mf/topology_loader.c | 31 +++++-- 3 files changed, 209 insertions(+), 7 deletions(-) diff --git a/dlls/mf/main.c b/dlls/mf/main.c index 537db3174cf..7b10bb94ffd 100644 --- a/dlls/mf/main.c +++ b/dlls/mf/main.c @@ -36,6 +36,189 @@ WINE_DEFAULT_DEBUG_CHANNEL(mfplat); extern const GUID CLSID_FileSchemePlugin; +struct guid_def +{ + const GUID *guid; + const char *name; +}; + +static int __cdecl debug_compare_guid(const void *a, const void *b) +{ + const GUID *guid = a; + const struct guid_def *guid_def = b; + return memcmp(guid, guid_def->guid, sizeof(*guid)); +} + +/* copied from mfplat module */ +const char *debugstr_mf_guid(const GUID *guid) +{ + static const struct guid_def guid_defs[] = + { +#define X(g) { &(g), #g } + X(MFAudioFormat_ADTS), + X(MFAudioFormat_PCM), + X(MFAudioFormat_PCM_HDCP), + X(MFAudioFormat_Float), + X(MFAudioFormat_DTS), + X(MFAudioFormat_DRM), + X(MFAudioFormat_MSP1), + X(MFAudioFormat_Vorbis), + X(MFAudioFormat_AAC), + X(MFVideoFormat_RGB24), + X(MFVideoFormat_ARGB32), + X(MFVideoFormat_RGB32), + X(MFVideoFormat_RGB565), + X(MFVideoFormat_RGB555), + X(MF_DEVSOURCE_ATTRIBUTE_SOURCE_TYPE_AUDCAP_GUID), + X(MFT_CATEGORY_MULTIPLEXER), + X(MFVideoFormat_A2R10G10B10), + X(MFT_CATEGORY_VIDEO_EFFECT), + X(MFMediaType_Script), + X(MFMediaType_Image), + X(MFMediaType_HTML), + X(MFMediaType_Binary), + X(MFVideoFormat_MPEG2), + X(MFMediaType_FileTransfer), + X(MFVideoFormat_RGB8), + X(MFAudioFormat_Dolby_AC3), + X(MFVideoFormat_L8), + X(MFAudioFormat_LPCM), + X(MFVideoFormat_420O), + X(MFVideoFormat_AI44), + X(MFVideoFormat_AV1), + X(MFVideoFormat_AYUV), + X(MFVideoFormat_H263), + X(MFVideoFormat_H264), + X(MFVideoFormat_H265), + X(MFVideoFormat_HEVC), + X(MFVideoFormat_HEVC_ES), + X(MFT_CATEGORY_AUDIO_EFFECT), + X(MFVideoFormat_I420), + X(MFVideoFormat_IYUV), + X(MFT_CATEGORY_VIDEO_DECODER), + X(MFVideoFormat_M4S2), + X(MFVideoFormat_MJPG), + X(MFVideoFormat_MP43), + X(MFVideoFormat_MP4S), + X(MFVideoFormat_MP4V), + X(MFVideoFormat_MPG1), + X(MFVideoFormat_MSS1), + X(MFVideoFormat_MSS2), + X(MFVideoFormat_NV11), + X(MFVideoFormat_NV12), + X(MFVideoFormat_ORAW), + X(MFAudioFormat_Opus), + X(MFAudioFormat_MPEG), + X(MFVideoFormat_D16), + X(MFVideoFormat_P010), + X(MFVideoFormat_P016), + X(MFVideoFormat_P210), + X(MFVideoFormat_P216), + X(MFVideoFormat_L16), + X(MFAudioFormat_MP3), + X(MFVideoFormat_UYVY), + X(MFVideoFormat_VP10), + X(MFVideoFormat_VP80), + X(MFVideoFormat_VP90), + X(MFVideoFormat_WMV1), + X(MFVideoFormat_WMV2), + X(MFVideoFormat_WMV3), + X(MFVideoFormat_WVC1), + X(MFT_CATEGORY_OTHER), + X(MFVideoFormat_Y210), + X(MFVideoFormat_Y216), + X(MFVideoFormat_Y410), + X(MFVideoFormat_Y416), + X(MFVideoFormat_Y41P), + X(MFVideoFormat_Y41T), + X(MFVideoFormat_Y42T), + X(MFVideoFormat_YUY2), + X(MFVideoFormat_YV12), + X(MFVideoFormat_YVU9), + X(MFVideoFormat_YVYU), + X(MFAudioFormat_WMAudioV8), + X(MFAudioFormat_ALAC), + X(MFAudioFormat_AMR_NB), + X(MFMediaType_Audio), + X(MFAudioFormat_WMAudioV9), + X(MFAudioFormat_AMR_WB), + X(MFAudioFormat_WMAudio_Lossless), + X(MFAudioFormat_AMR_WP), + X(MFAudioFormat_WMASPDIF), + X(MFVideoFormat_DV25), + X(MFVideoFormat_DV50), + X(MFVideoFormat_DVC), + X(MFVideoFormat_DVH1), + X(MFVideoFormat_DVHD), + X(MFVideoFormat_DVSD), + X(MFVideoFormat_DVSL), + X(MFVideoFormat_A16B16G16R16F), + X(MFVideoFormat_v210), + X(MFVideoFormat_v216), + X(MFVideoFormat_v410), + X(MFMediaType_Video), + X(MFAudioFormat_AAC_HDCP), + X(MFT_CATEGORY_DEMULTIPLEXER), + X(MF_DEVSOURCE_ATTRIBUTE_SOURCE_TYPE_VIDCAP_GUID), + X(MFT_CATEGORY_VIDEO_ENCODER), + X(MFAudioFormat_Dolby_AC3_HDCP), + X(MFMediaType_Subtitle), + X(MFMediaType_Stream), + X(MFAudioFormat_Dolby_AC3_SPDIF), + X(MFAudioFormat_Float_SpatialObjects), + X(MFMediaType_SAMI), + X(MFAudioFormat_ADTS_HDCP), + X(MFAudioFormat_FLAC), + X(MFAudioFormat_Dolby_DDPlus), + X(MFMediaType_MultiplexedFrames), + X(MFT_CATEGORY_AUDIO_DECODER), + X(MFAudioFormat_Base_HDCP), + X(MFT_CATEGORY_AUDIO_ENCODER), + X(MFVideoFormat_Base_HDCP), + X(MFVideoFormat_H264_HDCP), + X(MFVideoFormat_HEVC_HDCP), + X(MFMediaType_Default), + X(MFMediaType_Protected), + X(MFVideoFormat_H264_ES), + X(MFMediaType_Perception), + X(MFT_CATEGORY_VIDEO_PROCESSOR), +#undef X + }; + struct guid_def *ret = NULL; + + if (guid) + ret = bsearch(guid, guid_defs, ARRAY_SIZE(guid_defs), sizeof(*guid_defs), debug_compare_guid); + + return ret ? wine_dbg_sprintf("%s", ret->name) : wine_dbgstr_guid(guid); +} + +const char *debugstr_media_type(IMFMediaType *media_type) +{ + UINT channels, sps; + GUID subtype = {0}; + UINT64 frame_size; + + if (!media_type) + return "{null}"; + + IMFMediaType_GetGUID(media_type, &MF_MT_SUBTYPE, &subtype); + + if (SUCCEEDED(IMFMediaType_GetUINT64(media_type, &MF_MT_FRAME_SIZE, &frame_size)) && frame_size) + { + return wine_dbg_sprintf("{%p %s %ux%u}", media_type, debugstr_mf_guid(&subtype), (UINT)(frame_size >> 32), (UINT32)frame_size); + } + else if (SUCCEEDED(IMFMediaType_GetUINT32(media_type, &MF_MT_AUDIO_NUM_CHANNELS, &channels)) && channels) + { + if (SUCCEEDED(IMFMediaType_GetUINT32(media_type, &MF_MT_AUDIO_SAMPLES_PER_SECOND, &sps))) + return wine_dbg_sprintf("{%p %s %u-ch %u}", media_type, debugstr_mf_guid(&subtype), channels, sps); + return wine_dbg_sprintf("{%p %s %u-ch}", media_type, debugstr_mf_guid(&subtype), channels); + } + else + { + return wine_dbg_sprintf("{%p %s ?}", media_type, debugstr_mf_guid(&subtype)); + } +} + struct activate_object { IMFActivate IMFActivate_iface; diff --git a/dlls/mf/mf_private.h b/dlls/mf/mf_private.h index 1f2ef17a8c9..2647d3b5b44 100644 --- a/dlls/mf/mf_private.h +++ b/dlls/mf/mf_private.h @@ -113,6 +113,8 @@ static inline const char *debugstr_propvar(const PROPVARIANT *v) } } +extern const char *debugstr_media_type(IMFMediaType *media_type); + extern HRESULT file_scheme_handler_construct(REFIID riid, void **obj); extern HRESULT urlmon_scheme_handler_construct(REFIID riid, void **obj); diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index 73f70e62bc6..14da02d206a 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -147,9 +147,26 @@ struct topology_branch struct list entry; }; +static const char *debugstr_topology_node_type(IMFTopologyNode *node) +{ + MF_TOPOLOGY_TYPE type = MF_TOPOLOGY_MAX; + + IMFTopologyNode_GetNodeType(node, &type); + switch (type) + { + case MF_TOPOLOGY_OUTPUT_NODE: return "sink"; + case MF_TOPOLOGY_SOURCESTREAM_NODE: return "source"; + case MF_TOPOLOGY_TRANSFORM_NODE: return "transform"; + case MF_TOPOLOGY_TEE_NODE: return "tee"; + default: return "unknown"; + } +} + static const char *debugstr_topology_branch(struct topology_branch *branch) { - return wine_dbg_sprintf("%p:%lu to %p:%lu", branch->up.node, branch->up.stream, branch->down.node, branch->down.stream); + return wine_dbg_sprintf("%s %p:%lu to %s %p:%lu", debugstr_topology_node_type(branch->up.node), + branch->up.node, branch->up.stream, debugstr_topology_node_type(branch->down.node), + branch->down.node, branch->down.stream); } static HRESULT topology_branch_create(IMFTopologyNode *source, DWORD source_stream, @@ -282,8 +299,8 @@ static HRESULT topology_branch_connect_indirect(IMFTopology *topology, MF_CONNEC GUID category, guid; HRESULT hr; - TRACE("topology %p, method_mask %#x, branch %s, up_type %p, down_type %p.\n", - topology, method_mask, debugstr_topology_branch(branch), up_type, down_type); + TRACE("topology %p, method_mask %#x, branch %s, up_type %s, down_type %s.\n", topology, method_mask, + debugstr_topology_branch(branch), debugstr_media_type(up_type), debugstr_media_type(down_type)); if (FAILED(hr = IMFMediaType_GetMajorType(up_type, &input_info.guidMajorType))) return hr; @@ -420,8 +437,8 @@ static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_ME DWORD flags; HRESULT hr; - TRACE("topology %p, method_mask %#x, branch %s, up_type %p.\n", - topology, method_mask, debugstr_topology_branch(branch), up_type); + TRACE("topology %p, method_mask %#x, branch %s, up_type %s.\n", + topology, method_mask, debugstr_topology_branch(branch), debugstr_media_type(up_type)); if (FAILED(IMFTopologyNode_GetUINT32(branch->down.node, &MF_TOPONODE_CONNECT_METHOD, &method))) method = MF_CONNECT_ALLOW_DECODER; @@ -432,14 +449,14 @@ static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_ME if (SUCCEEDED(hr = get_first_supported_media_type(down_handler, &down_type)) && IMFMediaType_IsEqual(up_type, down_type, &flags) == S_OK) { - TRACE("Connecting branch %s with current type %p.\n", debugstr_topology_branch(branch), up_type); + TRACE("Connecting branch %s with current type %s.\n", debugstr_topology_branch(branch), debugstr_media_type(up_type)); hr = IMFTopologyNode_ConnectOutput(branch->up.node, branch->up.stream, branch->down.node, branch->down.stream); goto done; } if (SUCCEEDED(hr = IMFMediaTypeHandler_IsMediaTypeSupported(down_handler, up_type, NULL))) { - TRACE("Connected branch %s with upstream type %p.\n", debugstr_topology_branch(branch), up_type); + TRACE("Connected branch %s with upstream type %s.\n", debugstr_topology_branch(branch), debugstr_media_type(up_type)); if (SUCCEEDED(IMFTopologyNode_GetNodeType(branch->down.node, &type)) && type == MF_TOPOLOGY_TRANSFORM_NODE && FAILED(hr = IMFMediaTypeHandler_SetCurrentMediaType(down_handler, up_type))) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10009
From: Conor McCarthy <cmccarthy@codeweavers.com> This is not necessary and results in some branches being connected twice. --- dlls/mf/topology_loader.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index 14da02d206a..0ec9c148d98 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -555,7 +555,6 @@ static HRESULT topology_branch_connect(IMFTopology *topology, MF_CONNECT_METHOD static HRESULT topology_loader_resolve_branches(struct topoloader_context *context, struct list *branches, BOOL enumerate_source_types) { - struct list new_branches = LIST_INIT(new_branches); struct topology_branch *branch, *next; MF_TOPOLOGY_TYPE node_type; HRESULT hr = S_OK; @@ -564,9 +563,7 @@ static HRESULT topology_loader_resolve_branches(struct topoloader_context *conte { list_remove(&branch->entry); - if (FAILED(hr = topology_node_list_branches(branch->down.node, &new_branches))) - WARN("Failed to list branches from branch %s\n", debugstr_topology_branch(branch)); - else if (FAILED(hr = IMFTopologyNode_GetNodeType(branch->up.node, &node_type))) + if (FAILED(hr = IMFTopologyNode_GetNodeType(branch->up.node, &node_type))) WARN("Failed to get source node type for branch %s\n", debugstr_topology_branch(branch)); else if (FAILED(hr = topology_branch_clone_nodes(context, branch))) WARN("Failed to clone nodes for branch %s\n", debugstr_topology_branch(branch)); @@ -582,7 +579,6 @@ static HRESULT topology_loader_resolve_branches(struct topoloader_context *conte break; } - list_move_tail(branches, &new_branches); return hr; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10009
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/mf/topology_loader.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index 0ec9c148d98..011428b47d1 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -124,18 +124,6 @@ struct transform_output_type IMFActivate *activate; }; -struct connect_context -{ - struct topoloader_context *context; - IMFTopologyNode *upstream_node; - IMFTopologyNode *sink; - IMFMediaTypeHandler *sink_handler; - unsigned int output_index; - unsigned int input_index; - GUID converter_category; - GUID decoder_category; -}; - struct topology_branch { struct -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10009
From: Conor McCarthy <cmccarthy@codeweavers.com> This method is used only for connecting source nodes down, and MF_CONNECT_RESOLVE_INDEPENDENT_OUTPUTTYPES is the only valid flag. --- dlls/mf/topology_loader.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index 011428b47d1..bbd0a6f1993 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -271,6 +271,17 @@ static HRESULT update_media_type_from_upstream(IMFMediaType *media_type, IMFMedi return hr; } +static MF_TOPOLOGY_TYPE topology_node_get_type(IMFTopologyNode *node) +{ + MF_TOPOLOGY_TYPE node_type = 0; + HRESULT hr; + + if (FAILED(hr = IMFTopologyNode_GetNodeType(node, &node_type))) + ERR("Failed to get node type, hr %#lx.\n", hr); + + return node_type; +} + static HRESULT topology_branch_connect(IMFTopology *topology, MF_CONNECT_METHOD method_mask, struct topology_branch *branch, BOOL enumerate_source_types); static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_METHOD method_mask, @@ -499,16 +510,20 @@ static HRESULT topology_branch_foreach_up_types(IMFTopology *topology, MF_CONNEC static HRESULT topology_branch_connect(IMFTopology *topology, MF_CONNECT_METHOD method_mask, struct topology_branch *branch, BOOL enumerate_source_types) { + MF_TOPOLOGY_TYPE up_node_type; UINT32 method; HRESULT hr; TRACE("topology %p, method_mask %#x, branch %s.\n", topology, method_mask, debugstr_topology_branch(branch)); - if (FAILED(IMFTopologyNode_GetUINT32(branch->up.node, &MF_TOPONODE_CONNECT_METHOD, &method))) - method = MF_CONNECT_DIRECT; + up_node_type = topology_node_get_type(branch->up.node); if (enumerate_source_types) { + if (up_node_type != MF_TOPOLOGY_SOURCESTREAM_NODE + || FAILED(IMFTopologyNode_GetUINT32(branch->up.node, &MF_TOPONODE_CONNECT_METHOD, &method))) + method = MF_CONNECT_DIRECT; + if (method & MF_CONNECT_RESOLVE_INDEPENDENT_OUTPUTTYPES) hr = topology_branch_foreach_up_types(topology, method_mask & MF_CONNECT_ALLOW_DECODER, branch); else -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10009
From: Conor McCarthy <cmccarthy@codeweavers.com> Windows does not use MFT_SET_TYPE_TEST_ONLY when checking for transform type support. --- dlls/mf/topology.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/mf/topology.c b/dlls/mf/topology.c index 572d4d426d3..48ce941f2dd 100644 --- a/dlls/mf/topology.c +++ b/dlls/mf/topology.c @@ -1887,9 +1887,9 @@ static HRESULT WINAPI type_handler_IsMediaTypeSupported(IMFMediaTypeHandler *ifa if (handler->transform) { if (handler->output) - return IMFTransform_SetOutputType(handler->transform, handler->stream, in_type, MFT_SET_TYPE_TEST_ONLY); + return IMFTransform_SetOutputType(handler->transform, handler->stream, in_type, 0); else - return IMFTransform_SetInputType(handler->transform, handler->stream, in_type, MFT_SET_TYPE_TEST_ONLY); + return IMFTransform_SetInputType(handler->transform, handler->stream, in_type, 0); } if (FAILED(hr = IMFMediaTypeHandler_GetCurrentMediaType(iface, &type))) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10009
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/mf/tests/mf.c | 1 - dlls/mf/tests/topology.c | 14 ++--- dlls/mf/topology_loader.c | 123 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 123 insertions(+), 15 deletions(-) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 89877fa40ac..1576f5dd57a 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -2964,7 +2964,6 @@ static void test_media_session_events(void) ok(propvar.punkVal != (IUnknown *)topology, "got punkVal %p\n", propvar.punkVal); PropVariantClear(&propvar); - todo_wine ok(!handler->enum_count, "got %lu GetMediaTypeByIndex\n", handler->enum_count); ok(handler->set_current_count, "got %lu SetCurrentMediaType\n", handler->set_current_count); handler->enum_count = handler->set_current_count = 0; diff --git a/dlls/mf/tests/topology.c b/dlls/mf/tests/topology.c index d2c1b047c52..b57baa8e3a2 100644 --- a/dlls/mf/tests/topology.c +++ b/dlls/mf/tests/topology.c @@ -2265,7 +2265,6 @@ enum loader_test_flags LOADER_EXPECT_MFT_OUTPUT_ENUMERATED = 0x4000, LOADER_EXPECT_MFT_INPUT_ENUMERATED = 0x8000, LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO = 0x10000, - LOADER_TODO_MFT_IN_TYPE = 0x20000, LOADER_TODO_MFT_OUT_TYPE = 0x40000, }; @@ -2661,7 +2660,7 @@ static void test_topology_loader(void) .mft_output_types = {&audio_float_minimal, &audio_pcm_minimal, &audio_float_48000, &audio_pcm_48000}, .mft_current_output = &audio_pcm_minimal, .expected_result = MF_E_INVALIDMEDIATYPE, - .flags = LOADER_ADD_TEST_MFT | LOADER_TODO_MFT_IN_TYPE | LOADER_TODO_MFT_OUT_TYPE | LOADER_TODO, + .flags = LOADER_ADD_TEST_MFT | LOADER_TODO_MFT_OUT_TYPE | LOADER_TODO, }, { /* PCM -> PCM, different enumerated bps, add test MFT */ @@ -2676,7 +2675,7 @@ static void test_topology_loader(void) .mft_output_types = {&audio_float_minimal, &audio_pcm_minimal, &audio_float_48000, &audio_pcm_48000_resampler}, .mft_current_output = &audio_pcm_48000, .expected_result = S_OK, - .flags = LOADER_ADD_TEST_MFT | LOADER_TODO_MFT_IN_TYPE | LOADER_TODO_MFT_OUT_TYPE, + .flags = LOADER_ADD_TEST_MFT, }, { /* PCM -> PCM, different enumerated bps, add test MFT, configure MFT, no output types */ @@ -2684,7 +2683,7 @@ static void test_topology_loader(void) .mft_output_types = {&audio_float_minimal, &audio_pcm_minimal, &audio_float_48000, &audio_pcm_48000_resampler}, .mft_current_output = &audio_pcm_48000, .expected_result = S_OK, - .flags = LOADER_ADD_TEST_MFT | LOADER_NO_CURRENT_OUTPUT | LOADER_TODO_MFT_IN_TYPE | LOADER_TODO_MFT_OUT_TYPE, + .flags = LOADER_ADD_TEST_MFT | LOADER_NO_CURRENT_OUTPUT, }, { /* MP3 -> PCM, different enumerated bps, add test MFT, configure MFT */ @@ -2692,7 +2691,7 @@ static void test_topology_loader(void) .mft_input_type = &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_TODO | LOADER_TODO_MFT_OUT_TYPE, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, }, { /* MP3 -> PCM, different enumerated bps, add incompatible test MFT, configure MFT */ @@ -2837,14 +2836,14 @@ static void test_topology_loader(void) .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}, .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, .converter_class = CLSID_CColorConvertDMO, - .flags = LOADER_ADD_TEST_MFT | LOADER_TODO_MFT_OUT_TYPE | LOADER_TEST_MFT_EXPECT_CONVERTER | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, + .flags = LOADER_ADD_TEST_MFT | LOADER_TEST_MFT_EXPECT_CONVERTER | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, }, { /* 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}, .expected_result = S_OK, .decoder_class = CLSID_CMSH264DecoderMFT, - .flags = LOADER_ADD_TEST_MFT | LOADER_TODO_MFT_OUT_TYPE | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, }, }; @@ -3374,7 +3373,6 @@ todo_wine { 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), "got transform output_enum_complete %u\n", test_transform->output_enum_complete); - todo_wine_if(test->flags & LOADER_TODO_MFT_IN_TYPE) ok(test_transform->input_type_set == (test->expected_result != MF_E_TOPO_CODEC_NOT_FOUND), "Got transform input_type_set %u.\n", test_transform->input_type_set); todo_wine_if(test->flags & LOADER_TODO_MFT_OUT_TYPE) diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index bbd0a6f1993..01786ac5475 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -130,6 +130,7 @@ struct topology_branch { IMFTopologyNode *node; DWORD stream; + BOOL enumerate; } up, down; struct list entry; @@ -282,6 +283,108 @@ static MF_TOPOLOGY_TYPE topology_node_get_type(IMFTopologyNode *node) return node_type; } +struct type_enumerator +{ + IMFMediaTypeHandler *handler; + BOOL enumerate; + BOOL have_current; + int i; +}; + +static void type_enumerator_init(struct type_enumerator *enumerator, IMFMediaTypeHandler *handler, BOOL enumerate) +{ + enumerator->handler = handler; + enumerator->enumerate = enumerate; + enumerator->have_current = FALSE; + enumerator->i = enumerate - 1; +} + +static HRESULT type_enumerator_get_next_media_type(struct type_enumerator *enumerator, IMFMediaType **type) +{ + int i = enumerator->i++; + + if (i < 0) + { + if (SUCCEEDED(IMFMediaTypeHandler_GetCurrentMediaType(enumerator->handler, type))) + { + enumerator->have_current = TRUE; + return S_OK; + } + i = enumerator->i++; + } + + /* If we are not enumerating and we have a current type, do not check index 0. */ + if ((!i && !enumerator->have_current) || enumerator->enumerate) + return IMFMediaTypeHandler_GetMediaTypeByIndex(enumerator->handler, i, type); + + /* return MF_E_INVALIDMEDIATYPE if not enumerating types */ + return MF_E_INVALIDMEDIATYPE; +} + +static HRESULT topology_branch_complete_connection(struct topology_branch *branch, IMFMediaTypeHandler *up_handler, + IMFMediaType *up_type) +{ + HRESULT hr = S_OK; + + /* Current type is set for sources when enumerating, and always for transforms. */ + if (branch->up.enumerate || topology_node_get_type(branch->up.node) == MF_TOPOLOGY_TRANSFORM_NODE) + hr = IMFMediaTypeHandler_SetCurrentMediaType(up_handler, up_type); + + if (SUCCEEDED(hr)) + { + TRACE("Connected branch %s with up type %s.\n", debugstr_topology_branch(branch), debugstr_media_type(up_type)); + hr = IMFTopologyNode_ConnectOutput(branch->up.node, branch->up.stream, branch->down.node, branch->down.stream); + } + + return hr; +} + +static HRESULT topology_branch_connect_up_types(struct topology_branch *branch, IMFMediaType **first_up_type) +{ + IMFMediaTypeHandler *up_handler, *down_handler; + struct type_enumerator enumerator; + IMFMediaType *up_type; + HRESULT hr; + + TRACE("branch %s.\n", debugstr_topology_branch(branch)); + + *first_up_type = NULL; + + if (FAILED(hr = topology_node_get_type_handler(branch->up.node, branch->up.stream, TRUE, &up_handler))) + return hr; + if (FAILED(hr = topology_node_get_type_handler(branch->down.node, branch->down.stream, FALSE, &down_handler))) + { + IMFMediaTypeHandler_Release(up_handler); + return hr; + } + + /* The downstream current media type is not used here. Setting it in advance has no effect on the type used. + * TODO: unless MF_TOPONODE_LOCKED is set? + * Up and down transform media types are always set, even if they already match. IsMediaTypeSupported() does + * that here if the down node is a transform. + * If enumeration is disabled and upstream current type is null, the available type at index 0 is used. */ + + type_enumerator_init(&enumerator, up_handler, branch->up.enumerate); + while (SUCCEEDED(hr = type_enumerator_get_next_media_type(&enumerator, &up_type))) + { + if (!*first_up_type) + IMFMediaType_AddRef(*first_up_type = up_type); + + if (SUCCEEDED(hr = IMFMediaTypeHandler_IsMediaTypeSupported(down_handler, up_type, NULL))) + hr = topology_branch_complete_connection(branch, up_handler, up_type); + + IMFMediaType_Release(up_type); + + if (SUCCEEDED(hr)) + break; + } + + IMFMediaTypeHandler_Release(up_handler); + IMFMediaTypeHandler_Release(down_handler); + + return hr; +} + static HRESULT topology_branch_connect(IMFTopology *topology, MF_CONNECT_METHOD method_mask, struct topology_branch *branch, BOOL enumerate_source_types); static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_METHOD method_mask, @@ -540,15 +643,23 @@ static HRESULT topology_branch_connect(IMFTopology *topology, MF_CONNECT_METHOD IMFMediaTypeHandler *up_handler; IMFMediaType *up_type; - if (FAILED(hr = topology_node_get_type_handler(branch->up.node, branch->up.stream, TRUE, &up_handler))) - return hr; - if (SUCCEEDED(hr = IMFMediaTypeHandler_GetCurrentMediaType(up_handler, &up_type)) - || SUCCEEDED(hr = IMFMediaTypeHandler_GetMediaTypeByIndex(up_handler, 0, &up_type))) + if (up_node_type != MF_TOPOLOGY_SOURCESTREAM_NODE) { + if (FAILED(hr = topology_node_get_type_handler(branch->up.node, branch->up.stream, TRUE, &up_handler))) + return hr; + + if (SUCCEEDED(hr = IMFMediaTypeHandler_GetCurrentMediaType(up_handler, &up_type))) + IMFMediaType_Release(up_type); + branch->up.enumerate = FAILED(hr); + + IMFMediaTypeHandler_Release(up_handler); + } + + if (FAILED(hr = topology_branch_connect_up_types(branch, &up_type)) && up_type) hr = topology_branch_connect_down(topology, method_mask, branch, up_type); + + if (up_type) IMFMediaType_Release(up_type); - } - IMFMediaTypeHandler_Release(up_handler); } TRACE("returning %#lx\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10009
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/mf/topology_loader.c | 42 +++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index 01786ac5475..f1e7840596d 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -387,7 +387,7 @@ static HRESULT topology_branch_connect_up_types(struct topology_branch *branch, static HRESULT topology_branch_connect(IMFTopology *topology, MF_CONNECT_METHOD method_mask, struct topology_branch *branch, BOOL enumerate_source_types); -static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_METHOD method_mask, +static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_METHOD method, struct topology_branch *branch, IMFMediaType *up_type); static HRESULT topology_branch_connect_indirect(IMFTopology *topology, MF_CONNECT_METHOD method_mask, struct topology_branch *branch, IMFMediaType *up_type, IMFMediaType *down_type) @@ -529,21 +529,17 @@ HRESULT topology_node_init_media_type(IMFTopologyNode *node, DWORD stream, BOOL return hr; } -static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_METHOD method_mask, +static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_METHOD method, struct topology_branch *branch, IMFMediaType *up_type) { IMFMediaTypeHandler *down_handler; IMFMediaType *down_type = NULL; MF_TOPOLOGY_TYPE type; - UINT32 method; DWORD flags; HRESULT hr; - TRACE("topology %p, method_mask %#x, branch %s, up_type %s.\n", - topology, method_mask, debugstr_topology_branch(branch), debugstr_media_type(up_type)); - - if (FAILED(IMFTopologyNode_GetUINT32(branch->down.node, &MF_TOPONODE_CONNECT_METHOD, &method))) - method = MF_CONNECT_ALLOW_DECODER; + TRACE("topology %p, method %#x, branch %s, up_type %s.\n", + topology, method, debugstr_topology_branch(branch), debugstr_media_type(up_type)); if (FAILED(hr = topology_node_get_type_handler(branch->down.node, branch->down.stream, FALSE, &down_handler))) return hr; @@ -569,11 +565,11 @@ static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_ME goto done; } - if (FAILED(hr) && (method & method_mask & MF_CONNECT_ALLOW_CONVERTER) == MF_CONNECT_ALLOW_CONVERTER) + if (FAILED(hr) && (method & MF_CONNECT_ALLOW_CONVERTER) == MF_CONNECT_ALLOW_CONVERTER) hr = topology_branch_connect_indirect(topology, MF_CONNECT_ALLOW_CONVERTER, branch, up_type, down_type); - if (FAILED(hr) && (method & method_mask & MF_CONNECT_ALLOW_DECODER) == MF_CONNECT_ALLOW_DECODER) + if (FAILED(hr) && (method & MF_CONNECT_ALLOW_DECODER) == MF_CONNECT_ALLOW_DECODER) hr = topology_branch_connect_indirect(topology, MF_CONNECT_ALLOW_DECODER, branch, up_type, down_type); @@ -585,7 +581,7 @@ done: return hr; } -static HRESULT topology_branch_foreach_up_types(IMFTopology *topology, MF_CONNECT_METHOD method_mask, +static HRESULT topology_branch_foreach_up_types(IMFTopology *topology, MF_CONNECT_METHOD method, struct topology_branch *branch) { IMFMediaTypeHandler *handler; @@ -598,7 +594,7 @@ static HRESULT topology_branch_foreach_up_types(IMFTopology *topology, MF_CONNEC while (SUCCEEDED(hr = IMFMediaTypeHandler_GetMediaTypeByIndex(handler, index++, &type))) { - hr = topology_branch_connect_down(topology, method_mask, branch, type); + hr = topology_branch_connect_down(topology, method, branch, type); if (SUCCEEDED(hr)) hr = IMFMediaTypeHandler_SetCurrentMediaType(handler, type); IMFMediaType_Release(type); @@ -614,28 +610,32 @@ static HRESULT topology_branch_connect(IMFTopology *topology, MF_CONNECT_METHOD struct topology_branch *branch, BOOL enumerate_source_types) { MF_TOPOLOGY_TYPE up_node_type; - UINT32 method; + UINT32 up_method, down_method; HRESULT hr; TRACE("topology %p, method_mask %#x, branch %s.\n", topology, method_mask, debugstr_topology_branch(branch)); up_node_type = topology_node_get_type(branch->up.node); + if (FAILED(IMFTopologyNode_GetUINT32(branch->down.node, &MF_TOPONODE_CONNECT_METHOD, &down_method))) + down_method = MF_CONNECT_ALLOW_DECODER; + down_method &= method_mask; + if (enumerate_source_types) { if (up_node_type != MF_TOPOLOGY_SOURCESTREAM_NODE - || FAILED(IMFTopologyNode_GetUINT32(branch->up.node, &MF_TOPONODE_CONNECT_METHOD, &method))) - method = MF_CONNECT_DIRECT; + || FAILED(IMFTopologyNode_GetUINT32(branch->up.node, &MF_TOPONODE_CONNECT_METHOD, &up_method))) + up_method = MF_CONNECT_DIRECT; - if (method & MF_CONNECT_RESOLVE_INDEPENDENT_OUTPUTTYPES) - hr = topology_branch_foreach_up_types(topology, method_mask & MF_CONNECT_ALLOW_DECODER, branch); + if (up_method & MF_CONNECT_RESOLVE_INDEPENDENT_OUTPUTTYPES) + hr = topology_branch_foreach_up_types(topology, down_method & MF_CONNECT_ALLOW_DECODER, branch); else { - hr = topology_branch_foreach_up_types(topology, method_mask & MF_CONNECT_DIRECT, branch); + hr = topology_branch_foreach_up_types(topology, down_method & MF_CONNECT_DIRECT, branch); if (FAILED(hr)) - hr = topology_branch_foreach_up_types(topology, method_mask & MF_CONNECT_ALLOW_CONVERTER, branch); + hr = topology_branch_foreach_up_types(topology, down_method & MF_CONNECT_ALLOW_CONVERTER, branch); if (FAILED(hr)) - hr = topology_branch_foreach_up_types(topology, method_mask & MF_CONNECT_ALLOW_DECODER, branch); + hr = topology_branch_foreach_up_types(topology, down_method & MF_CONNECT_ALLOW_DECODER, branch); } } else @@ -656,7 +656,7 @@ static HRESULT topology_branch_connect(IMFTopology *topology, MF_CONNECT_METHOD } if (FAILED(hr = topology_branch_connect_up_types(branch, &up_type)) && up_type) - hr = topology_branch_connect_down(topology, method_mask, branch, up_type); + hr = topology_branch_connect_down(topology, down_method, branch, up_type); if (up_type) IMFMediaType_Release(up_type); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10009
On Tue Mar 24 16:14:35 2026 +0000, Rémi Bernon wrote:
Possible, but even looking at the initial MR changes I still feel like the whole stateful logic with index0 / current / enumerate cached data and then calls to `type_enumerator_reset` spread around makes it difficult to follow, and it needs to be made simpler. The `topology_branch` structure could be used to keep / cache information if we find it useful, instead of introducing a separate context structure, but a lot of the cached information doesn't seem to really deserve caching IMO. It would be better to only store characteristics of the connection mode that are slightly complicated to compute (for instance whether we need to enumerate types or not, but other info like node type, current or first media type doesn't seem useful). I removed the context.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10009#note_133972
Rémi Bernon (@rbernon) commented about dlls/mf/topology.c:
if (handler->transform) { if (handler->output) - return IMFTransform_SetOutputType(handler->transform, handler->stream, in_type, MFT_SET_TYPE_TEST_ONLY); + return IMFTransform_SetOutputType(handler->transform, handler->stream, in_type, 0); else - return IMFTransform_SetInputType(handler->transform, handler->stream, in_type, MFT_SET_TYPE_TEST_ONLY); + return IMFTransform_SetInputType(handler->transform, handler->stream, in_type, 0);
Are we sure about that? Do we really want to match this behavior? This looks a bit unfortunate and potentially costly with a lot of potential decoder creation / destruction. It also modifies the current media type, which means we might have to restore it to unset after failed attempts? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10009#note_133993
On Fri Mar 27 09:21:37 2026 +0000, Rémi Bernon wrote:
Are we sure about that? Do we really want to match this behavior? This looks a bit unfortunate and potentially costly with a lot of potential decoder creation / destruction. It also modifies the current media type, which means we might have to restore it to unset after failed attempts? We only need it for inputs. Setting the input type can be necessary because some transforms change their available output types based on it. Test 17 currently breaks if the input is not set. There may be more later.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10009#note_134059
On Fri Mar 27 13:51:40 2026 +0000, Conor McCarthy wrote:
We only need it for inputs. Setting the input type can be necessary because some transforms change their available output types based on it. Test 17 currently breaks if the input is not set. There may be more later. Can we keep using the flag on the output side to avoid actually creating the transforms, which may be heavy?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10009#note_134060
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Rémi Bernon (＠rbernon)