[FFmpeg-devel] [PATCH 01/24] avcodec: add color_range to AVCodec struct

wm4 nfxjfg at googlemail.com
Wed May 2 05:40:31 EEST 2018


On Tue,  1 May 2018 16:46:37 -0400
Vittorio Giovara <vittorio.giovara at gmail.com> wrote:

> --
> 
> On 5/1/2018 4:39 PM, Paul B Mahol wrote:
> > Signed-off-by: Paul B Mahol <onemda at gmail.com>
> > ---
> >  libavcodec/avcodec.h | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index fb0c6fae70..3a8f69243c 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -3433,6 +3433,7 @@ typedef struct AVCodec {
> >      uint8_t max_lowres;                     ///< maximum value for lowres supported by the decoder
> >      const AVClass *priv_class;              ///< AVClass for the private context
> >      const AVProfile *profiles;              ///< array of recognized profiles, or NULL if unknown, array is terminated by {FF_PROFILE_UNKNOWN}
> > +    const enum AVColorRange *color_ranges;  ///< array of supported color ranges by encoder, or NULL if unknown, array is terminated by AVCOL_RANGE_UNSPECIFIED
> >  
> >      /**
> >       * Group name of the codec implementation.
> >   
> 
> While I appreciate this effort to remove the year-long deprecated J-pixel
> format, I have a feeling that this is not the right way to do it.
> 
> I have no doubt that the code presented here works as expected, however
> adding YetAnotherField to the AVCodec structure is a slippery road. Namely
> only adding color range as an extra feature of the pixel format is incomplete.
> 
> We should add every other single color parameter in this structure, insert
> the appropriate conversion filters (which swscale is not fully able to
> produce) and handle correct format negotiation across the chain, which is
> not exactly trivial (ie. which color space should be favoured? which one has
> a larger gamut? and so on).
> 
> This creates a precedent that, I think, is bad API-wise and user-wise.
> I would rather keep the J-format around than creating a years long user-facing
> problem. Additionally having this field (or these fields if we are to include
> the other color properties) around makes the intrisic problem even harder
> to properly fix.
> 
> I am aware that the goal of this patchset is not to properly support all
> color conversion properties, and it's just to being able to drop the J-formats,
> but I hope that this feedback will give a larger picture of the problem
> involved, that it will help you in making the right decision about this set.

I think the patch at hand is OK if it can finally get rid of the J
formats. To me as an API users these actually cause nothing but
annoyance, so it's really great to get rid of them.

Sure, this doesn't solve negotiating of video attributes in
libavfilter, or accurately exposing all encoder capabilities. There are
plenty of other such cases (ask if you want examples). Solving this in
a general and complete way is _hard_. It's not just about adding
missing fields. On a side note, we should think of something that
doesn't require us duplicating all these metadata fields in AVCodec,
AVCodecContext, AVFrame, avfilter buffer sink, avfilter buffer src, and
having to touch every single to "negotiate" the attribute.

So just adding support for color range for the sole purpose of removing
the J formats sounds good to me.

What do you suggest?


More information about the ffmpeg-devel mailing list