PR #25245 opened by rajeshgk URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25245 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25245.patch
## Summary Propagate negative errors returned by libswscale graph pass callbacks instead of silently ignoring execution failures. ## Changes - Change `SwsPassFunc` from `void` to `int`, returning zero on success or a negative error code on failure. - Propagate callback errors through both direct and slice-threaded graph execution. - Prevent subsequent graph passes from executing after an earlier pass fails. - Preserve negative errors from legacy scaled and unscaled conversion callbacks. - Propagate Vulkan execution failures while correctly discarding abandoned command recordings. - Add the `fate-sws-graph-error` regression test. ## Validation - Graph regression tests: 13/13 passed. - libswscale FATE suite: passed. - ASAN/UBSAN regression tests: 13/13 passed. - ASAN/UBSAN targeted FATE: passed. - Valgrind memory checks: passed. - Vulkan-enabled compilation: passed. - Git whitespace validation: passed. **Limitation:** Vulkan runtime execution was not tested. ## Scope This patch focuses on graph pass execution error propagation and associated regression coverage. It does not introduce unrelated changes to scaling algorithms. >From 1bcc8f7712ef4166863a91d2558861c04b41f1af Mon Sep 17 00:00:00 2001 From: Rajesh Gopinath Kumar <[email protected]> Date: Mon, 19 Oct 2026 22:50:56 -0500 Subject: [PATCH] swscale/graph: propagate pass execution errors Propagate negative errors returned by swscale graph pass callbacks instead of silently discarding them. Change SwsPassFunc to return an integer status and forward failures through direct and slice-threaded execution. Stop subsequent passes when an earlier pass fails. Preserve negative errors from legacy scaled and unscaled converters. Handle Vulkan execution failures and discard abandoned command recordings without double-discarding submission failures. Add FATE regression coverage for callback failures, setup failures, downstream-pass suppression, threaded execution, and recovery. --- libswscale/Makefile | 3 +- libswscale/graph.c | 59 ++++-- libswscale/graph.h | 4 +- libswscale/ops_dispatch.c | 6 +- libswscale/tests/graph_error.c | 348 +++++++++++++++++++++++++++++++++ libswscale/vulkan/ops.c | 44 +++-- tests/fate/libswscale.mak | 5 + 7 files changed, 437 insertions(+), 32 deletions(-) create mode 100644 libswscale/tests/graph_error.c diff --git a/libswscale/Makefile b/libswscale/Makefile index f6dbed3c086..2d2fb406ccf 100644 --- a/libswscale/Makefile +++ b/libswscale/Makefile @@ -49,7 +49,8 @@ SHLIBOBJS += log2_tab.o half2float.o # Windows resource file SHLIBOBJS-$(HAVE_GNU_WINDRES) += swscaleres.o -TESTPROGS = colorspace \ +TESTPROGS = graph_error \ + colorspace \ floatimg_cmp \ pixdesc_query \ swscale \ diff --git a/libswscale/graph.c b/libswscale/graph.c index 664dd1c1c37..bffbea48c8e 100644 --- a/libswscale/graph.c +++ b/libswscale/graph.c @@ -279,7 +279,7 @@ static void frame_shift(const SwsFrame *f, const int y, uint8_t *data[4]) } } -static void run_copy(const SwsFrame *out, const SwsFrame *in, int y, int h, +static int run_copy(const SwsFrame *out, const SwsFrame *in, int y, int h, const SwsPass *pass) { uint8_t *in_data[4], *out_data[4]; @@ -303,9 +303,11 @@ static void run_copy(const SwsFrame *out, const SwsFrame *in, int y, int h, } } } + + return 0; } -static void run_rgb0(const SwsFrame *out, const SwsFrame *in, int y, int h, +static int run_rgb0(const SwsFrame *out, const SwsFrame *in, int y, int h, const SwsPass *pass) { SwsInternal *c = pass->priv; @@ -324,24 +326,30 @@ static void run_rgb0(const SwsFrame *out, const SwsFrame *in, int y, int h, src += src_stride; dst += dst_stride; } + + return 0; } -static void run_xyz2rgb(const SwsFrame *out, const SwsFrame *in, int y, int h, +static int run_xyz2rgb(const SwsFrame *out, const SwsFrame *in, int y, int h, const SwsPass *pass) { const SwsInternal *c = pass->priv; c->xyz12Torgb48(c, out->data[0] + y * out->linesize[0], out->linesize[0], in->data[0] + y * in->linesize[0], in->linesize[0], out->width, h); + + return 0; } -static void run_rgb2xyz(const SwsFrame *out, const SwsFrame *in, int y, int h, +static int run_rgb2xyz(const SwsFrame *out, const SwsFrame *in, int y, int h, const SwsPass *pass) { const SwsInternal *c = pass->priv; c->rgb48Toxyz12(c, out->data[0] + y * out->linesize[0], out->linesize[0], in->data[0] + y * in->linesize[0], in->linesize[0], out->width, h); + + return 0; } /*********************************************************************** @@ -391,7 +399,7 @@ static inline SwsContext *slice_ctx(const SwsPass *pass, int y) return sws; } -static void run_legacy_unscaled(const SwsFrame *out, const SwsFrame *in, +static int run_legacy_unscaled(const SwsFrame *out, const SwsFrame *in, int y, int h, const SwsPass *pass) { SwsContext *sws = slice_ctx(pass, y); @@ -399,11 +407,16 @@ static void run_legacy_unscaled(const SwsFrame *out, const SwsFrame *in, uint8_t *in_data[4]; frame_shift(in, y, in_data); - c->convert_unscaled(c, (const uint8_t *const *) in_data, in->linesize, y, h, - out->data, out->linesize); + int ret = c->convert_unscaled(c, (const uint8_t *const *) in_data, + in->linesize, y, h, + out->data, out->linesize); + if (ret < 0) + return ret; + + return 0; } -static void run_legacy_swscale(const SwsFrame *out, const SwsFrame *in, +static int run_legacy_swscale(const SwsFrame *out, const SwsFrame *in, int y, int h, const SwsPass *pass) { SwsContext *sws = slice_ctx(pass, y); @@ -411,11 +424,16 @@ static void run_legacy_swscale(const SwsFrame *out, const SwsFrame *in, uint8_t *out_data[4]; frame_shift(out, y, out_data); - ff_swscale(c, (const uint8_t *const *) in->data, in->linesize, 0, - sws->src_h, out_data, out->linesize, y, h); + int ret = ff_swscale(c, (const uint8_t *const *) in->data, + in->linesize, 0, sws->src_h, out_data, + out->linesize, y, h); + if (ret < 0) + return ret; + + return 0; } -static void run_legacy_lut3d(const SwsFrame *out, const SwsFrame *in, +static int run_legacy_lut3d(const SwsFrame *out, const SwsFrame *in, int y, int h, const SwsPass *pass) { const SwsLut3D *lut = pass->graph->lut3d; @@ -425,6 +443,8 @@ static void run_legacy_lut3d(const SwsFrame *out, const SwsFrame *in, ff_sws_lut3d_apply_rgba64(lut, in_data[0], in->linesize[0], out_data[0], out->linesize[0], out->width, h); + + return 0; } static void legacy_chr_pos(SwsGraph *graph, int *chr_pos, int override, int *warned) @@ -865,8 +885,8 @@ static int sws_graph_worker(void *priv, int jobnr, int threadnr, int nb_jobs, const int slice_y = jobnr * pass->slice_h; const int slice_h = FFMIN(pass->slice_h, pass->lines - slice_y); - pass->run(graph->exec.output, graph->exec.input, slice_y, slice_h, pass); - return 0; + return pass->run(graph->exec.output, graph->exec.input, + slice_y, slice_h, pass); } SwsGraph *ff_sws_graph_alloc(void) @@ -1043,17 +1063,22 @@ int ff_sws_graph_run(SwsGraph *graph, const AVFrame *dst, const AVFrame *src) graph->exec.pass = pass; graph->exec.input = pass->input ? &pass->input->output->frame : &src_field; graph->exec.output = pass->output->avframe ? &pass->output->frame : &dst_field; + int ret; + if (pass->setup) { - int ret = pass->setup(graph->exec.output, graph->exec.input, pass); + ret = pass->setup(graph->exec.output, graph->exec.input, pass); if (ret < 0) return ret; } - if (pass->num_slices == 1) { - pass->run(graph->exec.output, graph->exec.input, 0, pass->lines, pass); + ret = pass->run(graph->exec.output, graph->exec.input, + 0, pass->lines, pass); } else { - avpriv_slicethread_execute2(graph->slicethread, pass->num_slices, 0); + ret = avpriv_slicethread_execute2(graph->slicethread, + pass->num_slices, 0); } + if (ret < 0) + return ret; } return 0; diff --git a/libswscale/graph.h b/libswscale/graph.h index 7380a991ce5..774b0dd4d0b 100644 --- a/libswscale/graph.h +++ b/libswscale/graph.h @@ -42,8 +42,10 @@ typedef struct SwsGraph SwsGraph; /** * Output `h` lines of filtered data. `out` and `in` point to the * start of the image buffer for this pass. + * + * Return 0 on success or a negative AVERROR code on failure. */ -typedef void (*SwsPassFunc)(const SwsFrame *out, const SwsFrame *in, +typedef int (*SwsPassFunc)(const SwsFrame *out, const SwsFrame *in, int y, int h, const SwsPass *pass); /** diff --git a/libswscale/ops_dispatch.c b/libswscale/ops_dispatch.c index a0d049f6164..f029e39c644 100644 --- a/libswscale/ops_dispatch.c +++ b/libswscale/ops_dispatch.c @@ -366,7 +366,7 @@ static int op_pass_setup(const SwsFrame *out, const SwsFrame *in, return 0; } -static void op_pass_run(const SwsFrame *out, const SwsFrame *in, const int y, +static int op_pass_run(const SwsFrame *out, const SwsFrame *in, const int y, const int h, const SwsPass *pass) { const SwsOpPass *p = pass->priv; @@ -401,7 +401,7 @@ static void op_pass_run(const SwsFrame *out, const SwsFrame *in, const int y, if (!memcpy_in && !memcpy_out) { /* Fast path (fully aligned/padded inputs and outputs) */ comp->func(&exec, comp->priv, 0, y, num_blocks, y + h); - return; + return 0; } /* Non-aligned case (slow path); process main blocks as normal, and @@ -466,6 +466,8 @@ static void op_pass_run(const SwsFrame *out, const SwsFrame *in, const int y, copy_lines(exec.out[i], exec.out_stride[i], tail.out[i], tail.out_stride[i], lines, p->tail_size_out); } + + return 0; } /* Updates the plane copy (no-op) map for this operation list */ diff --git a/libswscale/tests/graph_error.c b/libswscale/tests/graph_error.c new file mode 100644 index 00000000000..d7fd2a09f42 --- /dev/null +++ b/libswscale/tests/graph_error.c @@ -0,0 +1,348 @@ +/* + * swscale graph error propagation regression tests + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + */ + +#include <stdatomic.h> +#include <errno.h> +#include <stdio.h> +#include <string.h> + +#include "libavutil/error.h" +#include "libavutil/frame.h" +#include "libavutil/pixfmt.h" +#include "libavutil/slicethread.h" +#include "libswscale/graph.h" + +typedef struct TestState { + int result; + int setup_result; + int calls; + int setup_calls; + int last_y; + int last_h; +} TestState; + +static int test_run(const SwsFrame *out, const SwsFrame *in, + int y, int h, const SwsPass *pass) +{ + TestState *state = pass->priv; + + state->calls++; + state->last_y = y; + state->last_h = h; + + return state->result; +} + +static int test_setup(const SwsFrame *out, const SwsFrame *in, + const SwsPass *pass) +{ + TestState *state = pass->priv; + + state->setup_calls++; + return state->setup_result; +} + +static int run_case(const char *name, + int height, + int first_result, + int first_setup_result, + int second_result, + int use_second, + int expected, + int expected_first_calls, + int expected_second_calls) +{ + TestState first = { + .result = first_result, + .setup_result = first_setup_result, + }; + TestState second = { + .result = second_result, + }; + + SwsPassBuffer buffer1 = {0}; + SwsPassBuffer buffer2 = {0}; + + SwsPass pass1 = { + .run = test_run, + .setup = test_setup, + .priv = &first, + .lines = height, + .num_slices = 1, + .output = &buffer1, + }; + + SwsPass pass2 = { + .run = test_run, + .priv = &second, + .lines = height, + .num_slices = 1, + .output = &buffer2, + }; + + SwsPass *passes[2] = { &pass1, &pass2 }; + + SwsGraph graph = { + .passes = passes, + .num_passes = use_second ? 2 : 1, + }; + + AVFrame src = { + .format = AV_PIX_FMT_GRAY8, + .width = 4, + .height = height, + }; + + AVFrame dst = src; + + graph.src.format = AV_PIX_FMT_GRAY8; + graph.dst.format = AV_PIX_FMT_GRAY8; + + int ret = ff_sws_graph_run(&graph, &dst, &src); + + if (ret != expected || + first.calls != expected_first_calls || + second.calls != expected_second_calls || + first.setup_calls != 1 || + (first.calls && (first.last_y != 0 || + first.last_h != height))) { + fprintf(stderr, + "FAIL %-30s ret=%d expected=%d " + "calls=%d/%d expected=%d/%d setup=%d\n", + name, ret, expected, + first.calls, second.calls, + expected_first_calls, expected_second_calls, + first.setup_calls); + return 1; + } + + printf("PASS %s\n", name); + return 0; +} + + +typedef struct ThreadState { + atomic_int calls; + atomic_int failed; + atomic_int last_slice_height; + int error; + int fail_y; +} ThreadState; + +static int threaded_run(const SwsFrame *out, const SwsFrame *in, + int y, int h, const SwsPass *pass) +{ + ThreadState *state = pass->priv; + + atomic_fetch_add(&state->calls, 1); + + if (y == pass->lines - h) + atomic_store(&state->last_slice_height, h); + + if (state->error && y == state->fail_y) { + atomic_fetch_add(&state->failed, 1); + return state->error; + } + + return 0; +} + +static int threaded_worker(void *priv, int jobnr, int threadnr, + int nb_jobs, int nb_threads) +{ + SwsGraph *graph = priv; + const SwsPass *pass = graph->exec.pass; + const int y = jobnr * pass->slice_h; + const int h = FFMIN(pass->slice_h, pass->lines - y); + + return pass->run(graph->exec.output, graph->exec.input, + y, h, pass); +} + +static int run_threaded_case(const char *name, int height, + int slice_height, int error, + int fail_y, int use_second, + int expected, int expect_recovery) +{ + ThreadState state = { + .error = error, + .fail_y = fail_y, + }; + ThreadState downstream = {0}; + + SwsPassBuffer buffer1 = {0}; + SwsPassBuffer buffer2 = {0}; + + SwsPass pass1 = { + .run = threaded_run, + .priv = &state, + .lines = height, + .slice_h = slice_height, + .num_slices = (height + slice_height - 1) / slice_height, + .output = &buffer1, + }; + + SwsPass pass2 = { + .run = threaded_run, + .priv = &downstream, + .lines = height, + .num_slices = 1, + .output = &buffer2, + }; + + SwsPass *passes[2] = { &pass1, &pass2 }; + + SwsGraph graph = { + .passes = passes, + .num_passes = use_second ? 2 : 1, + }; + + AVFrame src = { + .format = AV_PIX_FMT_GRAY8, + .width = 4, + .height = height, + }; + + AVFrame dst = src; + int ret; + int failed = 0; + + graph.src.format = AV_PIX_FMT_GRAY8; + graph.dst.format = AV_PIX_FMT_GRAY8; + + ret = avpriv_slicethread_create2(&graph.slicethread, + &graph, threaded_worker, + NULL, 4); + if (ret < 0) { + fprintf(stderr, "FAIL %s: thread creation %d\n", name, ret); + return 1; + } + + ret = ff_sws_graph_run(&graph, &dst, &src); + + if (ret != expected) { + fprintf(stderr, "FAIL %s: ret=%d expected=%d\n", + name, ret, expected); + failed = 1; + } + + if (error && + atomic_load(&state.failed) == 0) { + fprintf(stderr, "FAIL %s: failure callback not executed\n", + name); + failed = 1; + } + + if (expected < 0 && + atomic_load(&downstream.calls) != 0) { + fprintf(stderr, "FAIL %s: downstream pass executed\n", + name); + failed = 1; + } + + if (!error && + atomic_load(&state.calls) != pass1.num_slices) { + fprintf(stderr, "FAIL %s: expected %d slices, got %d\n", + name, pass1.num_slices, atomic_load(&state.calls)); + failed = 1; + } + + if (!error && + atomic_load(&state.last_slice_height) != + height - (pass1.num_slices - 1) * slice_height) { + fprintf(stderr, "FAIL %s: incorrect final slice height\n", + name); + failed = 1; + } + + if (expect_recovery) { + state.error = 0; + atomic_store(&state.calls, 0); + atomic_store(&state.failed, 0); + + ret = ff_sws_graph_run(&graph, &dst, &src); + + if (ret != 0 || + atomic_load(&state.calls) != pass1.num_slices) { + fprintf(stderr, "FAIL %s: recovery ret=%d calls=%d\n", + name, ret, atomic_load(&state.calls)); + failed = 1; + } + } + + avpriv_slicethread_free(&graph.slicethread); + + if (!failed) + printf("PASS %s\n", name); + + return failed; +} + +int main(void) +{ + int failures = 0; + + failures += run_case("single-success", + 8, 0, 0, 0, 0, 0, 1, 0); + + failures += run_case("single-einval", + 8, AVERROR(EINVAL), 0, 0, 0, + AVERROR(EINVAL), 1, 0); + + failures += run_case("single-enomem", + 8, AVERROR(ENOMEM), 0, 0, 0, + AVERROR(ENOMEM), 1, 0); + + failures += run_case("setup-error", + 8, 0, AVERROR(EINVAL), 0, 0, + AVERROR(EINVAL), 0, 0); + + failures += run_case("first-pass-error", + 8, AVERROR(EINVAL), 0, 0, 1, + AVERROR(EINVAL), 1, 0); + + failures += run_case("two-pass-success", + 8, 0, 0, 0, 1, 0, 1, 1); + + failures += run_case("single-line-boundary", + 1, 0, 0, 0, 0, 0, 1, 0); + + + failures += run_threaded_case("threaded-success", + 8, 2, 0, 0, 0, 0, 0); + + failures += run_threaded_case("threaded-einval", + 8, 2, AVERROR(EINVAL), 0, + 0, AVERROR(EINVAL), 0); + + failures += run_threaded_case("threaded-enomem", + 8, 2, AVERROR(ENOMEM), 0, + 0, AVERROR(ENOMEM), 0); + + failures += run_threaded_case("threaded-downstream-skip", + 8, 2, AVERROR(EINVAL), 0, + 1, AVERROR(EINVAL), 0); + + failures += run_threaded_case("threaded-recovery", + 8, 2, AVERROR(EINVAL), 0, + 0, AVERROR(EINVAL), 1); + + failures += run_threaded_case("threaded-odd-height", + 9, 2, 0, 0, 0, 0, 0); + + if (failures) { + fprintf(stderr, "%d graph regression tests failed\n", failures); + return 1; + } + + printf("All graph error propagation tests passed\n"); + return 0; +} diff --git a/libswscale/vulkan/ops.c b/libswscale/vulkan/ops.c index 3f8c34ecda3..6023851624d 100644 --- a/libswscale/vulkan/ops.c +++ b/libswscale/vulkan/ops.c @@ -91,27 +91,41 @@ typedef struct VulkanPriv { int interlaced; } VulkanPriv; -static void process(const SwsFrame *dst, const SwsFrame *src, int y, int h, +static int process(const SwsFrame *dst, const SwsFrame *src, int y, int h, const SwsPass *pass) { VulkanPriv *p = (VulkanPriv *) pass->priv; FFVkExecContext *ec = ff_vk_exec_get(&p->s->vkctx, &p->e); FFVulkanFunctions *vk = &p->s->vkctx.vkfn; - ff_vk_exec_start(&p->s->vkctx, ec); + int ret = ff_vk_exec_start(&p->s->vkctx, ec); + if (ret < 0) + return ret; AVFrame *src_f = (AVFrame *) src->avframe; AVFrame *dst_f = (AVFrame *) dst->avframe; - ff_vk_exec_add_dep_frame(&p->s->vkctx, ec, src_f, - VK_PIPELINE_STAGE_2_ALL_COMMANDS_BIT, - VK_PIPELINE_STAGE_2_COMPUTE_SHADER_BIT); - ff_vk_exec_add_dep_frame(&p->s->vkctx, ec, dst_f, - VK_PIPELINE_STAGE_2_ALL_COMMANDS_BIT, - VK_PIPELINE_STAGE_2_COMPUTE_SHADER_BIT); + ret = ff_vk_exec_add_dep_frame(&p->s->vkctx, ec, src_f, + VK_PIPELINE_STAGE_2_ALL_COMMANDS_BIT, + VK_PIPELINE_STAGE_2_COMPUTE_SHADER_BIT); + if (ret < 0) + goto fail; + + ret = ff_vk_exec_add_dep_frame(&p->s->vkctx, ec, dst_f, + VK_PIPELINE_STAGE_2_ALL_COMMANDS_BIT, + VK_PIPELINE_STAGE_2_COMPUTE_SHADER_BIT); + if (ret < 0) + goto fail; VkImageView src_views[AV_NUM_DATA_POINTERS]; VkImageView dst_views[AV_NUM_DATA_POINTERS]; - ff_vk_create_imageviews(&p->s->vkctx, ec, src_views, src_f, p->src_rep); - ff_vk_create_imageviews(&p->s->vkctx, ec, dst_views, dst_f, p->dst_rep); + ret = ff_vk_create_imageviews(&p->s->vkctx, ec, src_views, src_f, + p->src_rep); + if (ret < 0) + goto fail; + + ret = ff_vk_create_imageviews(&p->s->vkctx, ec, dst_views, dst_f, + p->dst_rep); + if (ret < 0) + goto fail; ff_vk_shader_update_img_array(&p->s->vkctx, ec, &p->shd, src_f, src_views, 0, 0, VK_IMAGE_LAYOUT_GENERAL, VK_NULL_HANDLE); @@ -152,8 +166,16 @@ static void process(const SwsFrame *dst, const SwsFrame *src, int y, int h, FFALIGN(dst->height, p->shd.lg_size[1])/p->shd.lg_size[1], 1); - ff_vk_exec_submit(&p->s->vkctx, ec); + ret = ff_vk_exec_submit(&p->s->vkctx, ec); + if (ret < 0) + return ret; + ff_vk_exec_wait(&p->s->vkctx, ec); + return 0; + +fail: + ff_vk_exec_discard(&p->s->vkctx, ec); + return ret; } static void free_fn(void *priv) diff --git a/tests/fate/libswscale.mak b/tests/fate/libswscale.mak index 42501feca60..3e57ae95c03 100644 --- a/tests/fate/libswscale.mak +++ b/tests/fate/libswscale.mak @@ -1,3 +1,8 @@ +FATE_LIBSWSCALE += fate-sws-graph-error +fate-sws-graph-error: libswscale/tests/graph_error$(EXESUF) +fate-sws-graph-error: CMD = run libswscale/tests/graph_error$(EXESUF) +fate-sws-graph-error: CMP = null + FATE_LIBSWSCALE += fate-sws-pixdesc-query fate-sws-pixdesc-query: libswscale/tests/pixdesc_query$(EXESUF) fate-sws-pixdesc-query: CMD = run libswscale/tests/pixdesc_query$(EXESUF) -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
