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

[Michael Niedermayer]

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 ...

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 ...

now iam still not sure what exactly you are complaining about and maybe even
you yourself dont know that ;)

is the names you disslike?
is it the fact that some code plain doesnt work with your favorite pix_fmt?
is it that native endian pix_fmt are available instead of littering the code
with #ifdef WORDS_BIGENDIAN ?

the lack of native endian indepandant names cant it be as there simply is no
such lack and the one or 2 missed pix_fmts if there are any can trivially
be added non native #define based names ...

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070409/bcfab2b4/attachment.pgp>