[prev in list] [next in list] [prev in thread] [next in thread]
List: wine-devel
Subject: Re: [PATCH 1/4] quartz: AsyncReader is always expected to have output pin
From: Andrew Eikum <aeikum () codeweavers ! com>
Date: 2017-05-31 14:32:10
Message-ID: 20170531143210.GD2524 () foghorn ! codeweavers ! com
[Download RAW message or body]
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
>
>
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic