Re: [PATCH 1/5] qedit: Mostly stubbed out implementation of SampleGrabber filter

[Nikolay Sivov]

On 2/2/2010 16:21, Paul Chitescu wrote:
> Changelog:
>    qedit: Mostly stubbed out implementation of SampleGrabber filter
>
> Until the last patch component is applied most application will crash as they
> rarely check for errors.
> - if (IsEqualIID(riid,&IID_IUnknown) ||
> -
>    IsEqualIID(riid,&IID_IPersist) ||
> -
>    IsEqualIID(riid,&IID_IMediaFilter) ||
> -
>    IsEqualIID(riid,&IID_IBaseFilter)) {
> -
>    InterlockedIncrement(&This->refCount);
> -
>    *ppvObject =&(This->IBaseFilter_Vtbl);
> -
>    return S_OK;
> - }

You should use ISampleGrabber_AddRef() here on success to keep increment 
call in one place.
It's a common practice as far as i can see in Wine code.

[PATCH 1/5] qedit: Mostly stubbed out implementation of SampleGrabber filter

[Paul Chitescu]

On Tuesday 02 February 2010 03:30:20 pm Nikolay Sivov wrote:
> On 2/2/2010 16:21, Paul Chitescu wrote:
> > Changelog:
> >    qedit: Mostly stubbed out implementation of SampleGrabber filter
> >
> > Until the last patch component is applied most application will crash as

they
> > rarely check for errors.
> > - if (IsEqualIID(riid,&IID_IUnknown) ||
> > -
> >    IsEqualIID(riid,&IID_IPersist) ||
> > -
> >    IsEqualIID(riid,&IID_IMediaFilter) ||
> > -
> >    IsEqualIID(riid,&IID_IBaseFilter)) {
> > -
> >    InterlockedIncrement(&This->refCount);
> > -
> >    *ppvObject =&(This->IBaseFilter_Vtbl);
> > -
> >    return S_OK;
> > - }
>
> You should use ISampleGrabber_AddRef() here on success to keep increment 
> call in one place.
> It's a common practice as far as i can see in Wine code.

It's not a global practice though, code in qcap and qedit use 
InterlockedIncrement() in QueryInterface - probably to avoid having two TRACE 
lines in output just like I did.