* [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context
@ 2023-10-27 17:04 Niklas Haas
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 2/8] avfilter/vf_extractplanes: tag alpha plane as full range Niklas Haas
` (7 more replies)
0 siblings, 8 replies; 20+ messages in thread
From: Niklas Haas @ 2023-10-27 17:04 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Niklas Haas
From: Niklas Haas <git@haasn.dev>
More commonly, this fixes the case of sws_setColorspaceDetails after
sws_getContext, since the latter implies sws_init_context.
The problem here is that sws_init_context sets up the range conversion
and fast path tables based on the values of srcRange/dstRange at init
time. This may result in locking in a "wrong" path (either using
unscaled fast path when range conversion later required, or using
scaled slow path when range conversion becomes no longer required).
There are two way outs:
1. Always initialize range conversion and unscaled converters, even if
they will be unused, and extend the runtime check.
2. Re-do initialization if the values change after
sws_setColorspaceDetails.
I opted for approach 1 because it was simpler and easier to reason
about.
---
libswscale/swscale.c | 2 +-
libswscale/utils.c | 5 +----
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/libswscale/swscale.c b/libswscale/swscale.c
index 90e5b299ab..46ba68fe6a 100644
--- a/libswscale/swscale.c
+++ b/libswscale/swscale.c
@@ -1016,7 +1016,7 @@ static int scale_internal(SwsContext *c,
reset_ptr(src2, c->srcFormat);
reset_ptr((void*)dst2, c->dstFormat);
- if (c->convert_unscaled) {
+ if (c->convert_unscaled && !c->lumConvertRange && !c->chrConvertRange) {
int offset = srcSliceY_internal;
int slice_h = srcSliceH;
diff --git a/libswscale/utils.c b/libswscale/utils.c
index e1ad685972..455955e907 100644
--- a/libswscale/utils.c
+++ b/libswscale/utils.c
@@ -1728,9 +1728,7 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
}
/* unscaled special cases */
- if (unscaled && !usesHFilter && !usesVFilter &&
- (c->srcRange == c->dstRange || isAnyRGB(dstFormat) ||
- isFloat(srcFormat) || isFloat(dstFormat))){
+ if (unscaled && !usesHFilter && !usesVFilter) {
ff_get_unscaled_swscale(c);
if (c->convert_unscaled) {
@@ -1738,7 +1736,6 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
av_log(c, AV_LOG_INFO,
"using unscaled %s -> %s special converter\n",
av_get_pix_fmt_name(srcFormat), av_get_pix_fmt_name(dstFormat));
- return 0;
}
}
--
2.42.0
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* [FFmpeg-devel] [PATCH 2/8] avfilter/vf_extractplanes: tag alpha plane as full range
2023-10-27 17:04 [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context Niklas Haas
@ 2023-10-27 17:04 ` Niklas Haas
2023-10-28 2:02 ` Vittorio Giovara
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 3/8] avfilter/vf_alphamerge: warn if input not " Niklas Haas
` (6 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Niklas Haas @ 2023-10-27 17:04 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Niklas Haas
From: Niklas Haas <git@haasn.dev>
Alpha planes are explicitly full range, even for limited range YUVA
formats. Mark them as such.
---
| 2 ++
1 file changed, 2 insertions(+)
--git a/libavfilter/vf_extractplanes.c b/libavfilter/vf_extractplanes.c
index 7b7149ab24..ca406ff323 100644
--- a/libavfilter/vf_extractplanes.c
+++ b/libavfilter/vf_extractplanes.c
@@ -312,6 +312,8 @@ static int extract_plane(AVFilterLink *outlink, AVFrame *frame)
if (!out)
return AVERROR(ENOMEM);
av_frame_copy_props(out, frame);
+ if (idx == 3 /* alpha */)
+ out->color_range = AVCOL_RANGE_JPEG;
if (s->is_packed) {
extract_from_packed(out->data[0], out->linesize[0],
--
2.42.0
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* [FFmpeg-devel] [PATCH 3/8] avfilter/vf_alphamerge: warn if input not full range
2023-10-27 17:04 [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context Niklas Haas
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 2/8] avfilter/vf_extractplanes: tag alpha plane as full range Niklas Haas
@ 2023-10-27 17:04 ` Niklas Haas
2023-10-27 21:29 ` Michael Niedermayer
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 4/8] avfilter/vf_scale: properly respect to input colorimetry Niklas Haas
` (5 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Niklas Haas @ 2023-10-27 17:04 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Niklas Haas
From: Niklas Haas <git@haasn.dev>
Alpha planes must always be full range, so complain loudly if fed
limited range grayscale input.
---
libavfilter/vf_alphamerge.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/libavfilter/vf_alphamerge.c b/libavfilter/vf_alphamerge.c
index 4bbc06da36..b814aa64f0 100644
--- a/libavfilter/vf_alphamerge.c
+++ b/libavfilter/vf_alphamerge.c
@@ -60,6 +60,12 @@ static int do_alphamerge(FFFrameSync *fs)
if (!alpha_buf)
return ff_filter_frame(ctx->outputs[0], main_buf);
+ if (alpha_buf->color_range == AVCOL_RANGE_MPEG) {
+ av_log(ctx, AV_LOG_WARNING, "alpha plane color range tagged as %s, "
+ "output will be wrong!",
+ av_color_range_name(alpha_buf->color_range));
+ }
+
if (s->is_packed_rgb) {
int x, y;
uint8_t *pin, *pout;
--
2.42.0
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* [FFmpeg-devel] [PATCH 4/8] avfilter/vf_scale: properly respect to input colorimetry
2023-10-27 17:04 [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context Niklas Haas
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 2/8] avfilter/vf_extractplanes: tag alpha plane as full range Niklas Haas
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 3/8] avfilter/vf_alphamerge: warn if input not " Niklas Haas
@ 2023-10-27 17:04 ` Niklas Haas
2023-10-27 21:01 ` Michael Niedermayer
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 5/8] avfilter/vf_scale: preserve YUV range by default Niklas Haas
` (4 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Niklas Haas @ 2023-10-27 17:04 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Niklas Haas
From: Niklas Haas <git@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
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* [FFmpeg-devel] [PATCH 5/8] avfilter/vf_scale: preserve YUV range by default
2023-10-27 17:04 [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context Niklas Haas
` (2 preceding siblings ...)
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 4/8] avfilter/vf_scale: properly respect to input colorimetry Niklas Haas
@ 2023-10-27 17:04 ` Niklas Haas
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 6/8] avfilter/vf_scale: simplify color matrix parsing logic Niklas Haas
` (3 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: Niklas Haas @ 2023-10-27 17:04 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Niklas Haas
From: Niklas Haas <git@haasn.dev>
YUV->YUV conversions should preserve input range, if the output range is
unspecified. Ensures full-range YUV input comes out as full-range YUV
output by default, even through YUV->YUV pixel format conversions.
---
libavfilter/vf_scale.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index 19c91cb86e..f9b0746072 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -687,6 +687,28 @@ static int scale_field(ScaleContext *scale, AVFrame *dst, AVFrame *src,
return 0;
}
+static int is_regular_yuv(enum AVPixelFormat fmt)
+{
+ const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(fmt);
+ if (desc->flags & (AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_PAL))
+ return 0;
+ if (desc->name && av_strstart(desc->name, "xyz", NULL))
+ return 0;
+ if (desc->nb_components < 3)
+ return 0; /* grayscale is forced full range inside libswscale */
+ switch (fmt) {
+ case AV_PIX_FMT_YUVJ420P:
+ case AV_PIX_FMT_YUVJ422P:
+ case AV_PIX_FMT_YUVJ444P:
+ case AV_PIX_FMT_YUVJ440P:
+ case AV_PIX_FMT_YUVJ411P:
+ return 0;
+ default:
+ return 1;
+ }
+}
+
+
static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out)
{
AVFilterContext *ctx = link->dst;
@@ -794,6 +816,8 @@ scale:
in_full = in->color_range == AVCOL_RANGE_JPEG;
if (scale->out_range != AVCOL_RANGE_UNSPECIFIED)
out_full = scale->out_range == AVCOL_RANGE_JPEG;
+ else if (is_regular_yuv(in->format) && is_regular_yuv(outlink->format))
+ out_full = in_full; /* preserve pixel range by default */
if (in->width == outlink->w &&
in->height == outlink->h &&
--
2.42.0
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* [FFmpeg-devel] [PATCH 6/8] avfilter/vf_scale: simplify color matrix parsing logic
2023-10-27 17:04 [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context Niklas Haas
` (3 preceding siblings ...)
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 5/8] avfilter/vf_scale: preserve YUV range by default Niklas Haas
@ 2023-10-27 17:04 ` Niklas Haas
2023-10-28 13:31 ` Niklas Haas
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 7/8] avfilter/vf_scale: tag output color space Niklas Haas
` (2 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Niklas Haas @ 2023-10-27 17:04 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Niklas Haas
From: Niklas Haas <git@haasn.dev>
No need to write a custom string parser when we can just use an integer
option with preset values. The various bits of fallback logic are wholly
redundant with equivalent logic already inside sws_getCoefficients.
---
libavfilter/vf_scale.c | 60 +++++++++++++-----------------------------
1 file changed, 19 insertions(+), 41 deletions(-)
diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index f9b0746072..4a2f0bd1f1 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -139,8 +139,8 @@ typedef struct ScaleContext {
char *flags_str;
- char *in_color_matrix;
- char *out_color_matrix;
+ int in_color_matrix;
+ int out_color_matrix;
int in_range;
int out_range;
@@ -407,30 +407,6 @@ static int query_formats(AVFilterContext *ctx)
return 0;
}
-static const int *parse_yuv_type(const char *s, enum AVColorSpace colorspace)
-{
- if (!s)
- s = "bt601";
-
- if (s && strstr(s, "bt709")) {
- colorspace = AVCOL_SPC_BT709;
- } else if (s && strstr(s, "fcc")) {
- colorspace = AVCOL_SPC_FCC;
- } else if (s && strstr(s, "smpte240m")) {
- colorspace = AVCOL_SPC_SMPTE240M;
- } else if (s && (strstr(s, "bt601") || strstr(s, "bt470") || strstr(s, "smpte170m"))) {
- colorspace = AVCOL_SPC_BT470BG;
- } else if (s && strstr(s, "bt2020")) {
- colorspace = AVCOL_SPC_BT2020_NCL;
- }
-
- if (colorspace < 1 || colorspace > 10 || colorspace == 8) {
- colorspace = AVCOL_SPC_BT470BG;
- }
-
- return sws_getCoefficients(colorspace);
-}
-
static int scale_eval_dimensions(AVFilterContext *ctx)
{
ScaleContext *scale = ctx->priv;
@@ -803,11 +779,13 @@ scale:
(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)
+ if (scale->in_color_matrix == -1 /* auto */)
+ inv_table = sws_getCoefficients(in->colorspace);
+ else if (scale->in_color_matrix != AVCOL_SPC_UNSPECIFIED)
+ inv_table = sws_getCoefficients(scale->in_color_matrix);
+ if (scale->out_color_matrix != AVCOL_SPC_UNSPECIFIED)
+ table = sws_getCoefficients(scale->out_color_matrix);
+ else if (scale->in_color_matrix != AVCOL_SPC_UNSPECIFIED)
table = inv_table;
if (scale->in_range != AVCOL_RANGE_UNSPECIFIED)
@@ -993,16 +971,16 @@ static const AVOption scale_options[] = {
{ "interl", "set interlacing", OFFSET(interlaced), AV_OPT_TYPE_BOOL, {.i64 = 0 }, -1, 1, FLAGS },
{ "size", "set video size", OFFSET(size_str), AV_OPT_TYPE_STRING, {.str = NULL}, 0, FLAGS },
{ "s", "set video size", OFFSET(size_str), AV_OPT_TYPE_STRING, {.str = NULL}, 0, FLAGS },
- { "in_color_matrix", "set input YCbCr type", OFFSET(in_color_matrix), AV_OPT_TYPE_STRING, { .str = "auto" }, .flags = FLAGS, "color" },
- { "out_color_matrix", "set output YCbCr type", OFFSET(out_color_matrix), AV_OPT_TYPE_STRING, { .str = NULL }, .flags = FLAGS, "color"},
- { "auto", NULL, 0, AV_OPT_TYPE_CONST, { .str = "auto" }, 0, 0, FLAGS, "color" },
- { "bt601", NULL, 0, AV_OPT_TYPE_CONST, { .str = "bt601" }, 0, 0, FLAGS, "color" },
- { "bt470", NULL, 0, AV_OPT_TYPE_CONST, { .str = "bt470" }, 0, 0, FLAGS, "color" },
- { "smpte170m", NULL, 0, AV_OPT_TYPE_CONST, { .str = "smpte170m" }, 0, 0, FLAGS, "color" },
- { "bt709", NULL, 0, AV_OPT_TYPE_CONST, { .str = "bt709" }, 0, 0, FLAGS, "color" },
- { "fcc", NULL, 0, AV_OPT_TYPE_CONST, { .str = "fcc" }, 0, 0, FLAGS, "color" },
- { "smpte240m", NULL, 0, AV_OPT_TYPE_CONST, { .str = "smpte240m" }, 0, 0, FLAGS, "color" },
- { "bt2020", NULL, 0, AV_OPT_TYPE_CONST, { .str = "bt2020" }, 0, 0, FLAGS, "color" },
+ { "in_color_matrix", "set input YCbCr type", OFFSET(in_color_matrix), AV_OPT_TYPE_INT, { .i64 = AVCOL_SPC_UNSPECIFIED }, 0, AVCOL_SPC_NB-1, .flags = FLAGS, "color" },
+ { "out_color_matrix", "set output YCbCr type", OFFSET(out_color_matrix), AV_OPT_TYPE_INT, { .i64 = AVCOL_SPC_UNSPECIFIED }, 0, AVCOL_SPC_NB-1, .flags = FLAGS, "color"},
+ { "auto", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = -1 }, 0, 0, FLAGS, "color" },
+ { "bt601", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_BT470BG }, 0, 0, FLAGS, "color" },
+ { "bt470", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_BT470BG }, 0, 0, FLAGS, "color" },
+ { "smpte170m", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_BT470BG }, 0, 0, FLAGS, "color" },
+ { "bt709", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_BT709 }, 0, 0, FLAGS, "color" },
+ { "fcc", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_FCC }, 0, 0, FLAGS, "color" },
+ { "smpte240m", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_SMPTE240M }, 0, 0, FLAGS, "color" },
+ { "bt2020", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_BT2020_NCL }, 0, 0, FLAGS, "color" },
{ "in_range", "set input color range", OFFSET( in_range), AV_OPT_TYPE_INT, {.i64 = AVCOL_RANGE_UNSPECIFIED }, 0, 2, FLAGS, "range" },
{ "out_range", "set output color range", OFFSET(out_range), AV_OPT_TYPE_INT, {.i64 = AVCOL_RANGE_UNSPECIFIED }, 0, 2, FLAGS, "range" },
{ "auto", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_RANGE_UNSPECIFIED }, 0, 0, FLAGS, "range" },
--
2.42.0
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* [FFmpeg-devel] [PATCH 7/8] avfilter/vf_scale: tag output color space
2023-10-27 17:04 [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context Niklas Haas
` (4 preceding siblings ...)
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 6/8] avfilter/vf_scale: simplify color matrix parsing logic Niklas Haas
@ 2023-10-27 17:04 ` Niklas Haas
2023-10-28 2:04 ` Vittorio Giovara
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 8/8] avcodec/pnm: explicitly tag color range Niklas Haas
2023-10-27 21:42 ` [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context Michael Niedermayer
7 siblings, 1 reply; 20+ messages in thread
From: Niklas Haas @ 2023-10-27 17:04 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Niklas Haas
From: Niklas Haas <git@haasn.dev>
When using vf_scale to force a specific output color space, also tag
this on the AVFrame. (Mirroring existing logic for output range)
---
libavfilter/vf_scale.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index 4a2f0bd1f1..d79d67c413 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -821,6 +821,9 @@ scale:
out->width = outlink->w;
out->height = outlink->h;
out->color_range = out_full ? AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG;
+ if (scale->out_color_matrix >= 0 &&
+ scale->out_color_matrix != AVCOL_SPC_UNSPECIFIED)
+ out->colorspace = scale->out_color_matrix;
// Sanity checks:
// 1. If the output is RGB, set the matrix coefficients to RGB.
--
2.42.0
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* [FFmpeg-devel] [PATCH 8/8] avcodec/pnm: explicitly tag color range
2023-10-27 17:04 [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context Niklas Haas
` (5 preceding siblings ...)
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 7/8] avfilter/vf_scale: tag output color space Niklas Haas
@ 2023-10-27 17:04 ` Niklas Haas
2023-10-27 18:34 ` Vittorio Giovara
2023-10-27 21:42 ` [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context Michael Niedermayer
7 siblings, 1 reply; 20+ messages in thread
From: Niklas Haas @ 2023-10-27 17:04 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Niklas Haas
From: Niklas Haas <git@haasn.dev>
PGMYUV seems to be always limited range. This was a format originally
invented by FFmpeg at a time when YUVJ distinguished limited from full
range YUV, and this codec never appeared to output YUVJ in any
circumstance, so hard-coding limited range preserves the status quo.
The other formats are explicitly documented to be full range RGB/gray
formats. That said, don't tag them yet, due to outstanding bugs w.r.t
grayscale formats and color range handling.
This change in behavior updates a bunch of FATE tests in trivial ways
(added tagging being the only difference).
---
libavcodec/pnm.c | 5 ++++
tests/ref/lavf/mkv | 4 ++--
tests/ref/lavf/mkv_attachment | 4 ++--
tests/ref/lavf/mxf | 6 ++---
tests/ref/lavf/y4m | 4 ++--
tests/ref/seek/lavf-mkv | 44 +++++++++++++++++------------------
tests/ref/seek/lavf-y4m | 22 +++++++++---------
7 files changed, 47 insertions(+), 42 deletions(-)
diff --git a/libavcodec/pnm.c b/libavcodec/pnm.c
index 77d24eeaf7..bc51f94b1c 100644
--- a/libavcodec/pnm.c
+++ b/libavcodec/pnm.c
@@ -240,5 +240,10 @@ int ff_pnm_decode_header(AVCodecContext *avctx, PNMContext * const s)
h /= 3;
avctx->height = h;
}
+
+ /* PGMYUV is always limited range */
+ if (avctx->codec_id == AV_CODEC_ID_PGMYUV)
+ avctx->color_range = AVCOL_RANGE_MPEG;
+
return 0;
}
diff --git a/tests/ref/lavf/mkv b/tests/ref/lavf/mkv
index a8c3fd13e8..5a3c3b931e 100644
--- a/tests/ref/lavf/mkv
+++ b/tests/ref/lavf/mkv
@@ -1,3 +1,3 @@
-6224bc0893bd0bb8a789e74bbd38c9c7 *tests/data/lavf/lavf.mkv
-320440 tests/data/lavf/lavf.mkv
+dd709c2b5e173eaca39cdd4a10aac3ec *tests/data/lavf/lavf.mkv
+320447 tests/data/lavf/lavf.mkv
tests/data/lavf/lavf.mkv CRC=0xec6c3c68
diff --git a/tests/ref/lavf/mkv_attachment b/tests/ref/lavf/mkv_attachment
index 4c958af162..1a086a4f24 100644
--- a/tests/ref/lavf/mkv_attachment
+++ b/tests/ref/lavf/mkv_attachment
@@ -1,3 +1,3 @@
-05132b99d16128e552c1a6f1619be8b7 *tests/data/lavf/lavf.mkv_attachment
-472590 tests/data/lavf/lavf.mkv_attachment
+7cd7b06892b74d66da217c8dda90bfac *tests/data/lavf/lavf.mkv_attachment
+472597 tests/data/lavf/lavf.mkv_attachment
tests/data/lavf/lavf.mkv_attachment CRC=0xec6c3c68
diff --git a/tests/ref/lavf/mxf b/tests/ref/lavf/mxf
index fdd1ef5c9c..4cf3388afa 100644
--- a/tests/ref/lavf/mxf
+++ b/tests/ref/lavf/mxf
@@ -1,9 +1,9 @@
-9ec1ad83b3400de11ca2f70b3b2d4c4d *tests/data/lavf/lavf.mxf
+fac1fb467168a374e96ea12755558869 *tests/data/lavf/lavf.mxf
526393 tests/data/lavf/lavf.mxf
tests/data/lavf/lavf.mxf CRC=0x8dddfaab
-3edfabe839a29f5902969c15ebac6d8d *tests/data/lavf/lavf.mxf
+d711481c4f81f6466fd92bdc7ed6c968 *tests/data/lavf/lavf.mxf
551481 tests/data/lavf/lavf.mxf
tests/data/lavf/lavf.mxf CRC=0xf091e687
-5bd0ce691510e6fae969886c32ad7a14 *tests/data/lavf/lavf.mxf
+7f4f8048c4f2d714e45947d4f39b8ea3 *tests/data/lavf/lavf.mxf
526393 tests/data/lavf/lavf.mxf
tests/data/lavf/lavf.mxf CRC=0x8dddfaab
diff --git a/tests/ref/lavf/y4m b/tests/ref/lavf/y4m
index 82c7087673..3c3fa830ad 100644
--- a/tests/ref/lavf/y4m
+++ b/tests/ref/lavf/y4m
@@ -1,3 +1,3 @@
-ec8178cb152f9cdbfd9cb724d977db2e *tests/data/lavf/lavf.y4m
-3801808 tests/data/lavf/lavf.y4m
+54f4ebcffedc886835444bb9d6aba3fb *tests/data/lavf/lavf.y4m
+3801828 tests/data/lavf/lavf.y4m
tests/data/lavf/lavf.y4m CRC=0x0a941f26
diff --git a/tests/ref/seek/lavf-mkv b/tests/ref/seek/lavf-mkv
index b8028dd075..e327959058 100644
--- a/tests/ref/seek/lavf-mkv
+++ b/tests/ref/seek/lavf-mkv
@@ -1,48 +1,48 @@
-ret: 0 st: 1 flags:1 dts:-0.011000 pts:-0.011000 pos: 682 size: 208
+ret: 0 st: 1 flags:1 dts:-0.011000 pts:-0.011000 pos: 689 size: 208
ret: 0 st:-1 flags:0 ts:-1.000000
-ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 898 size: 27837
+ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 905 size: 27837
ret: 0 st:-1 flags:1 ts: 1.894167
-ret: 0 st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292314 size: 27834
+ret: 0 st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292321 size: 27834
ret: 0 st: 0 flags:0 ts: 0.788000
-ret: 0 st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292314 size: 27834
+ret: 0 st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292321 size: 27834
ret: 0 st: 0 flags:1 ts:-0.317000
-ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 898 size: 27837
+ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 905 size: 27837
ret:-1 st: 1 flags:0 ts: 2.577000
ret: 0 st: 1 flags:1 ts: 1.471000
-ret: 0 st: 1 flags:1 dts: 0.982000 pts: 0.982000 pos: 320158 size: 209
+ret: 0 st: 1 flags:1 dts: 0.982000 pts: 0.982000 pos: 320165 size: 209
ret: 0 st:-1 flags:0 ts: 0.365002
-ret: 0 st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos: 146866 size: 27925
+ret: 0 st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos: 146873 size: 27925
ret: 0 st:-1 flags:1 ts:-0.740831
-ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 898 size: 27837
+ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 905 size: 27837
ret:-1 st: 0 flags:0 ts: 2.153000
ret: 0 st: 0 flags:1 ts: 1.048000
-ret: 0 st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292314 size: 27834
+ret: 0 st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292321 size: 27834
ret: 0 st: 1 flags:0 ts:-0.058000
-ret: 0 st: 1 flags:1 dts:-0.011000 pts:-0.011000 pos: 682 size: 208
+ret: 0 st: 1 flags:1 dts:-0.011000 pts:-0.011000 pos: 689 size: 208
ret: 0 st: 1 flags:1 ts: 2.836000
-ret: 0 st: 1 flags:1 dts: 0.982000 pts: 0.982000 pos: 320158 size: 209
+ret: 0 st: 1 flags:1 dts: 0.982000 pts: 0.982000 pos: 320165 size: 209
ret:-1 st:-1 flags:0 ts: 1.730004
ret: 0 st:-1 flags:1 ts: 0.624171
-ret: 0 st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos: 146866 size: 27925
+ret: 0 st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos: 146873 size: 27925
ret: 0 st: 0 flags:0 ts:-0.482000
-ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 898 size: 27837
+ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 905 size: 27837
ret: 0 st: 0 flags:1 ts: 2.413000
-ret: 0 st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292314 size: 27834
+ret: 0 st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292321 size: 27834
ret:-1 st: 1 flags:0 ts: 1.307000
ret: 0 st: 1 flags:1 ts: 0.201000
-ret: 0 st: 1 flags:1 dts:-0.011000 pts:-0.011000 pos: 682 size: 208
+ret: 0 st: 1 flags:1 dts:-0.011000 pts:-0.011000 pos: 689 size: 208
ret: 0 st:-1 flags:0 ts:-0.904994
-ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 898 size: 27837
+ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 905 size: 27837
ret: 0 st:-1 flags:1 ts: 1.989173
-ret: 0 st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292314 size: 27834
+ret: 0 st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292321 size: 27834
ret: 0 st: 0 flags:0 ts: 0.883000
-ret: 0 st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292314 size: 27834
+ret: 0 st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292321 size: 27834
ret: 0 st: 0 flags:1 ts:-0.222000
-ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 898 size: 27837
+ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 905 size: 27837
ret:-1 st: 1 flags:0 ts: 2.672000
ret: 0 st: 1 flags:1 ts: 1.566000
-ret: 0 st: 1 flags:1 dts: 0.982000 pts: 0.982000 pos: 320158 size: 209
+ret: 0 st: 1 flags:1 dts: 0.982000 pts: 0.982000 pos: 320165 size: 209
ret: 0 st:-1 flags:0 ts: 0.460008
-ret: 0 st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos: 146866 size: 27925
+ret: 0 st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos: 146873 size: 27925
ret: 0 st:-1 flags:1 ts:-0.645825
-ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 898 size: 27837
+ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 905 size: 27837
diff --git a/tests/ref/seek/lavf-y4m b/tests/ref/seek/lavf-y4m
index c416b4657b..67793ea40c 100644
--- a/tests/ref/seek/lavf-y4m
+++ b/tests/ref/seek/lavf-y4m
@@ -1,19 +1,19 @@
-ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 64 size:152064
+ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 84 size:152064
ret:-1 st:-1 flags:0 ts:-1.000000
ret: 0 st:-1 flags:1 ts: 1.894167
ret:-EOF
ret: 0 st: 0 flags:0 ts: 0.800000
-ret: 0 st: 0 flags:1 dts: 0.800000 pts: 0.800000 pos:3041464 size:152064
+ret: 0 st: 0 flags:1 dts: 0.800000 pts: 0.800000 pos:3041484 size:152064
ret: 0 st: 0 flags:1 ts:-0.320000
-ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 64 size:152064
+ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 84 size:152064
ret: 0 st:-1 flags:0 ts: 2.576668
ret:-EOF
ret: 0 st:-1 flags:1 ts: 1.470835
ret:-EOF
ret: 0 st: 0 flags:0 ts: 0.360000
-ret: 0 st: 0 flags:1 dts: 0.360000 pts: 0.360000 pos:1368694 size:152064
+ret: 0 st: 0 flags:1 dts: 0.360000 pts: 0.360000 pos:1368714 size:152064
ret: 0 st: 0 flags:1 ts:-0.760000
-ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 64 size:152064
+ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 84 size:152064
ret: 0 st:-1 flags:0 ts: 2.153336
ret:-EOF
ret: 0 st:-1 flags:1 ts: 1.047503
@@ -24,26 +24,26 @@ ret:-EOF
ret: 0 st:-1 flags:0 ts: 1.730004
ret:-EOF
ret: 0 st:-1 flags:1 ts: 0.624171
-ret: 0 st: 0 flags:1 dts: 0.600000 pts: 0.600000 pos:2281114 size:152064
+ret: 0 st: 0 flags:1 dts: 0.600000 pts: 0.600000 pos:2281134 size:152064
ret:-1 st: 0 flags:0 ts:-0.480000
ret: 0 st: 0 flags:1 ts: 2.400000
ret:-EOF
ret: 0 st:-1 flags:0 ts: 1.306672
ret:-EOF
ret: 0 st:-1 flags:1 ts: 0.200839
-ret: 0 st: 0 flags:1 dts: 0.160000 pts: 0.160000 pos: 608344 size:152064
+ret: 0 st: 0 flags:1 dts: 0.160000 pts: 0.160000 pos: 608364 size:152064
ret:-1 st: 0 flags:0 ts:-0.920000
ret: 0 st: 0 flags:1 ts: 2.000000
ret:-EOF
ret: 0 st:-1 flags:0 ts: 0.883340
-ret: 0 st: 0 flags:1 dts: 0.880000 pts: 0.880000 pos:3345604 size:152064
+ret: 0 st: 0 flags:1 dts: 0.880000 pts: 0.880000 pos:3345624 size:152064
ret: 0 st:-1 flags:1 ts:-0.222493
-ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 64 size:152064
+ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 84 size:152064
ret: 0 st: 0 flags:0 ts: 2.680000
ret:-EOF
ret: 0 st: 0 flags:1 ts: 1.560000
ret:-EOF
ret: 0 st:-1 flags:0 ts: 0.460008
-ret: 0 st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos:1824904 size:152064
+ret: 0 st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos:1824924 size:152064
ret: 0 st:-1 flags:1 ts:-0.645825
-ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 64 size:152064
+ret: 0 st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos: 84 size:152064
--
2.42.0
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [FFmpeg-devel] [PATCH 8/8] avcodec/pnm: explicitly tag color range
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 8/8] avcodec/pnm: explicitly tag color range Niklas Haas
@ 2023-10-27 18:34 ` Vittorio Giovara
0 siblings, 0 replies; 20+ messages in thread
From: Vittorio Giovara @ 2023-10-27 18:34 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Niklas Haas
On Fri, Oct 27, 2023 at 10:06 AM Niklas Haas <ffmpeg@haasn.xyz> wrote:
> From: Niklas Haas <git@haasn.dev>
>
> PGMYUV seems to be always limited range. This was a format originally
> invented by FFmpeg at a time when YUVJ distinguished limited from full
> range YUV, and this codec never appeared to output YUVJ in any
> circumstance, so hard-coding limited range preserves the status quo.
>
> The other formats are explicitly documented to be full range RGB/gray
> formats. That said, don't tag them yet, due to outstanding bugs w.r.t
> grayscale formats and color range handling.
>
> This change in behavior updates a bunch of FATE tests in trivial ways
> (added tagging being the only difference).
> ---
> libavcodec/pnm.c | 5 ++++
> tests/ref/lavf/mkv | 4 ++--
> tests/ref/lavf/mkv_attachment | 4 ++--
> tests/ref/lavf/mxf | 6 ++---
> tests/ref/lavf/y4m | 4 ++--
> tests/ref/seek/lavf-mkv | 44 +++++++++++++++++------------------
> tests/ref/seek/lavf-y4m | 22 +++++++++---------
> 7 files changed, 47 insertions(+), 42 deletions(-)
>
> diff --git a/libavcodec/pnm.c b/libavcodec/pnm.c
> index 77d24eeaf7..bc51f94b1c 100644
> --- a/libavcodec/pnm.c
> +++ b/libavcodec/pnm.c
> @@ -240,5 +240,10 @@ int ff_pnm_decode_header(AVCodecContext *avctx,
> PNMContext * const s)
> h /= 3;
> avctx->height = h;
> }
> +
> + /* PGMYUV is always limited range */
> + if (avctx->codec_id == AV_CODEC_ID_PGMYUV)
> + avctx->color_range = AVCOL_RANGE_MPEG;
>
Why does this check for the codec_id?
--
Vittorio
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [FFmpeg-devel] [PATCH 4/8] avfilter/vf_scale: properly respect to input colorimetry
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 4/8] avfilter/vf_scale: properly respect to input colorimetry Niklas Haas
@ 2023-10-27 21:01 ` Michael Niedermayer
2023-10-28 10:50 ` Niklas Haas
0 siblings, 1 reply; 20+ messages in thread
From: Michael Niedermayer @ 2023-10-27 21:01 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 1658 bytes --]
On Fri, Oct 27, 2023 at 07:04:42PM +0200, Niklas Haas wrote:
> From: Niklas Haas <git@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(-)
This breaks
tickets/2939/lena.jpg
(color looks slightly wrong compared to reference lena image)
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/8] avfilter/vf_alphamerge: warn if input not full range
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 3/8] avfilter/vf_alphamerge: warn if input not " Niklas Haas
@ 2023-10-27 21:29 ` Michael Niedermayer
0 siblings, 0 replies; 20+ messages in thread
From: Michael Niedermayer @ 2023-10-27 21:29 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 540 bytes --]
On Fri, Oct 27, 2023 at 07:04:41PM +0200, Niklas Haas wrote:
> From: Niklas Haas <git@haasn.dev>
>
> Alpha planes must always be full range, so complain loudly if fed
> limited range grayscale input.
> ---
> libavfilter/vf_alphamerge.c | 6 ++++++
> 1 file changed, 6 insertions(+)
should be ok
[...]
thx
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context
2023-10-27 17:04 [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context Niklas Haas
` (6 preceding siblings ...)
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 8/8] avcodec/pnm: explicitly tag color range Niklas Haas
@ 2023-10-27 21:42 ` Michael Niedermayer
2023-10-28 10:46 ` Niklas Haas
2023-10-28 14:38 ` Niklas Haas
7 siblings, 2 replies; 20+ messages in thread
From: Michael Niedermayer @ 2023-10-27 21:42 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 3006 bytes --]
On Fri, Oct 27, 2023 at 07:04:39PM +0200, Niklas Haas wrote:
> From: Niklas Haas <git@haasn.dev>
>
> More commonly, this fixes the case of sws_setColorspaceDetails after
> sws_getContext, since the latter implies sws_init_context.
>
> The problem here is that sws_init_context sets up the range conversion
> and fast path tables based on the values of srcRange/dstRange at init
> time. This may result in locking in a "wrong" path (either using
> unscaled fast path when range conversion later required, or using
> scaled slow path when range conversion becomes no longer required).
>
> There are two way outs:
>
> 1. Always initialize range conversion and unscaled converters, even if
> they will be unused, and extend the runtime check.
> 2. Re-do initialization if the values change after
> sws_setColorspaceDetails.
>
> I opted for approach 1 because it was simpler and easier to reason
> about.
> ---
> libswscale/swscale.c | 2 +-
> libswscale/utils.c | 5 +----
> 2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/libswscale/swscale.c b/libswscale/swscale.c
> index 90e5b299ab..46ba68fe6a 100644
> --- a/libswscale/swscale.c
> +++ b/libswscale/swscale.c
> @@ -1016,7 +1016,7 @@ static int scale_internal(SwsContext *c,
> reset_ptr(src2, c->srcFormat);
> reset_ptr((void*)dst2, c->dstFormat);
>
> - if (c->convert_unscaled) {
> + if (c->convert_unscaled && !c->lumConvertRange && !c->chrConvertRange) {
> int offset = srcSliceY_internal;
> int slice_h = srcSliceH;
>
> diff --git a/libswscale/utils.c b/libswscale/utils.c
> index e1ad685972..455955e907 100644
> --- a/libswscale/utils.c
> +++ b/libswscale/utils.c
> @@ -1728,9 +1728,7 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
> }
>
> /* unscaled special cases */
> - if (unscaled && !usesHFilter && !usesVFilter &&
> - (c->srcRange == c->dstRange || isAnyRGB(dstFormat) ||
> - isFloat(srcFormat) || isFloat(dstFormat))){
> + if (unscaled && !usesHFilter && !usesVFilter) {
> ff_get_unscaled_swscale(c);
>
> if (c->convert_unscaled) {
> @@ -1738,7 +1736,6 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
> av_log(c, AV_LOG_INFO,
> "using unscaled %s -> %s special converter\n",
> av_get_pix_fmt_name(srcFormat), av_get_pix_fmt_name(dstFormat));
> - return 0;
> }
> }
the av_log() message will be wrong if this path is unused
also this ties convert_unscaled to never do range conversion, if thats
intended, i guess thats ok. Otherwise that maybe is a restriction
we dont want to add.
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
"I am not trying to be anyone's saviour, I'm trying to think about the
future and not be sad" - Elon Musk
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/8] avfilter/vf_extractplanes: tag alpha plane as full range
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 2/8] avfilter/vf_extractplanes: tag alpha plane as full range Niklas Haas
@ 2023-10-28 2:02 ` Vittorio Giovara
0 siblings, 0 replies; 20+ messages in thread
From: Vittorio Giovara @ 2023-10-28 2:02 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Niklas Haas
On Fri, Oct 27, 2023 at 10:05 AM Niklas Haas <ffmpeg@haasn.xyz> wrote:
> From: Niklas Haas <git@haasn.dev>
>
> Alpha planes are explicitly full range, even for limited range YUVA
> formats. Mark them as such.
> ---
> libavfilter/vf_extractplanes.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/libavfilter/vf_extractplanes.c
> b/libavfilter/vf_extractplanes.c
> index 7b7149ab24..ca406ff323 100644
> --- a/libavfilter/vf_extractplanes.c
> +++ b/libavfilter/vf_extractplanes.c
> @@ -312,6 +312,8 @@ static int extract_plane(AVFilterLink *outlink,
> AVFrame *frame)
> if (!out)
> return AVERROR(ENOMEM);
> av_frame_copy_props(out, frame);
> + if (idx == 3 /* alpha */)
> + out->color_range = AVCOL_RANGE_JPEG;
>
this might be overkill, but do you think the check here should be made more
generic, in case the alpha plane is not in position #3?
--
Vittorio
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [FFmpeg-devel] [PATCH 7/8] avfilter/vf_scale: tag output color space
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 7/8] avfilter/vf_scale: tag output color space Niklas Haas
@ 2023-10-28 2:04 ` Vittorio Giovara
0 siblings, 0 replies; 20+ messages in thread
From: Vittorio Giovara @ 2023-10-28 2:04 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Niklas Haas
On Fri, Oct 27, 2023 at 10:06 AM Niklas Haas <ffmpeg@haasn.xyz> wrote:
> From: Niklas Haas <git@haasn.dev>
>
> When using vf_scale to force a specific output color space, also tag
> this on the AVFrame. (Mirroring existing logic for output range)
> ---
> libavfilter/vf_scale.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index 4a2f0bd1f1..d79d67c413 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -821,6 +821,9 @@ scale:
> out->width = outlink->w;
> out->height = outlink->h;
> out->color_range = out_full ? AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG;
> + if (scale->out_color_matrix >= 0 &&
> + scale->out_color_matrix != AVCOL_SPC_UNSPECIFIED)
> + out->colorspace = scale->out_color_matrix;
>
what about the other color properties?
--
Vittorio
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context
2023-10-27 21:42 ` [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context Michael Niedermayer
@ 2023-10-28 10:46 ` Niklas Haas
2023-10-28 21:53 ` Michael Niedermayer
2023-10-28 14:38 ` Niklas Haas
1 sibling, 1 reply; 20+ messages in thread
From: Niklas Haas @ 2023-10-28 10:46 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Fri, 27 Oct 2023 23:42:41 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Fri, Oct 27, 2023 at 07:04:39PM +0200, Niklas Haas wrote:
> > From: Niklas Haas <git@haasn.dev>
> >
> > More commonly, this fixes the case of sws_setColorspaceDetails after
> > sws_getContext, since the latter implies sws_init_context.
> >
> > The problem here is that sws_init_context sets up the range conversion
> > and fast path tables based on the values of srcRange/dstRange at init
> > time. This may result in locking in a "wrong" path (either using
> > unscaled fast path when range conversion later required, or using
> > scaled slow path when range conversion becomes no longer required).
> >
> > There are two way outs:
> >
> > 1. Always initialize range conversion and unscaled converters, even if
> > they will be unused, and extend the runtime check.
> > 2. Re-do initialization if the values change after
> > sws_setColorspaceDetails.
> >
> > I opted for approach 1 because it was simpler and easier to reason
> > about.
> > ---
> > libswscale/swscale.c | 2 +-
> > libswscale/utils.c | 5 +----
> > 2 files changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/libswscale/swscale.c b/libswscale/swscale.c
> > index 90e5b299ab..46ba68fe6a 100644
> > --- a/libswscale/swscale.c
> > +++ b/libswscale/swscale.c
> > @@ -1016,7 +1016,7 @@ static int scale_internal(SwsContext *c,
> > reset_ptr(src2, c->srcFormat);
> > reset_ptr((void*)dst2, c->dstFormat);
> >
> > - if (c->convert_unscaled) {
> > + if (c->convert_unscaled && !c->lumConvertRange && !c->chrConvertRange) {
> > int offset = srcSliceY_internal;
> > int slice_h = srcSliceH;
> >
> > diff --git a/libswscale/utils.c b/libswscale/utils.c
> > index e1ad685972..455955e907 100644
> > --- a/libswscale/utils.c
> > +++ b/libswscale/utils.c
> > @@ -1728,9 +1728,7 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
> > }
> >
> > /* unscaled special cases */
> > - if (unscaled && !usesHFilter && !usesVFilter &&
> > - (c->srcRange == c->dstRange || isAnyRGB(dstFormat) ||
> > - isFloat(srcFormat) || isFloat(dstFormat))){
> > + if (unscaled && !usesHFilter && !usesVFilter) {
> > ff_get_unscaled_swscale(c);
> >
> > if (c->convert_unscaled) {
> > @@ -1738,7 +1736,6 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
> > av_log(c, AV_LOG_INFO,
> > "using unscaled %s -> %s special converter\n",
> > av_get_pix_fmt_name(srcFormat), av_get_pix_fmt_name(dstFormat));
> > - return 0;
> > }
> > }
>
> the av_log() message will be wrong if this path is unused
Indeed. Perhaps the only way to fix that would be to instead try
approach 2 and go through full reinit if any params change after
setColorspaceDetails.
> also this ties convert_unscaled to never do range conversion, if thats
> intended, i guess thats ok. Otherwise that maybe is a restriction
> we dont want to add.
In the original code, c->convert_unscaled is set behind a `c->srcRange
== c->dstRange` check, so how is that a change in behavior?
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [FFmpeg-devel] [PATCH 4/8] avfilter/vf_scale: properly respect to input colorimetry
2023-10-27 21:01 ` Michael Niedermayer
@ 2023-10-28 10:50 ` Niklas Haas
2023-10-28 11:18 ` Niklas Haas
0 siblings, 1 reply; 20+ messages in thread
From: Niklas Haas @ 2023-10-28 10:50 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Fri, 27 Oct 2023 23:01:25 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Fri, Oct 27, 2023 at 07:04:42PM +0200, Niklas Haas wrote:
> > From: Niklas Haas <git@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(-)
>
> This breaks
> tickets/2939/lena.jpg
>
> (color looks slightly wrong compared to reference lena image)
How do I reproduce this? I tested with:
ffmpeg -i lena.jpg lena.png
But the resulting checksum was identical before and after this commit.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [FFmpeg-devel] [PATCH 4/8] avfilter/vf_scale: properly respect to input colorimetry
2023-10-28 10:50 ` Niklas Haas
@ 2023-10-28 11:18 ` Niklas Haas
0 siblings, 0 replies; 20+ messages in thread
From: Niklas Haas @ 2023-10-28 11:18 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Sat, 28 Oct 2023 12:50:14 +0200 Niklas Haas <ffmpeg@haasn.xyz> wrote:
> On Fri, 27 Oct 2023 23:01:25 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote:
> > On Fri, Oct 27, 2023 at 07:04:42PM +0200, Niklas Haas wrote:
> > > From: Niklas Haas <git@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(-)
> >
> > This breaks
> > tickets/2939/lena.jpg
> >
> > (color looks slightly wrong compared to reference lena image)
>
> How do I reproduce this? I tested with:
>
> ffmpeg -i lena.jpg lena.png
>
> But the resulting checksum was identical before and after this commit.
Nvm, I see the issue. Actually, the bug is in patch 1 - the logic is
insufficient to cover all cases, since ff_sws_init_range_convert is
never re-called after changing range.
I will rewrite that commit, which should fix this one also.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [FFmpeg-devel] [PATCH 6/8] avfilter/vf_scale: simplify color matrix parsing logic
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 6/8] avfilter/vf_scale: simplify color matrix parsing logic Niklas Haas
@ 2023-10-28 13:31 ` Niklas Haas
0 siblings, 0 replies; 20+ messages in thread
From: Niklas Haas @ 2023-10-28 13:31 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Niklas Haas
On Fri, 27 Oct 2023 19:04:44 +0200 Niklas Haas <ffmpeg@haasn.xyz> wrote:
> - { "in_color_matrix", "set input YCbCr type", OFFSET(in_color_matrix), AV_OPT_TYPE_STRING, { .str = "auto" }, .flags = FLAGS, "color" },
> - { "out_color_matrix", "set output YCbCr type", OFFSET(out_color_matrix), AV_OPT_TYPE_STRING, { .str = NULL }, .flags = FLAGS, "color"},
> + { "in_color_matrix", "set input YCbCr type", OFFSET(in_color_matrix), AV_OPT_TYPE_INT, { .i64 = AVCOL_SPC_UNSPECIFIED }, 0, AVCOL_SPC_NB-1, .flags = FLAGS, "color" },
> + { "out_color_matrix", "set output YCbCr type", OFFSET(out_color_matrix), AV_OPT_TYPE_INT, { .i64 = AVCOL_SPC_UNSPECIFIED }, 0, AVCOL_SPC_NB-1, .flags = FLAGS, "color"},
Bug: Should be set to -1 by default, to preserve old behavior. Will fix
in v2.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context
2023-10-27 21:42 ` [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context Michael Niedermayer
2023-10-28 10:46 ` Niklas Haas
@ 2023-10-28 14:38 ` Niklas Haas
1 sibling, 0 replies; 20+ messages in thread
From: Niklas Haas @ 2023-10-28 14:38 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Fri, 27 Oct 2023 23:42:41 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote:
> the av_log() message will be wrong if this path is unused
Well, this is a problem even if we move the log printing to context
reinit after sws_setColorspaceDetails, because it will still get printed
on the initial init (before range is known), unless we move it to
sws_scale_frame() itself. But then we would need to keep track also of
whether or not we already printed this.
Since this is just a developer-facing debug message, I'll probably just
reword it.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context
2023-10-28 10:46 ` Niklas Haas
@ 2023-10-28 21:53 ` Michael Niedermayer
0 siblings, 0 replies; 20+ messages in thread
From: Michael Niedermayer @ 2023-10-28 21:53 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 4298 bytes --]
On Sat, Oct 28, 2023 at 12:46:38PM +0200, Niklas Haas wrote:
> On Fri, 27 Oct 2023 23:42:41 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote:
> > On Fri, Oct 27, 2023 at 07:04:39PM +0200, Niklas Haas wrote:
> > > From: Niklas Haas <git@haasn.dev>
> > >
> > > More commonly, this fixes the case of sws_setColorspaceDetails after
> > > sws_getContext, since the latter implies sws_init_context.
> > >
> > > The problem here is that sws_init_context sets up the range conversion
> > > and fast path tables based on the values of srcRange/dstRange at init
> > > time. This may result in locking in a "wrong" path (either using
> > > unscaled fast path when range conversion later required, or using
> > > scaled slow path when range conversion becomes no longer required).
> > >
> > > There are two way outs:
> > >
> > > 1. Always initialize range conversion and unscaled converters, even if
> > > they will be unused, and extend the runtime check.
> > > 2. Re-do initialization if the values change after
> > > sws_setColorspaceDetails.
> > >
> > > I opted for approach 1 because it was simpler and easier to reason
> > > about.
> > > ---
> > > libswscale/swscale.c | 2 +-
> > > libswscale/utils.c | 5 +----
> > > 2 files changed, 2 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/libswscale/swscale.c b/libswscale/swscale.c
> > > index 90e5b299ab..46ba68fe6a 100644
> > > --- a/libswscale/swscale.c
> > > +++ b/libswscale/swscale.c
> > > @@ -1016,7 +1016,7 @@ static int scale_internal(SwsContext *c,
> > > reset_ptr(src2, c->srcFormat);
> > > reset_ptr((void*)dst2, c->dstFormat);
> > >
> > > - if (c->convert_unscaled) {
> > > + if (c->convert_unscaled && !c->lumConvertRange && !c->chrConvertRange) {
> > > int offset = srcSliceY_internal;
> > > int slice_h = srcSliceH;
> > >
> > > diff --git a/libswscale/utils.c b/libswscale/utils.c
> > > index e1ad685972..455955e907 100644
> > > --- a/libswscale/utils.c
> > > +++ b/libswscale/utils.c
> > > @@ -1728,9 +1728,7 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
> > > }
> > >
> > > /* unscaled special cases */
> > > - if (unscaled && !usesHFilter && !usesVFilter &&
> > > - (c->srcRange == c->dstRange || isAnyRGB(dstFormat) ||
> > > - isFloat(srcFormat) || isFloat(dstFormat))){
> > > + if (unscaled && !usesHFilter && !usesVFilter) {
> > > ff_get_unscaled_swscale(c);
> > >
> > > if (c->convert_unscaled) {
> > > @@ -1738,7 +1736,6 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
> > > av_log(c, AV_LOG_INFO,
> > > "using unscaled %s -> %s special converter\n",
> > > av_get_pix_fmt_name(srcFormat), av_get_pix_fmt_name(dstFormat));
> > > - return 0;
> > > }
> > > }
> >
> > the av_log() message will be wrong if this path is unused
>
> Indeed. Perhaps the only way to fix that would be to instead try
> approach 2 and go through full reinit if any params change after
> setColorspaceDetails.
>
> > also this ties convert_unscaled to never do range conversion, if thats
> > intended, i guess thats ok. Otherwise that maybe is a restriction
> > we dont want to add.
>
> In the original code, c->convert_unscaled is set behind a `c->srcRange
> == c->dstRange` check, so how is that a change in behavior?
not "change in behavior" but it may be more complex if a
yuv->yuv unscaled convert is added that supports range convert
because before convert_unscaled is set only when it can be used
afterwards convert_unscaled is always set but sometimes it then
can be used with differing yuv ranges sometimes not.
what i meant is that the conditions related to convert_unscaled
could become more dispersed and duplciated _IF_ specific future
code is added
also its not just c->srcRange == c->dstRange, the condition is
more complex
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2023-10-28 21:53 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-27 17:04 [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context Niklas Haas
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 2/8] avfilter/vf_extractplanes: tag alpha plane as full range Niklas Haas
2023-10-28 2:02 ` Vittorio Giovara
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 3/8] avfilter/vf_alphamerge: warn if input not " Niklas Haas
2023-10-27 21:29 ` Michael Niedermayer
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 4/8] avfilter/vf_scale: properly respect to input colorimetry Niklas Haas
2023-10-27 21:01 ` Michael Niedermayer
2023-10-28 10:50 ` Niklas Haas
2023-10-28 11:18 ` Niklas Haas
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 5/8] avfilter/vf_scale: preserve YUV range by default Niklas Haas
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 6/8] avfilter/vf_scale: simplify color matrix parsing logic Niklas Haas
2023-10-28 13:31 ` Niklas Haas
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 7/8] avfilter/vf_scale: tag output color space Niklas Haas
2023-10-28 2:04 ` Vittorio Giovara
2023-10-27 17:04 ` [FFmpeg-devel] [PATCH 8/8] avcodec/pnm: explicitly tag color range Niklas Haas
2023-10-27 18:34 ` Vittorio Giovara
2023-10-27 21:42 ` [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context Michael Niedermayer
2023-10-28 10:46 ` Niklas Haas
2023-10-28 21:53 ` Michael Niedermayer
2023-10-28 14:38 ` Niklas Haas
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git