[FFmpeg-devel] [PATCH] avfilter/setpts: add command support

[Stefano Sabatini]

On date Monday 2023-05-01 22:01:05 +0100, Oleg Afanasyev wrote:
> I'm using setpts to generate timelapses with slowdowns in the middle.
> Using setpts filter requires complicated expr to handle intervals. This
> patch allows commands to change expr and also adds a constant that
> provides time of last command applications to allow specifying gradual
> changes using difference between time and cmd time.
> 
> ----------------------
> with best regards
> Oleg Afanasyev

> From a714a0957a57c1d392feca0ba675ba5ac7c875ee Mon Sep 17 00:00:00 2001
> From: Oleg <oafanasiev at gmail.com>
> Date: Sat, 29 Apr 2023 19:56:46 +0100
> Subject: [PATCH] avfilter/setpts: add command support
> 
> Add support for changing expr on the fly.
> 
> Signed-off-by: Oleg <oafanasiev at gmail.com>
> ---
>  doc/filters.texi     |  7 +++++
>  libavfilter/setpts.c | 68 +++++++++++++++++++++++++++++++++-----------
>  2 files changed, 58 insertions(+), 17 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 50e1682144..fbdb1f8ecf 100644
> diff --git a/libavfilter/setpts.c b/libavfilter/setpts.c
> index 5bcc0c2dcf..7b09ce7707 100644
> --- a/libavfilter/setpts.c
> +++ b/libavfilter/setpts.c
[...]
> +static int process_command(AVFilterContext *ctx, const char *cmd, const char *arg,
> +                           char *res, int res_len, int flags)
> +{
> +    SetPTSContext *setpts = ctx->priv;
> +    int ret;
> +
> +    ret = ff_filter_process_command(ctx, cmd, arg, res, res_len, flags);
> +
> +    if (ret < 0)
> +        return ret;
> +

> +    if (!strcmp(cmd, "expr")) {
> +        av_expr_free(setpts->expr);
> +        ret = av_expr_parse(&setpts->expr, arg, var_names, NULL, NULL, NULL, NULL, 0, ctx);
> +        if (ret < 0) {
> +            av_log(ctx, AV_LOG_ERROR, "Error while parsing expression '%s'\n", arg);
> +        }

what happens in case setpts->expr is freed and this fails?

probably it should keep a reference to expr and remove it only in case
the new expression was successfully parsed

[...]

Looks good to me otherwise.