[FFmpeg-devel] [PATCH 1/2] avfilter/vf_blackdetect: support full-range YUV

[FFmpeg-devel] [PATCH 1/2] avfilter/vf_blackdetect: support full-range YUV

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

This filter currently makes the distinction between limited and full
range by testing for the deprecated YUVJ pixel formats at link setup
time. This is deprecated and should be improved to perform the detection
based on the per-frame metadata.

Rewrite it to calculate the black pixel threshold at the time of
filtering a frame, when metadata about the frame's color range is known.
Doing it this way has the small side benefit of being able to handle
streams with variable metadata, and is not a meaningful performance
cost.

Signed-off-by: Niklas Haas <git@haasn.dev>
---
 libavfilter/vf_blackdetect.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/libavfilter/vf_blackdetect.c b/libavfilter/vf_blackdetect.c
index 99ff1ac606..c937248169 100644
--- a/libavfilter/vf_blackdetect.c
+++ b/libavfilter/vf_blackdetect.c
@@ -102,8 +102,6 @@ static int config_input(AVFilterLink *inlink)
     BlackDetectContext *s = ctx->priv;
     const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format);
     const int depth = desc->comp[0].depth;
-    const int max = (1 << depth) - 1;
-    const int factor = (1 << (depth - 8));
 
     s->depth = depth;
     s->nb_threads = ff_filter_get_nb_threads(ctx);
@@ -113,16 +111,10 @@ static int config_input(AVFilterLink *inlink)
     if (!s->counter)
         return AVERROR(ENOMEM);
 
-    s->pixel_black_th_i = ff_fmt_is_in(inlink->format, yuvj_formats) ?
-        // luminance_minimum_value + pixel_black_th * luminance_range_size
-             s->pixel_black_th *  max :
-        16 * factor + s->pixel_black_th * (235 - 16) * factor;
-
     av_log(s, AV_LOG_VERBOSE,
-           "black_min_duration:%s pixel_black_th:%f pixel_black_th_i:%d picture_black_ratio_th:%f\n",
+           "black_min_duration:%s pixel_black_th:%f picture_black_ratio_th:%f\n",
            av_ts2timestr(s->black_min_duration, &s->time_base),
-           s->pixel_black_th, s->pixel_black_th_i,
-           s->picture_black_ratio_th);
+           s->pixel_black_th, s->picture_black_ratio_th);
     return 0;
 }
 
@@ -182,6 +174,14 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
     AVFilterContext *ctx = inlink->dst;
     BlackDetectContext *s = ctx->priv;
     double picture_black_ratio = 0;
+    const int max = (1 << s->depth) - 1;
+    const int factor = (1 << (s->depth - 8));
+    const int full = picref->color_range == AVCOL_RANGE_JPEG ||
+                     ff_fmt_is_in(picref->format, yuvj_formats);
+
+    s->pixel_black_th_i = full ? s->pixel_black_th * max :
+        // luminance_minimum_value + pixel_black_th * luminance_range_size
+        16 * factor + s->pixel_black_th * (235 - 16) * factor;
 
     ff_filter_execute(ctx, black_counter, picref, NULL,
                       FFMIN(inlink->h, s->nb_threads));
-- 
2.38.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/2] avfilter/vf_tinterlace: support full-range YUV

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

This filter, when used in the "pad" mode, currently makes the
distinction between limited and full range solely by testing for YUVJ
pixel formats at link setup time. This is deprecated and should be
improved to perform the detection based on the per-frame metadata.

In order to make this distinction based on color range metadata, which
is only known at the time of filtering frames, for simplicity, we simply
allocate two copies of the "black" frame - one for limited range and the
other for full range metadata. This could be done more dynamically (e.g.
as-needed or simply by blitting the appropriate pixel value directly),
but this change is relatively simple and preserves the structure of the
existing code.

This commit actually fixes a bug in FATE - the new output is correct for
the first time. The previous md5 ref was of a frame that incorrectly
combined full-range pixel data with limited-range black fields. The
corresponding result has been updated.

Signed-off-by: Niklas Haas <git@haasn.dev>
---
 libavfilter/tinterlace.h                     |  2 +-
 libavfilter/vf_tinterlace.c                  | 26 ++++++++++++++------
 tests/ref/fate/filter-pixfmts-tinterlace_pad |  2 +-
 3 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/libavfilter/tinterlace.h b/libavfilter/tinterlace.h
index 4059ebf81a..37b6c10c08 100644
--- a/libavfilter/tinterlace.h
+++ b/libavfilter/tinterlace.h
@@ -70,7 +70,7 @@ typedef struct TInterlaceContext {
     int vsub;                   ///< chroma vertical subsampling
     AVFrame *cur;
     AVFrame *next;
-    uint8_t *black_data[4];     ///< buffer used to fill padded lines
+    uint8_t *black_data[2][4];  ///< buffer used to fill padded lines (limited/full)
     int black_linesize[4];
     FFDrawContext draw;
     FFDrawColor color;
diff --git a/libavfilter/vf_tinterlace.c b/libavfilter/vf_tinterlace.c
index 7c54861de4..032629279a 100644
--- a/libavfilter/vf_tinterlace.c
+++ b/libavfilter/vf_tinterlace.c
@@ -201,7 +201,8 @@ static av_cold void uninit(AVFilterContext *ctx)
 
     av_frame_free(&tinterlace->cur );
     av_frame_free(&tinterlace->next);
-    av_freep(&tinterlace->black_data[0]);
+    av_freep(&tinterlace->black_data[0][0]);
+    av_freep(&tinterlace->black_data[1][0]);
 }
 
 static int config_out_props(AVFilterLink *outlink)
@@ -225,14 +226,22 @@ static int config_out_props(AVFilterLink *outlink)
         int ret;
         ff_draw_init(&tinterlace->draw, outlink->format, 0);
         ff_draw_color(&tinterlace->draw, &tinterlace->color, black);
-        if (ff_fmt_is_in(outlink->format, full_scale_yuvj_pix_fmts))
-            tinterlace->color.comp[0].u8[0] = 0;
-        ret = av_image_alloc(tinterlace->black_data, tinterlace->black_linesize,
+        /* limited range */
+        if (!ff_fmt_is_in(outlink->format, full_scale_yuvj_pix_fmts)) {
+            ret = av_image_alloc(tinterlace->black_data[0], tinterlace->black_linesize,
+                                 outlink->w, outlink->h, outlink->format, 16);
+            if (ret < 0)
+                return ret;
+            ff_fill_rectangle(&tinterlace->draw, &tinterlace->color, tinterlace->black_data[0],
+                              tinterlace->black_linesize, 0, 0, outlink->w, outlink->h);
+        }
+        /* full range */
+        tinterlace->color.comp[0].u8[0] = 0;
+        ret = av_image_alloc(tinterlace->black_data[1], tinterlace->black_linesize,
                              outlink->w, outlink->h, outlink->format, 16);
         if (ret < 0)
             return ret;
-
-        ff_fill_rectangle(&tinterlace->draw, &tinterlace->color, tinterlace->black_data,
+        ff_fill_rectangle(&tinterlace->draw, &tinterlace->color, tinterlace->black_data[1],
                           tinterlace->black_linesize, 0, 0, outlink->w, outlink->h);
     }
     if (tinterlace->flags & (TINTERLACE_FLAG_VLPF | TINTERLACE_FLAG_CVLPF)
@@ -360,7 +369,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
     AVFilterLink *outlink = ctx->outputs[0];
     TInterlaceContext *tinterlace = ctx->priv;
     AVFrame *cur, *next, *out;
-    int field, tff, ret;
+    int field, tff, full, ret;
 
     av_frame_free(&tinterlace->cur);
     tinterlace->cur  = tinterlace->next;
@@ -418,6 +427,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
         out->sample_aspect_ratio = av_mul_q(cur->sample_aspect_ratio, av_make_q(2, 1));
 
         field = (1 + outlink->frame_count_in) & 1 ? FIELD_UPPER : FIELD_LOWER;
+        full = out->color_range == AVCOL_RANGE_JPEG || ff_fmt_is_in(out->format, full_scale_yuvj_pix_fmts);
         /* copy upper and lower fields */
         copy_picture_field(tinterlace, out->data, out->linesize,
                            (const uint8_t **)cur->data, cur->linesize,
@@ -425,7 +435,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
                            FIELD_UPPER_AND_LOWER, 1, field, tinterlace->flags);
         /* pad with black the other field */
         copy_picture_field(tinterlace, out->data, out->linesize,
-                           (const uint8_t **)tinterlace->black_data, tinterlace->black_linesize,
+                           (const uint8_t **)tinterlace->black_data[full], tinterlace->black_linesize,
                            inlink->format, inlink->w, inlink->h,
                            FIELD_UPPER_AND_LOWER, 1, !field, tinterlace->flags);
         break;
diff --git a/tests/ref/fate/filter-pixfmts-tinterlace_pad b/tests/ref/fate/filter-pixfmts-tinterlace_pad
index 81a6961215..29321e542b 100644
--- a/tests/ref/fate/filter-pixfmts-tinterlace_pad
+++ b/tests/ref/fate/filter-pixfmts-tinterlace_pad
@@ -1,4 +1,4 @@
-gray                7ef396fecd8d1c9fe32173e4415ba671
+gray                227a6fe36a31fbef80210823454131ea
 yuv410p             35bc11d0d32efc9e9a969be7d720f4e6
 yuv411p             17ef3cd22a74f7368b5e02f68779f294
 yuv420p             93d5b6a4c44d67e4d4447e8dd0bf3d33
-- 
2.38.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] avfilter/vf_blackdetect: support full-range YUV

[Niklas Haas]

Any objections to these two patches?

On Fri, 09 Dec 2022 01:28:05 +0100 Niklas Haas <ffmpeg@haasn.xyz> wrote:
> From: Niklas Haas <git@haasn.dev>
> 
> This filter currently makes the distinction between limited and full
> range by testing for the deprecated YUVJ pixel formats at link setup
> time. This is deprecated and should be improved to perform the detection
> based on the per-frame metadata.
> 
> Rewrite it to calculate the black pixel threshold at the time of
> filtering a frame, when metadata about the frame's color range is known.
> Doing it this way has the small side benefit of being able to handle
> streams with variable metadata, and is not a meaningful performance
> cost.
> 
> Signed-off-by: Niklas Haas <git@haasn.dev>
> ---
>  libavfilter/vf_blackdetect.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/libavfilter/vf_blackdetect.c b/libavfilter/vf_blackdetect.c
> index 99ff1ac606..c937248169 100644
> --- a/libavfilter/vf_blackdetect.c
> +++ b/libavfilter/vf_blackdetect.c
> @@ -102,8 +102,6 @@ static int config_input(AVFilterLink *inlink)
>      BlackDetectContext *s = ctx->priv;
>      const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format);
>      const int depth = desc->comp[0].depth;
> -    const int max = (1 << depth) - 1;
> -    const int factor = (1 << (depth - 8));
>  
>      s->depth = depth;
>      s->nb_threads = ff_filter_get_nb_threads(ctx);
> @@ -113,16 +111,10 @@ static int config_input(AVFilterLink *inlink)
>      if (!s->counter)
>          return AVERROR(ENOMEM);
>  
> -    s->pixel_black_th_i = ff_fmt_is_in(inlink->format, yuvj_formats) ?
> -        // luminance_minimum_value + pixel_black_th * luminance_range_size
> -             s->pixel_black_th *  max :
> -        16 * factor + s->pixel_black_th * (235 - 16) * factor;
> -
>      av_log(s, AV_LOG_VERBOSE,
> -           "black_min_duration:%s pixel_black_th:%f pixel_black_th_i:%d picture_black_ratio_th:%f\n",
> +           "black_min_duration:%s pixel_black_th:%f picture_black_ratio_th:%f\n",
>             av_ts2timestr(s->black_min_duration, &s->time_base),
> -           s->pixel_black_th, s->pixel_black_th_i,
> -           s->picture_black_ratio_th);
> +           s->pixel_black_th, s->picture_black_ratio_th);
>      return 0;
>  }
>  
> @@ -182,6 +174,14 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
>      AVFilterContext *ctx = inlink->dst;
>      BlackDetectContext *s = ctx->priv;
>      double picture_black_ratio = 0;
> +    const int max = (1 << s->depth) - 1;
> +    const int factor = (1 << (s->depth - 8));
> +    const int full = picref->color_range == AVCOL_RANGE_JPEG ||
> +                     ff_fmt_is_in(picref->format, yuvj_formats);
> +
> +    s->pixel_black_th_i = full ? s->pixel_black_th * max :
> +        // luminance_minimum_value + pixel_black_th * luminance_range_size
> +        16 * factor + s->pixel_black_th * (235 - 16) * factor;
>  
>      ff_filter_execute(ctx, black_counter, picref, NULL,
>                        FFMIN(inlink->h, s->nb_threads));
> -- 
> 2.38.1
> 
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] avfilter/vf_tinterlace: support full-range YUV

