[FFmpeg-devel] [PATCH v2] avfilter/vf_scale: translate and verify swscale internal range flag

[Michael Niedermayer]

On Wed, Sep 16, 2020 at 11:18:48PM +0300, Jan Ekström wrote:
> This value - while it looks like the actual range of the content -
> is nothing but the internal value of swscale.
> 
> Thus, if we have RGB content, force the value to 1. Swscale will
> ignore it, but at least the value of the output AVFrame will now
> properly be "full range" instead of "limited range", as it is right
> now.
> 
> Additionally, after calling sws_setColorspaceDetails double-check
> the configured internal flag for the color range. Warn if this is
> different to the requested value.
> 
> Finally, utilize the translated configured output value into the
> output AVFrame's color_range.
> ---
>  libavfilter/vf_scale.c | 51 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index 58eee96744..592e4a344e 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -750,11 +750,30 @@ scale:
>          || in_range != AVCOL_RANGE_UNSPECIFIED
>          || scale->out_range != AVCOL_RANGE_UNSPECIFIED) {
>          int in_full, out_full, brightness, contrast, saturation;
> +        int configured_in_full_range_flag, configured_out_full_range_flag;
>          const int *inv_table, *table;
> +        const AVPixFmtDescriptor *in_desc  = av_pix_fmt_desc_get(in->format);
> +        const AVPixFmtDescriptor *out_desc = av_pix_fmt_desc_get(out->format);

> +        if (!in_desc || !out_desc) {
> +            av_log(ctx, AV_LOG_ERROR,
> +                   "Failed to get one or more of the pixel format descriptors "
> +                   "for formats - in: %d (%s), out: %d (%s)!\n",
> +                   in->format, in_desc ? "OK" : "bad",
> +                   out->format, out_desc ? "OK": "bad");
> +            av_frame_free(&in);
> +            av_frame_free(frame_out);
> +            return AVERROR_INVALIDDATA;
> +        }

can this be true ?


>  
>          sws_getColorspaceDetails(scale->sws, (int **)&inv_table, &in_full,
>                                   (int **)&table, &out_full,
>                                   &brightness, &contrast, &saturation);

> +        // translate the swscale internal range flags to hold true
> +        // for RGB

The range field of AVFrame is documented as
"MPEG vs JPEG YUV range."

so it is not defined for RGB and cannot be "wrong" for RGB
maybe iam nitpicking here but if you want to use the range for RGB
the API docs need to be changed first.

<super nitpick>I mean you could set it to 1 for RGB thats ok without a API
docs change. But writing in a comment that one way is correct for RGB is not
compatible with the current documentation</super nitpick>

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The day soldiers stop bringing you their problems is the day you have stopped 
leading them. They have either lost confidence that you can help or concluded 
you do not care. Either case is a failure of leadership. - Colin Powell
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200917/23c6dcf5/attachment.sig>