[PATCH 0/3] MR3386: quartz, amstream: Three patches for amstream tests.

[Zebediah Figura (＠zfigura)]

The leak fixes seem to resolve bug 55283. It's very concerning that address
space pressure is already that high, but...

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

[PATCH 1/3] quartz: Call EnumMatchingFilters() once in autoplug().

[Zebediah Figura]

From: Zebediah Figura <zfigura(a)codeweavers.com>

Instead of iterating through types one by one and calling EnumMatchingFilters()
on each one, build a list of all media types exposed by the pin and call the
function once.

This avoids trying to pointless autoplug the same filter multiple times, which
speeds up autoplugging greatly for cases when a given filter connects less than
instantaneously. The most prominent example of such a filter is the AVI
decompressor, which has to call ICLocate() on connection. ICLocate() is not a
fast API on Wine, and it is even slower on Windows; this reduces the number of
times we try to call it greatly.

--

I noticed, while trying to debug the amstream tests, that they were extremely
slow on my machine, taking about 40 seconds to execute. This machine is not a
particularly powerful one, but even some testbot machines struggle with
amstream, taking 20 seconds to complete.

ICLocate() has to enumerate the registry *and* system.ini *and* load and query
every driver, which makes it kind of slow—about 15-20 ms on this machine
(assuming it fails). It's slower on Windows, though—a VM on the same machine was
consistently yielding twice the execution time. But amstream_test.exe wasn't
nearly as slow.

Autoplugging the decodebin parser to the AVI decompressor means trying to
connect up to 13 times, once for each type the decodebin parser is capable of.
With current autoplug(), we would try to autoplug the decodebin parser 13 times,
once for each type (and the AVI parser once). And, for the icing on the cake, we
would try to autoplug the decodebin parser twice, because the file source
exports *two* media types (the autodetected stream type, and GUID_NULL).
Combined all together, that's over 330 calls to ICLocate(), meaning that if
autoplugging fails, it would take over five seconds to do so.

This patch ends up cutting the execution time of amstream from 40 seconds down
to 17 seconds on my machine. Much of the rest seems to be overhead of wined3d
capability detection, which itself is about 200 ms per adapter here.
---
 dlls/quartz/filtergraph.c       | 189 ++++++++++++++++++++------------
 dlls/quartz/tests/filtergraph.c |   2 +-
 2 files changed, 122 insertions(+), 69 deletions(-)

diff --git a/dlls/quartz/filtergraph.c b/dlls/quartz/filtergraph.c
index 8c8f89ce2ac..e0e126d8ef0 100644
--- a/dlls/quartz/filtergraph.c
+++ b/dlls/quartz/filtergraph.c
@@ -1195,16 +1195,79 @@ static HRESULT autoplug_through_filter(struct filter_graph *graph, IPin *source,
     return VFW_E_CANNOT_CONNECT;
 }
 
