[FFmpeg-devel,1/2] avfilter: steps to ditch YUVJ formats

Submitted by Paul B Mahol on Dec. 8, 2017, 11:12 a.m.

Details

Message ID 20171208111258.29346-1-onemda@gmail.com
State New
Headers show

Commit Message

Paul B Mahol Dec. 8, 2017, 11:12 a.m.
Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 fftools/ffmpeg.c         |  1 +
 fftools/ffmpeg.h         |  2 ++
 fftools/ffmpeg_filter.c  | 18 +++++++++++++++---
 libavcodec/avcodec.h     |  1 +
 libavcodec/utils.c       |  2 ++
 libavfilter/avfilter.c   |  2 ++
 libavfilter/avfilter.h   |  2 ++
 libavfilter/buffersink.c |  1 +
 libavfilter/buffersink.h |  1 +
 libavfilter/buffersrc.c  | 13 +++++++++++++
 libavfilter/buffersrc.h  |  5 +++++
 libavfilter/vf_scale.c   |  9 +++++++++
 libavfilter/video.c      |  8 +++++++-
 13 files changed, 61 insertions(+), 4 deletions(-)

Comments

Michael Niedermayer Dec. 8, 2017, 8:39 p.m.
On Fri, Dec 08, 2017 at 12:12:57PM +0100, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  fftools/ffmpeg.c         |  1 +
>  fftools/ffmpeg.h         |  2 ++
>  fftools/ffmpeg_filter.c  | 18 +++++++++++++++---
>  libavcodec/avcodec.h     |  1 +
>  libavcodec/utils.c       |  2 ++
>  libavfilter/avfilter.c   |  2 ++
>  libavfilter/avfilter.h   |  2 ++
>  libavfilter/buffersink.c |  1 +
>  libavfilter/buffersink.h |  1 +
>  libavfilter/buffersrc.c  | 13 +++++++++++++
>  libavfilter/buffersrc.h  |  5 +++++
>  libavfilter/vf_scale.c   |  9 +++++++++
>  libavfilter/video.c      |  8 +++++++-
>  13 files changed, 61 insertions(+), 4 deletions(-)
[...]

> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 5db6a81320..4ff5fe8aa1 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}
> +    const int color_range;                  ///< value of color range encoder supports, 0 if all is supported
>  
>      /*****************************************************************
>       * No fields below this line are part of the public API. They

Looking just at libavcodec (i didnt look indepth at the rest)

the API looks ok i think

an alternative may be a list similar to others that lists supported
ranges and we would have differen values for 8bit and 10bit full and
subset ranges. That way codecs could list support for one without the
other. But that may be overkill 


> 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])

If the user did set a avctx->color_range for an encoder then this
should warn and or fail if its unsupported instead of overriding
at least at higher strict_std_compliance

[...]
Michael Niedermayer Dec. 8, 2017, 8:48 p.m.
On Fri, Dec 08, 2017 at 12:12:57PM +0100, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  fftools/ffmpeg.c         |  1 +
>  fftools/ffmpeg.h         |  2 ++
>  fftools/ffmpeg_filter.c  | 18 +++++++++++++++---
>  libavcodec/avcodec.h     |  1 +
>  libavcodec/utils.c       |  2 ++
>  libavfilter/avfilter.c   |  2 ++
>  libavfilter/avfilter.h   |  2 ++
>  libavfilter/buffersink.c |  1 +
>  libavfilter/buffersink.h |  1 +
>  libavfilter/buffersrc.c  | 13 +++++++++++++
>  libavfilter/buffersrc.h  |  5 +++++
>  libavfilter/vf_scale.c   |  9 +++++++++
>  libavfilter/video.c      |  8 +++++++-
>  13 files changed, 61 insertions(+), 4 deletions(-)

as a more general comment,
If doing all this across all libs is too hard, doing one lib at a
time could be easier.

That is fully support color_range without yuvj in a lib while
providing support for deprecated yuvj formats.

that lib could be tested by some hack that intercepts yuvj and translates
it to color_range, such hack would of course not be commited

then do the next lib

just a random idea, if it simplifies something that would be great,
if not just ignore this mail

thx

[...]
Rostislav Pehlivanov Dec. 9, 2017, 1:36 a.m.
On 8 December 2017 at 20:48, Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Fri, Dec 08, 2017 at 12:12:57PM +0100, Paul B Mahol wrote:
> > Signed-off-by: Paul B Mahol <onemda@gmail.com>
> > ---
> >  fftools/ffmpeg.c         |  1 +
> >  fftools/ffmpeg.h         |  2 ++
> >  fftools/ffmpeg_filter.c  | 18 +++++++++++++++---
> >  libavcodec/avcodec.h     |  1 +
> >  libavcodec/utils.c       |  2 ++
> >  libavfilter/avfilter.c   |  2 ++
> >  libavfilter/avfilter.h   |  2 ++
> >  libavfilter/buffersink.c |  1 +
> >  libavfilter/buffersink.h |  1 +
> >  libavfilter/buffersrc.c  | 13 +++++++++++++
> >  libavfilter/buffersrc.h  |  5 +++++
> >  libavfilter/vf_scale.c   |  9 +++++++++
> >  libavfilter/video.c      |  8 +++++++-
> >  13 files changed, 61 insertions(+), 4 deletions(-)
>
> as a more general comment,
> If doing all this across all libs is too hard, doing one lib at a
> time could be easier.
>
> That is fully support color_range without yuvj in a lib while
> providing support for deprecated yuvj formats.
>
>
There's no point in keeping them. We're still in the middle of the bump and
if we have to we'll delay the bump until the YUVJ stuff is gone.

Patch hide | download patch | download mbox

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 6aff3366c5..4edf3ee52f 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -1864,6 +1864,7 @@  static void flush_encoders(void)
                         ifilter->width                  = par->width;
                         ifilter->height                 = par->height;
                         ifilter->sample_aspect_ratio    = par->sample_aspect_ratio;
+                        ifilter->color_range            = par->color_range;
                     }
                 }
 
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 4e73d59082..1ad0b1eff1 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -248,6 +248,7 @@  typedef struct InputFilter {
     int format;
 
     int width, height;
+    int color_range;
     AVRational sample_aspect_ratio;
 
     int sample_rate;
@@ -271,6 +272,7 @@  typedef struct OutputFilter {
 
     /* desired output stream properties */
     int width, height;
+    int color_range;
     AVRational frame_rate;
     int format;
     int sample_rate;
diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index 877fd670e6..3a279106a2 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -449,6 +449,7 @@  static int configure_output_video_filter(FilterGraph *fg, OutputFilter *ofilter,
     OutputStream *ost = ofilter->ost;
     OutputFile    *of = output_files[ost->file_index];
     AVFilterContext *last_filter = out->filter_ctx;
+    AVDictionaryEntry *cre = NULL;
     int pad_idx = out->pad_idx;
     int ret;
     char name[255];
@@ -461,7 +462,9 @@  static int configure_output_video_filter(FilterGraph *fg, OutputFilter *ofilter,
     if (ret < 0)
         return ret;
 
-    if (ofilter->width || ofilter->height) {
+    cre = av_dict_get(ost->encoder_opts, "color_range", NULL, 0);
+
+    if (ofilter->width || ofilter->height || (cre && cre->value) || ost->enc->color_range) {
         char args[255];
         AVFilterContext *filter;
         AVDictionaryEntry *e = NULL;
@@ -474,6 +477,12 @@  static int configure_output_video_filter(FilterGraph *fg, OutputFilter *ofilter,
             av_strlcatf(args, sizeof(args), ":%s=%s", e->key, e->value);
         }
 
+        if (cre && cre->value) {
+            av_strlcatf(args, sizeof(args), ":out_range=%s", cre->value);
+        } else if (ost->enc->color_range) {
+            av_strlcatf(args, sizeof(args), ":out_range=%s", av_color_range_name(ost->enc->color_range));
+        }
+
         snprintf(name, sizeof(name), "scaler_out_%d_%d",
                  ost->file_index, ost->index);
         if ((ret = avfilter_graph_create_filter(&filter, avfilter_get_by_name("scale"),
@@ -777,10 +786,11 @@  static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter,
     av_bprint_init(&args, 0, 1);
     av_bprintf(&args,
              "video_size=%dx%d:pix_fmt=%d:time_base=%d/%d:"
-             "pixel_aspect=%d/%d:sws_param=flags=%d",
+             "pixel_aspect=%d/%d:sws_param=flags=%d:color_range=%s",
              ifilter->width, ifilter->height, ifilter->format,
              tb.num, tb.den, sar.num, sar.den,
-             SWS_BILINEAR + ((ist->dec_ctx->flags&AV_CODEC_FLAG_BITEXACT) ? SWS_BITEXACT:0));
+             SWS_BILINEAR + ((ist->dec_ctx->flags&AV_CODEC_FLAG_BITEXACT) ? SWS_BITEXACT:0),
+             av_color_range_name(ist->dec_ctx->color_range));
     if (fr.num && fr.den)
         av_bprintf(&args, ":frame_rate=%d/%d", fr.num, fr.den);
     snprintf(name, sizeof(name), "graph %d input from stream %d:%d", fg->index,
@@ -1110,6 +1120,7 @@  int configure_filtergraph(FilterGraph *fg)
 
         ofilter->width  = av_buffersink_get_w(sink);
         ofilter->height = av_buffersink_get_h(sink);
+        ofilter->color_range = av_buffersink_get_color_range(sink);
 
         ofilter->sample_rate    = av_buffersink_get_sample_rate(sink);
         ofilter->channel_layout = av_buffersink_get_channel_layout(sink);
@@ -1182,6 +1193,7 @@  int ifilter_parameters_from_frame(InputFilter *ifilter, const AVFrame *frame)
     ifilter->width               = frame->width;
     ifilter->height              = frame->height;
     ifilter->sample_aspect_ratio = frame->sample_aspect_ratio;
+    ifilter->color_range         = frame->color_range;
 
     ifilter->sample_rate         = frame->sample_rate;
     ifilter->channels            = frame->channels;
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 5db6a81320..4ff5fe8aa1 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}
+    const int color_range;                  ///< value of color range encoder supports, 0 if all is supported
 
     /*****************************************************************
      * 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])
diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
index b98b32bacb..4a579bb49d 100644
--- a/libavfilter/avfilter.c
+++ b/libavfilter/avfilter.c
@@ -336,6 +336,8 @@  int avfilter_config_links(AVFilterContext *filter)
                         link->w = inlink->w;
                     if (!link->h)
                         link->h = inlink->h;
+                    if (!link->color_range)
+                        link->color_range = inlink->color_range;
                 } else if (!link->w || !link->h) {
                     av_log(link->src, AV_LOG_ERROR,
                            "Video source filters must set their output link's "
diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
index 47546c15e5..a5f4df74de 100644
--- a/libavfilter/avfilter.h
+++ b/libavfilter/avfilter.h
@@ -448,6 +448,8 @@  struct AVFilterLink {
      */
     AVRational time_base;
 
+    int color_range;
+
     /*****************************************************************
      * All fields below this line are not part of the public API. They
      * may not be used outside of libavfilter and can be changed and
diff --git a/libavfilter/buffersink.c b/libavfilter/buffersink.c
index 0f87b5439a..5f3c932d92 100644
--- a/libavfilter/buffersink.c
+++ b/libavfilter/buffersink.c
@@ -194,6 +194,7 @@  MAKE_AVFILTERLINK_ACCESSOR(AVRational       , frame_rate         )
 MAKE_AVFILTERLINK_ACCESSOR(int              , w                  )
 MAKE_AVFILTERLINK_ACCESSOR(int              , h                  )
 MAKE_AVFILTERLINK_ACCESSOR(AVRational       , sample_aspect_ratio)
+MAKE_AVFILTERLINK_ACCESSOR(int              , color_range        )
 
 MAKE_AVFILTERLINK_ACCESSOR(int              , channels           )
 MAKE_AVFILTERLINK_ACCESSOR(uint64_t         , channel_layout     )
diff --git a/libavfilter/buffersink.h b/libavfilter/buffersink.h
index 21d6bb505b..c0acee48d6 100644
--- a/libavfilter/buffersink.h
+++ b/libavfilter/buffersink.h
@@ -114,6 +114,7 @@  AVRational       av_buffersink_get_frame_rate          (const AVFilterContext *c
 int              av_buffersink_get_w                   (const AVFilterContext *ctx);
 int              av_buffersink_get_h                   (const AVFilterContext *ctx);
 AVRational       av_buffersink_get_sample_aspect_ratio (const AVFilterContext *ctx);
+int              av_buffersink_get_color_range         (const AVFilterContext *ctx);
 
 int              av_buffersink_get_channels            (const AVFilterContext *ctx);
 uint64_t         av_buffersink_get_channel_layout      (const AVFilterContext *ctx);
diff --git a/libavfilter/buffersrc.c b/libavfilter/buffersrc.c
index cd56f8ca45..67f8bdf158 100644
--- a/libavfilter/buffersrc.c
+++ b/libavfilter/buffersrc.c
@@ -52,6 +52,7 @@  typedef struct BufferSourceContext {
     int               w, h;
     enum AVPixelFormat  pix_fmt;
     AVRational        pixel_aspect;
+    int               color_range;
     char              *sws_param;
 
     AVBufferRef *hw_frames_ctx;
@@ -109,6 +110,8 @@  int av_buffersrc_parameters_set(AVFilterContext *ctx, AVBufferSrcParameters *par
             s->h = param->height;
         if (param->sample_aspect_ratio.num > 0 && param->sample_aspect_ratio.den > 0)
             s->pixel_aspect = param->sample_aspect_ratio;
+        if (param->color_range > 0)
+            s->color_range = param->color_range;
         if (param->frame_rate.num > 0 && param->frame_rate.den > 0)
             s->frame_rate = param->frame_rate;
         if (param->hw_frames_ctx) {
@@ -309,6 +312,15 @@  static const AVOption buffer_options[] = {
     { "time_base",     NULL,                     OFFSET(time_base),        AV_OPT_TYPE_RATIONAL, { .dbl = 0 }, 0, DBL_MAX, V },
     { "frame_rate",    NULL,                     OFFSET(frame_rate),       AV_OPT_TYPE_RATIONAL, { .dbl = 0 }, 0, DBL_MAX, V },
     { "sws_param",     NULL,                     OFFSET(sws_param),        AV_OPT_TYPE_STRING,                    .flags = V },
+    { "color_range",   NULL,                     OFFSET(color_range),      AV_OPT_TYPE_INT,      { .i64 = 0 }, 0, INT_MAX, V, "range" },
+    { "unspecified",   NULL,   0, AV_OPT_TYPE_CONST, {.i64=AVCOL_RANGE_UNSPECIFIED},  0, 0, V, "range"},
+    { "unknown",       NULL,   0, AV_OPT_TYPE_CONST, {.i64=AVCOL_RANGE_UNSPECIFIED},  0, 0, V, "range"},
+    { "limited",       NULL,   0, AV_OPT_TYPE_CONST, {.i64=AVCOL_RANGE_MPEG},         0, 0, V, "range"},
+    { "tv",            NULL,   0, AV_OPT_TYPE_CONST, {.i64=AVCOL_RANGE_MPEG},         0, 0, V, "range"},
+    { "mpeg",          NULL,   0, AV_OPT_TYPE_CONST, {.i64=AVCOL_RANGE_MPEG},         0, 0, V, "range"},
+    { "full",          NULL,   0, AV_OPT_TYPE_CONST, {.i64=AVCOL_RANGE_JPEG},         0, 0, V, "range"},
+    { "pc",            NULL,   0, AV_OPT_TYPE_CONST, {.i64=AVCOL_RANGE_JPEG},         0, 0, V, "range"},
+    { "jpeg",          NULL,   0, AV_OPT_TYPE_CONST, {.i64=AVCOL_RANGE_JPEG},         0, 0, V, "range"},
     { NULL },
 };
 
@@ -434,6 +446,7 @@  static int config_props(AVFilterLink *link)
         link->w = c->w;
         link->h = c->h;
         link->sample_aspect_ratio = c->pixel_aspect;
+        link->color_range = c->color_range;
 
         if (c->hw_frames_ctx) {
             link->hw_frames_ctx = av_buffer_ref(c->hw_frames_ctx);
diff --git a/libavfilter/buffersrc.h b/libavfilter/buffersrc.h
index 0652113f2b..1004121c4e 100644
--- a/libavfilter/buffersrc.h
+++ b/libavfilter/buffersrc.h
@@ -114,6 +114,11 @@  typedef struct AVBufferSrcParameters {
      * Audio only, the audio channel layout
      */
     uint64_t channel_layout;
+
+   /**
+     * Video only,
+     */
+    int color_range;
 } AVBufferSrcParameters;
 
 /**
diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index 2fd9c90d84..fdb94ed7eb 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -303,9 +303,15 @@  static int config_props(AVFilterLink *outlink)
             if (scale->in_range != AVCOL_RANGE_UNSPECIFIED)
                 av_opt_set_int(*s, "src_range",
                                scale->in_range == AVCOL_RANGE_JPEG, 0);
+            else
+                av_opt_set_int(*s, "src_range",
+                               inlink->color_range == AVCOL_RANGE_JPEG, 0);
             if (scale->out_range != AVCOL_RANGE_UNSPECIFIED)
                 av_opt_set_int(*s, "dst_range",
                                scale->out_range == AVCOL_RANGE_JPEG, 0);
+           else
+                av_opt_set_int(*s, "dst_range",
+                               outlink->color_range == AVCOL_RANGE_JPEG, 0);
 
             if (scale->opts) {
                 AVDictionaryEntry *e = NULL;
@@ -416,6 +422,7 @@  static int filter_frame(AVFilterLink *link, AVFrame *in)
     if(   in->width  != link->w
        || in->height != link->h
        || in->format != link->format
+       || in->color_range != link->color_range
        || in->sample_aspect_ratio.den != link->sample_aspect_ratio.den || in->sample_aspect_ratio.num != link->sample_aspect_ratio.num) {
         int ret;
 
@@ -429,6 +436,7 @@  static int filter_frame(AVFilterLink *link, AVFrame *in)
         link->dst->inputs[0]->format = in->format;
         link->dst->inputs[0]->w      = in->width;
         link->dst->inputs[0]->h      = in->height;
+        link->dst->inputs[0]->color_range = in->color_range;
 
         link->dst->inputs[0]->sample_aspect_ratio.den = in->sample_aspect_ratio.den;
         link->dst->inputs[0]->sample_aspect_ratio.num = in->sample_aspect_ratio.num;
@@ -578,6 +586,7 @@  static const AVOption scale_options[] = {
     {  "in_range", "set input color range",  OFFSET( in_range), AV_OPT_TYPE_INT, {.i64 = AVCOL_RANGE_UNSPECIFIED }, 0, 2, FLAGS, "range" },
     { "out_range", "set output color range", OFFSET(out_range), AV_OPT_TYPE_INT, {.i64 = AVCOL_RANGE_UNSPECIFIED }, 0, 2, FLAGS, "range" },
     { "auto",   NULL, 0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_RANGE_UNSPECIFIED }, 0, 0, FLAGS, "range" },
+    { "unknown", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_RANGE_UNSPECIFIED }, 0, 0, FLAGS, "range" },
     { "full",   NULL, 0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_RANGE_JPEG}, 0, 0, FLAGS, "range" },
     { "jpeg",   NULL, 0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_RANGE_JPEG}, 0, 0, FLAGS, "range" },
     { "mpeg",   NULL, 0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_RANGE_MPEG}, 0, 0, FLAGS, "range" },
diff --git a/libavfilter/video.c b/libavfilter/video.c
index 6f9020b9fe..8f12cb7080 100644
--- a/libavfilter/video.c
+++ b/libavfilter/video.c
@@ -43,6 +43,7 @@  AVFrame *ff_null_get_video_buffer(AVFilterLink *link, int w, int h)
 
 AVFrame *ff_default_get_video_buffer(AVFilterLink *link, int w, int h)
 {
+    AVFrame *frame = NULL;
     int pool_width = 0;
     int pool_height = 0;
     int pool_align = 0;
@@ -86,7 +87,12 @@  AVFrame *ff_default_get_video_buffer(AVFilterLink *link, int w, int h)
         }
     }
 
-    return ff_frame_pool_get(link->frame_pool);
+    frame = ff_frame_pool_get(link->frame_pool);
+    if (!frame)
+        return NULL;
+    frame->color_range = link->color_range;
+
+    return frame;
 }
 
 AVFrame *ff_get_video_buffer(AVFilterLink *link, int w, int h)