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

[Michael Niedermayer]

On Sun, Mar 21, 2010 at 07:54:21PM +0100, Reimar D?ffinger wrote:
> On Sun, Mar 21, 2010 at 07:24:46PM +0100, Michael Niedermayer wrote:
> > > +#define av_cold
> > > +#define CONFIG_HARDCODED_TABLES 0
> > 
> > ugly
> 
> Yes, I haven't come up with a good way to avoid it yet though.
> Thinks I've been thinking about:
> 1) adding it to tableprint.h
> 2) a new header file, possibly created by configure, with a few
> stubs like these and also some defines like CONFIG_SMALL etc.
> 
> > > +    {
> > > +        "static const uint8_t linear_to_ulaw[1 << 14]",
> > > +        write_uint8_array,
> > > +        linear_to_ulaw,
> > > +        1 << 14,
> > > +        0
> > > +    },
> > 
> > cleaner and with half the code:
> > 
> > printf("static const uint8_t linear_to_ulaw[1 << 14]= {\n");
> > for(i=0; i < 1<<14; i++)
> >     printf("%d,", linear_to_ulaw[i]);
> > printf("};\n");
> 
> Well, I can beat that easily:
> { "static const uint8_t linear_to_ulaw[1 << 14]",
>   write_uint8_array, linear_to_ulaw, 1 << 14 },

simplified by putting more things on the same line ...
anyway, new try:

printf("static const uint8_t inear_to_ulaw[SIZE]={%s};",
       pretty_print_array(linear_to_ulaw, sizeof(uint8_t), SIZE));



> 
> I don't see the point though, and that struct-based approach has
> a chance to be reused for other purposes e.g. analyzing the tables
> and give a somewhat human-readable result - 16k values in a single
> line certainly doesn't qualify as that.
> Human-readable is useful for getting an idea how likely it is possible
> to reduce the size otherwise for CONFIG_SMALL.
> Lastly, I consider it more scalable - even with a huge number of generators
> the maintenance effort should not grow much, with well-tested write_* functions
> available for almost all cases, and if necessary those struct
> definitions are easier to modify by a script than code that is likely to be
> slightly different for each case.

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

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.
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


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.


> 
> > > +#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();

</rant>

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100321/312797e9/attachment.pgp>