[Ffmpeg-devel] Re: [PATCH] Machine endian bytestream functions

[Ramiro Ribeiro Polla]

Hello,

[...]

M?ns Rullg?rd wrote:
> Ramiro Ribeiro Polla <ramiro at lisha.ufsc.br> writes:
>   
>> Do you suggest this test be dropped and we make a list of
>> architectures where unaligned access is possible and somewhat faster
>> than doing the shifting and adding? (this is what the previously
>> ok'd patch did)
>>     
>
> No, I'm suggesting this:
>
>   
>>> I'm thinking the unaligned16/32/64 macros from bitstream.h should be
>>> renamed and moved here instead.
>>>
>>>       
>> I'll take a look.
>>     
>
> This lets the compiler choose whatever access method is appropriate,
> and saves us the trouble of maintaining a fragile list.
>
>   

I didn't know what that __packed__ was doing there. But now I checked 
with an arm compiler...

This seems like the best alternative.

There are 2 macros/functions that do the same thing. LD, defined in 
intreadwrite.h, and unaligned, defined in bitstream.h

LD(16|32|64) are used in libavcodec/dsputil.[hc], libavcodec/sh4/qpel.c
unaligned(16|32|64) are used in libavcodec/mpegvideo.c, 
libavcodec/cavs.c, libavformat/oggparsevorbis.c and 
libavformat/oggparseogm.c
all uses of unaligned are the same as AV_R[LB], except in cavs.c. I'll 
probably send a patch for this.
ST(16|32|64) are used in libavcodec/dsputil.h

bitstream.h:
UNALIGNED_STORES_ARE_BAD is defined for some arches, but I think it 
isn't necessary.
unaligned reads are made with packed structs in compilers that allow it, 
or else it just reads normally.
the ARCH_X86 #ifdef isn't necessary, since gcc seems to know that x86 
accepts unaligned reads.

intreadwrite.h makes unaligned reads with packed structs with GNUC only, 
and also reads normally on other compilers.
intreadwrite.h implements the packed structs aesthetically better.

I think it would be best to fallback to reading bytes one at a time 
(like intreadwrite.h does) instead of using a normal read.

Before I start fiddling with this again, where is the preferred location 
and name for these functions?
I'd prefer unaligned in intreadwrite.h, and change all LD to unaligned.

Ramiro Polla