[MPlayer-dev-eng] [PATCH] Implement DirectShow filter graph

[Reimar Döffinger]

On Sat, Mar 06, 2010 at 01:16:48PM +0100, Steinar H. Gunderson wrote:
> @@ -0,0 +1,142 @@
> +/*
> + * Modified for use with MPlayer, detailed changelog at
> + * http://svn.mplayerhq.hu/mplayer/trunk/
> + */

Uh, if it's modified you should obviously also indicate the source.
And it should have a proper license header.

> +static int GraphKeeper = 0;

Documentation?

> +    if (memcmp(clsid, &CLSID_FilterGraph, sizeof(GUID)))

sizeof(*clsid) maybe?

> +    p = (IFilterGraph*) FilterGraphCreate();
> +    result = p->vt->QueryInterface((IUnknown*)p, iid, ppv);
> +    p->vt->Release((IUnknown*)p);

Wouldn't that make IUnknown the most appropriate type for p?

> +  

Trailing whitespace here and in many other places.

> +    FilterGraph* This = (FilterGraph*) malloc(sizeof(FilterGraph));
> +    This->vt = (IFilterGraph_vt*) malloc(sizeof(IFilterGraph_vt));

Useless casts. And I'd consider calloc to ensure everything's initialized.
And sizeof(*variable) has a lower chance of breaking with future changes
that sizeof(type), so I generally consider that preferable.

> +	graph=FilterGraphCreate();
> +	result = This->m_pFilter->vt->JoinFilterGraph(This->m_pFilter, (IFilterGraph*)graph, (short *)"F\0i\0l\0t\0e\0r\0");

Inconsistent space placement in casts.
Also I consider something like
static const uint16_t filtername[] = {'F', 'i', 'l', 't', 'e', 'r', 0};
more readable, especially since I think your variant
1) is incorrectly terminating (2 bytes 0 are needed)
2) is not necessarily properly aligned (though irrelevant unless there are DirectShow filters
for ARM/wince and someone manages to make the work with our loader, but it still isn't
nice code).

>  	// enumerate pins
>  	result = This->m_pFilter->vt->EnumPins(This->m_pFilter, &enum_pins);
>  	if (result || !enum_pins)

Mixed tabs and spaces.

I assume there's no way this might break compiling on Windows?