[FFmpeg-devel] [RFC] RGB32 pixfmts cleanup

[Stefano Sabatini]

On date Sunday 2009-03-22 16:45:50 +0100, Michael Niedermayer encoded:
> On Sun, Mar 22, 2009 at 04:20:38PM +0100, Stefano Sabatini wrote:
> > On date Thursday 2009-03-19 01:25:19 +0100, Michael Niedermayer encoded:
> > > On Thu, Mar 19, 2009 at 01:12:46AM +0100, Stefano Sabatini wrote:
> > [...]
> > > [..]
> > > > @@ -91,8 +89,10 @@
> > > >      PIX_FMT_NV12,      ///< planar YUV 4:2:0, 12bpp, 1 plane for Y and 1 for UV
> > > >      PIX_FMT_NV21,      ///< as above, but U and V bytes are swapped
> > > >  
> > > > -    PIX_FMT_RGB32_1,   ///< packed RGB 8:8:8, 32bpp, (msb)8R 8G 8B 8A(lsb), in CPU endianness
> > > > -    PIX_FMT_BGR32_1,   ///< packed RGB 8:8:8, 32bpp, (msb)8B 8G 8R 8A(lsb), in CPU endianness
> > > > +    PIX_FMT_ARGB,      ///< packed RGB 8:8:8, 32bpp, (msb)8A 8R 8G 8B(lsb), in CPU endianness
> > > > +    PIX_FMT_RGBA,      ///< packed RGB 8:8:8, 32bpp, (msb)8R 8G 8B 8A(lsb), in CPU endianness
> > > > +    PIX_FMT_ABGR,      ///< packed RGB 8:8:8, 32bpp, (msb)8A 8B 8G 8R(lsb), in CPU endianness
> > > > +    PIX_FMT_BGRA,      ///< packed RGB 8:8:8, 32bpp, (msb)8B 8G 8R 8A(lsb), in CPU endianness
> > > >  
> > > >      PIX_FMT_GRAY16BE,  ///<        Y        , 16bpp, big-endian
> > > >      PIX_FMT_GRAY16LE,  ///<        Y        , 16bpp, little-endian
> > > 
> > > the description is wrong, these are not in cpu endianness
> > 
> > Hi,
> > 
> > checking again the patch, the define change will broke regression,
> > since "rgb32", "rgb32_1" and friends have different meanings according
> > to the endianess of the system.
> > 
> > So we have:
> > 
> > name      |   BE   |  LE
> > ----------+--------+------
> > "rgb32"   |  ARGB  | BGRA     
> > "rgb32_1" |  RGBA  | ABGR
> > "bgr32"   |  ABGR  | RGBA
> > "bgr32_1" |  BGRA  | ARGB
> > 
> > That is a test for the format "rgba" will work differently in LE and
> > BE systems.
> > 
> > A possible solution would be to extend the avcodec_get_pix_fmt() so
> > that it will recognize also the "rgb32", "rgb32_1", etc. names, and
> > will return the right pixel format according to the previous table.
> 
> its ok to make t recognize "rgb32" "bgr32"
>
> > 
> > A cleaner solution may be to extend the PixFmtInfo/AVPixFmtDecriptor
> > to support also one or more aliases for each pixel format name.
> > Then we could define the various alias for "rgba", "bgra", etc
> > according to the system endianess.
> 
> iam against ading a alias field that will be unused by 98% of the
> entries and will complicate all code

Check the attached patches.

But I continue to prefer the previous solution, while it requires more
space, it's simpler and more extensible, also with it would be easy
to automatically expose the aliases (for example in
avcodec_get_pix_fmt_string), which doesn't look feasible with the second
solution.

Regards.
-- 
FFmpeg = Fostering Faithful Mysterious Programmable EniGma
-------------- next part --------------
A non-text attachment was scrubbed...
Name: extend-get-pix-fmt-name.patch
Type: text/x-diff
Size: 998 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090322/c30db470/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clean-rgba-pixfmts.patch
Type: text/x-diff
Size: 5579 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090322/c30db470/attachment-0001.patch>