[FFmpeg-devel] [PATCH] vf_colorspace: Add an option to clamp trc LUT output

[Nicolas George]

Drew Dunne via ffmpeg-devel (HE12025-08-14):
> Add a new flag to the vf_colorspace filter which provides the user an
> option to clamp the linear and delinear transfer characteristics LUT
> values to the [0, 1] represented range. This helps constrain the
> potential value range when converting between colorspaces.

Hi. Thanks for the patch. This filter seems orphaned. I will go at it.

> Signed-off-by: Drew Dunne <asdunne at google.com>
> ---
>  libavfilter/vf_colorspace.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)

Missing update to doc/filters.texi.


> 
> diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c
> index e1f4725f63..48aa267c07 100644
> --- a/libavfilter/vf_colorspace.c
> +++ b/libavfilter/vf_colorspace.c
> @@ -123,6 +123,7 @@ typedef struct ColorSpaceContext {
>      int fast_mode;
>      enum DitherMode dither;
>      enum WhitepointAdaptation wp_adapt;

> +    int clamplutoutput;

Other members in the structure have morereadablenames; please use
underscores.

>  
>      int16_t *rgb[3];
>      ptrdiff_t rgb_stride;
> @@ -215,7 +216,9 @@ static int fill_gamma_table(ColorSpaceContext *s)
>          } else {
>              d = out_alpha * pow(v, out_gamma) - (out_alpha - 1.0);
>          }

> -        s->delin_lut[n] = av_clip_int16(lrint(d * 28672.0));
> +        int d_int = av_clip_int16(lrint(d * 28672.0));
> +        s->delin_lut[n] =
> +            s->clamplutoutput ? FFMIN(FFMAX(0, d_int), 28672) : d_int; 

Please use av_clip() instead of chaining FFMIN() and FFMAX().

Also, avoid chaining av_clip_int16() and clipping to a smaller interval.

By the way, can you confirm it makes no significant difference in time?

>  
>          // linearize
>          if (v <= -in_beta * in_delta) {
> @@ -225,7 +228,9 @@ static int fill_gamma_table(ColorSpaceContext *s)
>          } else {
>              l = pow((v + in_alpha - 1.0) * in_ialpha, in_igamma);
>          }
> -        s->lin_lut[n] = av_clip_int16(lrint(l * 28672.0));
> +        int l_int = av_clip_int16(lrint(l * 28672.0));
> +        s->lin_lut[n] =
> +            s->clamplutoutput ? FFMIN(FFMAX(0, l_int), 28672) : l_int; 

Same as above.

>      }
>  
>      return 0;
> @@ -1000,6 +1005,11 @@ static const AVOption colorspace_options[] = {
>      ENUM("vonkries", WP_ADAPT_VON_KRIES, "wpadapt"),
>      ENUM("identity", WP_ADAPT_IDENTITY, "wpadapt"),
>  
> +    { "clamplutoutput", 
> +      "Clamps the linear and delinear LUT output values to the range [0, 1].",
> +      OFFSET(clamplutoutput), AV_OPT_TYPE_BOOL,  { .i64 = 0    },
> +      0, 1, FLAGS },
> +
>      { "iall",       "Set all input color properties together",
>        OFFSET(user_iall),   AV_OPT_TYPE_INT, { .i64 = CS_UNSPECIFIED },
>        CS_UNSPECIFIED, CS_NB - 1, FLAGS, .unit = "all" },

Regards,

-- 
  Nicolas George