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

[Ramiro Polla]

Hi,

On Fri, May 1, 2009 at 7:52 PM, Michael Niedermayer<michaelni at gmx.at> wrote:
> On Fri, May 01, 2009 at 11:31:38PM +0200, Stefano Sabatini wrote:
>> 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:
>> > > 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?
>
> a quick hack to solve the problem to 95% with minimal work, it was fabrices
> idea, they served us well but yes they should be removed once a more
> powerfull system is available
>
>
>>
>> > 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.
>
> yes
> its just seperating "how to store" from "what is stored"

I have a few questions about all this pixel format separation stuff.

- Would encoders need to specify AVPixFmtDescriptor and AVColorRange
pairs in .pix_fmts?
- Would imgconvert functions mostly only use AVPixFmtDescriptor?
- Would you expect in the future to have functions which convert from
one YUV Color Space to another? (like AVCOL_SPC_BT709 to
AVCOL_SPC_SMPTE170M)

And most importantly how would we pass around pixel format information
with multiple enums? Would we keep the current enum PixelFormat or
would the whole code need to be changed to pass around more than one
information at once? If the latter, do you expect a struct with
something similar to the colorspace macro-family Stefano suggested, or
an int64_t with (AVPixFmtDescriptor << x) | (AVColorRange < y), or ...
?

Ramiro Polla