[Ffmpeg-devel] [PATCH] BMP encoder

[Michel Bardiaux]

Michael Niedermayer wrote:
> Hi
> 
> On Tue, Oct 31, 2006 at 02:27:10PM +0100, Michel Bardiaux wrote:
>> Michael Niedermayer wrote:
>>> Hi
>>>
>>> On Mon, Oct 30, 2006 at 06:50:35PM +0100, Michel Bardiaux wrote:
>>> [...]
>>>
>>>> Note that I have some doubts about that code: it seems to assume that IF 
>>>> the compiler is not gcc THEN alignment is not an issue for 16 or 32 bits 
>>>> store. That is clearly wrong. Is dsputil.h supposed to be included by 
>>>> client code? 
>>> not really, at least ST*() isnt supposed to be used ...
>> Well, they'll be moved anyway, see other message. But I've just 
>> remembered that in some app I call imgconvert directly, no codec or 
>> whatever involved, and for that to work I had to call dsputil_init 
>> directly, and for that include dsputil.h.
>>
>> Move the prototype for dsputil_init to avcodec.h? Or call it from 
>> imgconvert (and maybe other places)?
> 
> as imgconert will be replaced by swscaler, i see no sense in fixing 
> such things ...

Then why is the current drive to relicense swscaler limited to the 
non-SIMD part?

> 
> 
>>>
>>>> If not, then why the 'else not GNUC clause?
>>> because we do support a few other compilers ...
>> But if dsputil.h is NOT supposed to be exported, then it has only to 
>> support gcc.
> 
> no because we do support other compilers, gcc is not the only c compiler 
> on the planet, we do not support c++ compilers but thats a differnt matter

Mmm, yes, gcc has been the only more-or-less C9X compiler for so long 
that one tends to forget there may be others (BTW do you know one?)

> 
> 
>> Besides, the non-gcc part is simply wrong: it assumes a naive unaligned 
>> store will work, and that's false on many architectures.
> 
> yes, this should be fixed with a #ifdef ARCH_X86 around the code and a
> slow byte per byte reading for the non gcc non x86 case

Then we agree.

> 
> [...]


-- 
Michel Bardiaux
R&D Director
T +32 [0] 2 790 29 41
F +32 [0] 2 790 29 02
E mailto:mbardiaux at mediaxim.be

Mediaxim NV/SA
Vorstlaan 191 Boulevard du Souverain
Brussel 1160 Bruxelles
http://www.mediaxim.com/