diff mbox series

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

Message ID CABtgRJverLAJWeArfXr_i7NFajne17F3UnkfFZSxs4q=y++_Ng@mail.gmail.com
State New
Headers show
Series [FFmpeg-devel] avfilter/setpts: add command support | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Oleg Afanasyev May 1, 2023, 9:01 p.m. UTC 
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

Comments

Stefano Sabatini May 7, 2023, 10:46 p.m. UTC | #1 
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@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@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.
Oleg Afanasyev May 9, 2023, 11:12 p.m. UTC | #2 
On Sun, 7 May 2023 at 23:47, Stefano Sabatini <stefasab@gmail.com> wrote:
>
> 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@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@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

Fixed! Didn't realize that encoding continues even if command fails,
so it was crashing with the previous expression still in place.

>
> [...]
>
> Looks good to me otherwise.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Stefano Sabatini May 14, 2023, 9:14 a.m. UTC | #3 
On date Wednesday 2023-05-10 00:12:52 +0100, Oleg Afanasyev wrote:
> On Sun, 7 May 2023 at 23:47, Stefano Sabatini <stefasab@gmail.com> wrote:
[...]
> >
> > 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
> 
> Fixed! Didn't realize that encoding continues even if command fails,
> so it was crashing with the previous expression still in place.
> 
> >
> > [...]
> >
> > Looks good to me otherwise.

Applied, thanks.
diff mbox series

Patch

From a714a0957a57c1d392feca0ba675ba5ac7c875ee Mon Sep 17 00:00:00 2001
From: Oleg <oafanasiev@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@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
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -29384,6 +29384,9 @@  The wallclock (RTC) time at the start of the movie in microseconds.
 @item TB
 The timebase of the input timestamps.
 
+@item T_CHANGE
+Time of the first frame after command was applied or time of the first frame if no commands.
+
 @end table
 
 @subsection Examples
@@ -29439,6 +29442,10 @@  asetpts=N/SR/TB
 
 @end itemize
 
+@subsection Commands
+
+Both filters support all above options as @ref{commands}.
+
 @section setrange
 
 Force color range for the output video frame.
diff --git a/libavfilter/setpts.c b/libavfilter/setpts.c
index 5bcc0c2dcf..7b09ce7707 100644
--- a/libavfilter/setpts.c
+++ b/libavfilter/setpts.c
@@ -63,6 +63,7 @@  static const char *const var_names[] = {
     "S",           //   Number of samples in the current frame
     "SR",          //   Audio sample rate
     "FR",          ///< defined only for constant frame-rate video
+    "T_CHANGE",    ///< time of first frame after latest command was applied
     NULL
 };
 
@@ -90,7 +91,8 @@  enum var_name {
     VAR_S,
     VAR_SR,
     VAR_FR,
-    VAR_VARS_NB
+    VAR_T_CHANGE,
+    VAR_VARS_NB,
 };
 
 typedef struct SetPTSContext {
@@ -120,6 +122,7 @@  static av_cold int init(AVFilterContext *ctx)
     setpts->var_values[VAR_PREV_OUTT]   = NAN;
     setpts->var_values[VAR_STARTPTS]    = NAN;
     setpts->var_values[VAR_STARTT]      = NAN;
+    setpts->var_values[VAR_T_CHANGE]    = NAN;
     return 0;
 }
 
@@ -163,6 +166,9 @@  static double eval_pts(SetPTSContext *setpts, AVFilterLink *inlink, AVFrame *fra
         setpts->var_values[VAR_STARTPTS] = TS2D(pts);
         setpts->var_values[VAR_STARTT  ] = TS2T(pts, inlink->time_base);
     }
+    if (isnan(setpts->var_values[VAR_T_CHANGE])) {
+        setpts->var_values[VAR_T_CHANGE] = TS2T(pts, inlink->time_base);
+    }
     setpts->var_values[VAR_PTS       ] = TS2D(pts);
     setpts->var_values[VAR_T         ] = TS2T(pts, inlink->time_base);
 #if FF_API_FRAME_PKT