[Thomas Mundt]

Am Fr., 9. Dez. 2022 um 01:28 Uhr schrieb Niklas Haas <ffmpeg@haasn.xyz>:

> From: Niklas Haas <git@haasn.dev>
>
> This filter, when used in the "pad" mode, currently makes the
> distinction between limited and full range solely by testing for YUVJ
> pixel formats at link setup time. This is deprecated and should be
> improved to perform the detection based on the per-frame metadata.
>
> In order to make this distinction based on color range metadata, which
> is only known at the time of filtering frames, for simplicity, we simply
> allocate two copies of the "black" frame - one for limited range and the
> other for full range metadata. This could be done more dynamically (e.g.
> as-needed or simply by blitting the appropriate pixel value directly),
> but this change is relatively simple and preserves the structure of the
> existing code.
>
> This commit actually fixes a bug in FATE - the new output is correct for
> the first time. The previous md5 ref was of a frame that incorrectly
> combined full-range pixel data with limited-range black fields. The
> corresponding result has been updated.
>
> Signed-off-by: Niklas Haas <git@haasn.dev>
> ---
>  libavfilter/tinterlace.h                     |  2 +-
>  libavfilter/vf_tinterlace.c                  | 26 ++++++++++++++------
>  tests/ref/fate/filter-pixfmts-tinterlace_pad |  2 +-
>  3 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/libavfilter/tinterlace.h b/libavfilter/tinterlace.h
> index 4059ebf81a..37b6c10c08 100644
> --- a/libavfilter/tinterlace.h
> +++ b/libavfilter/tinterlace.h
> @@ -70,7 +70,7 @@ typedef struct TInterlaceContext {
>      int vsub;                   ///< chroma vertical subsampling
>      AVFrame *cur;
>      AVFrame *next;
> -    uint8_t *black_data[4];     ///< buffer used to fill padded lines
> +    uint8_t *black_data[2][4];  ///< buffer used to fill padded lines
> (limited/full)
>      int black_linesize[4];
>      FFDrawContext draw;
>      FFDrawColor color;
> diff --git a/libavfilter/vf_tinterlace.c b/libavfilter/vf_tinterlace.c
> index 7c54861de4..032629279a 100644
> --- a/libavfilter/vf_tinterlace.c
> +++ b/libavfilter/vf_tinterlace.c
> @@ -201,7 +201,8 @@ static av_cold void uninit(AVFilterContext *ctx)
>
>      av_frame_free(&tinterlace->cur );
>      av_frame_free(&tinterlace->next);
> -    av_freep(&tinterlace->black_data[0]);
> +    av_freep(&tinterlace->black_data[0][0]);
> +    av_freep(&tinterlace->black_data[1][0]);
>  }
>
>  static int config_out_props(AVFilterLink *outlink)
> @@ -225,14 +226,22 @@ static int config_out_props(AVFilterLink *outlink)
>          int ret;
>          ff_draw_init(&tinterlace->draw, outlink->format, 0);
>          ff_draw_color(&tinterlace->draw, &tinterlace->color, black);
> -        if (ff_fmt_is_in(outlink->format, full_scale_yuvj_pix_fmts))
> -            tinterlace->color.comp[0].u8[0] = 0;
> -        ret = av_image_alloc(tinterlace->black_data,
> tinterlace->black_linesize,
> +        /* limited range */
> +        if (!ff_fmt_is_in(outlink->format, full_scale_yuvj_pix_fmts)) {
> +            ret = av_image_alloc(tinterlace->black_data[0],
> tinterlace->black_linesize,
> +                                 outlink->w, outlink->h, outlink->format,
> 16);
> +            if (ret < 0)
> +                return ret;
> +            ff_fill_rectangle(&tinterlace->draw, &tinterlace->color,
> tinterlace->black_data[0],
> +                              tinterlace->black_linesize, 0, 0,
> outlink->w, outlink->h);
> +        }
> +        /* full range */
> +        tinterlace->color.comp[0].u8[0] = 0;
> +        ret = av_image_alloc(tinterlace->black_data[1],
> tinterlace->black_linesize,
>                               outlink->w, outlink->h, outlink->format, 16);
>          if (ret < 0)
>              return ret;
> -
> -        ff_fill_rectangle(&tinterlace->draw, &tinterlace->color,
> tinterlace->black_data,
> +        ff_fill_rectangle(&tinterlace->draw, &tinterlace->color,
> tinterlace->black_data[1],
>                            tinterlace->black_linesize, 0, 0, outlink->w,
> outlink->h);
>      }
>      if (tinterlace->flags & (TINTERLACE_FLAG_VLPF | TINTERLACE_FLAG_CVLPF)
> @@ -360,7 +369,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *picref)
>      AVFilterLink *outlink = ctx->outputs[0];
>      TInterlaceContext *tinterlace = ctx->priv;
>      AVFrame *cur, *next, *out;
> -    int field, tff, ret;
> +    int field, tff, full, ret;
>
>      av_frame_free(&tinterlace->cur);
>      tinterlace->cur  = tinterlace->next;
> @@ -418,6 +427,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *picref)
>          out->sample_aspect_ratio = av_mul_q(cur->sample_aspect_ratio,
> av_make_q(2, 1));
>
>          field = (1 + outlink->frame_count_in) & 1 ? FIELD_UPPER :
> FIELD_LOWER;
> +        full = out->color_range == AVCOL_RANGE_JPEG ||
> ff_fmt_is_in(out->format, full_scale_yuvj_pix_fmts);
>          /* copy upper and lower fields */
>          copy_picture_field(tinterlace, out->data, out->linesize,
>                             (const uint8_t **)cur->data, cur->linesize,
> @@ -425,7 +435,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *picref)
>                             FIELD_UPPER_AND_LOWER, 1, field,
> tinterlace->flags);
>          /* pad with black the other field */
>          copy_picture_field(tinterlace, out->data, out->linesize,
> -                           (const uint8_t **)tinterlace->black_data,
> tinterlace->black_linesize,
> +                           (const uint8_t
> **)tinterlace->black_data[full], tinterlace->black_linesize,
>                             inlink->format, inlink->w, inlink->h,
>                             FIELD_UPPER_AND_LOWER, 1, !field,
> tinterlace->flags);
>          break;
> diff --git a/tests/ref/fate/filter-pixfmts-tinterlace_pad
> b/tests/ref/fate/filter-pixfmts-tinterlace_pad
> index 81a6961215..29321e542b 100644
> --- a/tests/ref/fate/filter-pixfmts-tinterlace_pad
> +++ b/tests/ref/fate/filter-pixfmts-tinterlace_pad
> @@ -1,4 +1,4 @@
> -gray                7ef396fecd8d1c9fe32173e4415ba671
> +gray                227a6fe36a31fbef80210823454131ea
>  yuv410p             35bc11d0d32efc9e9a969be7d720f4e6
>  yuv411p             17ef3cd22a74f7368b5e02f68779f294
>  yuv420p             93d5b6a4c44d67e4d4447e8dd0bf3d33
> --
> 2.38.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>

Looks OK for me.

Regards,
Thomas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".