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 #[email protected]
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 <[email protected]>
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 <[email protected]>
---
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 <[email protected]>
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 <[email protected]>
---
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 <cairo.h>
+#include <stdbool.h>
#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 #[email protected]
+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 #[email protected] (-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 <[email protected]>
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 <[email protected]>
---
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 <[email protected]>
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 <[email protected]>
---
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 <[email protected]>
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 <[email protected]>
---
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 <cairo.h>
#include <stdarg.h>
#include <stdio.h>
--
2.49.1
_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
