Message ID | 20200920103656.24728-1-jeebjp@gmail.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,v2] avfilter/vf_scale: translate and verify swscale internal range flag | expand |
Context | Check | Description |
---|---|---|
andriy/default | pending | |
andriy/make | success | Make finished |
andriy/make_fate | success | Make fate finished |
On Sun, Sep 20, 2020 at 01:36:56PM +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, translate the value to 1 which is what > at least this filter expects for RGB. Swscale will ignore this value > when set. > > 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 as the > output AVFrame's color_range. > --- > libavfilter/vf_scale.c | 44 +++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 43 insertions(+), 1 deletion(-) > > diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c > index 58eee96744..da8ce399cf 100644 > --- a/libavfilter/vf_scale.c > +++ b/libavfilter/vf_scale.c > @@ -647,6 +647,8 @@ static int scale_slice(AVFilterLink *link, AVFrame *out_buf, AVFrame *cur_pic, s > out,out_stride); > } > > +// swscale's internal range flag is 0 for RGB, which we have to override > +#define NORMALIZE_SWS_RANGE(format_flags, sws_range) (((format_flags) & AV_PIX_FMT_FLAG_RGB) ? 1 : (sws_range)) > static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out) > { > AVFilterContext *ctx = link->dst; > @@ -750,11 +752,19 @@ 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); > + av_assert0(in_desc && out_desc); > > sws_getColorspaceDetails(scale->sws, (int **)&inv_table, &in_full, > (int **)&table, &out_full, > &brightness, &contrast, &saturation); > + // translate the internal range flags according to this > + // filter's expectations for RGB. > + in_full = NORMALIZE_SWS_RANGE(in_desc->flags, in_full); > + out_full = NORMALIZE_SWS_RANGE(out_desc->flags, out_full); > > if (scale->in_color_matrix) > inv_table = parse_yuv_type(scale->in_color_matrix, in->colorspace); > @@ -782,7 +792,39 @@ scale: > table, out_full, > brightness, contrast, saturation); > > - out->color_range = out_full ? AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG; > + // double-check what was actually just configured, > + // since swscale can silently ignore the color range > + // value in sws_setColorspaceDetails. > + sws_getColorspaceDetails(scale->sws, (int **)&inv_table, > + &configured_in_full_range_flag, > + (int **)&table, > + &configured_out_full_range_flag, > + &brightness, &contrast, &saturation); > + > + // translate the actually configured internal range flags according > + // to this filter's expectations for RGB. > + configured_in_full_range_flag = \ > + NORMALIZE_SWS_RANGE(in_desc->flags, > + configured_in_full_range_flag); > + configured_out_full_range_flag = \ > + NORMALIZE_SWS_RANGE(out_desc->flags, > + configured_out_full_range_flag); > + > + if (in_full != configured_in_full_range_flag || > + out_full != configured_out_full_range_flag) { > + av_log(ctx, AV_LOG_WARNING, > + "swscale overrode set input/output range value as it " > + "considered it an invalid configuration! " > + "(input: requested: %s, configured: %s), " > + "(output: requested: %s, configured: %s)!\n", > + in_full ? "full" : "limited", > + configured_in_full_range_flag ? "full" : "limited", > + out_full ? "full" : "limited", > + configured_out_full_range_flag ? "full" : "limited"); > + } If i read this correctly, please correct me if i misread this swscale is setup with some ranges and ignores it. Is there a reason why swscale should not print a warning in this case ? iam asking as you print this from one user of swscale. wouldnt this be relevant to other users too ? thx [...]
On Sun, Sep 20, 2020 at 6:03 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Sun, Sep 20, 2020 at 01:36:56PM +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, translate the value to 1 which is what > > at least this filter expects for RGB. Swscale will ignore this value > > when set. > > > > 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 as the > > output AVFrame's color_range. > > --- > > libavfilter/vf_scale.c | 44 +++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 43 insertions(+), 1 deletion(-) > > > > diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c > > index 58eee96744..da8ce399cf 100644 > > --- a/libavfilter/vf_scale.c > > +++ b/libavfilter/vf_scale.c > > @@ -647,6 +647,8 @@ static int scale_slice(AVFilterLink *link, AVFrame *out_buf, AVFrame *cur_pic, s > > out,out_stride); > > } > > > > +// swscale's internal range flag is 0 for RGB, which we have to override > > +#define NORMALIZE_SWS_RANGE(format_flags, sws_range) (((format_flags) & AV_PIX_FMT_FLAG_RGB) ? 1 : (sws_range)) > > static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out) > > { > > AVFilterContext *ctx = link->dst; > > @@ -750,11 +752,19 @@ 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); > > + av_assert0(in_desc && out_desc); > > > > sws_getColorspaceDetails(scale->sws, (int **)&inv_table, &in_full, > > (int **)&table, &out_full, > > &brightness, &contrast, &saturation); > > + // translate the internal range flags according to this > > + // filter's expectations for RGB. > > + in_full = NORMALIZE_SWS_RANGE(in_desc->flags, in_full); > > + out_full = NORMALIZE_SWS_RANGE(out_desc->flags, out_full); > > > > if (scale->in_color_matrix) > > inv_table = parse_yuv_type(scale->in_color_matrix, in->colorspace); > > @@ -782,7 +792,39 @@ scale: > > table, out_full, > > brightness, contrast, saturation); > > > > - out->color_range = out_full ? AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG; > > + // double-check what was actually just configured, > > + // since swscale can silently ignore the color range > > + // value in sws_setColorspaceDetails. > > + sws_getColorspaceDetails(scale->sws, (int **)&inv_table, > > + &configured_in_full_range_flag, > > + (int **)&table, > > + &configured_out_full_range_flag, > > + &brightness, &contrast, &saturation); > > + > > + // translate the actually configured internal range flags according > > + // to this filter's expectations for RGB. > > + configured_in_full_range_flag = \ > > + NORMALIZE_SWS_RANGE(in_desc->flags, > > + configured_in_full_range_flag); > > + configured_out_full_range_flag = \ > > + NORMALIZE_SWS_RANGE(out_desc->flags, > > + configured_out_full_range_flag); > > + > > + if (in_full != configured_in_full_range_flag || > > + out_full != configured_out_full_range_flag) { > > + av_log(ctx, AV_LOG_WARNING, > > + "swscale overrode set input/output range value as it " > > + "considered it an invalid configuration! " > > + "(input: requested: %s, configured: %s), " > > + "(output: requested: %s, configured: %s)!\n", > > + in_full ? "full" : "limited", > > + configured_in_full_range_flag ? "full" : "limited", > > + out_full ? "full" : "limited", > > + configured_out_full_range_flag ? "full" : "limited"); > > + } > > If i read this correctly, please correct me if i misread this > swscale is setup with some ranges and ignores it. > Is there a reason why swscale should not print a warning in this case ? > iam asking as you print this from one user of swscale. > wouldnt this be relevant to other users too ? > Yes, but this is a bit different. In this case we compare the translated value (so if we got ignored in swscale because we tried to set 1 for full range for RGB, that by itself is not a warning). This will only warn if you f.ex. tried to set the value of limited range for RGB, and then as we translate swscale's internal value of 0 to 1 for RGB, you would get a warning since limited range RGB is not actually supported. Of course, me or someone else might have found this a bit more quickly if sws_setColorspaceDetails would indeed log a warning when it would override the value pushed to it. Alternatively, this could be partially fixed if sws_getColorspaceDetails would translate the internal value to 1 for RGB after taking it in from the internal configuration. Then the API user wouldn't have to translate the value for RGB to match how swscale assumes things to be. I am just not sure how deep I want to jump into swscale with this. Jan
diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c index 58eee96744..da8ce399cf 100644 --- a/libavfilter/vf_scale.c +++ b/libavfilter/vf_scale.c @@ -647,6 +647,8 @@ static int scale_slice(AVFilterLink *link, AVFrame *out_buf, AVFrame *cur_pic, s out,out_stride); } +// swscale's internal range flag is 0 for RGB, which we have to override +#define NORMALIZE_SWS_RANGE(format_flags, sws_range) (((format_flags) & AV_PIX_FMT_FLAG_RGB) ? 1 : (sws_range)) static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out) { AVFilterContext *ctx = link->dst; @@ -750,11 +752,19 @@ 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); + av_assert0(in_desc && out_desc); sws_getColorspaceDetails(scale->sws, (int **)&inv_table, &in_full, (int **)&table, &out_full, &brightness, &contrast, &saturation); + // translate the internal range flags according to this + // filter's expectations for RGB. + in_full = NORMALIZE_SWS_RANGE(in_desc->flags, in_full); + out_full = NORMALIZE_SWS_RANGE(out_desc->flags, out_full); if (scale->in_color_matrix) inv_table = parse_yuv_type(scale->in_color_matrix, in->colorspace); @@ -782,7 +792,39 @@ scale: table, out_full, brightness, contrast, saturation); - out->color_range = out_full ? AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG; + // double-check what was actually just configured, + // since swscale can silently ignore the color range + // value in sws_setColorspaceDetails. + sws_getColorspaceDetails(scale->sws, (int **)&inv_table, + &configured_in_full_range_flag, + (int **)&table, + &configured_out_full_range_flag, + &brightness, &contrast, &saturation); + + // translate the actually configured internal range flags according + // to this filter's expectations for RGB. + configured_in_full_range_flag = \ + NORMALIZE_SWS_RANGE(in_desc->flags, + configured_in_full_range_flag); + configured_out_full_range_flag = \ + NORMALIZE_SWS_RANGE(out_desc->flags, + configured_out_full_range_flag); + + if (in_full != configured_in_full_range_flag || + out_full != configured_out_full_range_flag) { + av_log(ctx, AV_LOG_WARNING, + "swscale overrode set input/output range value as it " + "considered it an invalid configuration! " + "(input: requested: %s, configured: %s), " + "(output: requested: %s, configured: %s)!\n", + in_full ? "full" : "limited", + configured_in_full_range_flag ? "full" : "limited", + out_full ? "full" : "limited", + configured_out_full_range_flag ? "full" : "limited"); + } + + out->color_range = configured_out_full_range_flag ? + AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG; } av_reduce(&out->sample_aspect_ratio.num, &out->sample_aspect_ratio.den,