From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id C86814C0EE for ; Mon, 8 Dec 2025 06:28:22 +0000 (UTC) Authentication-Results: ffbox; dkim=fail (body hash mismatch (got b'83qQJZX3kxDQ2ET3kujQFdh8gTHtKAeYzKNBJnSoQOA=', expected b'oi6EfLTZlLLzI2vN+68eURln4yOfahVF/9x7iZ3mMs8=')) header.d=ffmpeg.org header.i=@ffmpeg.org header.a=rsa-sha256 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ffmpeg.org; i=@ffmpeg.org; q=dns/txt; s=mail; t=1765175252; h=mime-version : to : date : message-id : reply-to : subject : list-id : list-archive : list-archive : list-help : list-owner : list-post : list-subscribe : list-unsubscribe : from : cc : content-type : content-transfer-encoding : from; bh=83qQJZX3kxDQ2ET3kujQFdh8gTHtKAeYzKNBJnSoQOA=; b=QRxtetvMiPDGXE7yv0f8hxa++iaqxpvxysg8aPFRytRtiVjfiQjQVyB8MzdHpHhYJhRJH 3NMdeCA5jLhD3u3P+0YzuEzurQfDSATNZoaxS3PQp5Ea421GOgogb4rj+8m/FExkeW4PDRe jzYM6ZPMw71vOPu8ee1sM9MCX/hItntldH9zX8/B0WY76Ozwi+cDvG1e1RzgF6/yTvHtYSs nZx39tI744ULl/LDIEBkzQ2L3CLw4wI4rFJt2IQ9dk1ZuJfUUNfa2WDdnTYldSlN+g0ugCC EmfpsT+oi+CI2JqUlb/Y+fUv89h9QBRtdkVW6SW+7LNGUCNvNTW1yZcCoEzw== Received: from [172.19.0.3] (unknown [172.19.0.3]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id 500E6690643; Mon, 8 Dec 2025 08:27:32 +0200 (EET) ARC-Seal: i=1; cv=none; a=rsa-sha256; d=ffmpeg.org; s=arc; t=1765175235; b=ZvgytOzFgdVOTH0bDXmipmpgu0M96QHWn6vRWZuARTLfxI+Amdo6ZlmlyR7cmAgJVaYuS pJRXhOltt7+hG++7+KyPgWr/TPTvqzLG83lnSqk6b3khXRKYTkN3YJ8bwU5KWKwrRyemBnX 3jtV5xifewJU3ClS1xJ2o5l3c67XRXDhgT46dnVHjJkzXpQX/u7z7ipVGwbEXgLzvzOjyWY w378xqgcATak+gurpdKzRiyioI9Ph4bmJPvUrYJLWVwOFPDq1q0xDD6esVO0a+D7/WObICc d184HVeYQQLcfCsm2CUTys6dO/JvjIIqOHAMyq/ZyFncBUoCjzt02GE3lJmw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=ffmpeg.org; s=arc; t=1765175235; h=from : sender : reply-to : subject : date : message-id : to : cc : mime-version : content-type : content-transfer-encoding : content-id : content-description : resent-date : resent-from : resent-sender : resent-to : resent-cc : resent-message-id : in-reply-to : references : list-id : list-help : list-unsubscribe : list-subscribe : list-post : list-owner : list-archive; bh=DuL45WYj2oivU64DM9JE+JZ6HT6EssY8AVoxmVPpozw=; b=sGdJFwbznnWGEX30lLhVJFI9ldFz/9+yROyKddG3ZqH1qCswLmp/bYXBOgPdogxK4f83h +Vj3kZfG16YDfxn2BAH1142x26alPuZSHGpqsY3mPSibyRchmSS6fstpnvMmCYKhssSoSXB vHvGqGnOLPPMRpnrPp7zqX44vsW2LrMVPmW5lx31S1DekcqFN24NEcHzmaic0hsrL2k/f8n JcQz5rkndHI+GabS4E0PLhi/CL5Or+c6sA/YGsITZjSJDtIL8gU1ebegXoXIBDLcn8dKZ68 pR9yFl434/HAONQEnWBnpXFtr2NbtVec6rtbRgeA93bMnlQ7+1cVHNma2fzQ== ARC-Authentication-Results: i=1; ffmpeg.org; dkim=pass header.d=ffmpeg.org header.i=@ffmpeg.org; arc=none; dmarc=pass header.from=ffmpeg.org policy.dmarc=quarantine Authentication-Results: ffmpeg.org; dkim=pass header.d=ffmpeg.org header.i=@ffmpeg.org; arc=none (Message is not ARC signed); dmarc=pass (Used From Domain Record) header.from=ffmpeg.org policy.dmarc=quarantine DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ffmpeg.org; i=@ffmpeg.org; q=dns/txt; s=mail; t=1765175226; h=content-type : mime-version : content-transfer-encoding : from : to : reply-to : subject : date : from; bh=oi6EfLTZlLLzI2vN+68eURln4yOfahVF/9x7iZ3mMs8=; b=QXuI8+iFSUFEsUau8ZTiTKuY93i1niMMPW7pkm5rkxWYSNREEc913KDSExZy0G7wqpg9p K94cx2d+O/L0Kg5k3nxYcQ/as65A0eu8/WI8yi08m8ZWg5Q3DYUov6uRSxXfCQocuGldMTg evpqFYYQEmf2x/qE4R+/MC9/WV3sFPqar118hfbg6DzzeRt4UpQ/qlbQCZpY6oqNqaXhfe5 QyYhBvzoBd8VtbPhlIodn0iXi0vSi33cPBbLO3zMCP2vivJBeLuHm2kie0P4bzf8IXG6IiN jOyugnI7w5RkmWIc+Tdp6DTUzTUpiLvOdF5KgqiMNXkYyu9+IYFsCJhsFwSw== Received: from 55ca25703178 (code.ffmpeg.org [188.245.149.3]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTPS id E70E96905DD for ; Mon, 8 Dec 2025 08:27:05 +0200 (EET) MIME-Version: 1.0 To: ffmpeg-devel@ffmpeg.org Date: Mon, 08 Dec 2025 06:27:05 -0000 Message-ID: <176517522621.39.2428572751279150650@2cb04c0e5124> Message-ID-Hash: SFXC6M5YWPILRUY5IQ33ZAWZO6LKF74X X-Message-ID-Hash: SFXC6M5YWPILRUY5IQ33ZAWZO6LKF74X X-MailFrom: code@ffmpeg.org X-Mailman-Rule-Hits: nonmember-moderation X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; header-match-ffmpeg-devel.ffmpeg.org-0; header-match-ffmpeg-devel.ffmpeg.org-1; header-match-ffmpeg-devel.ffmpeg.org-2; header-match-ffmpeg-devel.ffmpeg.org-3; emergency; member-moderation X-Mailman-Version: 3.3.10 Precedence: list Reply-To: FFmpeg development discussions and patches Subject: [FFmpeg-devel] [PATCH] drawvg filter: improve support for colors in variables (PR #21128) List-Id: FFmpeg development discussions and patches Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: From: "Ayose C. via ffmpeg-devel" Cc: "Ayose C." Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Archived-At: List-Archive: List-Post: PR #21128 opened by Ayose C. (ayosec) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21128 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21128.patch This pull-request implements the two proposed changes in the discussion of #21010, related to how colors are stored in variables: * Instead of writing a `0xRRGGBBAA` numeric value, colors from `defrgba` and `defhsla` are stored as a `double[4]`. Any Cairo function that uses a color (like [`cairo_pattern_create_rgba`](https://www.cairographics.org/manual/cairo-cairo-pattern-t.html#cairo-pattern-create-rgba) or [`cairo_pattern_add_color_stop_rgba`](https://www.cairographics.org/manual/cairo-cairo-pattern-t.html#cairo-pattern-add-color-stop-rgba)) expects each component in a `double` value between `0` and `1`. By storing the raw values (instead of converting them to a single `0xRRGGBBAA` number) we avoid problems like the ones detected in #21010. This method also simplifies the code to load colors from variables (no need for `av_be2ne32` and `color[i] / 255` when a variable is read). * Both `setvar` and `call` now accept colors as `#rrggbb` expressions. Now, instead of something like: ``` setvar someblue 0x1020FF7F call zigzag 0xABCDEFFF 100 ``` We can use a simpler syntax: ``` setvar someblue #1020FF@0.5 call zigzag #ABCDEF 100 ``` This adds some complexity to the parser, but it is much better for users. The color expression is converted to a `double[4]` by the parser (during the initialization of the filter). This array is allocated on the heap, so the size of the union in `VGSArgument` is not increased (i.e. it is still 8 bytes, instead of 32). The previous trick of using a `0xRRGGBBAA` value to define colors is not valid anymore. This change breaks a use-case that was included in the documentation, but the drawvg filter is not part of any release (it was merged just 6 weeks ago), so I expect this should not be a problem. ### Changes on `fate-filter-drawvg-video` The `p()` function is now checked in the `fate-filter-drawvg-video` test. The fix in #21030 was missing `TAG = COPY`. It is needed for the log message when `V=1` is not present: ```console $ make fate-filter-drawvg-video COPY tests/data/fate/drawvg.video TEST filter-drawvg-video ``` ### Fixed Warnings on Windows for `fate-filter-drawvg-interpreter` The test `fate-filter-drawvg-interpreter` requires [replacing the implementation of the `cairo_*` functions](https://code.ffmpeg.org/FFmpeg/FFmpeg/src/commit/c4d22f2d2c27ca6a078c126fbd371ded47b6ef7f/libavfilter/tests/drawvg.c#L60-L127) to print their arguments. When that code is built on a Windows system, the compiler was emitting a warning for every replaced function: ``` libavfilter/tests/drawvg.c:97:11: warning: 'cairo_arc' redeclared without dllimport attribute after being referenced with dll linkage 97 | MOCK_FN_5(cairo_arc); | ^~~~~~~~~ libavfilter/tests/drawvg.c:82:10: note: in definition of macro 'MOCK_FN_5' 82 | void func(cairo_t* cr, double a0, double a1, double a2, double a3, double a4) { \ | ^~~~ [...] ``` To fix this issue, the macro `CAIRO_WIN32_STATIC_BUILD` is defined before including `cairo.h`. Thus, the `dllimport` attribute [is not added to the declarations](https://gitlab.freedesktop.org/cairo/cairo/-/blob/1.18.4/src/cairo.h#L53-62). The `CAIRO_WIN32_STATIC_BUILD` has no effect on non-Windows systems, so there is no need to put it inside an `#ifdef WIN32` (or similar). >>From 23e2114d137be1aa20224dee6c7f7a07debc2347 Mon Sep 17 00:00:00 2001 From: Ayose Date: Fri, 5 Dec 2025 14:18:56 +0000 Subject: [PATCH 1/5] avfilter/vf_drawvg: skip conversions when a color is assigned to a variable. In libcairo, colors are defined as 4 separate components, and each one is double between 0 and 1. Before this commit, colors stored in variables (like `defhsla`) were converted to a `0xRRGGBBAA` value, which introduced some issues due to rounding errors. Now, when a color is assigned to a variable, the original values (a `double[4]`) are stored in a dedicated array (`color_vars`), so no conversion is needed. This change also reduces the cost of reading a color from a variable (no need for `av_be2ne32`, or the `color[i] / 255` operations). Signed-off-by: Ayose --- libavfilter/vf_drawvg.c | 49 +++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/libavfilter/vf_drawvg.c b/libavfilter/vf_drawvg.c index 5d3008084c..5ef04ca616 100644 --- a/libavfilter/vf_drawvg.c +++ b/libavfilter/vf_drawvg.c @@ -972,7 +972,7 @@ static int vgs_parse_statement( if (vgs_token_is_string(&token, parser->var_names[i])) { arg.type = ARG_COLOR_VAR; - arg.variable = i; + arg.variable = i - VAR_U0; break; } } @@ -1333,6 +1333,9 @@ fail: /// Number of different states for the `randomg` function. #define RANDOM_STATES 4 +/// Cairo requires each color component to be a double. +typedef double cairo_color[4]; + /// Block assigned to a procedure by a call to the `proc` command. struct VGSProcedure { const struct VGSProgram *program; @@ -1375,6 +1378,9 @@ struct VGSEvalState { /// executing each statement. double vars[VAR_COUNT]; + /// Colors stored in variables. + cairo_color color_vars[USER_VAR_COUNT]; + /// State for each index available for the `randomg` function. FFSFC64 random_state[RANDOM_STATES]; @@ -1551,9 +1557,13 @@ static int vgs_eval_state_init( return AVERROR(ENOMEM); } - for (int i = 0; i < VAR_COUNT; i++) + for (int i = 0; i < FF_ARRAY_ELEMS(state->vars); i++) state->vars[i] = NAN; + for (int i = 0; i < FF_ARRAY_ELEMS(state->color_vars); i++) + for (int j = 0; j < FF_ARRAY_ELEMS(state->color_vars[i]); j++) + state->color_vars[i][j] = NAN; + return 0; } @@ -1802,7 +1812,7 @@ static int vgs_eval( } while(0) double numerics[MAX_COMMAND_PARAMS]; - double colors[MAX_COMMAND_PARAMS][4]; + cairo_color colors[MAX_COMMAND_PARAMS]; double cx, cy; // Current point. @@ -1828,24 +1838,16 @@ static int vgs_eval( // Compute arguments. for (int arg = 0; arg < statement->args_count; arg++) { - uint8_t color[4]; - const struct VGSArgument *a = &statement->args[arg]; switch (a->type) { case ARG_COLOR: - case ARG_COLOR_VAR: - if (a->type == ARG_COLOR) { - memcpy(color, a->color, sizeof(color)); - } else { - uint32_t c = av_be2ne32((uint32_t)state->vars[a->variable]); - memcpy(color, &c, sizeof(color)); - } + for (int i = 0; i < FF_ARRAY_ELEMS(colors[arg]); i++) + colors[arg][i] = ((double)a->color[i]) / 255.0; + break; - colors[arg][0] = (double)(color[0]) / 255.0, - colors[arg][1] = (double)(color[1]) / 255.0, - colors[arg][2] = (double)(color[2]) / 255.0, - colors[arg][3] = (double)(color[3]) / 255.0; + case ARG_COLOR_VAR: + memcpy(&colors[arg], &state->color_vars[a->variable], sizeof(cairo_color)); break; case ARG_EXPR: @@ -1981,16 +1983,11 @@ static int vgs_eval( b = numerics[3]; } - #define C(v, o) ((uint32_t)lround(av_clipd(v, 0, 1) * 255) << o) - - state->vars[user_var] = (double)( - C(r, 24) - | C(g, 16) - | C(b, 8) - | C(numerics[4], 0) - ); - - #undef C + double *const color_var = state->color_vars[user_var - VAR_U0]; + color_var[0] = r; + color_var[1] = g; + color_var[2] = b; + color_var[3] = numerics[4]; break; } -- 2.49.1 >>From e4b5b3a9c7190b278a40923dd10038f045eab47e Mon Sep 17 00:00:00 2001 From: Ayose Date: Fri, 5 Dec 2025 14:19:05 +0000 Subject: [PATCH 2/5] avfilter/vf_drawvg: support color expressions as setvar/call arguments. The arguments for `setvar` and `call` commands can be colors (like `#rrggbb`). This replaces the previous trick of using `0xRRGGBBAA` values to use colors as procedure arguments. The parser stores colors as the value expected by Cairo (a `double[4]`). This array is allocated on the heap so the size of the union in `VGSArgument` is not increased (i.e. it is still 8 bytes, instead of 32). Signed-off-by: Ayose --- libavfilter/vf_drawvg.c | 159 +++++++++++++++++------ tests/ref/fate/filter-drawvg-interpreter | 10 ++ tests/ref/lavf/drawvg.all | 12 ++ 3 files changed, 140 insertions(+), 41 deletions(-) diff --git a/libavfilter/vf_drawvg.c b/libavfilter/vf_drawvg.c index 5ef04ca616..f89605185f 100644 --- a/libavfilter/vf_drawvg.c +++ b/libavfilter/vf_drawvg.c @@ -26,6 +26,7 @@ */ #include +#include #include "libavutil/avassert.h" #include "libavutil/avstring.h" @@ -220,6 +221,7 @@ struct VGSParameter { PARAM_END, PARAM_MAY_REPEAT, PARAM_NUMERIC, + PARAM_NUMERIC_COLOR, PARAM_NUMERIC_METADATA, PARAM_PROC_ARGS, PARAM_PROC_NAME, @@ -328,7 +330,7 @@ static const struct VGSCommandSpec vgs_commands[] = { { "setlinejoin", CMD_SET_LINE_JOIN, L(C(vgs_consts_line_join)) }, { "setlinewidth", CMD_SET_LINE_WIDTH, L(N) }, { "setrgba", CMD_SET_RGBA, L(N, N, N, N) }, - { "setvar", CMD_SET_VAR, L(V, N) }, + { "setvar", CMD_SET_VAR, L(V, { PARAM_NUMERIC_COLOR }) }, { "stroke", CMD_STROKE, NONE }, { "t", CMD_T_CURVE_TO_REL, R(N, N) }, { "translate", CMD_TRANSLATE, L(N, N) }, @@ -406,6 +408,22 @@ static int vgs_cmd_change_path(enum VGSCommand cmd) { } } + +/// Colors in cairo are defined by 4 values, between 0 and 1. Computed colors +/// (either by #RRGGBB expressions, or by commands like `defhsla`) are stored +/// in the values that will be sent to Cairo. +typedef double cairo_color[4]; + +static av_always_inline void color_copy(cairo_color *dest, const cairo_color *src) { + memcpy(dest, src, sizeof(cairo_color)); +} + +static av_always_inline void color_reset(cairo_color *const dest) { + for (int i = 0; i < FF_ARRAY_ELEMS(*dest); i++) + (*dest)[i] = NAN; +} + + /* * == VGS Parser == * @@ -631,7 +649,6 @@ next_token: struct VGSArgument { enum { ARG_COLOR = 1, - ARG_COLOR_VAR, ARG_CONST, ARG_EXPR, ARG_LITERAL, @@ -642,7 +659,7 @@ struct VGSArgument { } type; union { - uint8_t color[4]; + cairo_color *color; int constant; AVExpr *expr; double literal; @@ -686,6 +703,10 @@ static void vgs_statement_free(struct VGSStatement *stm) { struct VGSArgument *arg = &stm->args[j]; switch (arg->type) { + case ARG_COLOR: + av_freep(&arg->color); + break; + case ARG_EXPR: av_expr_free(arg->expr); break; @@ -720,6 +741,32 @@ static void vgs_free(struct VGSProgram *program) { } } +static int vgs_parse_color( + void *log_ctx, + struct VGSArgument *arg, + const struct VGSParser *parser, + const struct VGSParserToken *token +) { + uint8_t color[4]; + + const int ret = av_parse_color(color, token->lexeme, token->length, log_ctx); + if (ret != 0) { + vgs_log_invalid_token(log_ctx, parser, token, "Expected color."); + return ret; + } + + arg->type = ARG_COLOR; + arg->color = av_malloc(sizeof(cairo_color)); + + if (arg->color == NULL) + return AVERROR(ENOMEM); + + for (int i = 0; i < FF_ARRAY_ELEMS(*arg->color); i++) + (*arg->color)[i] = (double)color[i] / 255.0; + + return 0; +} + /// Consume the next argument as a numeric value, and store it in `arg`. /// /// Return `0` on success, and a negative `AVERROR` code on failure. @@ -727,7 +774,8 @@ static int vgs_parse_numeric_argument( void *log_ctx, struct VGSParser *parser, struct VGSArgument *arg, - int metadata + int metadata, + bool accept_colors ) { int ret; char stack_buf[64]; @@ -783,6 +831,14 @@ static int vgs_parse_numeric_argument( break; case TOKEN_WORD: + // If the token starts with `#` it is parsed as a color. If not, it + // must be a variable. + + if (accept_colors && lexeme[0] == '#') { + ret = vgs_parse_color(log_ctx, arg, parser, &token); + break; + } + ret = 1; for (int i = 0; i < VAR_COUNT; i++) { const char *var = parser->var_names[i]; @@ -825,9 +881,13 @@ static int vgs_parse_numeric_argument( return ret; } -/// Check if the next token is a numeric value, so the last command must be -/// repeated. -static int vgs_parser_can_repeat_cmd(void *log_ctx, struct VGSParser *parser) { +/// Check if the next token is a numeric value (or a color, if `accept_colors` +/// is true), so the last command must be repeated. +static int vgs_parser_can_repeat_cmd( + void *log_ctx, + struct VGSParser *parser, + bool accept_colors +) { struct VGSParserToken token = { 0 }; const int ret = vgs_parser_next_token(log_ctx, parser, &token, 0); @@ -842,12 +902,17 @@ static int vgs_parser_can_repeat_cmd(void *log_ctx, struct VGSParser *parser) { case TOKEN_WORD: // If the next token is a word, it will be considered to repeat - // the command only if it is a variable, and there is not - // known command with the same name. + // the command only if it is a variable, and there is no known + // command with the same name. + // + // Color expressions are also valid if `accept_colors` is true. if (vgs_get_command(token.lexeme, token.length) != NULL) return 1; + if (accept_colors && token.lexeme[0] == '#') + return 0; + for (int i = 0; i < VAR_COUNT; i++) { const char *var = parser->var_names[i]; if (var == NULL) @@ -925,7 +990,7 @@ static int vgs_parse_statement( // to append it to the current statement. if (statement.args_count < MAX_COMMAND_PARAMS - && vgs_parser_can_repeat_cmd(log_ctx, parser) == 0 + && vgs_parser_can_repeat_cmd(log_ctx, parser, false) == 0 ) { param--; } else { @@ -949,7 +1014,7 @@ static int vgs_parse_statement( // May repeat if the next token is numeric. if (param->type != PARAM_END - && vgs_parser_can_repeat_cmd(log_ctx, parser) == 0 + && vgs_parser_can_repeat_cmd(log_ctx, parser, false) == 0 ) { param = &decl->params[0]; statement.args = NULL; @@ -971,20 +1036,18 @@ static int vgs_parse_statement( break; if (vgs_token_is_string(&token, parser->var_names[i])) { - arg.type = ARG_COLOR_VAR; - arg.variable = i - VAR_U0; + arg.type = ARG_VARIABLE; + arg.variable = i; break; } } - if (arg.type == ARG_COLOR_VAR) + if (arg.type == ARG_VARIABLE) break; - ret = av_parse_color(arg.color, token.lexeme, token.length, log_ctx); - if (ret != 0) { - vgs_log_invalid_token(log_ctx, parser, &token, "Expected color."); + ret = vgs_parse_color(log_ctx, &arg, parser, &token); + if (ret != 0) FAIL(EINVAL); - } break; @@ -1023,7 +1086,7 @@ static int vgs_parse_statement( } case PARAM_PROC_ARGS: - if (vgs_parser_can_repeat_cmd(log_ctx, parser) != 0) { + if (vgs_parser_can_repeat_cmd(log_ctx, parser, true) != 0) { // No more arguments. Jump to next parameter. param++; continue; @@ -1038,12 +1101,14 @@ static int vgs_parse_statement( /* fallthrough */ case PARAM_NUMERIC: + case PARAM_NUMERIC_COLOR: case PARAM_NUMERIC_METADATA: ret = vgs_parse_numeric_argument( log_ctx, parser, &arg, - param->type == PARAM_NUMERIC_METADATA + param->type == PARAM_NUMERIC_METADATA, + param->type == PARAM_NUMERIC_COLOR || param->type == PARAM_PROC_ARGS ); if (ret != 0) @@ -1333,9 +1398,6 @@ fail: /// Number of different states for the `randomg` function. #define RANDOM_STATES 4 -/// Cairo requires each color component to be a double. -typedef double cairo_color[4]; - /// Block assigned to a procedure by a call to the `proc` command. struct VGSProcedure { const struct VGSProgram *program; @@ -1561,8 +1623,7 @@ static int vgs_eval_state_init( state->vars[i] = NAN; for (int i = 0; i < FF_ARRAY_ELEMS(state->color_vars); i++) - for (int j = 0; j < FF_ARRAY_ELEMS(state->color_vars[i]); j++) - state->color_vars[i][j] = NAN; + color_reset(&state->color_vars[i]); return 0; } @@ -1842,12 +1903,8 @@ static int vgs_eval( switch (a->type) { case ARG_COLOR: - for (int i = 0; i < FF_ARRAY_ELEMS(colors[arg]); i++) - colors[arg][i] = ((double)a->color[i]) / 255.0; - break; - - case ARG_COLOR_VAR: - memcpy(&colors[arg], &state->color_vars[a->variable], sizeof(cairo_color)); + numerics[arg] = NAN; + color_copy(&colors[arg], a->color); break; case ARG_EXPR: @@ -1861,6 +1918,12 @@ static int vgs_eval( case ARG_VARIABLE: av_assert0(a->variable < VAR_COUNT); numerics[arg] = state->vars[a->variable]; + + if (a->variable >= VAR_U0) + color_copy(&colors[arg], &state->color_vars[a->variable - VAR_U0]); + else + color_reset(&colors[arg]); + break; default: @@ -2165,26 +2228,38 @@ static int vgs_eval( av_log(state->log_ctx, AV_LOG_ERROR, "Missing body for procedure '%s'\n", proc_name); } else { - int ret; double current_vars[MAX_PROC_ARGS] = { 0 }; + cairo_color current_color_vars[MAX_PROC_ARGS]; // Set variables for the procedure arguments for (int i = 0; i < proc_args; i++) { const int var = proc->args[i]; - if (var != -1) { - current_vars[i] = state->vars[var]; - state->vars[var] = numerics[i + 1]; - } + if (var == -1) + continue; + + const int color_var = var - VAR_U0; + + // Assign both color and numeric values. + + current_vars[i] = state->vars[var]; + color_copy(¤t_color_vars[i], &state->color_vars[color_var]); + + state->vars[var] = numerics[i + 1]; + color_copy(&state->color_vars[color_var], &colors[i + 1]); } - ret = vgs_eval(state, proc->program); + const int ret = vgs_eval(state, proc->program); // Restore variable values. for (int i = 0; i < proc_args; i++) { const int var = proc->args[i]; - if (var != -1) { - state->vars[var] = current_vars[i]; - } + if (var == -1) + continue; + + const int color_var = var - VAR_U0; + + color_copy(&state->color_vars[color_var], ¤t_color_vars[i]); + state->vars[var] = current_vars[i]; } if (ret != 0) @@ -2399,9 +2474,11 @@ static int vgs_eval( ASSERT_ARGS(2); const int user_var = statement->args[0].constant; - av_assert0(user_var >= VAR_U0 && user_var < (VAR_U0 + USER_VAR_COUNT)); + + color_copy(&state->color_vars[user_var - VAR_U0], &colors[1]); state->vars[user_var] = numerics[1]; + break; } diff --git a/tests/ref/fate/filter-drawvg-interpreter b/tests/ref/fate/filter-drawvg-interpreter index 3fc33e9c07..d05c7428ee 100644 --- a/tests/ref/fate/filter-drawvg-interpreter +++ b/tests/ref/fate/filter-drawvg-interpreter @@ -76,6 +76,16 @@ cairo_fill cairo_set_source #a8d8f0e6 cairo_set_fill_rule 0 cairo_fill +cairo_set_source #abcdef66 +cairo_stroke +cairo_set_source #123456ff +cairo_stroke +cairo_set_source #abcdef66 +cairo_stroke +av_log[32]: [104:45] a0 = 1.000000 | [104:48] a2 = 123.000000 +cairo_set_source #50a0f033 +cairo_stroke +av_log[32]: [104:45] a0 = -1.000000 | [104:48] a2 = -123.000000 cairo_rel_line_to 1.0 3.0 cairo_rel_line_to nan 0.0 diff --git a/tests/ref/lavf/drawvg.all b/tests/ref/lavf/drawvg.all index 9603c8ed8f..9b1fc9234b 100644 --- a/tests/ref/lavf/drawvg.all +++ b/tests/ref/lavf/drawvg.all @@ -93,6 +93,18 @@ defhsla c1 200 0.7 0.8 0.9 setcolor c0 fill setcolor c1 fill +// Colors as arguments for setvar/call. +setvar color0 #123456 +setvar color1 #abcdef@0.4 +setvar color2 color1 +setvar a 123 + +setcolor color2 stroke +setcolor color0 stroke +proc f3 a0 a1 a2 { setcolor a1 stroke print a0 a2 } +call f3 1 color1 a +call f3 -1 #50A0F0@0.2 (-a) + // Frame metadata getmetadata md0 m.a getmetadata md1 m.b -- 2.49.1 >>From abe07b731b853a7db659986bfd88b51f3e9e9d78 Mon Sep 17 00:00:00 2001 From: Ayose Date: Fri, 5 Dec 2025 14:19:49 +0000 Subject: [PATCH 3/5] avfilter/vf_drawvg: values from the p() function can be used as colors. To be able to reuse colors from the original frame, the last value returned by `p()` is tracked in the eval state, and if it is assigned to a variable, the original color components are copied to `color_vars`. Thus, commands like `setcolor` and `colorstop` can use those variables: setvar pixel (p(0, 0)) ... setcolor pixel `fate-filter-drawvg-video` now also verifies the `p()` function. Signed-off-by: Ayose --- libavfilter/vf_drawvg.c | 49 +++++++++++++++++++++++++----- tests/fate/filter-video.mak | 9 +++--- tests/ref/fate/filter-drawvg-video | 2 +- tests/ref/lavf/drawvg.lines | 10 ------ tests/ref/lavf/drawvg.video | 13 ++++++++ 5 files changed, 61 insertions(+), 22 deletions(-) delete mode 100644 tests/ref/lavf/drawvg.lines create mode 100644 tests/ref/lavf/drawvg.video diff --git a/libavfilter/vf_drawvg.c b/libavfilter/vf_drawvg.c index f89605185f..99280366f3 100644 --- a/libavfilter/vf_drawvg.c +++ b/libavfilter/vf_drawvg.c @@ -1443,6 +1443,12 @@ struct VGSEvalState { /// Colors stored in variables. cairo_color color_vars[USER_VAR_COUNT]; + /// Track last color read by the `p()` function. + struct { + double numeric; + uint8_t components[4]; + } last_fn_p_color; + /// State for each index available for the `randomg` function. FFSFC64 random_state[RANDOM_STATES]; @@ -1555,7 +1561,7 @@ static double vgs_fn_randomg(void *data, double arg) { /// /// If the coordinates are outside the frame, return NAN. static double vgs_fn_p(void* data, double x0, double y0) { - const struct VGSEvalState *state = (struct VGSEvalState *)data; + struct VGSEvalState *state = (struct VGSEvalState *)data; const AVFrame *frame = state->frame; if (frame == NULL || !isfinite(x0) || !isfinite(y0)) @@ -1575,8 +1581,6 @@ static double vgs_fn_p(void* data, double x0, double y0) { for (int c = 0; c < desc->nb_components; c++) { uint32_t pixel; - const int depth = desc->comp[c].depth; - av_read_image_line2( &pixel, (void*)frame->data, @@ -1589,14 +1593,35 @@ static double vgs_fn_p(void* data, double x0, double y0) { 4 // dst_element_size ); - if (depth != 8) { + const int depth = desc->comp[c].depth; + if (depth != 8) pixel = pixel * 255 / ((1 << depth) - 1); - } color[c] = pixel; } - return color[0] << 24 | color[1] << 16 | color[2] << 8 | color[3]; + // A common use-case of `p()` is to store its result in a variable, so it + // can be used later with commands like `setcolor` or `colorstop`: + // + // setvar pixel (p(0, 0)) + // ... + // setcolor pixel + // + // Since we can't pass custom values through `av_expr_eval`, we store the + // color in the `VGSEvalState` instance. Then, when a variable is assigned + // in `setvar`, it checks if the value is the same as the output of `p()`. + // In such case, it copies the color components to the slot in `color_vars`. + // + // This solution is far from perfect, but it works in all the documented + // use-cases. + + const double num = color[0] << 24 | color[1] << 16 | color[2] << 8 | color[3]; + + state->last_fn_p_color.numeric = num; + for (int i = 0; i < FF_ARRAY_ELEMS(color); i++) + state->last_fn_p_color.components[i] = (uint8_t)color[i]; + + return num; } static int vgs_eval_state_init( @@ -1609,7 +1634,9 @@ static int vgs_eval_state_init( state->log_ctx = log_ctx; state->frame = frame; + state->rcp.status = RCP_NONE; + state->last_fn_p_color.numeric = NAN; if (program->proc_names != NULL) { state->procedures = av_calloc(sizeof(struct VGSProcedure), program->proc_names_count); @@ -2476,9 +2503,17 @@ static int vgs_eval( const int user_var = statement->args[0].constant; av_assert0(user_var >= VAR_U0 && user_var < (VAR_U0 + USER_VAR_COUNT)); - color_copy(&state->color_vars[user_var - VAR_U0], &colors[1]); state->vars[user_var] = numerics[1]; + // Take the color from `p()`, if any. + cairo_color *color = &state->color_vars[user_var - VAR_U0]; + if (state->last_fn_p_color.numeric == numerics[1]) { + for (int i = 0; i < FF_ARRAY_ELEMS(*color); i++) + (*color)[i] = state->last_fn_p_color.components[i] / 255.0; + } else { + color_copy(color, &colors[1]); + } + break; } diff --git a/tests/fate/filter-video.mak b/tests/fate/filter-video.mak index 9f3c92a395..b6462cca38 100644 --- a/tests/fate/filter-video.mak +++ b/tests/fate/filter-video.mak @@ -617,13 +617,14 @@ fate-filter-tiltandshift-410: CMD = framecrc -c:v pgmyuv -i $(SRC) -flags +bitex fate-filter-tiltandshift-422: CMD = framecrc -c:v pgmyuv -i $(SRC) -flags +bitexact -vf scale=sws_flags=+accurate_rnd+bitexact,format=yuv422p,tiltandshift fate-filter-tiltandshift-444: CMD = framecrc -c:v pgmyuv -i $(SRC) -flags +bitexact -vf scale=sws_flags=+accurate_rnd+bitexact,format=yuv444p,tiltandshift -DRAWVG_SCRIPT_LINES = tests/data/fate/drawvg.lines -$(DRAWVG_SCRIPT_LINES): $(SRC_PATH)/tests/ref/lavf/drawvg.lines +DRAWVG_SCRIPT_VIDEO = tests/data/fate/drawvg.video +$(DRAWVG_SCRIPT_VIDEO): TAG = COPY +$(DRAWVG_SCRIPT_VIDEO): $(SRC_PATH)/tests/ref/lavf/drawvg.video $(M)cp $< $@ FATE_FILTER_VSYNTH_VIDEO_FILTER-$(CONFIG_DRAWVG_FILTER) += fate-filter-drawvg-video -fate-filter-drawvg-video: $(DRAWVG_SCRIPT_LINES) -fate-filter-drawvg-video: CMD = video_filter scale,format=bgr0,drawvg=file=$(DRAWVG_SCRIPT_LINES) +fate-filter-drawvg-video: $(DRAWVG_SCRIPT_VIDEO) +fate-filter-drawvg-video: CMD = video_filter scale,format=bgr0,drawvg=file=$(DRAWVG_SCRIPT_VIDEO) tests/pixfmts.mak: TAG = GEN tests/pixfmts.mak: ffmpeg$(PROGSSUF)$(EXESUF) | tests diff --git a/tests/ref/fate/filter-drawvg-video b/tests/ref/fate/filter-drawvg-video index 0a646f6e2e..bd339be9b2 100644 --- a/tests/ref/fate/filter-drawvg-video +++ b/tests/ref/fate/filter-drawvg-video @@ -1 +1 @@ -drawvg-video caa7642950ab2fb1367bd28c287f31bd +drawvg-video 0f73962365b69779a604c5d41a4b7ae3 diff --git a/tests/ref/lavf/drawvg.lines b/tests/ref/lavf/drawvg.lines deleted file mode 100644 index e145052d50..0000000000 --- a/tests/ref/lavf/drawvg.lines +++ /dev/null @@ -1,10 +0,0 @@ -// Render a square, for `make fate-filter-drawvg-video`. - -M 10 10 -l 0 10 -h 10 -v -10 -h -10 -z -setcolor blue -stroke diff --git a/tests/ref/lavf/drawvg.video b/tests/ref/lavf/drawvg.video new file mode 100644 index 0000000000..89f6123385 --- /dev/null +++ b/tests/ref/lavf/drawvg.video @@ -0,0 +1,13 @@ +// Render a square, for `make fate-filter-drawvg-video`. + +setvar pixel (p(10, 10)) + +M 10 10 +l 0 100 +h 100 +v -100 +h -100 +z + +setcolor pixel +fill -- 2.49.1 >>From a50268eab3a4507656f62755dd04191d3769f99d Mon Sep 17 00:00:00 2001 From: Ayose Date: Fri, 5 Dec 2025 14:19:57 +0000 Subject: [PATCH 4/5] doc/drawvg-reference: changes on color syntax. Colors expressions (like `#RRGGBB`) can now be used as arguments for `setvar` and `call`. The trick of setting a variable with a `0xRRGGBBAA` value is not valid anymore. Signed-off-by: Ayose --- doc/drawvg-reference.texi | 83 ++++++++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 31 deletions(-) diff --git a/doc/drawvg-reference.texi b/doc/drawvg-reference.texi index 7d381915e8..c503bcec06 100644 --- a/doc/drawvg-reference.texi +++ b/doc/drawvg-reference.texi @@ -450,29 +450,34 @@ Optionally, an @code{@@a} suffix can be added to set the alpha value, where @code{a} is a number between @code{0} and @code{1}. @end itemize -The color can be a variable name. In that case, its value is interpreted -as a @code{0xRRGGBBAA} code. - @example -circle 75 100 50 +circle 70 70 60 setcolor #FF0000 fill -circle 125 100 50 -setvar CustomGreen 0x90EEAAFF -setcolor CustomGreen -fill - -circle 175 100 50 +circle 170 170 60 setcolor blue@@0.5 fill @end example -The commands @vgscmd{setrgba} and @vgscmd{sethsla} allow setting colors -using expressions. +The color can be a variable name. In that case, it must be assigned with +@vgscmd{defrgba}, @vgscmd{defhsla}, or @vgscmd{setvar} and a color. -@vgscmd{defrgba} and @vgscmd{defhsla} compute the color and store it in a -variable. +@example +circle 70 70 60 +setvar CustomGreen #22FF44 +setcolor CustomGreen +fill + +circle 170 170 60 +defhsla CustomBlue 200 0.7 0.5 1 +setcolor CustomBlue +fill +@end example + +The commands @vgscmd{setrgba} and @vgscmd{sethsla} allow setting colors using +expressions. Similar to @vgscmd{defrgba} and @vgscmd{defhsla}, but with no +intermediate variable. @subsection Constants @@ -845,13 +850,13 @@ fill @subsection Variables @vgscmd{setcolor} and @vgscmd{colorstop} accept a variable name as the -argument. When a variable is used, its value is interpreted as a -@code{0xRRGGBBAA} code. +argument. The variable must be assigned with @vgscmd{defrgba}, +@vgscmd{defhsla}, or @vgscmd{setvar} and a color. @codeexample{ @example // Use color #1020FF, alpha = 50% -setvar someblue 0x1020FF7F +setvar someblue #1020FF@@0.5 setcolor someblue @@ -872,15 +877,15 @@ setcolor teal rect 30 30 120 120 fill -setvar teal 0x70AAAAFF // Now, `teal` is #70AAAA -setcolor teal +setvar teal #70AAAA +setcolor teal // Use the new color for `teal`. rect 90 90 120 120 fill @end example } -@vgscmd{defrgba} and @vgscmd{defhsla} compute the @code{0xRRGGBBAA} value -for a color given its color components: +@vgscmd{defrgba} and @vgscmd{defhsla} assign a color to a variable, by providing +an expression for each color component: @itemize @item @@ -1241,9 +1246,9 @@ proc zigzag color y @{ stroke @} -call zigzag 0x40C0FFFF 60 -call zigzag 0x00AABBFF 120 -call zigzag 0x20F0B7FF 180 +call zigzag #40C0FF 60 +call zigzag #00AABB 120 +call zigzag #20F0B7 180 @end example } @@ -1328,9 +1333,26 @@ There are some functions specific to drawvg available in @ffexprs{}. @subsection Function @code{p} -@code{p(x, y)} returns the color of the pixel at coordinates -@code{x, y}, as a @code{0xRRGGBBAA} value. This value can be assigned to -a variable, which can be used later as the argument for @vgscmd{setcolor}. +@code{p(x, y)} returns the color of the pixel at coordinates @code{x, y}, as a +@code{0xRRGGBBAA} value. It can be assigned to a variable, so the color can be +available for @vgscmd{setcolor} and @vgscmd{colorstop} commands. + +If a single expression contains multiple calls to the function, it must return +the value of the last call in order to use it as a color. + +@codeexample{ +In this example, the first call to @code{p(0, 0)} is stored in the variable +@var{0} of the expression. Then, the same expression makes a second call to +@code{p(1, 1)}, and finally it returns the value in the variable @var{0}. + +@example +setvar pixel (st(0, p(0, 0)); p(1, 1); ld(0)) +@end example + +Since the result of the expression is not the last call to @code{p}, the +variable @var{pixel} can not be used as a color, but it still can be used as +a numeric @code{0xRRGGBBAA} value. +} If the coordinates are outside the frame, or any of the arguments is not a finite number (like @@ -1951,8 +1973,8 @@ point}. @signature{defhsla varname @var{h} @var{s} @var{l} @var{a}} Similar to @vgscmd{sethsla}, but instead of establishing the color for -stroke and fill operations, the computed color is stored as a -@code{0xRRGGBBAA} value in the variable @var{varname}. +stroke and fill operations, the computed color is assigned to the +variable @var{varname}. @var{varname} can then be used as a color for @vgscmd{setcolor} and @vgscmd{colorstop}. @@ -1965,8 +1987,7 @@ See @vgscmd{sethsla} for more details on how the color is computed. @signature{defrgba varname @var{r} @var{g} @var{b} @var{a}} Computes a color from the @emph{red}, @emph{green}, @emph{blue}, and -@emph{alpha} components, and assigns it to the variable @var{varname} -as a @code{0xRRGGBBAA} value. +@emph{alpha} components, and assigns it to the variable @var{varname}. All components are values between @code{0} and @code{1}. Values outside that range are clamped to it. -- 2.49.1 >>From 2ef71f4cae85be9ef158916dd30b5c0dd91858f2 Mon Sep 17 00:00:00 2001 From: Ayose Date: Sun, 7 Dec 2025 22:57:22 +0000 Subject: [PATCH 5/5] avfilter/tests/drawvg: fix warnings on WIN32 The compiler was emitting a warning on every Cairo function replaced by the `MOCK_FN_n` macros: warning: 'cairo_...': redeclared without dllimport attribute after being referenced with dll linkage The macro `CAIRO_WIN32_STATIC_BUILD` prevents the attribute `dllimport` on the declarations for these functions. Signed-off-by: Ayose --- libavfilter/tests/drawvg.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/libavfilter/tests/drawvg.c b/libavfilter/tests/drawvg.c index 9fb233d969..487b709ec5 100644 --- a/libavfilter/tests/drawvg.c +++ b/libavfilter/tests/drawvg.c @@ -16,6 +16,15 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ + +// Prevent the `dllimport` attribute in the functions declared by Cairo when +// the test is built on a Windows machine. +// +// This is needed to avoid the "redeclared without dllimport attribute after +// being referenced with dll linkage" warnings on every function redefined by +// the `MOCK_FN_n` macros below. +#define CAIRO_WIN32_STATIC_BUILD + #include #include #include -- 2.49.1 _______________________________________________ ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org