[PATCH v2 0/6] MR1262: mf: Set source and output nodes media types in the media session.
Supersedes: https://gitlab.winehq.org/wine/wine/-/merge_requests/789 This MR, with patches from Rémi should fix the no sound issue in media engine and also the black screen issue with MfPlayer.exe. -- v2: mf: Set media types for output nodes in the media session. mf: Move D3D awareness logic to branch resolver. mf/tests: Test for copier node in topology using evr. mf: Add some topology source node checks in IMFMediaSession_SetTopology. mf: Always enumerate branch source types for transform nodes. winegstreamer: Support MFT_SET_TYPE_TEST_ONLY flag in the MF transforms. https://gitlab.winehq.org/wine/wine/-/merge_requests/1262
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wma_decoder.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dlls/winegstreamer/wma_decoder.c b/dlls/winegstreamer/wma_decoder.c index 4a0d5f2592e..87793f9c29d 100644 --- a/dlls/winegstreamer/wma_decoder.c +++ b/dlls/winegstreamer/wma_decoder.c @@ -456,6 +456,11 @@ static HRESULT WINAPI transform_SetOutputType(IMFTransform *iface, DWORD id, IMF if (flags & MFT_SET_TYPE_TEST_ONLY) return S_OK; + if (FAILED(IMFMediaType_SetUINT32(decoder->input_type, &MF_MT_AUDIO_BITS_PER_SAMPLE, sample_size))) + return MF_E_INVALIDMEDIATYPE; + if (flags & MFT_SET_TYPE_TEST_ONLY) + return S_OK; + if (FAILED(IMFMediaType_SetUINT32(decoder->input_type, &MF_MT_AUDIO_BITS_PER_SAMPLE, sample_size))) return MF_E_INVALIDMEDIATYPE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1262
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/mf/tests/mf.c | 6 +++--- dlls/mf/topology_loader.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index c9f8fe96f8b..004f7b50f59 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -2986,13 +2986,13 @@ static void test_topology_loader(void) /* Float -> PCM, refuse input type, add converter */ .input_type = &audio_float_44100, .output_type = &audio_pcm_48000, .sink_method = MF_CONNECT_DIRECT, .source_method = -1, .expected_result = MF_E_NO_MORE_TYPES, - .flags = LOADER_SET_INVALID_INPUT | LOADER_ADD_RESAMPLER_MFT | LOADER_EXPECTED_CONVERTER | LOADER_TODO, + .flags = LOADER_SET_INVALID_INPUT | LOADER_ADD_RESAMPLER_MFT | LOADER_EXPECTED_CONVERTER, }, { /* Float -> PCM, refuse input type, add converter, allow resampler output type */ .input_type = &audio_float_44100, .output_type = &audio_pcm_48000_resampler, .sink_method = MF_CONNECT_DIRECT, .source_method = -1, .expected_result = S_OK, - .flags = LOADER_SET_INVALID_INPUT | LOADER_ADD_RESAMPLER_MFT | LOADER_EXPECTED_CONVERTER | LOADER_TODO, + .flags = LOADER_SET_INVALID_INPUT | LOADER_ADD_RESAMPLER_MFT | LOADER_EXPECTED_CONVERTER, }, { @@ -3449,7 +3449,7 @@ todo_wine { else ok(!handler.enum_count, "got %lu GetMediaTypeByIndex\n", handler.enum_count); - todo_wine_if((test->flags & LOADER_NO_CURRENT_OUTPUT) && !(test->flags & LOADER_SET_MEDIA_TYPES)) + todo_wine_if(test->flags & LOADER_NO_CURRENT_OUTPUT) ok(!handler.set_current_count, "got %lu SetCurrentMediaType\n", handler.set_current_count); if (handler.current_type) diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index ab694379237..0c42a93511b 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -498,7 +498,7 @@ static HRESULT topology_loader_resolve_branches(struct topoloader_context *conte WARN("Failed to clone nodes for branch %s\n", debugstr_topology_branch(branch)); else hr = topology_branch_connect(context->output_topology, MF_CONNECT_ALLOW_DECODER, - branch, enumerate_source_types); + branch, enumerate_source_types || node_type == MF_TOPOLOGY_TRANSFORM_NODE); topology_branch_destroy(branch); if (FAILED(hr)) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1262
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/mf/session.c | 126 ++++++++++++++++++++++++++++++++++++++++++--- dlls/mf/tests/mf.c | 17 ------ 2 files changed, 120 insertions(+), 23 deletions(-) diff --git a/dlls/mf/session.c b/dlls/mf/session.c index d5ff3384299..a20d411f17a 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -1939,20 +1939,134 @@ static HRESULT WINAPI mfsession_QueueEvent(IMFMediaSession *iface, MediaEventTyp return IMFMediaEventQueue_QueueEventParamVar(session->event_queue, event_type, ext_type, hr, value); } +static HRESULT session_check_stream_descriptor(IMFPresentationDescriptor *pd, IMFStreamDescriptor *sd) +{ + IMFStreamDescriptor *selected_sd; + DWORD i, count; + BOOL selected; + HRESULT hr; + + if (FAILED(hr = IMFPresentationDescriptor_GetStreamDescriptorCount(pd, &count))) + { + WARN("Failed to get stream descriptor count, hr %#lx.\n", hr); + return MF_E_TOPO_STREAM_DESCRIPTOR_NOT_SELECTED; + } + + for (i = 0; i < count; ++i) + { + if (FAILED(hr = IMFPresentationDescriptor_GetStreamDescriptorByIndex(pd, i, + &selected, &selected_sd))) + { + WARN("Failed to get stream descriptor %lu, hr %#lx.\n", i, hr); + return MF_E_TOPO_STREAM_DESCRIPTOR_NOT_SELECTED; + } + IMFStreamDescriptor_Release(selected_sd); + + if (selected_sd == sd) + { + if (selected) + return S_OK; + + WARN("Presentation descriptor %p stream %p is not selected.\n", pd, sd); + return MF_E_TOPO_STREAM_DESCRIPTOR_NOT_SELECTED; + } + } + + WARN("Failed to find stream descriptor %lu, hr %#lx.\n", i, hr); + return MF_E_TOPO_STREAM_DESCRIPTOR_NOT_SELECTED; +} + +static HRESULT session_check_topology(IMFTopology *topology) +{ + MF_TOPOLOGY_TYPE node_type; + IMFTopologyNode *node; + WORD node_count, i; + HRESULT hr; + + if (!topology) + return S_OK; + + if (FAILED(IMFTopology_GetNodeCount(topology, &node_count)) + || node_count == 0) + return E_INVALIDARG; + + for (i = 0; i < node_count; ++i) + { + if (FAILED(hr = IMFTopology_GetNode(topology, i, &node))) + break; + + if (FAILED(hr = IMFTopologyNode_GetNodeType(node, &node_type))) + { + IMFTopologyNode_Release(node); + break; + } + + switch (node_type) + { + case MF_TOPOLOGY_SOURCESTREAM_NODE: + { + IMFPresentationDescriptor *pd; + IMFStreamDescriptor *sd; + IMFMediaSource *source; + + if (FAILED(hr = IMFTopologyNode_GetUnknown(node, &MF_TOPONODE_SOURCE, &IID_IMFMediaSource, + (void **)&source))) + { + WARN("Missing MF_TOPONODE_SOURCE, hr %#lx.\n", hr); + IMFTopologyNode_Release(node); + return MF_E_TOPO_MISSING_SOURCE; + } + IMFMediaSource_Release(source); + + if (FAILED(hr = IMFTopologyNode_GetUnknown(node, &MF_TOPONODE_PRESENTATION_DESCRIPTOR, + &IID_IMFPresentationDescriptor, (void **)&pd))) + { + WARN("Missing MF_TOPONODE_PRESENTATION_DESCRIPTOR, hr %#lx.\n", hr); + IMFTopologyNode_Release(node); + return MF_E_TOPO_MISSING_PRESENTATION_DESCRIPTOR; + } + + if (FAILED(hr = IMFTopologyNode_GetUnknown(node, &MF_TOPONODE_STREAM_DESCRIPTOR, + &IID_IMFStreamDescriptor, (void **)&sd))) + { + WARN("Missing MF_TOPONODE_STREAM_DESCRIPTOR, hr %#lx.\n", hr); + IMFPresentationDescriptor_Release(pd); + IMFTopologyNode_Release(node); + return MF_E_TOPO_MISSING_STREAM_DESCRIPTOR; + } + + hr = session_check_stream_descriptor(pd, sd); + IMFPresentationDescriptor_Release(pd); + IMFStreamDescriptor_Release(sd); + if (FAILED(hr)) + { + IMFTopologyNode_Release(node); + return hr; + } + + break; + } + + default: + break; + } + + IMFTopologyNode_Release(node); + } + + return hr; +} + static HRESULT WINAPI mfsession_SetTopology(IMFMediaSession *iface, DWORD flags, IMFTopology *topology) { struct media_session *session = impl_from_IMFMediaSession(iface); struct session_op *op; - WORD node_count = 0; HRESULT hr; TRACE("%p, %#lx, %p.\n", iface, flags, topology); - if (topology) - { - if (FAILED(IMFTopology_GetNodeCount(topology, &node_count)) || node_count == 0) - return E_INVALIDARG; - } + if (FAILED(hr = session_check_topology(topology))) + return hr; if (FAILED(hr = create_session_op(SESSION_CMD_SET_TOPOLOGY, &op))) return hr; diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 004f7b50f59..ba4afddc7d2 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -2145,29 +2145,13 @@ static void test_media_session_events(void) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IMFMediaSession_SetTopology(session, 0, topology); - todo_wine ok(hr == MF_E_TOPO_MISSING_SOURCE, "Unexpected hr %#lx.\n", hr); - if (hr == S_OK) - { - hr = wait_media_event(session, callback, MESessionTopologySet, 1000, &propvar); - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - PropVariantClear(&propvar); - handler.enum_count = handler.set_current_count = 0; - } source = create_test_source(pd); init_source_node(source, -1, src_node, pd, sd); hr = IMFMediaSession_SetTopology(session, 0, topology); - todo_wine ok(hr == MF_E_TOPO_STREAM_DESCRIPTOR_NOT_SELECTED, "Unexpected hr %#lx.\n", hr); - if (hr == S_OK) - { - hr = wait_media_event(session, callback, MESessionTopologySet, 1000, &propvar); - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - PropVariantClear(&propvar); - handler.enum_count = handler.set_current_count = 0; - } hr = IMFMediaSession_ClearTopologies(session); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); @@ -2177,7 +2161,6 @@ static void test_media_session_events(void) hr = IMFMediaSession_Shutdown(session); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine ok(!media_sink.shutdown, "media sink is shutdown.\n"); media_sink.shutdown = FALSE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1262
From: Bernhard Kölbl <besentv(a)gmail.com> --- dlls/mf/tests/mf.c | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index ba4afddc7d2..2bbbe9a3762 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -3581,19 +3581,51 @@ static void test_topology_loader_evr(void) hr = IMFTopologyNode_GetNodeType(node, &node_type); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - if (node_type == MF_TOPOLOGY_OUTPUT_NODE) + switch (node_type) + { + case MF_TOPOLOGY_OUTPUT_NODE: { value = 1; hr = IMFTopologyNode_GetUINT32(node, &MF_TOPONODE_STREAMID, &value); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok(!value, "Unexpected stream id %u.\n", value); + break; + } + case MF_TOPOLOGY_TRANSFORM_NODE: + { + IMFAttributes *attrs; + IMFTransform *copier; + IUnknown *obj; + + hr = IMFTopologyNode_GetObject(node, (IUnknown **)&obj); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IUnknown_QueryInterface(obj, &IID_IMFTransform, (void **)&copier); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IMFTransform_GetAttributes(copier, &attrs); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + value = 0xdeadbeef; + hr = IMFAttributes_GetUINT32(attrs, &MFT_SUPPORT_DYNAMIC_FORMAT_CHANGE, &value); + ok(value == 1, "Unexpected dynamic state support state %u.\n", value); + + IMFAttributes_Release(attrs); + IMFTransform_Release(copier); + IUnknown_Release(obj); + break; } - else if (node_type == MF_TOPOLOGY_SOURCESTREAM_NODE) + case MF_TOPOLOGY_SOURCESTREAM_NODE: { value64 = 1; hr = IMFTopologyNode_GetUINT64(node, &MF_TOPONODE_MEDIASTART, &value64); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok(!value64, "Unexpected value.\n"); + break; + } + default: + ok(0, "Got unexpected node type %u.\n", node_type); + break; } IMFTopologyNode_Release(node); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1262
From: Bernhard Kölbl <besentv(a)gmail.com> To make it independent from set types on node endpoints. --- dlls/mf/topology_loader.c | 211 +++++++++++++++++--------------------- 1 file changed, 93 insertions(+), 118 deletions(-) diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index 0c42a93511b..b64e7ff99f2 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -359,12 +359,84 @@ static HRESULT topology_branch_get_current_type(IMFMediaTypeHandler *handler, IM return hr; } +static BOOL topology_loader_is_node_d3d_aware(IMFTopologyNode *node) +{ + IMFAttributes *attributes; + unsigned int d3d_aware = 0; + IMFTransform *transform; + + if (FAILED(topology_node_get_object(node, &IID_IMFAttributes, (void **)&attributes))) + return FALSE; + + IMFAttributes_GetUINT32(attributes, &MF_SA_D3D_AWARE, &d3d_aware); + IMFAttributes_Release(attributes); + + if (!d3d_aware && SUCCEEDED(topology_node_get_object(node, &IID_IMFTransform, (void **)&transform))) + { + d3d_aware = mf_is_sample_copier_transform(transform); + IMFTransform_Release(transform); + } + + return !!d3d_aware; +} + +static HRESULT topology_loader_create_copier(IMFMediaType *upstream_type, IMFMediaType *downstream_type, IMFTransform **copier) +{ + IMFTransform *transform; + HRESULT hr; + + if (FAILED(hr = MFCreateSampleCopierMFT(&transform))) + return hr; + + if (SUCCEEDED(hr) && FAILED(hr = IMFTransform_SetInputType(transform, 0, upstream_type, 0))) + WARN("Input type wasn't accepted, hr %#lx.\n", hr); + + if (SUCCEEDED(hr) && FAILED(hr = IMFTransform_SetOutputType(transform, 0, downstream_type, 0))) + WARN("Output type wasn't accepted, hr %#lx.\n", hr); + + if (SUCCEEDED(hr)) + { + *copier = transform; + IMFTransform_AddRef(*copier); + } + + IMFTransform_Release(transform); + + return hr; +} + +static HRESULT topology_branch_insert_copier(IMFTopology *topology, struct topology_branch *branch, + IMFMediaType *upstream_type, IMFMediaType *downstream_type) +{ + IMFTopologyNode *copier_node; + IMFTransform *copier; + HRESULT hr; + + if (FAILED(hr = topology_loader_create_copier(upstream_type, downstream_type, &copier))) + return hr; + + if (FAILED(hr = MFCreateTopologyNode(MF_TOPOLOGY_TRANSFORM_NODE, &copier_node))) + return hr; + + IMFTopologyNode_SetObject(copier_node, (IUnknown *)copier); + IMFTopology_AddNode(topology, copier_node); + IMFTopologyNode_ConnectOutput(branch->up.node, branch->up.stream, copier_node, 0); + IMFTopologyNode_ConnectOutput(copier_node, 0, branch->down.node, branch->down.stream); + + IMFTopologyNode_Release(copier_node); + + return S_OK; +} + static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_METHOD method_mask, struct topology_branch *branch, IMFMediaType *up_type) { IMFMediaTypeHandler *down_handler; IMFMediaType *down_type = NULL; + MFT_REGISTER_TYPE_INFO input_info; MF_CONNECT_METHOD method; + MF_TOPOLOGY_TYPE type; + BOOL need_copier = FALSE; DWORD flags; HRESULT hr; @@ -377,11 +449,25 @@ static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_ME if (FAILED(hr = topology_node_get_type_handler(branch->down.node, branch->down.stream, FALSE, &down_handler))) return hr; + /* Insert copier only for video streams. */ + if(SUCCEEDED(IMFTopologyNode_GetNodeType(branch->down.node, &type)) && type == MF_TOPOLOGY_OUTPUT_NODE + && SUCCEEDED(IMFMediaType_GetMajorType(up_type, &input_info.guidMajorType)) + && IsEqualGUID(&input_info.guidMajorType, &MFMediaType_Video)) + { + need_copier = (!topology_loader_is_node_d3d_aware(branch->up.node) + && topology_loader_is_node_d3d_aware(branch->down.node)); + } + if (SUCCEEDED(hr = topology_branch_get_current_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); - hr = IMFTopologyNode_ConnectOutput(branch->up.node, branch->up.stream, branch->down.node, branch->down.stream); + + if (need_copier) + hr = topology_branch_insert_copier(topology, branch, up_type, up_type); /* Up and downsteam type should be same here. */ + else + hr = IMFTopologyNode_ConnectOutput(branch->up.node, branch->up.stream, branch->down.node, branch->down.stream); + goto done; } @@ -389,7 +475,12 @@ static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_ME && SUCCEEDED(hr = IMFMediaTypeHandler_SetCurrentMediaType(down_handler, up_type))) { TRACE("Connected branch %s with upstream type %p.\n", debugstr_topology_branch(branch), up_type); - hr = IMFTopologyNode_ConnectOutput(branch->up.node, branch->up.stream, branch->down.node, branch->down.stream); + + if (need_copier) + hr = topology_branch_insert_copier(topology, branch, up_type, up_type); /* Up and downsteam type should be same here. */ + else + hr = IMFTopologyNode_ConnectOutput(branch->up.node, branch->up.stream, branch->down.node, branch->down.stream); + goto done; } @@ -509,124 +600,11 @@ static HRESULT topology_loader_resolve_branches(struct topoloader_context *conte return hr; } -static BOOL topology_loader_is_node_d3d_aware(IMFTopologyNode *node) -{ - IMFAttributes *attributes; - unsigned int d3d_aware = 0; - IMFTransform *transform; - - if (FAILED(topology_node_get_object(node, &IID_IMFAttributes, (void **)&attributes))) - return FALSE; - - IMFAttributes_GetUINT32(attributes, &MF_SA_D3D_AWARE, &d3d_aware); - IMFAttributes_Release(attributes); - - if (!d3d_aware && SUCCEEDED(topology_node_get_object(node, &IID_IMFTransform, (void **)&transform))) - { - d3d_aware = mf_is_sample_copier_transform(transform); - IMFTransform_Release(transform); - } - - return !!d3d_aware; -} - -static HRESULT topology_loader_create_copier(IMFTopologyNode *upstream_node, DWORD upstream_output, - IMFTopologyNode *downstream_node, unsigned int downstream_input, IMFTransform **copier) -{ - IMFMediaType *input_type = NULL, *output_type = NULL; - IMFTransform *transform; - HRESULT hr; - - if (FAILED(hr = MFCreateSampleCopierMFT(&transform))) - return hr; - - if (FAILED(hr = MFGetTopoNodeCurrentType(upstream_node, upstream_output, TRUE, &input_type))) - WARN("Failed to get upstream media type hr %#lx.\n", hr); - - if (SUCCEEDED(hr) && FAILED(hr = MFGetTopoNodeCurrentType(downstream_node, downstream_input, FALSE, &output_type))) - WARN("Failed to get downstream media type hr %#lx.\n", hr); - - if (SUCCEEDED(hr) && FAILED(hr = IMFTransform_SetInputType(transform, 0, input_type, 0))) - WARN("Input type wasn't accepted, hr %#lx.\n", hr); - - if (SUCCEEDED(hr) && FAILED(hr = IMFTransform_SetOutputType(transform, 0, output_type, 0))) - WARN("Output type wasn't accepted, hr %#lx.\n", hr); - - if (SUCCEEDED(hr)) - { - *copier = transform; - IMFTransform_AddRef(*copier); - } - - if (input_type) - IMFMediaType_Release(input_type); - if (output_type) - IMFMediaType_Release(output_type); - - IMFTransform_Release(transform); - - return hr; -} - -static HRESULT topology_loader_connect_copier(struct topoloader_context *context, IMFTopologyNode *upstream_node, - DWORD upstream_output, IMFTopologyNode *downstream_node, DWORD downstream_input, IMFTransform *copier) -{ - IMFTopologyNode *copier_node; - HRESULT hr; - - if (FAILED(hr = MFCreateTopologyNode(MF_TOPOLOGY_TRANSFORM_NODE, &copier_node))) - return hr; - - IMFTopologyNode_SetObject(copier_node, (IUnknown *)copier); - IMFTopology_AddNode(context->output_topology, copier_node); - IMFTopologyNode_ConnectOutput(upstream_node, upstream_output, copier_node, 0); - IMFTopologyNode_ConnectOutput(copier_node, 0, downstream_node, downstream_input); - - IMFTopologyNode_Release(copier_node); - - return S_OK; -} - -/* Right now this should be used for output nodes only. */ -static HRESULT topology_loader_connect_d3d_aware_input(struct topoloader_context *context, - IMFTopologyNode *node) -{ - IMFTopologyNode *upstream_node; - IMFTransform *copier = NULL; - IMFStreamSink *stream_sink; - DWORD upstream_output; - HRESULT hr; - - if (FAILED(hr = topology_node_get_object(node, &IID_IMFStreamSink, (void **)&stream_sink))) - return hr; - - if (topology_loader_is_node_d3d_aware(node)) - { - if (SUCCEEDED(IMFTopologyNode_GetInput(node, 0, &upstream_node, &upstream_output))) - { - if (!topology_loader_is_node_d3d_aware(upstream_node)) - { - if (SUCCEEDED(hr = topology_loader_create_copier(upstream_node, upstream_output, node, 0, &copier))) - { - hr = topology_loader_connect_copier(context, upstream_node, upstream_output, node, 0, copier); - IMFTransform_Release(copier); - } - } - IMFTopologyNode_Release(upstream_node); - } - } - - IMFStreamSink_Release(stream_sink); - - return hr; -} - static void topology_loader_resolve_complete(struct topoloader_context *context) { MF_TOPOLOGY_TYPE node_type; IMFTopologyNode *node; WORD i, node_count; - HRESULT hr; IMFTopology_GetNodeCount(context->output_topology, &node_count); @@ -641,9 +619,6 @@ static void topology_loader_resolve_complete(struct topoloader_context *context) /* Set MF_TOPONODE_STREAMID for all outputs. */ if (FAILED(IMFTopologyNode_GetItem(node, &MF_TOPONODE_STREAMID, NULL))) IMFTopologyNode_SetUINT32(node, &MF_TOPONODE_STREAMID, 0); - - if (FAILED(hr = topology_loader_connect_d3d_aware_input(context, node))) - WARN("Failed to connect D3D-aware input, hr %#lx.\n", hr); } else if (node_type == MF_TOPOLOGY_SOURCESTREAM_NODE) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1262
From: Rémi Bernon <rbernon(a)codeweavers.com> Instead of the topology loader. --- dlls/mf/mf_private.h | 1 + dlls/mf/session.c | 60 +++++++++++++++++++++++++++++++++++++++ dlls/mf/tests/mf.c | 4 --- dlls/mf/topology_loader.c | 28 +++++++++++++++--- 4 files changed, 85 insertions(+), 8 deletions(-) diff --git a/dlls/mf/mf_private.h b/dlls/mf/mf_private.h index bd1d6c7537b..69923154b79 100644 --- a/dlls/mf/mf_private.h +++ b/dlls/mf/mf_private.h @@ -117,3 +117,4 @@ extern BOOL mf_is_sample_copier_transform(IMFTransform *transform) DECLSPEC_HIDD extern BOOL mf_is_sar_sink(IMFMediaSink *sink) DECLSPEC_HIDDEN; extern HRESULT topology_node_get_object(IMFTopologyNode *node, REFIID riid, void **obj) DECLSPEC_HIDDEN; extern HRESULT topology_node_get_type_handler(IMFTopologyNode *node, DWORD stream, BOOL output, IMFMediaTypeHandler **handler) DECLSPEC_HIDDEN; +extern HRESULT topology_node_init_media_type(IMFTopologyNode *node, DWORD stream, BOOL output, IMFMediaType **type) DECLSPEC_HIDDEN; diff --git a/dlls/mf/session.c b/dlls/mf/session.c index a20d411f17a..4420910d4a0 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -596,6 +596,64 @@ static HRESULT session_bind_output_nodes(IMFTopology *topology) return hr; } +static HRESULT session_init_media_types(IMFTopology *topology) +{ + MF_TOPOLOGY_TYPE node_type; + WORD node_count, i, j; + IMFTopologyNode *node; + IMFMediaType *type; + DWORD input_count; + HRESULT hr; + + if (FAILED(hr = IMFTopology_GetNodeCount(topology, &node_count))) + return hr; + + for (i = 0; i < node_count; ++i) + { + if (FAILED(hr = IMFTopology_GetNode(topology, i, &node))) + break; + + if (FAILED(hr = IMFTopologyNode_GetNodeType(node, &node_type)) + || node_type != MF_TOPOLOGY_OUTPUT_NODE) + { + IMFTopologyNode_Release(node); + continue; + } + if (FAILED(hr = IMFTopologyNode_GetInputCount(node, &input_count))) + { + IMFTopologyNode_Release(node); + continue; + } + + for (j = 0; j < input_count; ++j) + { + IMFMediaTypeHandler *handler; + IMFTopologyNode *up_node; + DWORD input; + + if (SUCCEEDED(hr = IMFTopologyNode_GetInput(node, j, &up_node, &input))) + { + hr = topology_node_init_media_type(up_node, input, TRUE, &type); + IMFTopologyNode_Release(up_node); + } + if (FAILED(hr)) + break; + + if (SUCCEEDED(hr = topology_node_get_type_handler(node, j, FALSE, &handler))) + { + hr = IMFMediaTypeHandler_SetCurrentMediaType(handler, type); + IMFMediaTypeHandler_Release(handler); + } + + IMFMediaType_Release(type); + } + + IMFTopologyNode_Release(node); + } + + return hr; +} + static void session_set_caps(struct media_session *session, DWORD caps) { DWORD delta = session->caps ^ caps; @@ -1768,6 +1826,8 @@ static void session_set_topology(struct media_session *session, DWORD flags, IMF if (SUCCEEDED(hr)) hr = IMFTopoLoader_Load(session->topo_loader, topology, &resolved_topology, NULL /* FIXME? */); + if (SUCCEEDED(hr)) + hr = session_init_media_types(resolved_topology); if (SUCCEEDED(hr)) { diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 2bbbe9a3762..4ed044c1ee4 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -2263,7 +2263,6 @@ static void test_media_session_events(void) PropVariantClear(&propvar); ok(handler.enum_count, "got %lu GetMediaTypeByIndex\n", handler.enum_count); - todo_wine ok(handler.set_current_count, "got %lu SetCurrentMediaType\n", handler.set_current_count); handler.enum_count = handler.set_current_count = 0; @@ -2342,7 +2341,6 @@ static void test_media_session_events(void) PropVariantClear(&propvar); ok(!handler.enum_count, "got %lu GetMediaTypeByIndex\n", handler.enum_count); - todo_wine ok(handler.set_current_count, "got %lu SetCurrentMediaType\n", handler.set_current_count); handler.enum_count = handler.set_current_count = 0; @@ -3431,8 +3429,6 @@ todo_wine { ok(handler.enum_count, "got %lu GetMediaTypeByIndex\n", handler.enum_count); else ok(!handler.enum_count, "got %lu GetMediaTypeByIndex\n", handler.enum_count); - - todo_wine_if(test->flags & LOADER_NO_CURRENT_OUTPUT) ok(!handler.set_current_count, "got %lu SetCurrentMediaType\n", handler.set_current_count); if (handler.current_type) diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c index b64e7ff99f2..87bc9795647 100644 --- a/dlls/mf/topology_loader.c +++ b/dlls/mf/topology_loader.c @@ -306,6 +306,8 @@ static HRESULT topology_branch_connect_indirect(IMFTopology *topology, MF_CONNEC IMFTopologyNode_SetGUID(node, &MF_TOPONODE_TRANSFORM_OBJECTID, &guid); hr = topology_branch_connect_down(topology, MF_CONNECT_DIRECT, &up_branch, up_type); + if (SUCCEEDED(hr)) + hr = IMFTransform_SetInputType(transform, 0, up_type, 0); if (down_type) { if (SUCCEEDED(hr)) @@ -335,7 +337,7 @@ static HRESULT topology_branch_connect_indirect(IMFTopology *topology, MF_CONNEC return hr; } -static HRESULT topology_branch_get_current_type(IMFMediaTypeHandler *handler, IMFMediaType **type) +static HRESULT get_first_supported_media_type(IMFMediaTypeHandler *handler, IMFMediaType **type) { IMFMediaType *media_type; HRESULT hr; @@ -428,6 +430,21 @@ static HRESULT topology_branch_insert_copier(IMFTopology *topology, struct topol return S_OK; } +HRESULT topology_node_init_media_type(IMFTopologyNode *node, DWORD stream, BOOL output, IMFMediaType **type) +{ + IMFMediaTypeHandler *handler; + HRESULT hr; + + if (SUCCEEDED(hr = topology_node_get_type_handler(node, stream, output, &handler))) + { + if (SUCCEEDED(hr = get_first_supported_media_type(handler, type))) + hr = IMFMediaTypeHandler_SetCurrentMediaType(handler, *type); + IMFMediaTypeHandler_Release(handler); + } + + return hr; +} + static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_METHOD method_mask, struct topology_branch *branch, IMFMediaType *up_type) { @@ -458,7 +475,7 @@ static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_ME && topology_loader_is_node_d3d_aware(branch->down.node)); } - if (SUCCEEDED(hr = topology_branch_get_current_type(down_handler, &down_type)) + 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); @@ -471,11 +488,14 @@ static HRESULT topology_branch_connect_down(IMFTopology *topology, MF_CONNECT_ME goto done; } - if (SUCCEEDED(hr = IMFMediaTypeHandler_IsMediaTypeSupported(down_handler, up_type, NULL)) - && SUCCEEDED(hr = IMFMediaTypeHandler_SetCurrentMediaType(down_handler, up_type))) + 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); + if (SUCCEEDED(IMFTopologyNode_GetNodeType(branch->down.node, &type)) && type == MF_TOPOLOGY_TRANSFORM_NODE + && FAILED(hr = IMFMediaTypeHandler_SetCurrentMediaType(down_handler, up_type))) + WARN("Failed to set transform node media type, hr %#lx\n", hr); + if (need_copier) hr = topology_branch_insert_copier(topology, branch, up_type, up_type); /* Up and downsteam type should be same here. */ else -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1262
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/wma_decoder.c:
if (flags & MFT_SET_TYPE_TEST_ONLY) return S_OK;
+ if (FAILED(IMFMediaType_SetUINT32(decoder->input_type, &MF_MT_AUDIO_BITS_PER_SAMPLE, sample_size))) + return MF_E_INVALIDMEDIATYPE; + if (flags & MFT_SET_TYPE_TEST_ONLY) + return S_OK; +
This is already upstream, you can drop that commit. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1262#note_13327
Rémi Bernon (@rbernon) commented about dlls/mf/topology_loader.c:
return hr; }
+static BOOL topology_loader_is_node_d3d_aware(IMFTopologyNode *node)
Splitting these changes into a commit that only moves functions around and another one that changes the logic would make it more obvious what is changed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1262#note_13328
Rémi Bernon (@rbernon) commented about dlls/mf/topology_loader.c:
- - if (FAILED(hr = MFCreateTopologyNode(MF_TOPOLOGY_TRANSFORM_NODE, &copier_node))) - return hr; - - IMFTopologyNode_SetObject(copier_node, (IUnknown *)copier); - IMFTopology_AddNode(context->output_topology, copier_node); - IMFTopologyNode_ConnectOutput(upstream_node, upstream_output, copier_node, 0); - IMFTopologyNode_ConnectOutput(copier_node, 0, downstream_node, downstream_input); - - IMFTopologyNode_Release(copier_node); - - return S_OK; -} - -/* Right now this should be used for output nodes only. */ -static HRESULT topology_loader_connect_d3d_aware_input(struct topoloader_context *context, It's not completely obvious to me that moving the copier creation to the branch resolver is:
1) The right thing to do, 2) Is actually what's fixing the issue. I think the issue is fixed because you're now creating the copier with the same input and output types, using the upstream node output type, instead of querying the, possibly not yet initialized, downstream EVR node media type. I'd believe that a simpler change that only modifies the copier creation logic to ignore downstream type would fix it as well. It would be nice to add some more tests, I think to `test_topology_loader` with custom D3D aware nodes to try to figure the answers to the following questions: 1) Are copiers really added dynamically to any branch? (Like if you have `non-d3d source -> d3d mft -> non-d3d output`, do you get two copiers? 2) Is the copier output type really initialized to the upstream node output type? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1262#note_13329
Rémi Bernon (@rbernon) commented about dlls/mf/session.c:
+ for (i = 0; i < node_count; ++i) + { + if (FAILED(hr = IMFTopology_GetNode(topology, i, &node))) + break; + + if (FAILED(hr = IMFTopologyNode_GetNodeType(node, &node_type)) + || node_type != MF_TOPOLOGY_OUTPUT_NODE) + { + IMFTopologyNode_Release(node); + continue; + } + if (FAILED(hr = IMFTopologyNode_GetInputCount(node, &input_count))) + { + IMFTopologyNode_Release(node); + continue; + } I didn't do it for dubious style reasons but I think we could merge these two, into something like:
```c if (FAILED(hr = IMFTopologyNode_GetInputCount(node, &input_count)) || FAILED(hr = IMFTopologyNode_GetNodeType(node, &node_type)) || node_type != MF_TOPOLOGY_OUTPUT_NODE) { IMFTopologyNode_Release(node); continue; } ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1262#note_13330
Rémi Bernon (@rbernon) commented about dlls/mf/session.c:
+ IMFTopologyNode_Release(node); + continue; + } + if (FAILED(hr = IMFTopologyNode_GetInputCount(node, &input_count))) + { + IMFTopologyNode_Release(node); + continue; + } + + for (j = 0; j < input_count; ++j) + { + IMFMediaTypeHandler *handler; + IMFTopologyNode *up_node; + DWORD input; + + if (SUCCEEDED(hr = IMFTopologyNode_GetInput(node, j, &up_node, &input))) I think this should be named `up_output` or `up_index` instead, it's not the input index of the current node, but rather the output index of the upstream node.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1262#note_13331
On Sun Nov 6 10:42:58 2022 +0000, Rémi Bernon wrote:
It's not completely obvious to me that moving the copier creation to the branch resolver is: 1) The right thing to do, 2) Is actually what's fixing the issue. I think the issue is fixed because you're now creating the copier with the same input and output types, using the upstream node output type, instead of querying the, possibly not yet initialized, downstream EVR node media type. I'd believe that a simpler change that only modifies the copier creation logic to ignore downstream type would fix it as well. It would be nice to add some more tests, I think to `test_topology_loader` with custom D3D aware nodes to try to figure the answers to the following questions: 1) Are copiers really added dynamically to any branch? (Like if you have `non-d3d source -> d3d mft -> non-d3d output`, do you get two copiers? 2) Is the copier output type really initialized to the upstream node output type? I agree that added complexity is not obviously helpful. If the real issue is that some types are not configured somewhere, we could fix just that. Regarding adding copiers at any suitable point, it's not hard to test manually I suppose, with topoedit. Some generic solution checking if 'upstream_d3d_aware != downstream_d3d_aware then add copier', but first we need this checked on Windows, and second we will not get in this situation in wine in practice.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1262#note_13334
I think the issue is fixed because you're now creating the copier with the same input and output types, using the upstream node output type, instead of querying the, possibly not yet initialized, downstream EVR node media type.
I am using the same type because, from my understanding, it's given that down and up type are compatible at this point and thus the same here. This means I can just provide the up type for both input and output, as the `branch_connect_down` function already tested for format compatibility between MFT/Source and EVR. I surely can test it, I played a bit with topoedit, but Windows doesn't seem to like adding the copier in my case, or they renamed it to "color converter". Also, I'm only adding the copier between the output node and anything upstream from that, not anywhere else. Afaiu in Wine all nodes, except EVR are not d3d aware and as such it only makes sense to insert copier between the last node and EVR. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1262#note_13341
On Sun Nov 6 10:42:57 2022 +0000, Rémi Bernon wrote:
Splitting these changes into a commit that only moves functions around and another one that changes the logic would make it more obvious what is changed. Should I push with the split patches for easier review or already look for another solution?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1262#note_13342
If the real issue is that some types are not configured somewhere, we could fix just that.
We purposely break media types not being set in the topo resolver for output nodes, which in turn breaks the copier inserter code, that's why I moved it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1262#note_13343
On Sun Nov 6 10:43:00 2022 +0000, Rémi Bernon wrote:
I think this should be named `up_output` or `up_index` instead, it's not the input index of the current node, but rather the output index of the upstream node. Yep, I wasn't sure how to name it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1262#note_13344
On Sun Nov 6 16:53:38 2022 +0000, Bernhard Kölbl wrote:
Should I push with the split patches for easier review or already look for another solution? I did it locally to check the differences, so that's more a guideline for future changes.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1262#note_13348
I am using the same type because, from my understanding, it's given that down and up type are compatible at this point and thus the same here. This means I can just provide the up type for both input and output, as the branch_connect_down function already tested for format compatibility between MFT/Source and EVR.
Sure and it's very possible that it's the right thing to do, but I don't think you need to move the copier creation for that, you could just do the same where it's currently created. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1262#note_13349
On Sun Nov 6 20:23:34 2022 +0000, Rémi Bernon wrote:
I am using the same type because, from my understanding, it's given that down and up type are compatible at this point and thus the same here. This means I can just provide the up type for both input and output, as the branch_connect_down function already tested for format compatibility between MFT/Source and EVR. Sure and it's very possible that it's the right thing to do, but I don't think you need to move the copier creation for that, you could just do the same where it's currently created. By assuming the up type fits the down type?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1262#note_13350
On Sun Nov 6 20:31:41 2022 +0000, Bernhard Kölbl wrote:
By assuming the up type fits the down type? Yes it should already have been checked by the branch resolver.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1262#note_13351
participants (3)
-
Bernhard Kölbl -
Nikolay Sivov (＠nsivov)
-
Rémi Bernon