[Ffmpeg-devel] [PATCH/RFC] 1-7 and 9-15 bits per pixel PGM files

Baptiste Coudurier baptiste.coudurier
Mon Apr 9 23:51:16 CEST 2007


Michael Niedermayer a ?crit :
> Hi
> On Mon, Apr 09, 2007 at 07:32:13PM +0200, Baptiste Coudurier wrote:
>> Hi
>> Michael Niedermayer a ?crit :
>>> [...]
>>>> +        }
>>>> +    } else if (avctx->pix_fmt == PIX_FMT_GRAY16BE && s->bpp < 16) {
>>>> +        unsigned int j, val;
>>>> +        for(ptr = p->data[0], i = 0; i < avctx->height; i++, ptr += 
>>>> linesize) {
>>>> +            for(j = 0; j < avctx->width; j++) {
>>>> +                val = AV_RB16(&ptr[j<<1]);
>>>> +                val = val<<(16-s->bpp) | val>>(s->bpp-(16-s->bpp));
>>>> +                AV_WB16(&ptr[j<<1], val);
>>> 16bit formats should be in native endian internally (for obvious reasons)
>> Is it speed reason ?
>> pix_fmt IS external IMHO.
>> This "native" endian pix_fmt exporting is IMHO broken, and history prove 
>> that it is useless with RGB32 variants, since AVI and MOV used 
>> respectivly inversed order. Im not that wrong extrapolating that to 
>> GRAY16/ME/LE/BE, Im sure I can find exact same problem in tiff/targa/dpx.
>> Now in swscale you got pix_fmt only supported on one arch. 
>> PIX_FMT_RGB32_1 used by MOV, while clear name is obvious: 
>> PIX_FMT_RGBA8888 (not sure about right order for rgba else abgr, didnt 
>> double check specs), 32 might be used if it appears obvious that each 
>> comp will have 8 bits.
>> Correct design is to name it according to what endianness is stored INTO 
>> the file, and implement routines that deals with it, assigning correct 
>> function pointers at init, if "native" endianness can be used.
>> Since Im being ignored on this issue, just my 2 cents.
> you are being ignored as what you say makes no sense
> formats ARE named in both native endian and stored variants with #ifdefs
> and #defines setting one to the other depening on host endianness
> so you can use enirely host endian independant names if you wish ...

Well I agree that I may be confused myself ;)

> the namimg also is fairly consistent and you would likely agree if you
> tried to name all using a different scheme but as you look at just 1 out
> of 20 pix formats it surely looks confusing, try to name rgb555 & rgb565
> based on what is bytewise stored for example ...

Yes, mainly because those 2 (RGB32 and RGB656) are the only ones which I 
have issues with.

> now iam still not sure what exactly you are complaining about and maybe even
> you yourself dont know that ;)
> is the names you disslike?

About some, yes

> is it the fact that some code plain doesnt work with your favorite pix_fmt?

Well, even if that is true, that would be up to me to support it ;)

> is it that native endian pix_fmt are available instead of littering the code
> with #ifdef WORDS_BIGENDIAN ?

I might agree with that, but that is done atm is more complicated than 
simply avoiding define's.

What I don't like:



4 defines for 4 pixfmt is useless. Those can be specified correctly in 
the code !
Also RGBA is clearly better that BGR32_1 !

1 define for 2 pix_fmt is useful, I agree:
#define PIX_FMT_GRAY16 for LE/BE

Exporting NATIVE pix_fmt, if we solve RGB32 issue, only RGB555/565 will 
be left, and it will be the same issue as RGB32 since mov uses 555 on 16 
bits. About the naming, well I need to think, but I'll find something 
sooner or later, I never give up ;)


Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
checking for life_signs in -lkenny... no

More information about the ffmpeg-devel mailing list