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

[Ramiro Ribeiro Polla]

Michael Niedermayer wrote:
> Hi
>
> On Tue, Apr 17, 2007 at 01:13:04PM -0400, Rich Felker wrote:
>   
>> On Tue, Apr 17, 2007 at 06:43:34PM +0200, Michael Niedermayer wrote:
>>     
>>> 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?
>>>       
>> It can be documented in a porting file or something. But I strongly
>> object to having blatently nonportable code in the default build. If I
>> were trying to compile ffmpeg on a strange system and not a competent
>> coder/debugger, I would have no idea why it crashed and probably just
>> assume it was nonportable crapware rather than learning how to
>> investigate and "fix" the problem.
>>
>> An few alternatives/compromise ideas:
>>
>> 1. Enable the nonportable code by default but have a
>> --disable-nonportable-assumptions option to configure, or an
>> ARCH_GENERIC define included in the above list that configure would
>> define when the arch is unknown to inhibit all such optimizations.
>>
>> 2. Just have configure print a message directing users to a porting
>> document with references to this line and other similar lines when an
>> unknown arch is detected.
>>
>>     
>
> iam fine with these, iam just strongly against silently disabling it
>
>
>   
This is also being discussed in 
http://article.gmane.org/gmane.comp.video.ffmpeg.devel/48933

I'm just waiting for an answer to my questions at the end of the e-mail 
to make the changes and send a patch.

Ramiro Polla