[PATCH v2 0/7] MR10009: mf/topology_loader: Refactor the topology loader and fix a few test failures.
This changes the functionality relatively little, but the refactoring will allow greater changes in a followup MR which will fix more tests. -- v2: mf/topology_loader: Load the branch up connection method only for source nodes. mf/topology_loader: Use the branch connection context for down nodes. mf/topology_loader: Set the branch upstream node media type if not a source node. mf/topology_loader: Introduce a branch connection context. 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..931400cd50c 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_subtype(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..6b182e04d82 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_subtype(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..0c1b48ca1b2 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_subtype(up_type), debugstr_media_subtype(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_subtype(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_subtype(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_subtype(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 0c1b48ca1b2..5c691003621 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 5c691003621..c34e150f27a 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> --- dlls/mf/topology_loader.c | 95 ++++++++++++++++++++++++++++++--------- 1 file changed, 75 insertions(+), 20 deletions(-) diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index c34e150f27a..7a32a6a0a44 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -271,6 +271,67 @@ static HRESULT update_media_type_from_upstream(IMFMediaType *media_type, IMFMedi return hr; } +struct connection_context_type_enumerator +{ + IMFMediaTypeHandler *handler; + IMFMediaType *current; + IMFMediaType *index0; + BOOL enumerate; + int index; +}; + +static void type_enumerator_release(struct connection_context_type_enumerator *enumerator) +{ + if (enumerator->current) + IMFMediaType_Release(enumerator->current); + if (enumerator->index0) + IMFMediaType_Release(enumerator->index0); + if (enumerator->handler) + IMFMediaTypeHandler_Release(enumerator->handler); +} + +struct connection_context +{ + MF_TOPOLOGY_TYPE up_node_type; + struct connection_context_type_enumerator up_types; +}; + +static void connection_context_destroy(struct connection_context *context) +{ + type_enumerator_release(&context->up_types); +} + +static HRESULT connection_context_init(struct connection_context *context, IMFTopology *topology, + struct topology_branch *branch, MF_CONNECT_METHOD method, BOOL enumerate_source_types) +{ + IMFMediaType *up_type; + HRESULT hr; + + if (FAILED(hr = IMFTopologyNode_GetNodeType(branch->up.node, &context->up_node_type))) + return hr; + + if (FAILED(hr = topology_node_get_type_handler(branch->up.node, branch->up.stream, TRUE, &context->up_types.handler))) + return hr; + + enumerate_source_types = context->up_node_type == MF_TOPOLOGY_SOURCESTREAM_NODE && enumerate_source_types; + if (!(context->up_types.enumerate = enumerate_source_types)) + { + hr = IMFMediaTypeHandler_GetCurrentMediaType(context->up_types.handler, &context->up_types.current); + if (context->up_node_type != MF_TOPOLOGY_SOURCESTREAM_NODE) + context->up_types.enumerate = FAILED(hr); + } + + if (FAILED(hr = IMFMediaTypeHandler_GetMediaTypeByIndex(context->up_types.handler, 0, &up_type))) + goto failed; + context->up_types.index0 = up_type; + + return S_OK; + +failed: + connection_context_destroy(context); + 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, @@ -472,16 +533,13 @@ done: } static HRESULT topology_branch_foreach_up_types(IMFTopology *topology, MF_CONNECT_METHOD method_mask, - struct topology_branch *branch) + struct topology_branch *branch, struct connection_context *context) { - IMFMediaTypeHandler *handler; + IMFMediaTypeHandler *handler = context->up_types.handler; IMFMediaType *type; DWORD index = 0; HRESULT hr; - if (FAILED(hr = topology_node_get_type_handler(branch->up.node, branch->up.stream, TRUE, &handler))) - return hr; - while (SUCCEEDED(hr = IMFMediaTypeHandler_GetMediaTypeByIndex(handler, index++, &type))) { hr = topology_branch_connect_down(topology, method_mask, branch, type); @@ -492,50 +550,47 @@ static HRESULT topology_branch_foreach_up_types(IMFTopology *topology, MF_CONNEC break; } - IMFMediaTypeHandler_Release(handler); return hr; } static HRESULT topology_branch_connect(IMFTopology *topology, MF_CONNECT_METHOD method_mask, struct topology_branch *branch, BOOL enumerate_source_types) { + struct connection_context context = {0}; UINT32 method; HRESULT hr; TRACE("topology %p, method_mask %#x, branch %s.\n", topology, method_mask, debugstr_topology_branch(branch)); + if (FAILED(hr = connection_context_init(&context, topology, branch, method_mask, enumerate_source_types))) + return hr; + if (FAILED(IMFTopologyNode_GetUINT32(branch->up.node, &MF_TOPONODE_CONNECT_METHOD, &method))) method = MF_CONNECT_DIRECT; if (enumerate_source_types) { if (method & MF_CONNECT_RESOLVE_INDEPENDENT_OUTPUTTYPES) - hr = topology_branch_foreach_up_types(topology, method_mask & MF_CONNECT_ALLOW_DECODER, branch); + hr = topology_branch_foreach_up_types(topology, method_mask & MF_CONNECT_ALLOW_DECODER, branch, &context); else { - hr = topology_branch_foreach_up_types(topology, method_mask & MF_CONNECT_DIRECT, branch); + hr = topology_branch_foreach_up_types(topology, method_mask & MF_CONNECT_DIRECT, branch, &context); if (FAILED(hr)) - hr = topology_branch_foreach_up_types(topology, method_mask & MF_CONNECT_ALLOW_CONVERTER, branch); + hr = topology_branch_foreach_up_types(topology, method_mask & MF_CONNECT_ALLOW_CONVERTER, branch, &context); if (FAILED(hr)) - hr = topology_branch_foreach_up_types(topology, method_mask & MF_CONNECT_ALLOW_DECODER, branch); + hr = topology_branch_foreach_up_types(topology, method_mask & MF_CONNECT_ALLOW_DECODER, branch, &context); } } else { - 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))) - { - hr = topology_branch_connect_down(topology, method_mask, branch, up_type); - IMFMediaType_Release(up_type); - } - IMFMediaTypeHandler_Release(up_handler); + up_type = context.up_types.current ? context.up_types.current : context.up_types.index0; + hr = topology_branch_connect_down(topology, method_mask, branch, up_type); } + connection_context_destroy(&context); + TRACE("returning %#lx\n", hr); return hr; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10009
From: Conor McCarthy <cmccarthy@codeweavers.com> --- dlls/mf/tests/topology.c | 10 +++++----- dlls/mf/topology_loader.c | 3 +++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/dlls/mf/tests/topology.c b/dlls/mf/tests/topology.c index d2c1b047c52..e45c731e9e7 100644 --- a/dlls/mf/tests/topology.c +++ b/dlls/mf/tests/topology.c @@ -2676,7 +2676,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 | LOADER_TODO_MFT_IN_TYPE, }, { /* PCM -> PCM, different enumerated bps, add test MFT, configure MFT, no output types */ @@ -2684,7 +2684,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 | LOADER_TODO_MFT_IN_TYPE, }, { /* MP3 -> PCM, different enumerated bps, add test MFT, configure MFT */ @@ -2692,7 +2692,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 +2837,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, }, }; diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index 7a32a6a0a44..e10b627c1da 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -587,6 +587,9 @@ static HRESULT topology_branch_connect(IMFTopology *topology, MF_CONNECT_METHOD up_type = context.up_types.current ? context.up_types.current : context.up_types.index0; hr = topology_branch_connect_down(topology, method_mask, branch, up_type); + /* Upstream type is left unset only for an unenumerated media source. */ + if (SUCCEEDED(hr) && context.up_node_type != MF_TOPOLOGY_SOURCESTREAM_NODE) + hr = IMFMediaTypeHandler_SetCurrentMediaType(context.up_types.handler, up_type); } connection_context_destroy(&context); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10009
From: Conor McCarthy <cmccarthy@codeweavers.com> The todo_wine added to test_media_session_events() is for a sink with zero supported media types, not valid on recent Windows versions. --- dlls/mf/tests/mf.c | 2 +- dlls/mf/tests/topology.c | 14 ++++----- dlls/mf/topology_loader.c | 61 ++++++++++++++++++++++----------------- 3 files changed, 42 insertions(+), 35 deletions(-) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 89877fa40ac..8a368e06315 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; @@ -3038,6 +3037,7 @@ 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 e45c731e9e7..79febc250ba 100644 --- a/dlls/mf/tests/topology.c +++ b/dlls/mf/tests/topology.c @@ -2486,7 +2486,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 */ @@ -2513,7 +2512,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, }, { @@ -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, + .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, + .flags = LOADER_ADD_TEST_MFT | LOADER_NO_CURRENT_OUTPUT, }, { /* MP3 -> PCM, different enumerated bps, add test MFT, configure MFT */ @@ -2699,7 +2698,7 @@ static void test_topology_loader(void) .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}, .expected_result = MF_E_TOPO_CODEC_NOT_FOUND, - .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO | LOADER_TODO, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, }, { @@ -2830,7 +2829,7 @@ static void test_topology_loader(void) .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}, .expected_result = MF_E_TOPO_CODEC_NOT_FOUND, .decoder_class = CLSID_CMSH264DecoderMFT, - .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO | LOADER_TODO, + .flags = LOADER_ADD_TEST_MFT | LOADER_EXPECT_MFT_INPUT_ENUMERATED_TODO, }, { /* H264 -> RGB32, Video Processor media type, add test MFT */ @@ -3417,7 +3416,6 @@ todo_wine { ok(ref == 0, "Release returned %ld\n", ref); ref = IMFMediaType_Release(input_types[0]); - todo_wine ok(ref == 0, "Release returned %ld\n", ref); ref = IMFMediaType_Release(input_types[1]); ok(ref == 0, "Release returned %ld\n", ref); diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index e10b627c1da..c4ece38db7f 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -294,11 +294,13 @@ struct connection_context { MF_TOPOLOGY_TYPE up_node_type; struct connection_context_type_enumerator up_types; + struct connection_context_type_enumerator down_types; }; static void connection_context_destroy(struct connection_context *context) { type_enumerator_release(&context->up_types); + type_enumerator_release(&context->down_types); } static HRESULT connection_context_init(struct connection_context *context, IMFTopology *topology, @@ -312,6 +314,8 @@ static HRESULT connection_context_init(struct connection_context *context, IMFTo if (FAILED(hr = topology_node_get_type_handler(branch->up.node, branch->up.stream, TRUE, &context->up_types.handler))) return hr; + if (FAILED(hr = topology_node_get_type_handler(branch->down.node, branch->down.stream, FALSE, &context->down_types.handler))) + goto failed; enumerate_source_types = context->up_node_type == MF_TOPOLOGY_SOURCESTREAM_NODE && enumerate_source_types; if (!(context->up_types.enumerate = enumerate_source_types)) @@ -325,6 +329,9 @@ static HRESULT connection_context_init(struct connection_context *context, IMFTo goto failed; context->up_types.index0 = up_type; + hr = IMFMediaTypeHandler_GetCurrentMediaType(context->down_types.handler, &context->down_types.current); + context->down_types.enumerate = (hr == MF_E_NOT_INITIALIZED || hr == MF_E_TRANSFORM_TYPE_NOT_SET); + return S_OK; failed: @@ -332,12 +339,19 @@ failed: return hr; } +static void connection_context_init_down_type_enumeration(struct connection_context *context) +{ + /* Types are fetched by index only if GetCurrentMediaType() returned MF_E_NOT_INITIALIZED or MF_E_TRANSFORM_TYPE_NOT_SET. */ + if (context->down_types.enumerate && !context->down_types.index0) + IMFMediaTypeHandler_GetMediaTypeByIndex(context->down_types.handler, 0, &context->down_types.index0); +} + 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, - struct topology_branch *branch, IMFMediaType *up_type); + struct topology_branch *branch, IMFMediaType *up_type, struct connection_context *context); static HRESULT topology_branch_connect_indirect(IMFTopology *topology, MF_CONNECT_METHOD method_mask, - struct topology_branch *branch, IMFMediaType *up_type, IMFMediaType *down_type) + struct topology_branch *branch, BOOL enumerate, IMFMediaType *up_type, IMFMediaType *down_type) { BOOL decoder = (method_mask & MF_CONNECT_ALLOW_DECODER) == MF_CONNECT_ALLOW_DECODER; MFT_REGISTER_TYPE_INFO input_info, output_info; @@ -389,6 +403,7 @@ static HRESULT topology_branch_connect_indirect(IMFTopology *topology, MF_CONNEC { struct topology_branch down_branch = {.up.node = node, .down = branch->down}; struct topology_branch up_branch = {.up = branch->up, .down.node = node}; + struct connection_context up_context = {0}; MF_CONNECT_METHOD method = method_mask; IMFMediaType *media_type; @@ -400,7 +415,14 @@ static HRESULT topology_branch_connect_indirect(IMFTopology *topology, MF_CONNEC if (SUCCEEDED(IMFActivate_GetGUID(activates[i], &MFT_TRANSFORM_CLSID_Attribute, &guid))) IMFTopologyNode_SetGUID(node, &MF_TOPONODE_TRANSFORM_OBJECTID, &guid); - hr = topology_branch_connect_down(topology, MF_CONNECT_DIRECT, &up_branch, up_type); + if (FAILED(hr = connection_context_init(&up_context, topology, &up_branch, MF_CONNECT_DIRECT, enumerate))) + { + IMFTransform_Release(transform); + continue; + } + + hr = topology_branch_connect_down(topology, MF_CONNECT_DIRECT, &up_branch, up_type, &up_context); + connection_context_destroy(&up_context); if (down_type && SUCCEEDED(MFCreateMediaType(&media_type))) { if (SUCCEEDED(IMFMediaType_CopyAllItems(down_type, (IMFAttributes *)media_type)) @@ -477,13 +499,12 @@ HRESULT topology_node_init_media_type(IMFTopologyNode *node, DWORD stream, BOOL } static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_METHOD method_mask, - struct topology_branch *branch, IMFMediaType *up_type) + struct topology_branch *branch, IMFMediaType *up_type, struct connection_context *context) { - IMFMediaTypeHandler *down_handler; - IMFMediaType *down_type = NULL; + IMFMediaTypeHandler *down_handler = context->down_types.handler; + IMFMediaType *down_type; MF_TOPOLOGY_TYPE type; UINT32 method; - DWORD flags; HRESULT hr; TRACE("topology %p, method_mask %#x, branch %s, up_type %s.\n", @@ -492,17 +513,6 @@ static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_ME if (FAILED(IMFTopologyNode_GetUINT32(branch->down.node, &MF_TOPONODE_CONNECT_METHOD, &method))) method = MF_CONNECT_ALLOW_DECODER; - if (FAILED(hr = topology_node_get_type_handler(branch->down.node, branch->down.stream, FALSE, &down_handler))) - return hr; - - 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 %s.\n", debugstr_topology_branch(branch), debugstr_media_subtype(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 %s.\n", debugstr_topology_branch(branch), debugstr_media_subtype(up_type)); @@ -516,19 +526,18 @@ static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_ME goto done; } + connection_context_init_down_type_enumeration(context); + down_type = context->down_types.current ? context->down_types.current : context->down_types.index0; + if (FAILED(hr) && (method & method_mask & MF_CONNECT_ALLOW_CONVERTER) == MF_CONNECT_ALLOW_CONVERTER) hr = topology_branch_connect_indirect(topology, MF_CONNECT_ALLOW_CONVERTER, - branch, up_type, down_type); + branch, context->up_types.enumerate, up_type, down_type); if (FAILED(hr) && (method & method_mask & MF_CONNECT_ALLOW_DECODER) == MF_CONNECT_ALLOW_DECODER) hr = topology_branch_connect_indirect(topology, MF_CONNECT_ALLOW_DECODER, - branch, up_type, down_type); + branch, context->up_types.enumerate, up_type, down_type); done: - if (down_type) - IMFMediaType_Release(down_type); - IMFMediaTypeHandler_Release(down_handler); - return hr; } @@ -542,7 +551,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_mask, branch, type, context); if (SUCCEEDED(hr)) hr = IMFMediaTypeHandler_SetCurrentMediaType(handler, type); IMFMediaType_Release(type); @@ -586,7 +595,7 @@ static HRESULT topology_branch_connect(IMFTopology *topology, MF_CONNECT_METHOD IMFMediaType *up_type; up_type = context.up_types.current ? context.up_types.current : context.up_types.index0; - hr = topology_branch_connect_down(topology, method_mask, branch, up_type); + hr = topology_branch_connect_down(topology, method_mask, branch, up_type, &context); /* Upstream type is left unset only for an unenumerated media source. */ if (SUCCEEDED(hr) && context.up_node_type != MF_TOPOLOGY_SOURCESTREAM_NODE) hr = IMFMediaTypeHandler_SetCurrentMediaType(context.up_types.handler, up_type); -- 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 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index c4ece38db7f..abda4213786 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -574,7 +574,8 @@ static HRESULT topology_branch_connect(IMFTopology *topology, MF_CONNECT_METHOD if (FAILED(hr = connection_context_init(&context, topology, branch, method_mask, enumerate_source_types))) return hr; - if (FAILED(IMFTopologyNode_GetUINT32(branch->up.node, &MF_TOPONODE_CONNECT_METHOD, &method))) + if (context.up_node_type != MF_TOPOLOGY_SOURCESTREAM_NODE || !enumerate_source_types + || FAILED(IMFTopologyNode_GetUINT32(branch->up.node, &MF_TOPONODE_CONNECT_METHOD, &method))) method = MF_CONNECT_DIRECT; if (enumerate_source_types) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10009
Shortened it so it's mostly refactoring. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10009#note_131920
Rémi Bernon (@rbernon) commented about dlls/mf/main.c:
+ 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_subtype(IMFMediaType *media_type) Why subtype? What about simply `debugstr_media_type` instead?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10009#note_133374
Rémi Bernon (@rbernon) commented about dlls/mf/topology_loader.c:
+ { + hr = IMFMediaTypeHandler_GetCurrentMediaType(context->up_types.handler, &context->up_types.current); + if (context->up_node_type != MF_TOPOLOGY_SOURCESTREAM_NODE) + context->up_types.enumerate = FAILED(hr); + } + + if (FAILED(hr = IMFMediaTypeHandler_GetMediaTypeByIndex(context->up_types.handler, 0, &up_type))) + goto failed; + context->up_types.index0 = up_type; + + return S_OK; + +failed: + connection_context_destroy(context); + return hr; +} Fwiw I'm still reviewing the code, but I'm not completely sure to see how this context makes things simpler. It seems to me that it duplicates information that is already available otherwise. For instance up_node_type is already available from `IMFTopologyNode_GetNodeType(branch->up.node, &context->up_node_type)`, same thing for `up_types.index0`. It also adds an extra layer of indirection with more state to keep in mind when reading each function.
If we want to simplify some logic we could use extra helpers, but adding one stateful context makes it more difficult to understand the logic dependency between the various connection steps. For instance, whether down node type enumeration is dependent on up node type enumeration or not, becomes a bit blurry when they are kept together in this new context. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10009#note_133375
On Mon Mar 23 14:01:16 2026 +0000, Rémi Bernon wrote:
Fwiw I'm still reviewing the code, but I'm not completely sure to see how this context makes things simpler. It seems to me that it duplicates information that is already available otherwise. For instance up_node_type is already available from `IMFTopologyNode_GetNodeType(branch->up.node, &context->up_node_type)`, same thing for `up_types.index0`. It also adds an extra layer of indirection with more state to keep in mind when reading each function. If we want to simplify some logic we could use extra helpers, but adding one stateful context makes it more difficult to understand the logic dependency between the various connection steps. For instance, whether down node type enumeration is dependent on up node type enumeration or not, becomes a bit blurry when they are kept together in this new context. We can probably do without the context, but it will be very useful to have a stateful enumerator later. Where both up and down nodes are enumerated for connection, the current type is a complication, and we end up with multiple possibilities:
current up + current down current up + enumerated down enumerated up + current down enumerated up + enumerated down An enumerator which handles this internally allows us to use a nested loop where the connection logic appears once. That's not apparent in this MR though. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10009#note_133502
On Tue Mar 24 06:23:03 2026 +0000, Conor McCarthy wrote:
We can probably do without the context, but it will be very useful to have a stateful enumerator later. Where both up and down nodes are enumerated for connection, the current type is a complication, and we end up with multiple possibilities: current up + current down current up + enumerated down enumerated up + current down enumerated up + enumerated down An enumerator which handles this internally allows us to use a nested loop where the connection logic appears once. That's not apparent in this MR though. 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). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10009#note_133542
participants (4)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Rémi Bernon
-
Rémi Bernon (＠rbernon)