[MPlayer-dev-eng] shared liba52 and mm_accel macros

[Finn Thain]

Diego Biurrun wrote:

> On Fri, Mar 16, 2007 at 11:38:36AM +1100, Finn Thain wrote:
> > 
> > I've just finished porting mplayer's SIMD accelerated liba52 to the 
> > shared library (upstream 0.7.4) on my system. I had to add the 
> > necessary autotools guff, but it works fine on altivec.
> 
> So you're working on upstream liba52?  Nice to hear, I thought the 
> project was orphaned.

I'm not really working on upstream liba52 other than trying scratch an 
itch. That is, I needed a faster liba52 on my old powermac. Someone else 
did all the work (mplayer's liba52 seems to be the only one with altivec 
support), I really only autotooled it.
 
> > Only real problem is the MM_ACCEL_* macros. These values are checked 
> > at runtime, so the shared library has to agree with the apps that link 
> > with it, otherwise there's no acceleration (or worse).
> > 
[snip]
> > 
> > Note that there are a couple of issues here:
> > 
> > 1) 0x08000000 is MM_ACCEL_X86_3DNOWEXT and MM_ACCEL_X86_SSE2.
> > 
> > 2) MM_ACCEL_PPC_ALTIVEC is 0x00010000 and 0x04000000.
> > 
> > 
> > So my question is, is there an authoratitive source for these macros?
> 
> Not that I know of.

I did some digging on code.google.com, and it looks like they came from 
mpeg2dec originally.

> > If not, can mplayer and liba52 be changed to match xine? The reason I 
> > suggest that mplayer change rather than the others is that it seems 
> > mplayer doesn't link dynamically with other packages with respect to 
> > mm_accel values. (Correct me if I'm wrong.)
> > 
> > Of course, xine will have to change too, but I can't try to push any 
> > patches to them unless there is consensus upon the actual values. If 
> > there was, they could go into /usr/include/liba52/mm_accel.h (or 
> > whatever).
> 
> Changing MPlayer should be fine, just send a patch.

I decided against sending the patch. If I can't send it upstream first 
then there's no point. But it gets worse than that.

> > I would like to get mplayer's liba52 acceleration into the upstream 
> > code. Has anyone attempted that before? It seems that project hasn't 
> > made a release for years.
> 
> I don't think so..

I guess I'll have to try anyway. It would be nice to get the SIMD stuff 
upstream.

BTW, I noticed that upstream liba52 now defines MM_ACCEL_DJBFFT as 
0x00000001, even though older versions of liba52 and everyone else who 
took the mm_accel.h header from mpeg2dec still defines MM_ACCEL_MLIB 
0x00000001! Nice one.

And actually, there is a third possibility for MM_ACCEL_PPC_ALTIVEC, 
mpeg2dec was forked as mpeg2dec-avec, which defines it 0x10000000... I 
guess that was never sent upstream either.

I also discovered that although the values I used in my patch agree with 
DirectFB and xinelib, there is even a fourth possibility for 
MM_ACCEL_PPC_ALTIVEC (remember this is part of the library API) -- the 
original defintion from mpeg2dec and those projects that include that code 
(there are quite a few) generally have _both_ MM_ACCEL_PPC_ALTIVEC and 
MM_ACCEL_X86_MMX defined as 0x80000000.

The assumption here is that, come runtime, the architecture has been 
determined already, so reuse the same value. Well, I'm not comfortable 
with that, since I know that Mac OS X can run powerpc binaries alongside 
intel binaries (and I seem to recall that an emulated powerpc app can 
dynamically link with a native x86 dylib (?)).

So it seems that the upstream mpeg2dec source and most of the projects 
that forked it would need to be patched, as would upstream liba52's fork 
of that code... ouch.

I concluded that the whole idea of choosing an optimisation via a library 
API is broken -- in this case a52_init(uint32_t mm_accel). It should be 
taken care of by the library at compile time (it's not like we don't have 
the source) or else CPU features should be detected automatically at 
runtime by the library. It shouldn't be part of the API. (Or else 
libraries are intended to be forked and bundled as they are in mplayer et 
al.)

But I'm not going to write routines to detect CPU features for all the 
architectures involved (I've counted sparc, mips, intel, amd and ppc 
optimisations so far) -- all just to fix the library APIs.

You gotta laugh though. :)

-f

> 
> Diego