PR #22647 opened by Niklas Haas (haasn) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/22647 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/22647.patch
See https://code.ffmpeg.org/FFmpeg/FFmpeg/issues/22264 >From 26ad40355532ed4a1dde52994eb455cfde2837b9 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Fri, 27 Mar 2026 19:14:56 +0100 Subject: [PATCH 1/4] swscale/ops: add missing check on SwsDitherOp.y_offset Doesn't actually affect anything in the currently generated ops lists. Signed-off-by: Niklas Haas <[email protected]> --- libswscale/ops.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libswscale/ops.c b/libswscale/ops.c index ece49e299f..9414a4c8ad 100644 --- a/libswscale/ops.c +++ b/libswscale/ops.c @@ -330,8 +330,11 @@ void ff_sws_op_list_update_comps(SwsOpList *ops) break; case SWS_OP_DITHER: /* Strip zero flag because of the nonzero dithering offset */ - for (int i = 0; i < 4; i++) + for (int i = 0; i < 4; i++) { + if (op->dither.y_offset[i] < 0) + continue; op->comps.flags[i] = prev.flags[i] & ~SWS_COMP_ZERO; + } break; case SWS_OP_UNPACK: for (int i = 0; i < 4; i++) { -- 2.52.0 >From 99aba9c922d164fb2078f0144174606c4bb6d637 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Fri, 27 Mar 2026 19:05:10 +0100 Subject: [PATCH 2/4] swscale/ops: add min/max to SwsDitherOp This gives more accurate information to the range tracker. Signed-off-by: Niklas Haas <[email protected]> --- libswscale/format.c | 11 +++++++++++ libswscale/ops.h | 1 + 2 files changed, 12 insertions(+) diff --git a/libswscale/format.c b/libswscale/format.c index 3464a8af7c..4c5a0080b1 100644 --- a/libswscale/format.c +++ b/libswscale/format.c @@ -1256,6 +1256,8 @@ static int fmt_dither(SwsContext *ctx, SwsOpList *ops, .op = SWS_OP_DITHER, .type = type, .dither.matrix = bias, + .dither.min = *bias, + .dither.max = *bias, }); } else { return 0; /* No-op */ @@ -1271,6 +1273,15 @@ static int fmt_dither(SwsContext *ctx, SwsOpList *ops, if (!dither.matrix) return AVERROR(ENOMEM); + const int size = 1 << dither.size_log2; + dither.min = dither.max = dither.matrix[0]; + for (int i = 1; i < size * size; i++) { + if (av_cmp_q(dither.min, dither.matrix[i]) > 0) + dither.min = dither.matrix[i]; + if (av_cmp_q(dither.matrix[i], dither.max) > 0) + dither.max = dither.matrix[i]; + } + /* Brute-forced offsets; minimizes quantization error across a 16x16 * bayer dither pattern for standard RGBA and YUVA pixel formats */ const int offsets_16x16[4] = {0, 3, 2, 5}; diff --git a/libswscale/ops.h b/libswscale/ops.h index bc8e751f7b..66bf511f4f 100644 --- a/libswscale/ops.h +++ b/libswscale/ops.h @@ -140,6 +140,7 @@ typedef struct SwsConvertOp { typedef struct SwsDitherOp { AVRational *matrix; /* tightly packed dither matrix (refstruct) */ + AVRational min, max; /* minimum/maximum value in `matrix` */ int size_log2; /* size (in bits) of the dither matrix */ int8_t y_offset[4]; /* row offset for each component, or -1 for ignored */ } SwsDitherOp; -- 2.52.0 >From 15ac0420c3e70a24b88e99fb8ac84c39ced54a38 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Fri, 27 Mar 2026 19:18:43 +0100 Subject: [PATCH 3/4] swscale/ops: keep track of correct dither min/max Mostly, this just affects the metadata in benign ways, e.g.: rgb24 -> yuv444p: [ u8 XXXX -> +++X] SWS_OP_READ : 3 elem(s) packed >> 0 min: {0, 0, 0, _}, max: {255, 255, 255, _} [ u8 ...X -> +++X] SWS_OP_CONVERT : u8 -> f32 min: {0, 0, 0, _}, max: {255, 255, 255, _} [f32 ...X -> ...X] SWS_OP_LINEAR : matrix3+off3 [...] min: {16, 16, 16, _}, max: {235, 240, 240, _} [f32 ...X -> ...X] SWS_OP_DITHER : 16x16 matrix + {0 3 2 -1} - min: {33/2, 33/2, 33/2, _}, max: {471/2, 481/2, 481/2, _} + min: {16.001953, 16.001953, 16.001953, _}, max: {235.998047, 240.998047, 240.998047, _} [f32 ...X -> +++X] SWS_OP_CONVERT : f32 -> u8 min: {16, 16, 16, _}, max: {235, 240, 240, _} [ u8 ...X -> +++X] SWS_OP_WRITE : 3 elem(s) planar >> 0 min: {16, 16, 16, _}, max: {235, 240, 240, _} (X = unused, z = byteswapped, + = exact, 0 = zero) However, it surprisingly actually includes a semantic change, whenever converting from limited range to monob or monow: yuv444p -> monow: [ u8 XXXX -> +XXX] SWS_OP_READ : 1 elem(s) planar >> 0 min: {0, _, _, _}, max: {255, _, _, _} [ u8 .XXX -> +XXX] SWS_OP_CONVERT : u8 -> f32 min: {0, _, _, _}, max: {255, _, _, _} [f32 .XXX -> .XXX] SWS_OP_LINEAR : luma [...] min: {-20/219, _, _, _}, max: {235/219, _, _, _} [f32 .XXX -> .XXX] SWS_OP_DITHER : 16x16 matrix + {0 -1 -1 -1} - min: {179/438, _, _, _}, max: {689/438, _, _, _} + min: {-0.089371, _, _, _}, max: {2.071106, _, _, _} + [f32 .XXX -> .XXX] SWS_OP_MAX : {0 0 0 0} <= x + min: {0, _, _, _}, max: {2.071106, _, _, _} [f32 .XXX -> .XXX] SWS_OP_MIN : x <= {1 _ _ _} - min: {179/438, _, _, _}, max: {1, _, _, _} + min: {0, _, _, _}, max: {1, _, _, _} [f32 .XXX -> +XXX] SWS_OP_CONVERT : f32 -> u8 min: {0, _, _, _}, max: {1, _, _, _} [ u8 .XXX -> +XXX] SWS_OP_WRITE : 1 elem(s) planar >> 3 min: {0, _, _, _}, max: {1, _, _, _} (X = unused, z = byteswapped, + = exact, 0 = zero) Note the presence of an extra SWS_OP_MAX, to correctly clamp sub-blacks (values below 16) to 0.0, rather than underflowing. This was previously undetected because the dither was modelled as adding 0.5 to every pixel value, but that's only true on average - not always. Signed-off-by: Niklas Haas <[email protected]> --- libswscale/ops.c | 5 +++++ tests/ref/fate/sws-ops-list | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/libswscale/ops.c b/libswscale/ops.c index 9414a4c8ad..f8b25f8310 100644 --- a/libswscale/ops.c +++ b/libswscale/ops.c @@ -278,6 +278,7 @@ void ff_sws_op_list_update_comps(SwsOpList *ops) switch (op->op) { case SWS_OP_READ: case SWS_OP_LINEAR: + case SWS_OP_DITHER: case SWS_OP_SWAP_BYTES: case SWS_OP_UNPACK: break; /* special cases, handled below */ @@ -331,9 +332,13 @@ void ff_sws_op_list_update_comps(SwsOpList *ops) case SWS_OP_DITHER: /* Strip zero flag because of the nonzero dithering offset */ for (int i = 0; i < 4; i++) { + op->comps.min[i] = prev.min[i]; + op->comps.max[i] = prev.max[i]; if (op->dither.y_offset[i] < 0) continue; op->comps.flags[i] = prev.flags[i] & ~SWS_COMP_ZERO; + op->comps.min[i] = av_add_q(op->comps.min[i], op->dither.min); + op->comps.max[i] = av_add_q(op->comps.max[i], op->dither.max); } break; case SWS_OP_UNPACK: diff --git a/tests/ref/fate/sws-ops-list b/tests/ref/fate/sws-ops-list index c83105406f..a3bbfd994b 100644 --- a/tests/ref/fate/sws-ops-list +++ b/tests/ref/fate/sws-ops-list @@ -1 +1 @@ -121d60ee0261ca5d9650a8d1e9e8a060 +cdcfa182135b5b815b0a474422d7f949 -- 2.52.0 >From acc7d15c9cfe7db20446f44ea2db09bb4fe20e9b Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Fri, 27 Mar 2026 19:29:01 +0100 Subject: [PATCH 4/4] swscale/ops_chain: simplify ff_sws_compile_op_tables() with int index Instead of this needlessly complicated dance of allocating on-stack copies of SwsOpList only to iterate with AVERROR(EAGAIN). This was originally thought to be useful for compiling multiple ops at once, but even that can be solved in easier ways. Signed-off-by: Niklas Haas <[email protected]> --- libswscale/ops_backend.c | 18 ++++++------------ libswscale/ops_chain.c | 13 +++++-------- libswscale/ops_chain.h | 6 +++--- libswscale/x86/ops.c | 20 +++++++------------- 4 files changed, 21 insertions(+), 36 deletions(-) diff --git a/libswscale/ops_backend.c b/libswscale/ops_backend.c index cbc7eba1b4..e7aaeda9d4 100644 --- a/libswscale/ops_backend.c +++ b/libswscale/ops_backend.c @@ -66,20 +66,14 @@ static int compile(SwsContext *ctx, SwsOpList *ops, SwsCompiledOp *out) av_assert0(ops->num_ops > 0); const SwsPixelType read_type = ops->ops[0].type; - /* Make on-stack copy of `ops` to iterate over */ - SwsOpList rest = *ops; - do { + for (int i = 0; i < ops->num_ops; i++) { ret = ff_sws_op_compile_tables(ctx, tables, FF_ARRAY_ELEMS(tables), - &rest, SWS_BLOCK_SIZE, chain); - } while (ret == AVERROR(EAGAIN)); - - if (ret < 0) { - ff_sws_op_chain_free(chain); - if (rest.num_ops < ops->num_ops) { - av_log(ctx, AV_LOG_TRACE, "Uncompiled remainder:\n"); - ff_sws_op_list_print(ctx, AV_LOG_TRACE, AV_LOG_TRACE, &rest); + ops, i, SWS_BLOCK_SIZE, chain); + if (ret < 0) { + av_log(ctx, AV_LOG_TRACE, "Failed to compile op %d\n", i); + ff_sws_op_chain_free(chain); + return ret; } - return ret; } *out = (SwsCompiledOp) { diff --git a/libswscale/ops_chain.c b/libswscale/ops_chain.c index d159acb119..2b8e7622bb 100644 --- a/libswscale/ops_chain.c +++ b/libswscale/ops_chain.c @@ -194,15 +194,15 @@ static int op_match(const SwsOp *op, const SwsOpEntry *entry, const SwsComps nex } int ff_sws_op_compile_tables(SwsContext *ctx, const SwsOpTable *const tables[], - int num_tables, SwsOpList *ops, const int block_size, - SwsOpChain *chain) + int num_tables, SwsOpList *ops, int ops_index, + const int block_size, SwsOpChain *chain) { static const SwsOp dummy = { .comps.unused = { true, true, true, true }}; - const SwsOp *next = ops->num_ops > 1 ? &ops->ops[1] : &dummy; + const SwsOp *op = &ops->ops[ops_index]; + const SwsOp *next = ops->num_ops > ops_index ? op + 1 : &dummy; const unsigned cpu_flags = av_get_cpu_flags(); const SwsOpEntry *best = NULL; const SwsOpTable *best_table = NULL; - const SwsOp *op = &ops->ops[0]; int ret, best_score = 0; for (int n = 0; n < num_tables; n++) { @@ -249,10 +249,7 @@ int ff_sws_op_compile_tables(SwsContext *ctx, const SwsOpTable *const tables[], chain->cpu_flags |= best_table->cpu_flags; chain->over_read = FFMAX(chain->over_read, res.over_read); chain->over_write = FFMAX(chain->over_write, res.over_write); - - ops->ops++; - ops->num_ops--; - return ops->num_ops ? AVERROR(EAGAIN) : 0; + return 0; } #define q2pixel(type, q) ((q).den ? (type) (q).num / (q).den : 0) diff --git a/libswscale/ops_chain.h b/libswscale/ops_chain.h index 648625b7fe..82462b7fa0 100644 --- a/libswscale/ops_chain.h +++ b/libswscale/ops_chain.h @@ -160,10 +160,10 @@ struct SwsOpTable { * "Compile" a single op by looking it up in a list of fixed size op tables. * See `op_match` in `ops_chain.c` for details on how the matching works. * - * Returns 0, AVERROR(EAGAIN), or a negative error code. + * Returns 0 or a negative error code. */ int ff_sws_op_compile_tables(SwsContext *ctx, const SwsOpTable *const tables[], - int num_tables, SwsOpList *ops, const int block_size, - SwsOpChain *chain); + int num_tables, SwsOpList *ops, int ops_index, + const int block_size, SwsOpChain *chain); #endif diff --git a/libswscale/x86/ops.c b/libswscale/x86/ops.c index f4d35ec37b..e2922bfd91 100644 --- a/libswscale/x86/ops.c +++ b/libswscale/x86/ops.c @@ -720,11 +720,9 @@ static int compile(SwsContext *ctx, SwsOpList *ops, SwsCompiledOp *out) .block_size = 2 * FFMIN(mmsize, 32) / ff_sws_op_list_max_size(ops), }; - /* Make on-stack copy of `ops` to iterate over */ - SwsOpList rest = *ops; - do { + for (int i = 0; i < ops->num_ops; i++) { int op_block_size = out->block_size; - SwsOp *op = &rest.ops[0]; + SwsOp *op = &ops->ops[i]; if (op_is_type_invariant(op)) { if (op->op == SWS_OP_CLEAR) @@ -734,16 +732,12 @@ static int compile(SwsContext *ctx, SwsOpList *ops, SwsCompiledOp *out) } ret = ff_sws_op_compile_tables(ctx, tables, FF_ARRAY_ELEMS(tables), - &rest, op_block_size, chain); - } while (ret == AVERROR(EAGAIN)); - - if (ret < 0) { - ff_sws_op_chain_free(chain); - if (rest.num_ops < ops->num_ops) { - av_log(ctx, AV_LOG_TRACE, "Uncompiled remainder:\n"); - ff_sws_op_list_print(ctx, AV_LOG_TRACE, AV_LOG_TRACE, &rest); + ops, i, op_block_size, chain); + if (ret < 0) { + av_log(ctx, AV_LOG_TRACE, "Failed to compile op %d\n", i); + ff_sws_op_chain_free(chain); + return ret; } - return ret; } #define ASSIGN_PROCESS_FUNC(NAME) \ -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
