[Ffmpeg-devel] PATCH Blackfin UNALIGNED_STORES_ARE_BAD in bitstream.h

[mmh]

Michael Niedermayer writes:
 > Hi
 > 
 > On Tue, Apr 17, 2007 at 11:56:14AM -0400, Rich Felker wrote:
 > > On Tue, Apr 17, 2007 at 05:39:35PM +0200, Michael Niedermayer wrote:
 > > > > Index: bitstream.h
 > > > > ===================================================================
 > > > > --- bitstream.h	(revision 8596)
 > > > > +++ bitstream.h	(working copy)
 > > > > @@ -166,7 +166,7 @@
 > > > >      uint8_t run;
 > > > >  } RL_VLC_ELEM;
 > > > >  
 > > > > -#if defined(ARCH_SPARC) || defined(ARCH_ARMV4L) || defined(ARCH_MIPS)
 > > > > +#if defined(ARCH_SPARC) || defined(ARCH_ARMV4L) || defined(ARCH_MIPS) || defined(ARCH_BFIN)
 > > > >  #define UNALIGNED_STORES_ARE_BAD
 > > > 
 > > > looks ok
 > > 
 > > IMO we should turn this the other way around:
 > > 
 > > #if !defined(ARCH_X86) && !defined(...)
 > > 
 > > Then the code will ALWAYS work, and just be suboptimal on archs we
 > > haven't identified yet, rather than crashing on unknown archs.
 > 
 > i strongly object to this, noone would ever add any architecture besides x86
 > to it, how should anyone even know of this line?
 > there are simlar other things in the code like several differnt bitreaders
 > noone after _years_ has benchmarked them on non x86 with very few exceptions
 > 
 > if it crashes and the person sends a bugrport its trivial to add the arch
 > if its just slower than what it could be nothing will happen it will always
 > stay slow
 > 
 > if configure would print a big warning like
 > WARINING: configure doesnt know your architecture and so has disabled all
 > optimizations, please try the following switches: ... and report bechmarks
 > to ffmpeg-dev so we can automate the optimization selection

This is a great point. It does run the risk of being a hidden
bottleneck. So just apply my previous patch and I will remember to
optimize this later for higher bitrate contents.

While we are on this topic I was looking at the ALT_(READER/WRITER)
stuff.. And that seems to have been broken and is probably not
supported right now is that correct?

Thanks
Marc