@@ -269,14 +275,40 @@  static av_cold void uninit(AVFilterContext *ctx)
     setpts->expr = NULL;
 }
 
+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);
+        }
+        setpts->var_values[VAR_T_CHANGE] = NAN;
+    } else {
+        ret = AVERROR(EINVAL);
+    }
+
+    return ret;
+}
+
 #define OFFSET(x) offsetof(SetPTSContext, x)
 #define V AV_OPT_FLAG_VIDEO_PARAM
 #define A AV_OPT_FLAG_AUDIO_PARAM
+#define R AV_OPT_FLAG_RUNTIME_PARAM
 #define F AV_OPT_FLAG_FILTERING_PARAM
 
 #if CONFIG_SETPTS_FILTER
 static const AVOption setpts_options[] = {
-    { "expr", "Expression determining the frame timestamp", OFFSET(expr_str), AV_OPT_TYPE_STRING, { .str = "PTS" }, .flags = V|F },
+    { "expr", "Expression determining the frame timestamp", OFFSET(expr_str), AV_OPT_TYPE_STRING, { .str = "PTS" }, .flags = V|F|R },
     { NULL }
 };
 AVFILTER_DEFINE_CLASS(setpts);
@@ -297,12 +329,13 @@  static const AVFilterPad avfilter_vf_setpts_outputs[] = {
 };
 
 const AVFilter ff_vf_setpts = {
-    .name      = "setpts",
-    .description = NULL_IF_CONFIG_SMALL("Set PTS for the output video frame."),
-    .init      = init,
-    .activate  = activate,
-    .uninit    = uninit,
-    .flags     = AVFILTER_FLAG_METADATA_ONLY,
+    .name            = "setpts",
+    .description     = NULL_IF_CONFIG_SMALL("Set PTS for the output video frame."),
+    .init            = init,
+    .activate        = activate,
+    .uninit          = uninit,
+    .process_command = process_command,
+    .flags           = AVFILTER_FLAG_METADATA_ONLY,
 
     .priv_size = sizeof(SetPTSContext),
     .priv_class = &setpts_class,
@@ -315,7 +348,7 @@  const AVFilter ff_vf_setpts = {
 #if CONFIG_ASETPTS_FILTER
 
 static const AVOption asetpts_options[] = {
-    { "expr", "Expression determining the frame timestamp", OFFSET(expr_str), AV_OPT_TYPE_STRING, { .str = "PTS" }, .flags = A|F },
+    { "expr", "Expression determining the frame timestamp", OFFSET(expr_str), AV_OPT_TYPE_STRING, { .str = "PTS" }, .flags = A|F|R },
     { NULL }
 };
 AVFILTER_DEFINE_CLASS(asetpts);
@@ -336,14 +369,15 @@  static const AVFilterPad asetpts_outputs[] = {
 };
 
 const AVFilter ff_af_asetpts = {
-    .name        = "asetpts",
-    .description = NULL_IF_CONFIG_SMALL("Set PTS for the output audio frame."),
-    .init        = init,
-    .activate    = activate,
-    .uninit      = uninit,
-    .priv_size   = sizeof(SetPTSContext),
-    .priv_class  = &asetpts_class,
-    .flags       = AVFILTER_FLAG_METADATA_ONLY,
+    .name            = "asetpts",
+    .description     = NULL_IF_CONFIG_SMALL("Set PTS for the output audio frame."),
+    .init            = init,
+    .activate        = activate,
+    .uninit          = uninit,
+    .process_command = process_command,
+    .priv_size       = sizeof(SetPTSContext),
+    .priv_class      = &asetpts_class,
+    .flags           = AVFILTER_FLAG_METADATA_ONLY,
     FILTER_INPUTS(asetpts_inputs),
     FILTER_OUTPUTS(asetpts_outputs),
 };
-- 
2.40.0