+static HRESULT get_autoplug_types(IPin *source, unsigned int *ret_count, GUID **ret_types)
+{
+    unsigned int i, mt_count = 0, mt_capacity = 16;
+    AM_MEDIA_TYPE **mts = NULL;
+    IEnumMediaTypes *enummt;
+    GUID *types = NULL;
+    HRESULT hr;
+
+    if (FAILED(hr = IPin_EnumMediaTypes(source, &enummt)))
+    {
+        ERR("Failed to enumerate media types, hr %#lx.\n", hr);
+        return hr;
+    }
+
+    for (;;)
+    {
+        ULONG count;
+
+        if (!(mts = realloc(mts, mt_capacity * sizeof(*mts))))
+        {
+            hr = E_OUTOFMEMORY;
+            goto out;
+        }
+
+        if (FAILED(hr = IEnumMediaTypes_Next(enummt, mt_capacity - mt_count, mts + mt_count, &count)))
+        {
+            ERR("Failed to get media types, hr %#lx.\n", hr);
+            goto out;
+        }
+
+        mt_count += count;
+        if (hr == S_FALSE)
+            break;
+
+        mt_capacity *= 2;
+    }
+
+    if (!(types = malloc(mt_count * 2 * sizeof(*types))))
+    {
+        hr = E_OUTOFMEMORY;
+        goto out;
+    }
+
+    for (i = 0; i < mt_count; ++i)
+    {
+        types[i * 2] = mts[i]->majortype;
+        types[i * 2 + 1] = mts[i]->subtype;
+        DeleteMediaType(mts[i]);
+    }
+
+    *ret_count = mt_count;
+    *ret_types = types;
+
+    hr = S_OK;
+out:
+    free(mts);
+    IEnumMediaTypes_Release(enummt);
+    return hr;
+}
+
 /* Common helper for IGraphBuilder::Connect() and IGraphBuilder::Render(), which
  * share most of the same code. Render() calls this with a NULL sink. */
 static HRESULT autoplug(struct filter_graph *graph, IPin *source, IPin *sink,
         BOOL render_to_existing, unsigned int recursion_depth)
 {
     IAMGraphBuilderCallback *callback = NULL;
-    IEnumMediaTypes *enummt;
+    struct filter *graph_filter;
+    IEnumMoniker *enummoniker;
+    unsigned int type_count;
     IFilterMapper2 *mapper;
-    struct filter *filter;
-    AM_MEDIA_TYPE *mt;
+    IBaseFilter *filter;
+    GUID *types = NULL;
+    IMoniker *moniker;
     HRESULT hr;
 
     TRACE("Trying to autoplug %p to %p, recursion depth %u.\n", source, sink, recursion_depth);
@@ -1228,16 +1291,16 @@ static HRESULT autoplug(struct filter_graph *graph, IPin *source, IPin *sink,
     }
 
     /* Always prefer filters in the graph. */
-    LIST_FOR_EACH_ENTRY(filter, &graph->filters, struct filter, entry)
+    LIST_FOR_EACH_ENTRY(graph_filter, &graph->filters, struct filter, entry)
     {
-        if (SUCCEEDED(hr = autoplug_through_filter(graph, source, filter->filter,
+        if (SUCCEEDED(hr = autoplug_through_filter(graph, source, graph_filter->filter,
                 sink, render_to_existing, recursion_depth)))
             return hr;
     }
 
     IUnknown_QueryInterface(graph->punkFilterMapper2, &IID_IFilterMapper2, (void **)&mapper);
 
-    if (FAILED(hr = IPin_EnumMediaTypes(source, &enummt)))
+    if (FAILED(hr = get_autoplug_types(source, &type_count, &types)))
     {
         IFilterMapper2_Release(mapper);
         return hr;
@@ -1246,85 +1309,75 @@ static HRESULT autoplug(struct filter_graph *graph, IPin *source, IPin *sink,
     if (graph->pSite)
         IUnknown_QueryInterface(graph->pSite, &IID_IAMGraphBuilderCallback, (void **)&callback);
 
-    while (IEnumMediaTypes_Next(enummt, 1, &mt, NULL) == S_OK)
-    {
-        GUID types[2] = {mt->majortype, mt->subtype};
-        IEnumMoniker *enummoniker;
-        IBaseFilter *filter;
-        IMoniker *moniker;
-
-        DeleteMediaType(mt);
+    if (FAILED(hr = IFilterMapper2_EnumMatchingFilters(mapper, &enummoniker,
+            0, FALSE, MERIT_UNLIKELY, TRUE, type_count, types, NULL, NULL, FALSE,
+            render_to_existing, 0, NULL, NULL, NULL)))
+        goto out;
 
-        if (FAILED(hr = IFilterMapper2_EnumMatchingFilters(mapper, &enummoniker,
-                0, FALSE, MERIT_UNLIKELY, TRUE, 1, types, NULL, NULL, FALSE,
-                render_to_existing, 0, NULL, NULL, NULL)))
-            goto out;
+    while (IEnumMoniker_Next(enummoniker, 1, &moniker, NULL) == S_OK)
+    {
+        IPropertyBag *bag;
+        VARIANT var;
 
-        while (IEnumMoniker_Next(enummoniker, 1, &moniker, NULL) == S_OK)
+        VariantInit(&var);
+        IMoniker_BindToStorage(moniker, NULL, NULL, &IID_IPropertyBag, (void **)&bag);
+        hr = IPropertyBag_Read(bag, L"FriendlyName", &var, NULL);
+        IPropertyBag_Release(bag);
+        if (FAILED(hr))
         {
-            IPropertyBag *bag;
-            VARIANT var;
-
-            VariantInit(&var);
-            IMoniker_BindToStorage(moniker, NULL, NULL, &IID_IPropertyBag, (void **)&bag);
-            hr = IPropertyBag_Read(bag, L"FriendlyName", &var, NULL);
-            IPropertyBag_Release(bag);
-            if (FAILED(hr))
-            {
-                IMoniker_Release(moniker);
-                continue;
-            }
-
-            if (callback && FAILED(hr = IAMGraphBuilderCallback_SelectedFilter(callback, moniker)))
-            {
-                TRACE("Filter rejected by IAMGraphBuilderCallback::SelectedFilter(), hr %#lx.\n", hr);
-                IMoniker_Release(moniker);
-                continue;
-            }
-
-            hr = create_filter(graph, moniker, &filter);
             IMoniker_Release(moniker);
-            if (FAILED(hr))
-            {
-                ERR("Failed to create filter for %s, hr %#lx.\n", debugstr_w(V_BSTR(&var)), hr);
-                VariantClear(&var);
-                continue;
-            }
+            continue;
+        }
 
-            if (callback && FAILED(hr = IAMGraphBuilderCallback_CreatedFilter(callback, filter)))
-            {
-                TRACE("Filter rejected by IAMGraphBuilderCallback::CreatedFilter(), hr %#lx.\n", hr);
-                IBaseFilter_Release(filter);
-                continue;
-            }
+        if (callback && FAILED(hr = IAMGraphBuilderCallback_SelectedFilter(callback, moniker)))
+        {
+            TRACE("Filter rejected by IAMGraphBuilderCallback::SelectedFilter(), hr %#lx.\n", hr);
+            IMoniker_Release(moniker);
+            continue;
+        }
 
-            hr = IFilterGraph2_AddFilter(&graph->IFilterGraph2_iface, filter, V_BSTR(&var));
+        hr = create_filter(graph, moniker, &filter);
+        IMoniker_Release(moniker);
+        if (FAILED(hr))
+        {
+            ERR("Failed to create filter for %s, hr %#lx.\n", debugstr_w(V_BSTR(&var)), hr);
             VariantClear(&var);
-            if (FAILED(hr))
-            {
-                ERR("Failed to add filter, hr %#lx.\n", hr);
-                IBaseFilter_Release(filter);
-                continue;
-            }
+            continue;
+        }
 
-            hr = autoplug_through_filter(graph, source, filter, sink, render_to_existing, recursion_depth);
-            if (SUCCEEDED(hr))
-            {
-                IBaseFilter_Release(filter);
-                goto out;
-            }
+        if (callback && FAILED(hr = IAMGraphBuilderCallback_CreatedFilter(callback, filter)))
+        {
+            TRACE("Filter rejected by IAMGraphBuilderCallback::CreatedFilter(), hr %#lx.\n", hr);
+            IBaseFilter_Release(filter);
+            continue;
+        }
 
-            IFilterGraph2_RemoveFilter(&graph->IFilterGraph2_iface, filter);
+        hr = IFilterGraph2_AddFilter(&graph->IFilterGraph2_iface, filter, V_BSTR(&var));
+        VariantClear(&var);
+        if (FAILED(hr))
+        {
+            ERR("Failed to add filter, hr %#lx.\n", hr);
             IBaseFilter_Release(filter);
+            continue;
         }
-        IEnumMoniker_Release(enummoniker);
+
+        hr = autoplug_through_filter(graph, source, filter, sink, render_to_existing, recursion_depth);
+        if (SUCCEEDED(hr))
+        {
+            IBaseFilter_Release(filter);
+            goto out;
+        }
+
+        IFilterGraph2_RemoveFilter(&graph->IFilterGraph2_iface, filter);
+        IBaseFilter_Release(filter);
     }
+    IEnumMoniker_Release(enummoniker);
 
     hr = VFW_E_CANNOT_CONNECT;
 
 out:
+    free(types);
     if (callback) IAMGraphBuilderCallback_Release(callback);
-    IEnumMediaTypes_Release(enummt);
     IFilterMapper2_Release(mapper);
     return hr;
 }
diff --git a/dlls/quartz/tests/filtergraph.c b/dlls/quartz/tests/filtergraph.c
index 391f9777175..00232ea7a52 100644
--- a/dlls/quartz/tests/filtergraph.c
+++ b/dlls/quartz/tests/filtergraph.c
@@ -5344,7 +5344,7 @@ static void test_autoplug_uyvy(void)
      * failure to decode up to missing audio hardware, even though we're not
      * trying to render audio. */
     hr = IFilterGraph2_Render(graph, &source_pin.IPin_iface);
-    todo_wine ok(hr == S_OK || hr == VFW_E_NO_AUDIO_HARDWARE, "Got hr %#lx.\n", hr);
+    ok(hr == S_OK || hr == VFW_E_NO_AUDIO_HARDWARE, "Got hr %#lx.\n", hr);
 
     ref = IFilterGraph2_Release(graph);
     ok(!ref, "Got outstanding refcount %ld.\n", ref);
-- 
GitLab


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

[PATCH 2/3] amstream: Release the previous ddraw in ddraw_IAMMediaStream_Initialize().

[Zebediah Figura]

From: Zebediah Figura <zfigura(a)codeweavers.com>

---
 dlls/amstream/ddrawstream.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/dlls/amstream/ddrawstream.c b/dlls/amstream/ddrawstream.c
index 77795be47fa..b31466803f4 100644
--- a/dlls/amstream/ddrawstream.c
+++ b/dlls/amstream/ddrawstream.c
@@ -324,6 +324,10 @@ static HRESULT WINAPI ddraw_IAMMediaStream_Initialize(IAMMediaStream *iface, IUn
     stream->purpose_id = *purpose_id;
     stream->stream_type = stream_type;
 
+    if (stream->ddraw)
+        IDirectDraw_Release(stream->ddraw);
+    stream->ddraw = NULL;
+
     if (source_object
             && FAILED(hr = IUnknown_QueryInterface(source_object, &IID_IDirectDraw, (void **)&stream->ddraw)))
         FIXME("Stream object doesn't implement IDirectDraw interface, hr %#lx.\n", hr);
-- 
GitLab


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

[PATCH 3/3] amstream/tests: Fix a leak in test_ddrawstream_begin_flush_end_flush().

[Zebediah Figura]

From: Zebediah Figura <zfigura(a)codeweavers.com>

---
 dlls/amstream/tests/amstream.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c
index f15584f8948..a2f27c1b557 100644
--- a/dlls/amstream/tests/amstream.c
+++ b/dlls/amstream/tests/amstream.c
@@ -5991,6 +5991,9 @@ static void test_ddrawstream_begin_flush_end_flush(void)
     IDirectDrawMediaStream_Release(ddraw_stream);
     ref = IMediaStream_Release(stream);
     ok(!ref, "Got outstanding refcount %ld.\n", ref);
+    IPin_Release(pin2);
+    ref = IAMMediaStream_Release(stream2);
+    ok(!ref, "Got outstanding refcount %ld.\n", ref);
 }
 
 static void test_ddrawstream_new_segment(void)
-- 
GitLab

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