Some fixes for filtergraph.c (#2)
Maarten Lankhorst
m.b.lankhorst at gmail.com
Sun Apr 24 07:16:50 CDT 2005
Christian Costa wrote:
> 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 ?
Fixed
>
>> 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.
Fixed
>
>> 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.
Asleep? ;) You can't connect an input pin more then once
(VFW_E_ALREADY_CONNECTED), anyway this function always returned S_OK, I
fixed that too now... If you still think multiple connects should happen
http://msdn.microsoft.com/library/en-us/directshow/htm/ipinconnect.asp ;)
>
>> }
>> CoTaskMemFree(ppins);
>> + IBaseFilter_Release(pfilter);
>> + IPin_Release(ppinfilter);
>> + break;
>> }
>> - break;
>>
>> error:
>> + if (HaveEnumPin) IPin_Release(ppinfilter);
>>
> See above.
Fixed, even if the connect failed in the end the method would return S_OK...
>
>> 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.
Fixed
>
>> 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.
>
Yup, fixed..
>> }
>> 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
I also implemented MediaControl Pause and MediaControl Stop, msn was
crashing because those were missing, and it didn't have a FIXME for the
stub but a TRACE since your name was in the copyright shame on you! I
modified that part a little so I could call mediacontrol run, stop, and
pause without having to copy/paste too much code..
I hope this patch will get applied to cvs (would be my first patch hehe ;))
Cya,
Maarten
-------------- next part --------------
Index: filtergraph.c
===================================================================
RCS file: /home/wine/wine/dlls/quartz/filtergraph.c,v
retrieving revision 1.27
diff -u -p -r1.27 filtergraph.c
--- filtergraph.c 24 Mar 2005 21:01:37 -0000 1.27
+++ filtergraph.c 24 Apr 2005 11:52:05 -0000
@@ -556,8 +556,7 @@ static HRESULT GetInternalConnections(IB
IPin_QueryDirection(ppin, &pindir);
if (pindir == PINDIR_OUTPUT)
i++;
- else
- IPin_Release(ppin);
+ IPin_Release(ppin);
}
*pppins = CoTaskMemAlloc(sizeof(IPin*)*i);
/* Retrieve output pins */
@@ -629,7 +628,7 @@ static HRESULT WINAPI Graphbuilder_Conne
if (!nbmt) {
ERR("No media type found!\n");
- return S_OK;
+ return VFW_E_CANNOT_CONNECT;
}
TRACE("MajorType %s\n", debugstr_guid(&mt->majortype));
TRACE("SubType %s\n", debugstr_guid(&mt->subtype));
@@ -648,7 +647,7 @@ static HRESULT WINAPI Graphbuilder_Conne
VARIANT var;
GUID clsid;
IPin** ppins;
- IPin* ppinfilter;
+ IPin* ppinfilter = NULL;
IBaseFilter* pfilter = NULL;
hr = GetFilterInfo(pMoniker, &clsid, &var);
@@ -677,6 +676,7 @@ static HRESULT WINAPI Graphbuilder_Conne
ERR("Enumpins (%lx)\n", hr);
goto error;
}
+
hr = IEnumPins_Next(penumpins, 1, &ppinfilter, &pin);
if (FAILED(hr)) {
ERR("Next (%lx)\n", hr);
@@ -703,17 +703,23 @@ static HRESULT WINAPI Graphbuilder_Conne
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;
}
+ while (++i < nb) IPin_Release[i];
CoTaskMemFree(ppins);
+ if (FAILED(hr)) goto error;
+ IBaseFilter_Release(pfilter);
+ IPin_Release(ppinfilter);
+ break;
}
- break;
error:
+ if (ppinfilter) IPin_Release(ppinfilter);
if (pfilter) {
IGraphBuilder_RemoveFilter(iface, pfilter);
IBaseFilter_Release(pfilter);
@@ -722,7 +728,8 @@ error:
IEnumMediaTypes_Release(penummt);
DeleteMediaType(mt);
-
+
+ if (FAILED(hr)) return VFW_E_CANNOT_CONNECT;
return S_OK;
}
@@ -1167,7 +1174,9 @@ static HRESULT WINAPI Mediacontrol_Invok
return S_OK;
}
-static HRESULT ExploreGraph(IFilterGraphImpl* pGraph, IPin* pOutputPin, REFERENCE_TIME tStart)
+typedef HRESULT(WINAPI *fnFoundFilter)(IBaseFilter *);
+
+static HRESULT ExploreGraph(IFilterGraphImpl* pGraph, IPin* pOutputPin, fnFoundFilter FoundFilter)
{
HRESULT hr;
IPin* pInputPin;
@@ -1176,7 +1185,8 @@ static HRESULT ExploreGraph(IFilterGraph
ULONG i;
PIN_INFO PinInfo;
- TRACE("%p %p %lld\n", pGraph, pOutputPin, tStart);
+ TRACE("%p %p\n", pGraph, pOutputPin);
+ PinInfo.pFilter = NULL;
hr = IPin_ConnectedTo(pOutputPin, &pInputPin);
@@ -1201,20 +1211,33 @@ static HRESULT ExploreGraph(IFilterGraph
/* Explore the graph downstream from this pin
* 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);
+ ExploreGraph(pGraph, ppPins[i], FoundFilter);
+ IPin_Release(ppPins[i]);
}
CoTaskMemFree(ppPins);
}
- TRACE("Run filter %p\n", PinInfo.pFilter);
- IBaseFilter_Run(PinInfo.pFilter, tStart);
+ TRACE("Doing stuff with filter %p\n", PinInfo.pFilter);
+ FoundFilter(PinInfo.pFilter);
}
+ if (PinInfo.pFilter) IBaseFilter_Release(PinInfo.pFilter);
return hr;
}
-/*** IMediaControl methods ***/
-static HRESULT WINAPI Mediacontrol_Run(IMediaControl *iface) {
+static HRESULT WINAPI SendRun(IBaseFilter *pFilter) {
+ return IBaseFilter_Run(pFilter, 0);
+}
+
+static HRESULT WINAPI SendPause(IBaseFilter *pFilter) {
+ return IBaseFilter_Pause(pFilter);
+}
+
+static HRESULT WINAPI SendStop(IBaseFilter *pFilter) {
+ return IBaseFilter_Stop(pFilter);
+}
+
+static HRESULT WINAPI SendTheFilterAMessage(IMediaControl *iface, fnFoundFilter FoundFilter) {
ICOM_THIS_MULTI(IFilterGraphImpl, IMediaControl_vtbl, iface);
int i;
IBaseFilter* pfilter;
@@ -1223,17 +1246,9 @@ static HRESULT WINAPI Mediacontrol_Run(I
IPin* pPin;
LONG dummy;
PIN_DIRECTION dir;
-
TRACE("(%p/%p)->()\n", This, iface);
EnterCriticalSection(&This->cs);
-
- if (This->state == State_Running)
- {
- LeaveCriticalSection(&This->cs);
- return S_OK;
- }
-
/* Explorer the graph from source filters to renderers, determine renderers number and
* run filters from renderers to source filters */
This->nRenderers = 0;
@@ -1253,6 +1268,7 @@ static HRESULT WINAPI Mediacontrol_Run(I
while(IEnumPins_Next(pEnum, 1, &pPin, &dummy) == S_OK)
{
IPin_QueryDirection(pPin, &dir);
+ IPin_Release(pPin);
if (dir == PINDIR_INPUT)
{
source = FALSE;
@@ -1266,32 +1282,51 @@ static HRESULT WINAPI Mediacontrol_Run(I
while(IEnumPins_Next(pEnum, 1, &pPin, &dummy) == S_OK)
{
/* Explore the graph downstream from this pin */
- ExploreGraph(This, pPin, 0);
+ ExploreGraph(This, pPin, FoundFilter);
+ IPin_Release(pPin);
}
- IBaseFilter_Run(pfilter, 0);
+ FoundFilter(pfilter);
}
IEnumPins_Release(pEnum);
}
- This->state = State_Running;
-
LeaveCriticalSection(&This->cs);
-
+
+ return S_FALSE;
+}
+
+/*** IMediaControl methods ***/
+static HRESULT WINAPI Mediacontrol_Run(IMediaControl *iface) {
+ ICOM_THIS_MULTI(IFilterGraphImpl, IMediaControl_vtbl, iface);
+ TRACE("(%p/%p)->()\n", This, iface);
+
+ if (This->state == State_Running) return S_OK;
+
+ SendTheFilterAMessage(iface, SendRun);
+ This->state = State_Running;
return S_FALSE;
}
static HRESULT WINAPI Mediacontrol_Pause(IMediaControl *iface) {
ICOM_THIS_MULTI(IFilterGraphImpl, IMediaControl_vtbl, iface);
+ TRACE("(%p/%p)->()\n", This, iface);
- TRACE("(%p/%p)->(): stub !!!\n", This, iface);
+ if (This->state == State_Paused) return S_OK;
+
+ SendTheFilterAMessage(iface, SendPause);
+ This->state = State_Paused;
return S_OK;
}
static HRESULT WINAPI Mediacontrol_Stop(IMediaControl *iface) {
ICOM_THIS_MULTI(IFilterGraphImpl, IMediaControl_vtbl, iface);
+ TRACE("(%p/%p)->()\n", This, iface);
- TRACE("(%p/%p)->(): stub !!!\n", This, iface);
+ if (This->state == State_Stopped) return S_OK;
+ if (This->state == State_Running) SendTheFilterAMessage(iface, SendPause);
+ SendTheFilterAMessage(iface, SendStop);
+ This->state = State_Stopped;
return S_OK;
}
More information about the wine-patches
mailing list