diff mbox series

[FFmpeg-devel] avfilter/vf_fps: extend support for expressions

Message ID 20210605155213.3334-1-jamrial@gmail.com
State Accepted
Commit dd770883e976c91feeb8de58eacd97dfb4e8308e
Headers show
Series [FFmpeg-devel] avfilter/vf_fps: extend support for expressions
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

James Almer June 5, 2021, 3:52 p.m. UTC
AV_OPT_TYPE_VIDEO_RATE AVOption types are parsed as expressions, but in a
limited way. For example, name constants can only be parsed alone and not as
part of a longer expression.

This change allows usage like

ffmpeg -i IN -vf fps="if(eq(source_fps\,film)\,ntsc_film\,source_fps)" OUT

Suggested-by: ffmpeg@fb.com
Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavfilter/vf_fps.c | 65 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 60 insertions(+), 5 deletions(-)

Comments

James Almer June 13, 2021, 4:44 p.m. UTC | #1
On 6/5/2021 12:52 PM, James Almer wrote:
> AV_OPT_TYPE_VIDEO_RATE AVOption types are parsed as expressions, but in a
> limited way. For example, name constants can only be parsed alone and not as
> part of a longer expression.
> 
> This change allows usage like
> 
> ffmpeg -i IN -vf fps="if(eq(source_fps\,film)\,ntsc_film\,source_fps)" OUT
> 
> Suggested-by: ffmpeg@fb.com
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>   libavfilter/vf_fps.c | 65 ++++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 60 insertions(+), 5 deletions(-)

Will apply.
Gyan Doshi June 19, 2021, 7:56 p.m. UTC | #2
On 2021-06-05 21:22, James Almer wrote:
> AV_OPT_TYPE_VIDEO_RATE AVOption types are parsed as expressions, but in a
> limited way. For example, name constants can only be parsed alone and not as
> part of a longer expression.
>
> This change allows usage like
>
> ffmpeg -i IN -vf fps="if(eq(source_fps\,film)\,ntsc_film\,source_fps)" OUT
>
> Suggested-by: ffmpeg@fb.com
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>   libavfilter/vf_fps.c | 65 ++++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 60 insertions(+), 5 deletions(-)
>
> diff --git a/libavfilter/vf_fps.c b/libavfilter/vf_fps.c
> index b7b2d6f2db..29588a5f6e 100644
> --- a/libavfilter/vf_fps.c
> +++ b/libavfilter/vf_fps.c
> @@ -30,6 +30,7 @@
>   #include <stdint.h>
>   
>   #include "libavutil/avassert.h"
> +#include "libavutil/eval.h"
>   #include "libavutil/mathematics.h"
>   #include "libavutil/opt.h"
>   #include "avfilter.h"
> @@ -42,12 +43,47 @@ enum EOFAction {
>       EOF_ACTION_NB
>   };
>   
> +static const char *const var_names[] = {
> +  "source_fps",
> +  "ntsc",
> +  "pal",
> +  "qntsc",
> +  "qpal",
> +  "sntsc",
> +  "spal",
> +  "film",
> +  "ntsc_film",
> +  NULL
> +};
> +
> +enum var_name {
> +  VAR_SOURCE_FPS,
> +  VAR_FPS_NTSC,
> +  VAR_FPS_PAL,
> +  VAR_FPS_QNTSC,
> +  VAR_FPS_QPAL,
> +  VAR_FPS_SNTSC,
> +  VAR_FPS_SPAL,
> +  VAR_FPS_FILM,
> +  VAR_FPS_NTSC_FILM,
> +  VARS_NB
> +};
> +
> +static const double ntsc_fps = 30000.0 / 1001.0;
> +static const double pal_fps = 25.0;
> +static const double qntsc_fps = 30000.0 / 1001.0;
> +static const double qpal_fps = 25.0;
> +static const double sntsc_fps = 30000.0 / 1001.0;
> +static const double spal_fps = 25.0;
> +static const double film_fps = 24.0;
> +static const double ntsc_film_fps = 24000.0 / 1001.0;

