[FFmpeg-devel] [PATCH] hardcoding for pcm alaw/ulaw tables

[Reimar Döffinger]

On Sun, Mar 21, 2010 at 10:13:54PM +0100, Michael Niedermayer wrote:
> The point iam trying to make, and it seems quite unsccessfull is that
> the generators as thay are are just lots of red tape that is unreadable,
> and undocumented

Ok, but you must understand it doesn't feel like that to me (with the exception
of the av_cold/CONFIG_... defines) since I wrote
the code, and while I do copy existing code with this infrastructure it
usually takes me only minutes to get a working version.

> what is needed IMO is
> 1. the code must be readable by just looking at it without looking up
>    everything and just accepting that some statements are there and it
>    wont compile where they not.

While in principle I agree, due to having to support cross-compiling those
table generators are tricky, and I have some doubts it is possible to make
them quite as simple as you (and to a degree I) want them to be.

> 2. It must be possible to write a table generator from a single 80x25
>    page of documentation without having to refer to another generator
>    and copy & pasting from it because nothing makes any sense and nothing
>    is documented.
>    if you need more than 80x25 chars to document it then there is something
>    wrong IMHO

I guess a 80x25 documentation containing "copy and paste this" is not what you want :-)
I'll look into it - somewhen.

> if i just would compare what we have against checking c files with tables
> into svn and in addition having runtime table generator code too and
> switch per CONFIG_HARDCODED_TABLES. Its should be quite a bit simpler and
> cleaner.

I'd like to note that the original purpose was not to be better than or a
replacement for that in general, but to have a solution where the tables
are too large to be checked in.
That I am now using it for everything is imply due to the fact that it's faster
for me than getting the tables themselves to check in.

> > > > +#define pcm_alaw_tableinit()
> > > > +#define pcm_ulaw_tableinit()
> > > 
> > > ugly
> > 
> > What is ugly about it and what would you prefer instead?
> > E.g. declaring the functions always and putting the function
> > body under #if?
> > Or doing a if (CONFIG_HARDCODED_TABLES) return;
> > at the start of the function?
> > Your one-line "ugly" comments are of no help, I'm not writing ugly
> > code on purpose, I either don't consider it ugly or can't think
> > of a better way, neither of which such a statement alone is going
> > to change.
> 
> if(!CONFIG_HARDCODED_TABLES) pcm_alaw_tableinit();

Then instead of the
#define pcm_alaw_tableinit()
you would need an additional
static void pcm_alaw_tableinit(void);
still to make the compiler not complain about a missing
prototype (or what exactly that warning is), adding the
kind of "magic statements" you complained about above.
I'll happily change it, I just considered it the worst solution,
exposing detail of the table generation to the main code with
(from my point of view) neither need nor advantage.