[FFmpeg-devel] PixFmtInfo cleanup

[Stefano Sabatini]

On date Saturday 2009-03-14 02:13:49 +0100, Michael Niedermayer encoded:
> On Sat, Mar 14, 2009 at 01:30:24AM +0100, Stefano Sabatini wrote:
> > On date Thursday 2009-03-05 00:51:47 +0100, Michael Niedermayer encoded:
> > [...]
> > 
> > After another look:
> > 
> > * currently pixdesc.[hc] contains things like:
> >   PIX_FMT_RGB656_LE
> > 
> >   I think you meant:
> >   PIX_FMT_RGB656LE
> > 
> >   (without the underscore) instead, as it is more consistent with the
> >   others PIX_FMT_* and other stuff in libav*.
> 
> yes the "_" was a unintentional typo

Patch for that, togheter with another little one for expanding lsb in
the docs.
 
> > * Either the PIX_FMT_RGB565_LE description:
> > 
> >     [PIX_FMT_RGB565_LE] = {
> >         .nb_channels  = 3,
> >         .log2_chroma_w= 0,
> >         .log2_chroma_h= 0,
> >         .comp = {
> > 
> > //               shift
> > //                 v
> >             {0,1,1,0,4},
> >             {0,1,1,5,5},
> >             {0,1,2,3,4},
> >         },
> > 
> > is wrong, either the definition of shift:
> > uint16_t shift        :3;            ///< number of lsb that must be shifted away to get the value
> > 
> > in this case it should be the number of *most* significant bits to
> > shift away.
> 
> its possible i xchanged 0 and 3 by mistake

No it's correct, but it puzzled me since I wasn't really understanding
the meaning of offset, also the BGR order looks weird.
 
> > Also what's the interpretation of offset_plus1 if the value is 0?
> > Only example is
> > 
> >      [PIX_FMT_RGB565_BE] = {
> >         .nb_channels  = 3,
> >         .log2_chroma_w= 0,
> >         .log2_chroma_h= 0,
> >         .comp = {
> >             {0,1,1, 0,4},
> >             {0,1,1, 5,5},
> >             {0,1,0, 3,4},
> > //               ^
> > //            offset_plus1
> >         },
> >         .flags = PIX_FMT_BE,
> >     },
> > 
> > when I'd rather expect the same values of the corresponding LE case.
> 
> LE is stores like:
> gggbbbbb rrrrrggg 
> byte0    byte1
> 
> BE is stored like:
> rrrrrggg gggbbbbb 
> byte0    byte1
> 
> 
> for LE 
> blue is offset=0, green is 0, red is 1
> 
> for BE 
> blue is offset=0, green is 0, red is -1
> just try to do the 16bit LE & BE reads you will see this is correct

Uh, got it, it was tricky though...

Maybe a less terse explanation may help, now that I see it the
definition of offset makes sense but it was quite hard to grasp, at
least without reading the read_line() code...

Regards.
-- 
FFmpeg = Faithful and Fierce Moronic Portentous Elitarian Gymnast
-------------- next part --------------
A non-text attachment was scrubbed...
Name: expand-pixdesc-lsb.patch
Type: text/x-diff
Size: 852 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090314/ade6254e/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pixdesc-underscores.patch
Type: text/x-diff
Size: 712 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090314/ade6254e/attachment-0001.patch>