[FFmpeg-devel] [PATCH] Add colorspace properties to pixdesc

[Stefano Sabatini]

On date Thursday 2009-04-30 03:34:13 +0200, Michael Niedermayer encoded:
> On Thu, Apr 30, 2009 at 01:49:20AM +0200, Stefano Sabatini wrote:
> > Hi,
> > 
> > this seems required for colorutils.c, just a preliminary patch though.
> > 
> > In order to convert a color from one pixfmt to another one I need to
> > know the colorspace, then I should be able to apply the correct macro
> > from libavcodec/colorspace.h.
> > 
> > Then I'll need to eventually scale each component value, and finally
> > assign it to the correct component (for this I need the AVComponent
> > color).
> > 
> > I'm not sure this is the rigth approach, so before to fill the
> > pixdesc.c table I'd like to hear your opinion.
> 
> ideg
> this patch wins the most broken patch award of the year
> 
> let me explain (it should be obvious but it seems not)
> 
> One of the key goals of the pixfmt structure was 
> _TO_SEPERATE_THE_COLORSPACE_FROM_HOW_THE_BITS_OF_THE_PIXELS_ARE_STORED_
> 
> Why would we want that?
> again should be obvious but is obviously not :(
> These are 2 semmantically seperate things, we will not duplicate the list
> of all yuv formats 12 times because there are 6 yuv types and 2 (jpeg vs mpeg)
> style ranges.

So I wonder what's the point of the PIX_FMT_YUVJ* pixfmts? Should they
be removed?

> So repeating the forgotten discussions ...
> a colorspace enum should be added to AVCodecContext

So we could end up with this:
pixfmt/pixdesc define the disposition, depth, etc. of each component,
but gives no hint for what regards the current color and colorspace of
each component.

So we could have for example a PIX_FMT_YUV420, and the colorspace
could by YUV, YUV_JPEG, YUV_MJPEG, the colorspace info being stored
externally with respect to the pixfmt/pixdesc.

The we could follow some convenction, for which for example with image
in some YUV colorspace the component are also specified in the same
order Y/U/V, the same for R/G/B.

This also means that a pixfmt/pixdesc belongs anyway to some sort of
colorspace macro-family (I can't figure out a better term now),
e.g. YUV and RGB, each one with many variants, maybe would be useful
expose this property anyway.

For example we could have some filter which only work with YUV or RGB
macro-family colorspaces, having this information in the pixdescs may
help to implement the hypotetical make_format_list(int flags)
function.
  
> [...]
> > +enum AVColorspace {
> > +    AV_COLORSPACE_RGB,      /**< RGB color space */
> > +    AV_COLORSPACE_YUV,      /**< YUV color space. 16 <= Y <= 235, 16 <= U, V <= 240 */
> > +    AV_COLORSPACE_YUV_JPEG, /**< YUV color space. 0 <= Y <= 255, 0 <= U, V <= 255 */
> > +    AV_COLORSPACE_NB,
> > +};
> 
> see ff_yuv2rgb_coeffs, the mpeg4 and h264 specs
> there are many yuv types ...
> and refering to <= 255 for something that can be 65535 is not good
> the mpeg* specs and probably h264 have a better definition, though maybe
> theirs is overkill and can be simplified ...

Regards.
-- 
FFmpeg = Frenzy & Freak Mean Puristic Easy Gargoyle