New subject: [PATCH 1/4] quartz: AsyncReader is always expected to have output pin

31 May 2017

Thanks for working on this.  I tested your patch sequence against a
number of games as well as embedding videos in MSOffice 2007 and 2010
and found no regressions. I do have some comments, though, in-line
below.
Is this something you could write a test for? E.g. create the filter
and then query for its output pin immediately after creation.
On Thu, May 25, 2017 at 04:54:47PM +0200, Miklós Máté wrote:
...
Even between create() and Load(). This fixes a crash in DSPlayer.
Signed-off-by: Miklós Máté mtmkls@gmail.com
dlls/quartz/filesource.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/dlls/quartz/filesource.c b/dlls/quartz/filesource.c
index ed7e5ee201..200463ff5b 100644
--- a/dlls/quartz/filesource.c
+++ b/dlls/quartz/filesource.c
@@ -73,7 +73,8 @@ static const IFileSourceFilterVtbl FileSource_Vtbl;
 static const IAsyncReaderVtbl FileAsyncReader_Vtbl;
 static const IAMFilterMiscFlagsVtbl IAMFilterMiscFlags_Vtbl;
-static HRESULT FileAsyncReader_Construct(HANDLE hFile, IBaseFilter * pBaseFilter, LPCRITICAL_SECTION pCritSec, IPin ** ppPin);
+static HRESULT FileAsyncReader_Construct(IBaseFilter * pBaseFilter, LPCRITICAL_SECTION pCritSec, IPin ** ppPin);
+static HRESULT FileAsyncReader_SetFile(IAsyncReader * iface, HANDLE hFile);
static const WCHAR mediatype_name[] = {
     'M', 'e', 'd', 'i', 'a', ' ', 'T', 'y', 'p', 'e', 0 };
@@ -416,6 +417,7 @@ static const BaseFilterFuncTable BaseFuncTable = {
 HRESULT AsyncReader_create(IUnknown * pUnkOuter, LPVOID * ppv)
 {
     AsyncReader *pAsyncRead;

HRESULT hr;
if( pUnkOuter )
    return CLASS_E_NOAGGREGATION;


@@ -429,7 +431,8 @@ HRESULT AsyncReader_create(IUnknown * pUnkOuter, LPVOID * ppv)
 pAsyncRead->IFileSourceFilter_iface.lpVtbl = &FileSource_Vtbl;
 pAsyncRead->IAMFilterMiscFlags_iface.lpVtbl = &IAMFilterMiscFlags_Vtbl;


pAsyncRead->pOutputPin = NULL;


hr = FileAsyncReader_Construct(&pAsyncRead->filter.IBaseFilter_iface, &pAsyncRead->filter.csFilter, &pAsyncRead->pOutputPin);
BaseFilterImpl_IncrementPinVersion(&pAsyncRead->filter);

If this fails, I think we should free resources and return the error
immediately.
...
 pAsyncRead->pszFileName = NULL;
 pAsyncRead->pmt = NULL;

@@ -438,7 +441,7 @@ HRESULT AsyncReader_create(IUnknown * pUnkOuter, LPVOID * ppv)
 TRACE("-- created at %p\n", pAsyncRead);



return S_OK;


return hr;

}
/** IUnknown methods **/
@@ -606,7 +609,7 @@ static ULONG WINAPI FileSource_Release(IFileSourceFilter * iface)
static HRESULT WINAPI FileSource_Load(IFileSourceFilter * iface, LPCOLESTR pszFileName, const AM_MEDIA_TYPE * pmt)
 {

HRESULT hr;


HRESULT hr = E_FAIL;
HANDLE hFile;
IAsyncReader * pReader = NULL;
AsyncReader *This = impl_from_IFileSourceFilter(iface);

@@ -625,16 +628,15 @@ static HRESULT WINAPI FileSource_Load(IFileSourceFilter * iface, LPCOLESTR pszFi
         return HRESULT_FROM_WIN32(GetLastError());
     }

/* create pin */

hr = FileAsyncReader_Construct(hFile, &This->filter.IBaseFilter_iface, &This->filter.csFilter, &This->pOutputPin);

BaseFilterImpl_IncrementPinVersion(&This->filter);

if (SUCCEEDED(hr))



if (This->pOutputPin)

Since we now create the output pin during filter creation, this should
never be non-NULL, right?
...
     hr = IPin_QueryInterface(This->pOutputPin, &IID_IAsyncReader, (LPVOID *)&pReader);

 /* store file name & media type */
 if (SUCCEEDED(hr))
 {


   FileAsyncReader_SetFile(pReader, hFile);


   CoTaskMemFree(This->pszFileName);
   if (This->pmt)
       FreeMediaType(This->pmt);



@@ -939,7 +941,7 @@ static const BaseOutputPinFuncTable output_BaseOutputFuncTable = {
     BaseOutputPinImpl_BreakConnect
 };
-static HRESULT FileAsyncReader_Construct(HANDLE hFile, IBaseFilter * pBaseFilter, LPCRITICAL_SECTION pCritSec, IPin ** ppPin)
+static HRESULT FileAsyncReader_Construct(IBaseFilter * pBaseFilter, LPCRITICAL_SECTION pCritSec, IPin ** ppPin)
 {
     PIN_INFO piOutput;
     HRESULT hr;
@@ -952,9 +954,8 @@ static HRESULT FileAsyncReader_Construct(HANDLE hFile, IBaseFilter * pBaseFilter
 if (SUCCEEDED(hr))
 {


   FileAsyncReader *pPinImpl =  (FileAsyncReader *)*ppPin;




   FileAsyncReader *pPinImpl =  impl_from_IPin(*ppPin);
   pPinImpl->IAsyncReader_iface.lpVtbl = &FileAsyncReader_Vtbl;




   pPinImpl->hFile = hFile;
   pPinImpl->bFlushing = FALSE;
   pPinImpl->sample_list = NULL;
   pPinImpl->handle_list = NULL;



@@ -965,6 +966,14 @@ static HRESULT FileAsyncReader_Construct(HANDLE hFile, IBaseFilter * pBaseFilter
     return hr;
 }
+static HRESULT FileAsyncReader_SetFile(IAsyncReader * iface, HANDLE hFile)
+{

FileAsyncReader *This = impl_from_IAsyncReader(iface);

This->hFile = hFile;
return S_OK;

+}



Why is this a separate function instead of just setting hFile in
FileSource_Load?
...
/* IAsyncReader */
static HRESULT WINAPI FileAsyncReader_QueryInterface(IAsyncReader * iface, REFIID riid, LPVOID * ppv)
2.11.0

    

Re: [PATCH 1/4] quartz: AsyncReader is always expected to have output pin

Signed-off-by: Miklós Máté mtmkls@gmail.com

static HRESULT WINAPI FileAsyncReader_QueryInterface(IAsyncReader * iface, REFIID riid, LPVOID * ppv)

Signed-off-by: Miklós Máté mtmkls@gmail.com

static HRESULT WINAPI FileAsyncReader_QueryInterface(IAsyncReader * iface, REFIID riid, LPVOID * ppv)