[MPlayer-dev-eng] [PATCH] add support for AC3 endianness

[Reimar Döffinger]

Hello,
On Mon, Sep 17, 2007 at 02:58:23PM +0800, Ulion wrote:
> > --- ad_hwac3.c
> > +    AV_WB16(buf    , 0xF872);   // iec 61937 syncword 1
> > +    AV_WB16(buf + 2, 0x4E1F);   // iec 61937 syncword 2
> > +    buf[4] = sh_audio->a_in_buffer[5] & 0x7; // bsmod
> > +    buf[5] = 0x01;              // data-type ac3
> > +    AV_WB16(buf + 6, len << 3); // number of bits in payload
> 
> since no byte-order change needed here, maybe we can make it more explicitly.
> + buf[0] = 0xF8;
> + buf[1] = 0x72;
> + buf[2] = 0x4E;
> + buf[3] = 0x1F;
> + buf[4] = sh_audio->a_in_buffer[5] & 0x7; // bsmod
> + buf[5] = 0x01;              // data-type ac3
> + buf[6] = (len >> 5) & 0xFF; // number of bits in payload
> + buf[7] = (len << 3) & 0xFF;
> 
> dts part also make samilar changes, or keep the last one AV_WB16 for
> length as you like.

This is not more explicit, this is _less_ explicit, using AV_WB16 makes it
clear that the AC3 data is a sequence of 16 bit integers, it is _not_ a
byte stream like your code implies.
Using AV_WB16 also makes it obvious why we are creating AF_FORMAT_AC3_BE
and what needs to be changed to create AF_FORMAT_AC3_LE instead.

> I think AV_WB16 is some kind of implicative way to do what we want, it
> even use bswap_16 on little-endian byte-order platform, not directly
> just as assigning byte by byte ourselves.

This is not at all speed critical, so it doesn't really matter how
complicated AV_WB16 is, but it wouldn't be that hard to change AV_WB16
so it does the right thing for constants by using gcc extensions if
someone cares...

Greetings,
Reimar Döffinger