[MPlayer-dev-eng] [PATCH] libbs2b audio filter

[Andrew Savchenko]

Hi,

On Thursday 05 March 2009 16:39, Uoti Urpala wrote:
[...]
> > Necessity of using void* pointers and pointer to function
> > ensues from aiming to avoid six different play functions for
> > different types of audio data or even worse alternative:
> > switch statement inside single play function. Original
> > processing functions from
>
> You could use char * for the pointer arithmetic.

Afaik char is not garantied to be exactly 8 bit width. I used 
u_int8_t as Reimar suggested.

> > +typedef struct af_bs2b_s
>
> I'd use the struct type directly leave leave out the typedef.

This is a rather common practice to use typedef to avoid explicit 
usage of full structure type at every place. This doesn't hurt 
performance and, imho, make it easier to read the code.

> And in any case using a _s suffix for a struct name is just
> stupid. (Yes, there is existing stupid code in MPlayer, but that
> doesn't mean you should use it as an example.)

Structure name is unused there, I just removed it.

> > +    void (*bs2b)();
>
> You can't pretend that the function has no argument type by
> leaving it out of the declaration. As long as the library
> functions actually have different types it is NOT possible to
> call them from a single location.

Why? It isn't fast definitely, but it should work anyway if number 
and type of arguments will be compatible with those declared in 
the function prototype of actually called function.

> +        if (!sscanf(arg,"%i:%i", &opt[0], &opt[1]))
> +        {
>
> Put opening brace on the same line as the "if".

Why? IMO this kind of indentation makes it easier to read the code. 
Currently two different code styles -- opening brace at the end of 
line vs opening brace on the new line -- are mixed in this source. 
If it is necessary, I may change all code according to the latter 
style.

-- 
Best regards,
Andrew
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20090305/4c303083/attachment.pgp>