Some fixes for filtergraph.c

[Christian Costa]

Hi Maarten,

Maarten Lankhorst wrote:

> This doesn't fix everything, just theroutines i called.. some other 
> similar functions aren't fixed because filtergraph.c is really THAT 
> leaky..b mostly with reference counts.. 

Yes, Indeed. :-)

Here are some comments :

>
>------------------------------------------------------------------------
>
>--- wine-20050310/dlls/quartz/filtergraph.c	2005-03-02 11:12:12.000000000 +0100
>+++ wine/dlls/quartz/filtergraph.c	2005-04-24 00:50:30.000000000 +0200
>@@ -556,8 +556,7 @@
>             IPin_QueryDirection(ppin, &pindir);
>             if (pindir == PINDIR_OUTPUT)
>                 i++;
>-            else
>-                IPin_Release(ppin);
>+            IPin_Release(ppin);
>         }
>         *pppins = CoTaskMemAlloc(sizeof(IPin*)*i);
>         /* Retrieve output pins */
>@@ -650,6 +649,7 @@
>         IPin** ppins;
>         IPin* ppinfilter;
>         IBaseFilter* pfilter = NULL;
>+        BOOL HaveEnumPin = FALSE;
> 
>
Why don't you just set ppinfilter to NULL
and do "if (ppinfilter) IPin_Release(ppinfilter);" at the end ?

>         hr = GetFilterInfo(pMoniker, &clsid, &var);
>         IMoniker_Release(pMoniker);
>@@ -677,6 +677,7 @@
>             ERR("Enumpins (%lx)\n", hr);
>             goto error;
>         }
>+
>         hr = IEnumPins_Next(penumpins, 1, &ppinfilter, &pin);
>         if (FAILED(hr)) {
>             ERR("Next (%lx)\n", hr);
>@@ -687,6 +688,7 @@
>             goto error;
>         }
>         IEnumPins_Release(penumpins);
>+        HaveEnumPin = TRUE;
> 
>
See above.

>         hr = IPin_Connect(ppinOut, ppinfilter, NULL);
>         if (FAILED(hr)) {
>@@ -703,17 +705,21 @@
>             TRACE("pins to consider: %ld\n", nb);
>             for(i = 0; i < nb; i++) {
>                 TRACE("Processing pin %d\n", i);
>-                hr = IGraphBuilder_Connect(iface, ppins[0], ppinIn);
>+                hr = IGraphBuilder_Connect(iface, ppins[i], ppinIn);
>                 if (FAILED(hr)) {
>-                    TRACE("Cannot render pin %p (%lx)\n", ppinfilter, hr);
>-                    return hr;
>+                   TRACE("Cannot render pin %p (%lx)\n", ppinfilter, hr);
>                 }
>+                IPin_Release(ppins[i]);
>+                if (SUCCEEDED(hr)) break;
>
This is not correct, you must connect all pins, not just the first that 
succeeds.

>             }
>             CoTaskMemFree(ppins);
>+            IBaseFilter_Release(pfilter);
>+            IPin_Release(ppinfilter);
>+            break;
>         }
>-        break;
> 
> error:
>+        if (HaveEnumPin) IPin_Release(ppinfilter);
>
See above.

>         if (pfilter) {
>             IGraphBuilder_RemoveFilter(iface, pfilter);
>             IBaseFilter_Release(pfilter);
>@@ -1175,13 +1181,16 @@
>     ULONG nb;
>     ULONG i;
>     PIN_INFO PinInfo;
>+    BOOL HavePinInfo = FALSE;
> 
>     TRACE("%p %p %lld\n", pGraph, pOutputPin, tStart);
> 
>     hr = IPin_ConnectedTo(pOutputPin, &pInputPin);
> 
>-    if (SUCCEEDED(hr))
>+    if (SUCCEEDED(hr)) {
>+        HavePinInfo = TRUE;
>         hr = IPin_QueryPinInfo(pInputPin, &PinInfo);
>+    }
> 
>
Personally, I would set PinIno.pFilter to NULL on failure and do if 
(PinIno.pFilter) release... at the end.
But it's just a matter of taste.

>     if (SUCCEEDED(hr))
>         hr = GetInternalConnections(PinInfo.pFilter, pInputPin, &ppPins, &nb);
>@@ -1202,6 +1211,7 @@
> 		 * FIXME: We should prevent exploring from a pin more than once. This can happens when
> 		 * several input pins are connected to the same output (a MUX for instance). */
>                 ExploreGraph(pGraph, ppPins[i], tStart);
>+                IPin_Release(ppPins[i]);
>             }
> 
>             CoTaskMemFree(ppPins);
>@@ -1210,6 +1220,7 @@
>         IBaseFilter_Run(PinInfo.pFilter, tStart);
>     }
> 
>+    if (HavePinInfo) IBaseFilter_Release(PinInfo.pFilter);
>     return hr;
> }
> 
>@@ -1255,9 +1266,11 @@
>             IPin_QueryDirection(pPin, &dir);
>             if (dir == PINDIR_INPUT)
>             {
>+                IPin_Release(pPin);
>                 source = FALSE;
>                 break;
>             }
>+            IPin_Release(pPin);
>
That would be simplier to do a single call to IPinRelease after 
IPin_QueryDirection.

>         }
>         if (source == TRUE)
>         {
>@@ -1267,6 +1280,7 @@
>             {
>                 /* Explore the graph downstream from this pin */
>                 ExploreGraph(This, pPin, 0);
>+                IPin_Release(pPin);
>             }
>             IBaseFilter_Run(pfilter, 0);
>         }
>  
>
Nice patch, BTW! :-)

Bye,
Christian