Sorry, I missed this before you applied it but what's the q- and s- 
prefixed standards with identical rates?
I can't find any specs or docs online.


> +
>   typedef struct FPSContext {
>       const AVClass *class;
>   
>       double start_time;      ///< pts, in seconds, of the expected first frame
>   
> -    AVRational framerate;   ///< target framerate
> +    char *framerate;        ///< expression that defines the target framerate
>       int rounding;           ///< AVRounding method for timestamps
>       int eof_action;         ///< action performed for last frame in FIFO
>   
> @@ -76,7 +112,7 @@ typedef struct FPSContext {
>   #define V AV_OPT_FLAG_VIDEO_PARAM
>   #define F AV_OPT_FLAG_FILTERING_PARAM
>   static const AVOption fps_options[] = {
> -    { "fps", "A string describing desired output framerate", OFFSET(framerate), AV_OPT_TYPE_VIDEO_RATE, { .str = "25" }, 0, INT_MAX, V|F },
> +    { "fps", "A string describing desired output framerate", OFFSET(framerate), AV_OPT_TYPE_STRING, { .str = "25" }, 0, 0, V|F },
>       { "start_time", "Assume the first PTS should be this value.", OFFSET(start_time), AV_OPT_TYPE_DOUBLE, { .dbl = DBL_MAX}, -DBL_MAX, DBL_MAX, V|F },
>       { "round", "set rounding method for timestamps", OFFSET(rounding), AV_OPT_TYPE_INT, { .i64 = AV_ROUND_NEAR_INF }, 0, 5, V|F, "round" },
>           { "zero", "round towards 0",                 0, AV_OPT_TYPE_CONST, { .i64 = AV_ROUND_ZERO     }, 0, 0, V|F, "round" },
> @@ -99,7 +135,6 @@ static av_cold int init(AVFilterContext *ctx)
>       s->status_pts   = AV_NOPTS_VALUE;
>       s->next_pts     = AV_NOPTS_VALUE;
>   
> -    av_log(ctx, AV_LOG_VERBOSE, "fps=%d/%d\n", s->framerate.num, s->framerate.den);
>       return 0;
>   }
>   
> @@ -153,8 +188,26 @@ static int config_props(AVFilterLink* outlink)
>       AVFilterLink    *inlink = ctx->inputs[0];
>       FPSContext      *s      = ctx->priv;
>   
> -    outlink->time_base  = av_inv_q(s->framerate);
> -    outlink->frame_rate = s->framerate;
> +    double var_values[VARS_NB], res;
> +    int ret;
> +
> +    var_values[VAR_SOURCE_FPS]    = av_q2d(inlink->frame_rate);
> +    var_values[VAR_FPS_NTSC]      = ntsc_fps;
> +    var_values[VAR_FPS_PAL]       = pal_fps;
> +    var_values[VAR_FPS_QNTSC]     = qntsc_fps;
> +    var_values[VAR_FPS_QPAL]      = qpal_fps;
> +    var_values[VAR_FPS_SNTSC]     = sntsc_fps;
> +    var_values[VAR_FPS_SPAL]      = spal_fps;
> +    var_values[VAR_FPS_FILM]      = film_fps;
> +    var_values[VAR_FPS_NTSC_FILM] = ntsc_film_fps;
> +    ret = av_expr_parse_and_eval(&res, s->framerate,
> +                                 var_names, var_values,
> +                                 NULL, NULL, NULL, NULL, NULL, 0, ctx);
> +    if (ret < 0)
> +        return ret;
> +
> +    outlink->frame_rate = av_d2q(res, INT_MAX);
> +    outlink->time_base  = av_inv_q(outlink->frame_rate);
>   
>       /* Calculate the input and output pts offsets for start_time */
>       if (s->start_time != DBL_MAX && s->start_time != AV_NOPTS_VALUE) {
> @@ -173,6 +226,8 @@ static int config_props(AVFilterLink* outlink)
>                  s->in_pts_off, s->out_pts_off, s->start_time);
>       }
>   
> +    av_log(ctx, AV_LOG_VERBOSE, "fps=%d/%d\n", outlink->frame_rate.num, outlink->frame_rate.den);
> +
>       return 0;
>   }
>
James Almer June 19, 2021, 7:59 p.m. UTC | #3
On 6/19/2021 4:56 PM, Gyan Doshi wrote:
> 
> 
> On 2021-06-05 21:22, James Almer wrote:
>> +static const double ntsc_fps = 30000.0 / 1001.0;
>> +static const double pal_fps = 25.0;
>> +static const double qntsc_fps = 30000.0 / 1001.0;
>> +static const double qpal_fps = 25.0;
>> +static const double sntsc_fps = 30000.0 / 1001.0;
>> +static const double spal_fps = 25.0;
>> +static const double film_fps = 24.0;
>> +static const double ntsc_film_fps = 24000.0 / 1001.0;
> 
> Sorry, I missed this before you applied it but what's the q- and s- 
> prefixed standards with identical rates?
> I can't find any specs or docs online.

