[FFmpeg-devel] [PATCH 01/27] avcodec: add color_range to AVCodec struct and use it

[wm4]

On Mon, 11 Dec 2017 12:07:34 +0100
Paul B Mahol <onemda at gmail.com> wrote:

> On 12/11/17, wm4 <nfxjfg at googlemail.com> wrote:
> > On Sat,  9 Dec 2017 16:37:53 +0100
> > Paul B Mahol <onemda at gmail.com> wrote:
> >  
> >> Signed-off-by: Paul B Mahol <onemda at gmail.com>
> >> ---
> >>  libavcodec/avcodec.h | 1 +
> >>  libavcodec/utils.c   | 2 ++
> >>  2 files changed, 3 insertions(+)
> >>
> >> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> >> index 5db6a81320..e5de4797c8 100644
> >> --- a/libavcodec/avcodec.h
> >> +++ b/libavcodec/avcodec.h
> >> @@ -3376,6 +3376,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}
> >> +    int color_range;                        ///< supported color range by
> >> encoder, 0 means any is supported  
> >
> > I'd prefer if the doxygen said AVCOL_RANGE_UNSPECIFIED.
> >
> > Actually, I'd prefer if this were an array.  
> 
> What for? It can be MPEG or JPEG or UNKNOWN (one of previous two)
> 
> >
> > Actually, I'd prefer even more if there were some nicer way to export
> > supported encoder parameters, as currently it's mostly guess work on
> > the API user side (or hardcoding knowledge).
> >  
> >>
> >>      /*****************************************************************
> >>       * No fields below this line are part of the public API. They
> >> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> >> index baf09119fe..aa81c21ef3 100644
> >> --- a/libavcodec/utils.c
> >> +++ b/libavcodec/utils.c
> >> @@ -879,6 +879,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >>                  avctx->codec->pix_fmts[i] == AV_PIX_FMT_YUVJ444P)
> >>                  avctx->color_range = AVCOL_RANGE_JPEG;
> >>          }
> >> +        if (avctx->codec->color_range)
> >> +            avctx->color_range = avctx->codec->color_range;
> >>          if (avctx->codec->supported_samplerates) {
> >>              for (i = 0; avctx->codec->supported_samplerates[i] != 0; i++)
> >>                  if (avctx->sample_rate ==
> >> avctx->codec->supported_samplerates[i])  
> >
> > Fine, but it's inevitable that the encoder supports the J formats still
> > for a while.  
> 
> 
> Why are you all dismissive about this?

Well, to be honest, I don't mind if this whole J* format change
suddenly breaks all API users, but others might be not so happy about
it.