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

[Rostislav Pehlivanov]

On 1 May 2018 at 22:06, Paul B Mahol <onemda at gmail.com> wrote:

> On 5/1/18, 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.
>
> Then why are they deprecated at all?
>
> There is even warning displayed all the time.
>
> Not mentioning that color range does not work at all unless explicitly
> managed.
>
> Because you are not proposing solution, and just complaining, I will
> ignore this mail.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

I agree with you, they need to be gone. The purpose of the patchset also
isn't just to get rid of them, its to have a good api to handle color
ranges and how it ought to be handled by filters and codecs. His only
objection was that there's a stigma to adding more fields to avctx, which
can't be avoided in this case _because_ the color range is an essential
property of images.