[MPlayer-dev-eng] liba52 reorganization

[Alex Izvorski]

Hello,

I'd like to present a reorganization of the liba52 optimization patches
from mplayer.  This is not quite finished but it is sufficient to show
the idea.  It is a diff against liba52 version 0.7.4.  Hopefully a later
version of this will be a candidate for upstream merging.  The goal was
to have the absolute minimum possible set of changes to the original
liba52 code that allows the optimized versions to be hooked in, with
most of the new stuff split into separate new files, one per type of
optimization, and with (obsessively) consistent naming and organization
of everything.  One patch ("-changed-only") shows only the changes to
already existing files in liba52 (and yes, it can compile and run with
that, with default defines, it just doesn't include the optimizations -
relatively light set of changes). The other patch ("-all") shows newly
added files as well (and is pretty massive).  Finally, "-alt-bitstream"
includes everything related to ALT_BITSTREAM_READER, bitstream_skip, and
memalign.  The last one is just for reference, I think it would need a
bit of work before it is ready for upstreaming.

All is fairly self-explanatory, but here's a quick guided tour: 

- any function which can have different versions becomes a function
pointer stored in a52_state, instead of a global function pointer.
(amongst other things, this way there are fewer globals, and it would be
needed for a truly thread-safe version; there is no measurable speed
impact).

- a52_downmix_accel_init and a52_imdct_init (maybe that should be
a52_imdct_accel_init?) set those to the C versions (named foo_c) and
depending on mm_accel flags call optimization-specific init functions
which may replace those function pointers (with specific versions like
foo_sse) and perform other initialization.

- those init functions are (well, should be, and mostly are) the only
part of any optimized code visible from the rest.

- some functions (a52_downmix, ...) which did not take a52_state as an
argument now do, in order to be able to use the function pointers.

- the FOO-optimized code is only compiled if HAVE_FOO is defined (in
theory, not quite there in practice), and only runs if MM_ACCEL_FOO is
set.

TODO:
- how to set correct architecture/instruction set environment variables
like ARCH_X86 and HAVE_SSE if building outside of mplayer?  anyone want
to have a shot at adding these to a52dec's original configure.in?

- the optimized versions are single-precision-only, should be
conditional on ifndef LIBA52_DOUBLE.  also more things need to be
conditional on HAVE_FOO and less on architecture.

- static arrays with initializers... some of the optimized code (e.g.
imdct_sse) relies on tables which I'd like to make local to that
compilation unit (ie: internal linkage), to avoid namespace pollution.
unfortunately, they have an initializer, so if they are declared as
static (even static const), they fail to link.  not really seeing a
*good* way around this.

- copyrights: a lot of this code came with minimal or no attribution,
I've tried to put in reasonable copyright headers, but I need to contact
people and make sure it is correct.  If some of this is your code and it
is not correctly attributed, *please* let me know.

- what to do with srfftp?  in terms of code organization, that is a bit
of the odd-man-out at the moment.  maybe just move into imdct_3dnow.c?

- a lot more testing ;)  altivec is quite likely broken, other things
may be as well.

Comments welcome.

--Alex



P.S. The combination of "-all" and "-alt-bitstream" contains the
all the code from the current patch in mplayer (except for upmix_MMX and
resample).

-------------- next part --------------
A non-text attachment was scrubbed...
Name: a52dec-mplayer-patches-v7-all.diff
Type: text/x-patch
Size: 112053 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20060922/0e85cd8a/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: a52dec-mplayer-patches-v7-alt-bitstream.diff
Type: text/x-patch
Size: 7867 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20060922/0e85cd8a/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: a52dec-mplayer-patches-v7-changed-only.diff
Type: text/x-patch
Size: 21270 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20060922/0e85cd8a/attachment-0002.bin>