[FFmpeg-devel] [PATCH 4/8] avfilter/vf_scale: properly respect to input colorimetry
Niklas Haas
ffmpeg at haasn.xyz
Fri Oct 27 20:04:42 EEST 2023
From: Niklas Haas <git at haasn.dev>
This code, as written, skips sws_init_context if scale->in_range was not
set, even if scale->in_frame_range is set to something. So this would
hit the 'no sws context' fast path in scale_frame and skip color range
conversion even where technically required. This had the effect of, in
practice, effectively applying limited/full range conversion if and only
if you set e.g. a nonzero yuv color matrix, which is obviously
undesirable behavior.
Second, the assignment of out->color_range inside the branch would fail
to properly propagate tags for any actually applied conversion, for
example on conversion to a forced full range format.
Solve both of these problems and make the code much easier to understand
and follow by doing the following:
1. Always initialize sws context on get_props
2. Always use sws_getColorspaceDetails + sws_setColorspaceDetails to
fix the conversion matrices per-frame.
3. Rather than testing if the context exists, do the no-op test after
settling the above values and deciding what conversion to actually
perform.
---
libavfilter/vf_scale.c | 186 +++++++++++++++++------------------------
1 file changed, 76 insertions(+), 110 deletions(-)
diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index 23335cef4b..19c91cb86e 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -143,7 +143,6 @@ typedef struct ScaleContext {
char *out_color_matrix;
int in_range;
- int in_frame_range;
int out_range;
int out_h_chr_pos;
@@ -356,8 +355,6 @@ static av_cold int init(AVFilterContext *ctx)
if (!threads)
av_opt_set_int(scale->sws_opts, "threads", ff_filter_get_nb_threads(ctx), 0);
- scale->in_frame_range = AVCOL_RANGE_UNSPECIFIED;
-
return 0;
}
@@ -520,6 +517,7 @@ static int config_props(AVFilterLink *outlink)
const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format);
const AVPixFmtDescriptor *outdesc = av_pix_fmt_desc_get(outfmt);
ScaleContext *scale = ctx->priv;
+ struct SwsContext **swscs[3] = {&scale->sws, &scale->isws[0], &scale->isws[1]};
uint8_t *flags_val = NULL;
int ret;
@@ -552,65 +550,46 @@ static int config_props(AVFilterLink *outlink)
if (scale->isws[1])
sws_freeContext(scale->isws[1]);
scale->isws[0] = scale->isws[1] = scale->sws = NULL;
- if (inlink0->w == outlink->w &&
- inlink0->h == outlink->h &&
- !scale->out_color_matrix &&
- scale->in_range == scale->out_range &&
- inlink0->format == outlink->format)
- ;
- else {
- struct SwsContext **swscs[3] = {&scale->sws, &scale->isws[0], &scale->isws[1]};
- int i;
-
- for (i = 0; i < 3; i++) {
- int in_v_chr_pos = scale->in_v_chr_pos, out_v_chr_pos = scale->out_v_chr_pos;
- struct SwsContext *const s = sws_alloc_context();
- if (!s)
- return AVERROR(ENOMEM);
- *swscs[i] = s;
-
- ret = av_opt_copy(s, scale->sws_opts);
- if (ret < 0)
- return ret;
- av_opt_set_int(s, "srcw", inlink0 ->w, 0);
- av_opt_set_int(s, "srch", inlink0 ->h >> !!i, 0);
- av_opt_set_int(s, "src_format", inlink0->format, 0);
- av_opt_set_int(s, "dstw", outlink->w, 0);
- av_opt_set_int(s, "dsth", outlink->h >> !!i, 0);
- av_opt_set_int(s, "dst_format", outfmt, 0);
- if (scale->in_range != AVCOL_RANGE_UNSPECIFIED)
- av_opt_set_int(s, "src_range",
- scale->in_range == AVCOL_RANGE_JPEG, 0);
- else if (scale->in_frame_range != AVCOL_RANGE_UNSPECIFIED)
- av_opt_set_int(s, "src_range",
- scale->in_frame_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);
-
- /* Override chroma location default settings to have the correct
- * chroma positions. MPEG chroma positions are used by convention.
- * Note that this works for both MPEG-1/JPEG and MPEG-2/4 chroma
- * locations, since they share a vertical alignment */
- if (desc->log2_chroma_h == 1 && scale->in_v_chr_pos == -513) {
- in_v_chr_pos = (i == 0) ? 128 : (i == 1) ? 64 : 192;
- }
-
- if (outdesc->log2_chroma_h == 1 && scale->out_v_chr_pos == -513) {
- out_v_chr_pos = (i == 0) ? 128 : (i == 1) ? 64 : 192;
- }
-
- av_opt_set_int(s, "src_h_chr_pos", scale->in_h_chr_pos, 0);
- av_opt_set_int(s, "src_v_chr_pos", in_v_chr_pos, 0);
- av_opt_set_int(s, "dst_h_chr_pos", scale->out_h_chr_pos, 0);
- av_opt_set_int(s, "dst_v_chr_pos", out_v_chr_pos, 0);
-
- if ((ret = sws_init_context(s, NULL, NULL)) < 0)
- return ret;
- if (!scale->interlaced)
- break;
+ for (int i = 0; i < 3; i++) {
+ int in_v_chr_pos = scale->in_v_chr_pos, out_v_chr_pos = scale->out_v_chr_pos;
+ struct SwsContext *const s = sws_alloc_context();
+ if (!s)
+ return AVERROR(ENOMEM);
+ *swscs[i] = s;
+
+ ret = av_opt_copy(s, scale->sws_opts);
+ if (ret < 0)
+ return ret;
+
+ av_opt_set_int(s, "srcw", inlink0 ->w, 0);
+ av_opt_set_int(s, "srch", inlink0 ->h >> !!i, 0);
+ av_opt_set_int(s, "src_format", inlink0->format, 0);
+ av_opt_set_int(s, "dstw", outlink->w, 0);
+ av_opt_set_int(s, "dsth", outlink->h >> !!i, 0);
+ av_opt_set_int(s, "dst_format", outfmt, 0);
+
+ /* Override chroma location default settings to have the correct
+ * chroma positions. MPEG chroma positions are used by convention.
+ * Note that this works for both MPEG-1/JPEG and MPEG-2/4 chroma
+ * locations, since they share a vertical alignment */
+ if (desc->log2_chroma_h == 1 && scale->in_v_chr_pos == -513) {
+ in_v_chr_pos = (i == 0) ? 128 : (i == 1) ? 64 : 192;
+ }
+
+ if (outdesc->log2_chroma_h == 1 && scale->out_v_chr_pos == -513) {
+ out_v_chr_pos = (i == 0) ? 128 : (i == 1) ? 64 : 192;
}
+
+ av_opt_set_int(s, "src_h_chr_pos", scale->in_h_chr_pos, 0);
+ av_opt_set_int(s, "src_v_chr_pos", in_v_chr_pos, 0);
+ av_opt_set_int(s, "dst_h_chr_pos", scale->out_h_chr_pos, 0);
+ av_opt_set_int(s, "dst_v_chr_pos", out_v_chr_pos, 0);
+
+ if ((ret = sws_init_context(s, NULL, NULL)) < 0)
+ return ret;
+ if (!scale->interlaced)
+ break;
}
if (inlink0->sample_aspect_ratio.num){
@@ -716,9 +695,10 @@ static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out)
AVFrame *out;
const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);
char buf[32];
- int ret;
- int in_range;
+ int in_full, out_full, brightness, contrast, saturation;
+ const int *inv_table, *table;
int frame_changed;
+ int ret;
*frame_out = NULL;
if (in->colorspace == AVCOL_SPC_YCGCO)
@@ -730,13 +710,6 @@ static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out)
in->sample_aspect_ratio.den != link->sample_aspect_ratio.den ||
in->sample_aspect_ratio.num != link->sample_aspect_ratio.num;
- if (in->color_range != AVCOL_RANGE_UNSPECIFIED &&
- scale->in_range == AVCOL_RANGE_UNSPECIFIED &&
- in->color_range != scale->in_frame_range) {
- scale->in_frame_range = in->color_range;
- frame_changed = 1;
- }
-
if (scale->eval_mode == EVAL_MODE_FRAME || frame_changed) {
unsigned vars_w[VARS_NB] = { 0 }, vars_h[VARS_NB] = { 0 };
@@ -804,7 +777,30 @@ FF_ENABLE_DEPRECATION_WARNINGS
}
scale:
- if (!scale->sws) {
+ sws_getColorspaceDetails(scale->sws, (int **)&inv_table, &in_full,
+ (int **)&table, &out_full,
+ &brightness, &contrast, &saturation);
+
+ if (scale->in_color_matrix)
+ inv_table = parse_yuv_type(scale->in_color_matrix, in->colorspace);
+ if (scale->out_color_matrix)
+ table = parse_yuv_type(scale->out_color_matrix, AVCOL_SPC_UNSPECIFIED);
+ else if (scale->in_color_matrix)
+ table = inv_table;
+
+ if (scale->in_range != AVCOL_RANGE_UNSPECIFIED)
+ in_full = scale->in_range == AVCOL_RANGE_JPEG;
+ else if (in->color_range != AVCOL_RANGE_UNSPECIFIED)
+ in_full = in->color_range == AVCOL_RANGE_JPEG;
+ if (scale->out_range != AVCOL_RANGE_UNSPECIFIED)
+ out_full = scale->out_range == AVCOL_RANGE_JPEG;
+
+ if (in->width == outlink->w &&
+ in->height == outlink->h &&
+ in->format == outlink->format &&
+ inv_table == table &&
+ in_full == out_full) {
+ /* no conversion needed */
*frame_out = in;
return 0;
}
@@ -822,6 +818,7 @@ scale:
av_frame_copy_props(out, in);
out->width = outlink->w;
out->height = outlink->h;
+ out->color_range = out_full ? AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG;
// Sanity checks:
// 1. If the output is RGB, set the matrix coefficients to RGB.
@@ -838,48 +835,17 @@ scale:
if (scale->output_is_pal)
avpriv_set_systematic_pal2((uint32_t*)out->data[1], outlink->format == AV_PIX_FMT_PAL8 ? AV_PIX_FMT_BGR8 : outlink->format);
- in_range = in->color_range;
-
- if ( scale->in_color_matrix
- || scale->out_color_matrix
- || scale-> in_range != AVCOL_RANGE_UNSPECIFIED
- || in_range != AVCOL_RANGE_UNSPECIFIED
- || scale->out_range != AVCOL_RANGE_UNSPECIFIED) {
- int in_full, out_full, brightness, contrast, saturation;
- const int *inv_table, *table;
-
- sws_getColorspaceDetails(scale->sws, (int **)&inv_table, &in_full,
- (int **)&table, &out_full,
- &brightness, &contrast, &saturation);
-
- if (scale->in_color_matrix)
- inv_table = parse_yuv_type(scale->in_color_matrix, in->colorspace);
- if (scale->out_color_matrix)
- table = parse_yuv_type(scale->out_color_matrix, AVCOL_SPC_UNSPECIFIED);
- else if (scale->in_color_matrix)
- table = inv_table;
-
- if (scale-> in_range != AVCOL_RANGE_UNSPECIFIED)
- in_full = (scale-> in_range == AVCOL_RANGE_JPEG);
- else if (in_range != AVCOL_RANGE_UNSPECIFIED)
- in_full = (in_range == AVCOL_RANGE_JPEG);
- if (scale->out_range != AVCOL_RANGE_UNSPECIFIED)
- out_full = (scale->out_range == AVCOL_RANGE_JPEG);
-
- sws_setColorspaceDetails(scale->sws, inv_table, in_full,
+ sws_setColorspaceDetails(scale->sws, inv_table, in_full,
+ table, out_full,
+ brightness, contrast, saturation);
+ if (scale->isws[0])
+ sws_setColorspaceDetails(scale->isws[0], inv_table, in_full,
+ table, out_full,
+ brightness, contrast, saturation);
+ if (scale->isws[1])
+ sws_setColorspaceDetails(scale->isws[1], inv_table, in_full,
table, out_full,
brightness, contrast, saturation);
- if (scale->isws[0])
- sws_setColorspaceDetails(scale->isws[0], inv_table, in_full,
- table, out_full,
- brightness, contrast, saturation);
- if (scale->isws[1])
- sws_setColorspaceDetails(scale->isws[1], inv_table, in_full,
- table, out_full,
- brightness, contrast, saturation);
-
- out->color_range = out_full ? AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG;
- }
av_reduce(&out->sample_aspect_ratio.num, &out->sample_aspect_ratio.den,
(int64_t)in->sample_aspect_ratio.num * outlink->h * link->w,
--
2.42.0
More information about the ffmpeg-devel
mailing list