Re: 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