[PATCH 0/1] MR608: mf: Only query downstream current media type for indirect connection.

[Rémi Bernon]

Some transforms, such as the EVR mixer do not implement GetInputAvailableType
and may not have any current media type set, though they would accept upstream
media type right away.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/608

[PATCH 1/1] mf: Only query downstream current media type for indirect connection.

[Rémi Bernon]

From: R��mi Bernon <rbernon(a)codeweavers.com>

Some transforms, such as the EVR mixer do not implement GetInputAvailableType
and may not have any current media type set, though they would accept upstream
media type right away.
---
 dlls/mf/topology_loader.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/dlls/mf/topology_loader.c b/dlls/mf/topology_loader.c
index 08ee7922094..2c8fd10366c 100644
--- a/dlls/mf/topology_loader.c
+++ b/dlls/mf/topology_loader.c
@@ -342,29 +342,35 @@ 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;
 
-    if (SUCCEEDED(hr = IMFMediaTypeHandler_GetCurrentMediaType(down_handler, &down_type))
-            && IMFMediaType_IsEqual(up_type, down_type, &flags) == S_OK)
+    if (SUCCEEDED(hr = IMFMediaTypeHandler_GetCurrentMediaType(down_handler, &down_type)))
+    {
+        hr = IMFMediaType_IsEqual(up_type, down_type, &flags);
+        IMFMediaType_Release(down_type);
+    }
+
+    if (hr == S_OK)
     {
         TRACE("Connecting branch %s with current type %p.\n", debugstr_topology_branch(branch), up_type);
         IMFMediaTypeHandler_Release(down_handler);
-        IMFMediaType_Release(down_type);
 
         return IMFTopologyNode_ConnectOutput(branch->up.node, branch->up.stream, branch->down.node, branch->down.stream);
     }
 
-    if (FAILED(hr) && FAILED(hr = IMFMediaTypeHandler_GetMediaTypeByIndex(down_handler, 0, &down_type)))
-        return hr;
-
     if (SUCCEEDED(hr = IMFMediaTypeHandler_IsMediaTypeSupported(down_handler, up_type, NULL))
             && SUCCEEDED(hr = IMFMediaTypeHandler_SetCurrentMediaType(down_handler, up_type)))
     {
         TRACE("Connected branch %s with upstream type %p.\n", debugstr_topology_branch(branch), up_type);
         IMFMediaTypeHandler_Release(down_handler);
-        IMFMediaType_Release(down_type);
 
         return IMFTopologyNode_ConnectOutput(branch->up.node, branch->up.stream, branch->down.node, branch->down.stream);
     }
 
+    if (FAILED(IMFMediaTypeHandler_GetCurrentMediaType(down_handler, &down_type))
+            && FAILED(IMFMediaTypeHandler_GetMediaTypeByIndex(down_handler, 0, &down_type)))
+    {
+        IMFMediaTypeHandler_Release(down_handler);
+        return MF_E_NO_MORE_TYPES;
+    }
     IMFMediaTypeHandler_Release(down_handler);
 
     if (FAILED(hr) && (method & method_mask & MF_CONNECT_ALLOW_CONVERTER) == MF_CONNECT_ALLOW_CONVERTER)
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/608

[Nikolay Sivov (＠nsivov)]

Is it possible to have a test for this? Specifically with EVR, if that's how you found this problem.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/608#note_5907

[Rémi Bernon]

On Tue Aug  9 17:22:40 2022 +0000, Nikolay Sivov wrote:
> Is it possible to have a test for this? Specifically with EVR, if that's
> how you found this problem.
> I'm not sure to understand what you would like to test exactly? There are tests already for the mixer in `test_default_mixer_type_negotiation`, which confirm that `GetInputAvailableType` returns `E_NOTIMPL`.

This change only moves the call to `GetInputAvailableType` to be done only when needed, while it was done early just to make `down_type` release a bit easier, but it's causing an early failure that has no reason to be.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/608#note_5935

[Nikolay Sivov (＠nsivov)]

On Tue Aug  9 17:22:40 2022 +0000, R��mi Bernon wrote:
> I'm not sure to understand what you would like to test exactly? There
> are tests already for the mixer in
> `test_default_mixer_type_negotiation`, which confirm that
> `GetInputAvailableType` returns `E_NOTIMPL`.
> This change only moves the call to `GetInputAvailableType` to be done
> only when needed, while it was done early just to make `down_type`
> release a bit easier, but it's causing an early failure that has no
> reason to be.
> Ok, so it's only swapping the order. Description made me think it was fixing something that didn't work before.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/608#note_5939

[Rémi Bernon]

On Tue Aug  9 18:25:03 2022 +0000, Nikolay Sivov wrote:
> Ok, so it's only swapping the order. Description made me think it was
> fixing something that didn't work before.
> I'll add some tests, I think there's some interesting additional fixes to be made.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/608#note_6082