When IPin_QueryInternalConnections returns S_OK and nb > 0, a SEGFAULT occurs at dlls/quartz/filtergraph.c:2144 as the code is expecting ppPins to be an initialized array if SUCCEEDED(hr) is TRUE and nb > 0.
This patch ensures ppPins is an initialized array if SUCCEEDED(hr) is TRUE and nb > 0. Prior to this patch, ppPins was not being initialized when hr was S_OK and nb > 0.
The Microsoft documentation for IPin_QueryInternalConnections states: *** This method has another use that is now deprecated: The Filter Graph Manager treats a filter as being a renderer filter if at least one input pin implements this method but returns zero in nPin. If you are writing a new renderer filter, however, you should implement the IAMFilterMiscFlags interface instead of using this method to indicate that the filter is a renderer. ***
The code I changed was written back in 2004/2005. My guess is back then the deprecated behaviour would only return S_OK when nb == 0, but this no longer appears to be the case. See line 99 of https://chromium.googlesource.com/webm/webmdshow/+/master/webmsplit/webmspli... for an example.
Signed-off-by: Brendan McGrath brendan@redmandi.com --- dlls/quartz/filtergraph.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/dlls/quartz/filtergraph.c b/dlls/quartz/filtergraph.c index c8595646a03..536c48d346e 100644 --- a/dlls/quartz/filtergraph.c +++ b/dlls/quartz/filtergraph.c @@ -981,9 +981,7 @@ static HRESULT GetInternalConnections(IBaseFilter* pfilter, IPin* pinputpin, IPi
TRACE("(%p, %p, %p, %p)\n", pfilter, pinputpin, pppins, pnb); hr = IPin_QueryInternalConnections(pinputpin, NULL, &nb); - if (hr == S_OK) { - /* Rendered input */ - } else if (hr == S_FALSE) { + if (SUCCEEDED(hr) && nb > 0) { *pppins = CoTaskMemAlloc(sizeof(IPin*)*nb); hr = IPin_QueryInternalConnections(pinputpin, *pppins, &nb); if (hr != S_OK) {
On 08/10/18 02:06, Brendan McGrath wrote:
When IPin_QueryInternalConnections returns S_OK and nb > 0, a SEGFAULT occurs at dlls/quartz/filtergraph.c:2144 as the code is expecting ppPins to be an initialized array if SUCCEEDED(hr) is TRUE and nb > 0.
This patch ensures ppPins is an initialized array if SUCCEEDED(hr) is TRUE and nb > 0. Prior to this patch, ppPins was not being initialized when hr was S_OK and nb > 0.
The Microsoft documentation for IPin_QueryInternalConnections states:
This method has another use that is now deprecated: The Filter Graph Manager treats a filter as being a renderer filter if at least one input pin implements this method but returns zero in nPin. If you are writing a new renderer filter, however, you should implement the IAMFilterMiscFlags interface instead of using this method to indicate that the filter is a renderer.
The code I changed was written back in 2004/2005. My guess is back then the deprecated behaviour would only return S_OK when nb == 0, but this no longer appears to be the case. See line 99 of https://chromium.googlesource.com/webm/webmdshow/+/master/webmsplit/webmspli... for an example.
Signed-off-by: Brendan McGrath brendan@redmandi.com
dlls/quartz/filtergraph.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/dlls/quartz/filtergraph.c b/dlls/quartz/filtergraph.c index c8595646a03..536c48d346e 100644 --- a/dlls/quartz/filtergraph.c +++ b/dlls/quartz/filtergraph.c @@ -981,9 +981,7 @@ static HRESULT GetInternalConnections(IBaseFilter* pfilter, IPin* pinputpin, IPi
TRACE("(%p, %p, %p, %p)\n", pfilter, pinputpin, pppins, pnb); hr = IPin_QueryInternalConnections(pinputpin, NULL, &nb);
- if (hr == S_OK) {
/* Rendered input */- } else if (hr == S_FALSE) {
- if (SUCCEEDED(hr) && nb > 0) { *pppins = CoTaskMemAlloc(sizeof(IPin*)*nb); hr = IPin_QueryInternalConnections(pinputpin, *pppins, &nb); if (hr != S_OK) {
Currently the only use of this function is in ExploreGraph(). Native does this completely differently (i.e. it actually delivers state change requests to all filters regardless of whether they're connected, and ensures that they are received in order by topologically sorting them on connection). I suspect we should probably get rid of it and just use IBaseFilter_EnumPins() there directly.
ExploreGraph will now iterate each OUTPUT PIN connected to an INPUT PINs filter (whether it is internally connected to the INPUT PIN or not).
This is effectively the same as if IPin_QueryInternalConnections had returned E_NOTIMPL everytime.
The static function GetInternalConnections is therefore no longer required and has been removed. ---
This patch is a result of the feedback provided by Zebediah Figura to my original patch (Fix SEGFAULT when num pin > 0).
I'm not sure if this is exactly what was intended - but I figure if there should be a check for an internal connection then it would match the functionality of my original patch (which was much smaller; thus safer).
dlls/quartz/filtergraph.c | 98 ++++++++------------------------------- 1 file changed, 19 insertions(+), 79 deletions(-)
diff --git a/dlls/quartz/filtergraph.c b/dlls/quartz/filtergraph.c index 07d40dbb0ea..b2c3e0c78e2 100644 --- a/dlls/quartz/filtergraph.c +++ b/dlls/quartz/filtergraph.c @@ -974,68 +974,6 @@ static HRESULT GetFilterInfo(IMoniker* pMoniker, VARIANT* pvar) return hr; }
-static HRESULT GetInternalConnections(IBaseFilter* pfilter, IPin* pinputpin, IPin*** pppins, ULONG* pnb) -{ - HRESULT hr; - ULONG nb = 0; - - TRACE("(%p, %p, %p, %p)\n", pfilter, pinputpin, pppins, pnb); - hr = IPin_QueryInternalConnections(pinputpin, NULL, &nb); - if (hr == S_OK) { - /* Rendered input */ - } else if (hr == S_FALSE) { - *pppins = CoTaskMemAlloc(sizeof(IPin*)*nb); - hr = IPin_QueryInternalConnections(pinputpin, *pppins, &nb); - if (hr != S_OK) { - WARN("Error (%x)\n", hr); - } - } else if (hr == E_NOTIMPL) { - /* Input connected to all outputs */ - IEnumPins* penumpins; - IPin* ppin; - int i = 0; - TRACE("E_NOTIMPL\n"); - hr = IBaseFilter_EnumPins(pfilter, &penumpins); - if (FAILED(hr)) { - WARN("filter Enumpins failed (%x)\n", hr); - return hr; - } - i = 0; - /* Count output pins */ - while(IEnumPins_Next(penumpins, 1, &ppin, &nb) == S_OK) { - PIN_DIRECTION pindir; - IPin_QueryDirection(ppin, &pindir); - if (pindir == PINDIR_OUTPUT) - i++; - IPin_Release(ppin); - } - *pppins = CoTaskMemAlloc(sizeof(IPin*)*i); - /* Retrieve output pins */ - IEnumPins_Reset(penumpins); - i = 0; - while(IEnumPins_Next(penumpins, 1, &ppin, &nb) == S_OK) { - PIN_DIRECTION pindir; - IPin_QueryDirection(ppin, &pindir); - if (pindir == PINDIR_OUTPUT) - (*pppins)[i++] = ppin; - else - IPin_Release(ppin); - } - IEnumPins_Release(penumpins); - nb = i; - if (FAILED(hr)) { - WARN("Next failed (%x)\n", hr); - return hr; - } - } else if (FAILED(hr)) { - WARN("Cannot get internal connection (%x)\n", hr); - return hr; - } - - *pnb = nb; - return S_OK; -} - /* Attempt to connect one of the output pins on filter to sink. Helper for * FilterGraph2_Connect(). */ static HRESULT connect_output_pin(IFilterGraphImpl *graph, IBaseFilter *filter, IPin *sink) @@ -2089,10 +2027,8 @@ static HRESULT ExploreGraph(IFilterGraphImpl* pGraph, IPin* pOutputPin, fnFoundF IMediaSeeking *seeking; HRESULT hr; IPin* pInputPin; - IPin** ppPins; - ULONG nb; - ULONG i; PIN_INFO PinInfo; + IEnumPins* pEnumPins;
TRACE("%p %p\n", pGraph, pOutputPin); PinInfo.pFilter = NULL; @@ -2102,27 +2038,31 @@ static HRESULT ExploreGraph(IFilterGraphImpl* pGraph, IPin* pOutputPin, fnFoundF if (SUCCEEDED(hr)) { hr = IPin_QueryPinInfo(pInputPin, &PinInfo); - if (SUCCEEDED(hr)) - hr = GetInternalConnections(PinInfo.pFilter, pInputPin, &ppPins, &nb); IPin_Release(pInputPin); }
if (SUCCEEDED(hr)) { - if (nb) - { - for(i = 0; i < nb; i++) - { - /* 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], FoundFilter, data); - IPin_Release(ppPins[i]); - } + hr = IBaseFilter_EnumPins(PinInfo.pFilter, &pEnumPins); + }
- CoTaskMemFree(ppPins); + if (SUCCEEDED(hr)) + { + IPin* pPin; + /* Explore the graph downstream from this pin's filter + * FIXME: We should prevent exploring from a pin more than once. This can happen when + * several input pins are connected to the same output (a MUX for instance). */ + while(IEnumPins_Next(pEnumPins, 1, &pPin, NULL) == S_OK) + { + PIN_DIRECTION pinDir; + IPin_QueryDirection(pPin, &pinDir); + if (pinDir == PINDIR_OUTPUT) + ExploreGraph(pGraph, pPin, FoundFilter, data); + IPin_Release(pPin); } - TRACE("Doing stuff with filter %p\n", PinInfo.pFilter); + + IEnumPins_Release(pEnumPins); + TRACE("Doing stuff with filter %s (%p)\n", debugstr_w(PinInfo.achName), PinInfo.pFilter);
if (SUCCEEDED(IBaseFilter_QueryInterface(PinInfo.pFilter, &IID_IAMFilterMiscFlags, (void **)&flags)))
-
Brendan McGrath -
Zebediah Figura