It's taken from video_rate_abbrs in libavutil/parseutils.c
The description is "VCD compliant" for q and "square pixel" for s.
Gyan Doshi June 20, 2021, 6:56 a.m. UTC | #4
On 2021-06-20 01:29, James Almer wrote:
> On 6/19/2021 4:56 PM, Gyan Doshi wrote:
>>
>>
>> On 2021-06-05 21:22, James Almer wrote:
>>> +static const double ntsc_fps = 30000.0 / 1001.0;
>>> +static const double pal_fps = 25.0;
>>> +static const double qntsc_fps = 30000.0 / 1001.0;
>>> +static const double qpal_fps = 25.0;
>>> +static const double sntsc_fps = 30000.0 / 1001.0;
>>> +static const double spal_fps = 25.0;
>>> +static const double film_fps = 24.0;
>>> +static const double ntsc_film_fps = 24000.0 / 1001.0;
>>
>> Sorry, I missed this before you applied it but what's the q- and s- 
>> prefixed standards with identical rates?
>> I can't find any specs or docs online.
>
> It's taken from video_rate_abbrs in libavutil/parseutils.c
> The description is "VCD compliant" for q and "square pixel" for s.

Right. So originally, these labels were created to denote a distinct 
*permutation* of resolution and framerate, in ba2a8cb40b
They are not part of any distinct variant of any standard or convention, 
outside of ffmpeg, and frankly not even internally.

These should be removed to avoid any confusion.

Regards,
Gyan
diff mbox series

Patch

diff --git a/libavfilter/vf_fps.c b/libavfilter/vf_fps.c
index b7b2d6f2db..29588a5f6e 100644
--- a/libavfilter/vf_fps.c
+++ b/libavfilter/vf_fps.c
@@ -30,6 +30,7 @@ 
 #include <stdint.h>
 
 #include "libavutil/avassert.h"
+#include "libavutil/eval.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/opt.h"
 #include "avfilter.h"
@@ -42,12 +43,47 @@  enum EOFAction {
     EOF_ACTION_NB
 };
 
