[MPlayer-dev-eng] ARM and liba52

[Siarhei Siamashka]

On Thursday 01 May 2008, Ivan Kalvachev wrote:
> >  ------------------------------------------------------------------------
> >  r10490 | alex | 2003-07-28 02:02:58 +0300 (Mon, 28 Jul 2003) | 2 lines
> >  Changed paths:
> >    M /trunk/liba52/bitstream.h
> >
> >  Changed swab32 from macro to inline function, this fixes compilation on
> > alpha (with gcc2.95). Based on patch by KotH
>
> I think the proper fix would be to don't use ALT_BITSTREAM_READER on
> architectures that can't read unaligned words. It seems that the code
> in question uses some kind of black magic trying to make unaligned
> access problem of the compiler, instead of reading it byte by byte.

That's a different topic (more related to ffmpeg), but ALT_BITSTREAM_READER 
is fine on ARM and other architectures. As you need to swap bytes in word 
on little endian architecture, just reading 4 bytes and doing 3 shifted AND
operations is quite fast. With a small modification this code can be turned
into 3 byte reads and two ANDs as we don't need all 32 bits and can make code
smaller and faster. That's what I'm looking at now, and that's why this
problem in liba52 got detected (initially not by testing with real AC3 files,
but just spotting this part of suspicious code on reading it).

In addition, there is ALT_BITSTREAM_READER_LE reader which is used
unconditionally for some decoders.

> I think a patch that disables ALT_* on all ARMs without native
> unaligned access would be quickly committed.

What's the problem with just reverting old broken commit? A change of
bitstream reader will require some benchmarks done. And it would be
optimization and not a bugfix.

> p.s.
> the black magic code is present in lavc bitreader too, but it looks
> like it is not used anywhere. Time for cleanup :)

Don't hurry with the cleanup. Better try to understand the code first :)

-- 
Best regards,
Siarhei Siamashka