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