PR #24538 opened by Niklas Haas (haasn) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24538 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24538.patch
We can re-complicate this at a later point in time if the benchmarks justify it and the logic has settled, but I'm leaning towards this being a non-issue >From 113bb9b24acd275490b67851a306581533405a56 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Tue, 15 Sep 2026 15:46:22 +0200 Subject: [PATCH 1/5] swscale/ops_dispatch: cosmetic Signed-off-by: Niklas Haas <[email protected]> --- libswscale/ops_dispatch.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/libswscale/ops_dispatch.c b/libswscale/ops_dispatch.c index a1f43a867c..1ec8c1e85a 100644 --- a/libswscale/ops_dispatch.c +++ b/libswscale/ops_dispatch.c @@ -595,7 +595,9 @@ static int compile_single(const CompileArgs *args, const SwsOpList *ops, p->planes_out = rw_data_planes(write); p->pixel_bits_out = rw_pixel_bits(write); p->palette_idx = -1; - p->exec_base = (SwsOpExec) { + + SwsOpExec *exec = &p->exec_base; + *exec = (SwsOpExec) { .width = dst->width, .height = dst->height, }; @@ -624,9 +626,9 @@ static int compile_single(const CompileArgs *args, const SwsOpList *ops, const int chroma = idx == 1 || idx == 2; const int sub_x = chroma ? indesc->log2_chroma_w : 0; const int sub_y = chroma ? indesc->log2_chroma_h : 0; - p->exec_base.in_sub_x[i] = sub_x; - p->exec_base.in_sub_y[i] = sub_y; - p->exec_base.block_size_in[i] = block_bits_in >> 3; + exec->in_sub_x[i] = sub_x; + exec->in_sub_y[i] = sub_y; + exec->block_size_in[i] = block_bits_in >> 3; p->idx_in[i] = idx; } @@ -635,9 +637,9 @@ static int compile_single(const CompileArgs *args, const SwsOpList *ops, const int chroma = idx == 1 || idx == 2; const int sub_x = chroma ? outdesc->log2_chroma_w : 0; const int sub_y = chroma ? outdesc->log2_chroma_h : 0; - p->exec_base.out_sub_x[i] = sub_x; - p->exec_base.out_sub_y[i] = sub_y; - p->exec_base.block_size_out[i] = block_bits_out >> 3; + exec->out_sub_x[i] = sub_x; + exec->out_sub_y[i] = sub_y; + exec->block_size_out[i] = block_bits_out >> 3; p->idx_out[i] = idx; } @@ -660,16 +662,16 @@ static int compile_single(const CompileArgs *args, const SwsOpList *ops, line = next; } bump[filter->dst_size - 1] = 0; - p->exec_base.in_bump_y = bump; + exec->in_bump_y = bump; } else if (read && read->rw.filter.op == SWS_OP_FILTER_H) { /* Compute pixel offset map for each output line */ - const int pixels = FFALIGN(filter->dst_size, p->comp.block_size); + const int pixels = FFALIGN(filter->dst_size, comp->block_size); int32_t *offset = av_malloc_array(pixels, sizeof(*offset)); if (!offset) { ret = AVERROR(ENOMEM); goto fail; } - p->exec_base.in_offset_x = offset; + exec->in_offset_x = offset; for (int x = 0; x < filter->dst_size; x++) { /* Sanity check; if the tap would land on a half-pixel, we cannot @@ -685,7 +687,7 @@ static int compile_single(const CompileArgs *args, const SwsOpList *ops, for (int x = filter->dst_size; x < pixels; x++) offset[x] = offset[filter->dst_size - 1]; for (int i = 0; i < 4; i++) - p->exec_base.block_size_in[i] = 0; /* ptr does not advance */ + exec->block_size_in[i] = 0; /* ptr does not advance */ p->filter_size_h = filter->filter_size; } -- 2.52.0 >From 548aa7442fbd96dc20755e58bcee901183590832 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Tue, 15 Sep 2026 15:49:28 +0200 Subject: [PATCH 2/5] swscale/ops_dispatch: pre-compute pass block count This depends only on the compiled op and output size, which are known in advance. Signed-off-by: Niklas Haas <[email protected]> --- libswscale/ops_dispatch.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libswscale/ops_dispatch.c b/libswscale/ops_dispatch.c index 1ec8c1e85a..e0d0d34b31 100644 --- a/libswscale/ops_dispatch.c +++ b/libswscale/ops_dispatch.c @@ -218,11 +218,8 @@ static int op_pass_setup(const SwsFrame *out, const SwsFrame *in, /* Set up main loop parameters */ const unsigned block_size = comp->block_size; - const size_t num_blocks = (width + block_size - 1) / block_size; + const size_t num_blocks = p->num_blocks; const size_t aligned_w = num_blocks * block_size; - if (aligned_w < width) /* overflow */ - return AVERROR(EINVAL); - p->num_blocks = num_blocks; p->memcpy_first = false; p->memcpy_last = false; p->memcpy_out = false; @@ -595,6 +592,9 @@ static int compile_single(const CompileArgs *args, const SwsOpList *ops, p->planes_out = rw_data_planes(write); p->pixel_bits_out = rw_pixel_bits(write); p->palette_idx = -1; + p->num_blocks = (dst->width + comp->block_size - 1) / comp->block_size; + if (p->num_blocks * comp->block_size < (unsigned) dst->width) + return AVERROR(ERANGE); SwsOpExec *exec = &p->exec_base; *exec = (SwsOpExec) { -- 2.52.0 >From 39bd07713a6eaf3812bdb8c5edb3f81ed99be7b7 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Wed, 16 Sep 2026 14:16:10 +0200 Subject: [PATCH 3/5] swscale/ops_dispatch: simplify tail copy logic Instead of only copying the first/last slices, just copy the input for all slices. This only results in a measurable difference when using a very narrow frame, with a large number of threads. (e.g. 128x8192) For regular 16:9 content, the difference is negligible; the extra memory bandwidth is about ~0.3% of the total frame size. Signed-off-by: Niklas Haas <[email protected]> --- libswscale/ops_dispatch.c | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/libswscale/ops_dispatch.c b/libswscale/ops_dispatch.c index e0d0d34b31..f604199b1e 100644 --- a/libswscale/ops_dispatch.c +++ b/libswscale/ops_dispatch.c @@ -55,8 +55,7 @@ typedef struct SwsOpPass { int *offsets_y; int filter_size_h; int filter_size_v; - bool memcpy_first; - bool memcpy_last; + bool memcpy_in; bool memcpy_out; size_t tail_blocks; uint8_t *tail_buf; /* extra memory for fixing unpadded tails */ @@ -220,9 +219,8 @@ static int op_pass_setup(const SwsFrame *out, const SwsFrame *in, const unsigned block_size = comp->block_size; const size_t num_blocks = p->num_blocks; const size_t aligned_w = num_blocks * block_size; - p->memcpy_first = false; - p->memcpy_last = false; - p->memcpy_out = false; + p->memcpy_in = false; + p->memcpy_out = false; size_t safe_blocks = num_blocks; for (int i = 0; i < p->planes_in; i++) { @@ -247,9 +245,8 @@ static int op_pass_setup(const SwsFrame *out, const SwsFrame *in, } if (safe_blocks_in < num_blocks) { - p->memcpy_first |= in->linesize[idx] < 0; - p->memcpy_last |= in->linesize[idx] > 0; - safe_blocks = FFMIN(safe_blocks, safe_blocks_in); + p->memcpy_in = true; + safe_blocks = FFMIN(safe_blocks, safe_blocks_in); } size_t loop_size = num_blocks * exec->block_size_in[i]; @@ -278,8 +275,7 @@ static int op_pass_setup(const SwsFrame *out, const SwsFrame *in, exec->in_stride[1] = exec->in_bump[1] = 0; } - const bool memcpy_in = p->memcpy_first || p->memcpy_last; - if (!memcpy_in && !p->memcpy_out) { + if (!p->memcpy_in && !p->memcpy_out) { av_assert0(safe_blocks == num_blocks); return 0; } @@ -306,7 +302,7 @@ static int op_pass_setup(const SwsFrame *out, const SwsFrame *in, } const size_t alloc_width = aligned_w - safe_width; - for (int i = 0; memcpy_in && i < p->planes_in; i++) { + for (int i = 0; p->memcpy_in && i < p->planes_in; i++) { size_t needed_size; if (exec->in_offset_x) { /* The input offset map is already padded to multiples of the block @@ -330,7 +326,7 @@ static int op_pass_setup(const SwsFrame *out, const SwsFrame *in, alloc_size += tail->out_stride[i] * out->height; } - if (memcpy_in && exec->in_offset_x) { + if (p->memcpy_in && exec->in_offset_x) { /* `in_offset_x` is indexed relative to the line start, not the start * of the section being processed; so we need to over-allocate this * array to the full width of the image, even though we will only @@ -343,7 +339,7 @@ static int op_pass_setup(const SwsFrame *out, const SwsFrame *in, return AVERROR(ENOMEM); uint8_t *tail_buf = p->tail_buf; - for (int i = 0; memcpy_in && i < p->planes_in; i++) { + for (int i = 0; p->memcpy_in && i < p->planes_in; i++) { tail->in[i] = tail_buf; tail_buf += tail->in_stride[i] * in->height; } @@ -353,7 +349,7 @@ static int op_pass_setup(const SwsFrame *out, const SwsFrame *in, tail_buf += tail->out_stride[i] * out->height; } - if (memcpy_in && exec->in_offset_x) { + if (p->memcpy_in && exec->in_offset_x) { tail->in_offset_x = (int32_t *) tail_buf; for (int i = safe_width; i < aligned_w; i++) tail->in_offset_x[i] = exec->in_offset_x[i] - p->tail_off_in; @@ -398,11 +394,8 @@ static void op_pass_run(const SwsFrame *out, const SwsFrame *in, const int y, * memcpy the last column on the output side if unpadded. */ - const int y_in_first = p->offsets_y ? p->offsets_y[y] : y; - const int y_in_last = p->offsets_y ? p->offsets_y[y + h - 1] + p->filter_size_v - 1 - : y + h - 1; - const bool memcpy_in = p->memcpy_last && y_in_last == in->height - 1 || - p->memcpy_first && y_in_first == 0; + const int y_in = p->offsets_y ? p->offsets_y[y] : y; + const bool memcpy_in = p->memcpy_in; const bool memcpy_out = p->memcpy_out; const size_t num_blocks = p->num_blocks; const size_t tail_blocks = p->tail_blocks; @@ -436,7 +429,7 @@ static void op_pass_run(const SwsFrame *out, const SwsFrame *in, const int y, /* Input offsets are relative to the base pointer */ if (!exec.in_offset_x || memcpy_in) exec.in[i] += p->tail_off_in; - tail.in[i] += (y_in_first >> exec.in_sub_y[i]) * tail.in_stride[i]; + tail.in[i] += (y_in >> exec.in_sub_y[i]) * tail.in_stride[i]; } for (int i = 0; i < p->planes_out; i++) { exec.out[i] += p->tail_off_out; -- 2.52.0 >From e846cba579c6717c0d75378b70dc6d959be0b4e2 Mon Sep 17 00:00:00 2001 From: Michael Niedermayer <[email protected]> Date: Sun, 13 Sep 2026 00:33:26 +0200 Subject: [PATCH 4/5] cosmetics: move copy_lines() above op_pass_setup() --- libswscale/ops_dispatch.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/libswscale/ops_dispatch.c b/libswscale/ops_dispatch.c index f604199b1e..f15287537c 100644 --- a/libswscale/ops_dispatch.c +++ b/libswscale/ops_dispatch.c @@ -204,6 +204,17 @@ static size_t safe_blocks_offset(size_t num_blocks, unsigned block_size, return safe_blocks; } +static void copy_lines(uint8_t *dst, const ptrdiff_t dst_stride, + const uint8_t *src, const ptrdiff_t src_stride, + const int h, const size_t bytes) +{ + for (int y = 0; y < h; y++) { + memcpy(dst, src, bytes); + dst += dst_stride; + src += src_stride; + } +} + static int op_pass_setup(const SwsFrame *out, const SwsFrame *in, const SwsPass *pass) { @@ -358,17 +369,6 @@ static int op_pass_setup(const SwsFrame *out, const SwsFrame *in, return 0; } -static void copy_lines(uint8_t *dst, const ptrdiff_t dst_stride, - const uint8_t *src, const ptrdiff_t src_stride, - const int h, const size_t bytes) -{ - for (int y = 0; y < h; y++) { - memcpy(dst, src, bytes); - dst += dst_stride; - src += src_stride; - } -} - static void op_pass_run(const SwsFrame *out, const SwsFrame *in, const int y, const int h, const SwsPass *pass) { -- 2.52.0 >From 650ebff5bf3a82c50446455faddb6d40e742f0c6 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Wed, 16 Sep 2026 18:27:04 +0200 Subject: [PATCH 5/5] swscale/ops_dispatch: copy input lines during setup() when filtering This fixes breakage when vertically filtering unpadded sources, because the previous loop did not account for extra lines from the vertical filter itself. Alternate approach to #24382. Signed-off-by: Niklas Haas <[email protected]> See-Also: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24382 --- libswscale/ops_dispatch.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/libswscale/ops_dispatch.c b/libswscale/ops_dispatch.c index f15287537c..a0d049f616 100644 --- a/libswscale/ops_dispatch.c +++ b/libswscale/ops_dispatch.c @@ -158,18 +158,6 @@ static inline void get_row_data(const SwsOpPass *p, const int y_dst, out[i] = base->out[i] + (y_dst >> base->out_sub_y[i]) * base->out_stride[i]; } -static inline int get_lines_in(const SwsOpPass *p, const int y, const int h, - const int plane) -{ - const SwsOpExec *base = &p->exec_base; - if (!p->offsets_y) - return h >> base->in_sub_y[plane]; - - const int y0 = p->offsets_y[y] >> base->in_sub_y[plane]; - const int y1 = (p->offsets_y[y + h - 1] + p->filter_size_v - 1) >> base->in_sub_y[plane]; - return y1 - y0 + 1; -} - static inline size_t pixel_bytes(size_t pixels, int pixel_bits, enum AVRounding rounding) { @@ -366,6 +354,15 @@ static int op_pass_setup(const SwsFrame *out, const SwsFrame *in, tail->in_offset_x[i] = exec->in_offset_x[i] - p->tail_off_in; } + /* If vertically filtering, we need to copy all lines before processing + * any slice, because filters may read past the current slice bounds */ + for (int i = 0; p->memcpy_in && p->offsets_y && i < p->planes_in; i++) { + const int lines = AV_CEIL_RSHIFT(in->height, exec->in_sub_y[i]); + copy_lines((uint8_t *) tail->in[i], tail->in_stride[i], + exec->in[i] + p->tail_off_in, exec->in_stride[i], + lines, p->tail_size_in); + } + return 0; } @@ -438,9 +435,11 @@ static void op_pass_run(const SwsFrame *out, const SwsFrame *in, const int y, for (int i = 0; i < p->planes_in; i++) { if (memcpy_in) { - const int lines = get_lines_in(p, y, h, i); - copy_lines((uint8_t *) tail.in[i], tail.in_stride[i], - exec.in[i], exec.in_stride[i], lines, p->tail_size_in); + if (!p->offsets_y) { /* already copied by op_pass_setup() */ + const int lines = h >> exec.in_sub_y[i]; + copy_lines((uint8_t *) tail.in[i], tail.in_stride[i], + exec.in[i], exec.in_stride[i], lines, p->tail_size_in); + } } else { /* Reuse input pointers directly */ const size_t loop_size = tail_blocks * exec.block_size_in[i]; -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