+static const char *const var_names[] = {
+  "source_fps",
+  "ntsc",
+  "pal",
+  "qntsc",
+  "qpal",
+  "sntsc",
+  "spal",
+  "film",
+  "ntsc_film",
+  NULL
+};
+
+enum var_name {
+  VAR_SOURCE_FPS,
+  VAR_FPS_NTSC,
+  VAR_FPS_PAL,
+  VAR_FPS_QNTSC,
+  VAR_FPS_QPAL,
+  VAR_FPS_SNTSC,
+  VAR_FPS_SPAL,
+  VAR_FPS_FILM,
+  VAR_FPS_NTSC_FILM,
+  VARS_NB
+};
+
+static const double ntsc_fps = 30000.0 / 1001.0;
+static const double pal_fps = 25.0;
+static const double qntsc_fps = 30000.0 / 1001.0;
+static const double qpal_fps = 25.0;
+static const double sntsc_fps = 30000.0 / 1001.0;
+static const double spal_fps = 25.0;
+static const double film_fps = 24.0;
+static const double ntsc_film_fps = 24000.0 / 1001.0;
+
 typedef struct FPSContext {
     const AVClass *class;
 
     double start_time;      ///< pts, in seconds, of the expected first frame
 
-    AVRational framerate;   ///< target framerate
+    char *framerate;        ///< expression that defines the target framerate
     int rounding;           ///< AVRounding method for timestamps
     int eof_action;         ///< action performed for last frame in FIFO
 
@@ -76,7 +112,7 @@  typedef struct FPSContext {
 #define V AV_OPT_FLAG_VIDEO_PARAM
 #define F AV_OPT_FLAG_FILTERING_PARAM
 static const AVOption fps_options[] = {
-    { "fps", "A string describing desired output framerate", OFFSET(framerate), AV_OPT_TYPE_VIDEO_RATE, { .str = "25" }, 0, INT_MAX, V|F },
+    { "fps", "A string describing desired output framerate", OFFSET(framerate), AV_OPT_TYPE_STRING, { .str = "25" }, 0, 0, V|F },
     { "start_time", "Assume the first PTS should be this value.", OFFSET(start_time), AV_OPT_TYPE_DOUBLE, { .dbl = DBL_MAX}, -DBL_MAX, DBL_MAX, V|F },
     { "round", "set rounding method for timestamps", OFFSET(rounding), AV_OPT_TYPE_INT, { .i64 = AV_ROUND_NEAR_INF }, 0, 5, V|F, "round" },
         { "zero", "round towards 0",                 0, AV_OPT_TYPE_CONST, { .i64 = AV_ROUND_ZERO     }, 0, 0, V|F, "round" },
@@ -99,7 +135,6 @@  static av_cold int init(AVFilterContext *ctx)
     s->status_pts   = AV_NOPTS_VALUE;
     s->next_pts     = AV_NOPTS_VALUE;
 
-    av_log(ctx, AV_LOG_VERBOSE, "fps=%d/%d\n", s->framerate.num, s->framerate.den);
     return 0;
 }
 
@@ -153,8 +188,26 @@  static int config_props(AVFilterLink* outlink)
     AVFilterLink    *inlink = ctx->inputs[0];
     FPSContext      *s      = ctx->priv;
 
-    outlink->time_base  = av_inv_q(s->framerate);
-    outlink->frame_rate = s->framerate;
+    double var_values[VARS_NB], res;
+    int ret;
+
+    var_values[VAR_SOURCE_FPS]    = av_q2d(inlink->frame_rate);
+    var_values[VAR_FPS_NTSC]      = ntsc_fps;
+    var_values[VAR_FPS_PAL]       = pal_fps;
+    var_values[VAR_FPS_QNTSC]     = qntsc_fps;
+    var_values[VAR_FPS_QPAL]      = qpal_fps;
+    var_values[VAR_FPS_SNTSC]     = sntsc_fps;
+    var_values[VAR_FPS_SPAL]      = spal_fps;
+    var_values[VAR_FPS_FILM]      = film_fps;
+    var_values[VAR_FPS_NTSC_FILM] = ntsc_film_fps;
+    ret = av_expr_parse_and_eval(&res, s->framerate,
+                                 var_names, var_values,
+                                 NULL, NULL, NULL, NULL, NULL, 0, ctx);
+    if (ret < 0)
+        return ret;
+
+    outlink->frame_rate = av_d2q(res, INT_MAX);
+    outlink->time_base  = av_inv_q(outlink->frame_rate);
 
     /* Calculate the input and output pts offsets for start_time */
     if (s->start_time != DBL_MAX && s->start_time != AV_NOPTS_VALUE) {
@@ -173,6 +226,8 @@  static int config_props(AVFilterLink* outlink)
                s->in_pts_off, s->out_pts_off, s->start_time);
     }
 
+    av_log(ctx, AV_LOG_VERBOSE, "fps=%d/%d\n", outlink->frame_rate.num, outlink->frame_rate.den);
+
     return 0;
 }