Re: quartz: Add stub IAMStreamSelect to MPEG-1 Stream Splitter. (try 2, resend)

[Nikolay Sivov]

On 22.10.2015 17:35, Anton Baskanov wrote:
> +    if ( IsEqualIID(riid, &IID_IUnknown)
> +      || IsEqualIID(riid, &IID_IPersist)
> +      || IsEqualIID(riid, &IID_IMediaFilter)
> +      || IsEqualIID(riid, &IID_IBaseFilter) )
> +        *ppv = iface;
> +
> +    if ( IsEqualIID(riid, &IID_IAMStreamSelect) )
> +        *ppv = &This->IAMStreamSelect_iface;

This is usually done with 'else if'.
> +static HRESULT WINAPI StreamSelect_QueryInterface(IAMStreamSelect *iface, REFIID riid, LPVOID *ppv)
> +{
> +    MPEGSplitterImpl *This = impl_from_IAMStreamSelect(iface);
> +
> +    return IBaseFilter_QueryInterface(&This->Parser.filter.IBaseFilter_iface, riid, ppv);
> +}

Unfortunately we don't have a written rule for that (I think), so I 
guess I'm just sharing my habits: I personally try to use exact 
interface names for implementation functions, with leading 'I' stripped, 
so AMStreamSelect_* in this case. This makes it clear what's this is 
about, and you don't need to invent another name that doesn't exist 
anywhere.
> +static const IAMStreamSelectVtbl StreamSelect_Vtbl =
> +{

Same here, AMStreamSelectVtbl is easier imho.

(quartz is not a best example in that regard)

Also please, no LPVOID.
> +    HRESULT Count(
> +        [out] DWORD * pcStreams);
> +
> +    HRESULT Info(
> +        [in] long lIndex,
> +        [out] AM_MEDIA_TYPE ** ppmt,
> +        [out] DWORD * pdwFlags,
> +        [out] LCID * plcid,
> +        [out] DWORD * pdwGroup,
> +        [out] WCHAR ** ppszName,
> +        [out] IUnknown ** ppObject,
> +        [out] IUnknown ** ppUnk);
> +
> +    HRESULT Enable(
> +        [in] long lIndex,
> +        [in] DWORD dwFlags);

You are using exact naming PSDK has, please do it in your own words, 
with better, cleaner names, like 'streams' or 'index', spacing in 
pointer notation could be fixed as well.