[FFmpeg-devel] [PATCH 33/47] fftools: remove parse_time_or_die()

[Anton Khirnov <anton@khirnov.net>]

Replace it with calling av_parse_time() directly, which provides
graceful error handling and more accurate error messages.
---
 fftools/cmdutils.c        | 19 ++++++-------------
 fftools/cmdutils.h        | 17 -----------------
 fftools/ffmpeg_mux_init.c | 30 ++++++++++++++++++++++++------
 fftools/ffmpeg_opt.c      | 17 ++++++++++++++---
 fftools/ffplay.c          | 16 ++--------------
 5 files changed, 46 insertions(+), 53 deletions(-)

diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index 48a81ca201..b401b8fb89 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -127,18 +127,6 @@ int parse_number(const char *context, const char *numstr, int type,
     return AVERROR(EINVAL);
 }
 
-int64_t parse_time_or_die(const char *context, const char *timestr,
-                          int is_duration)
-{
-    int64_t us;
-    if (av_parse_time(&us, timestr, is_duration) < 0) {
-        av_log(NULL, AV_LOG_FATAL, "Invalid %s specification for %s: %s\n",
-               is_duration ? "duration" : "date", context, timestr);
-        exit_program(1);
-    }
-    return us;
-}
-
 void show_help_options(const OptionDef *options, const char *msg, int req_flags,
                        int rej_flags, int alt_flags)
 {
@@ -304,7 +292,12 @@ static int write_option(void *optctx, const OptionDef *po, const char *opt,
 
         *(int64_t *)dst = num;
     } else if (po->flags & OPT_TIME) {
-        *(int64_t *)dst = parse_time_or_die(opt, arg, 1);
+        ret = av_parse_time(dst, arg, 1);
+        if (ret < 0) {
+            av_log(NULL, AV_LOG_ERROR, "Invalid duration for option %s: %s\n",
+                   opt, arg);
+            return ret;
+        }
     } else if (po->flags & OPT_FLOAT) {
         ret = parse_number(opt, arg, OPT_FLOAT, -INFINITY, INFINITY, &num);
         if (ret < 0)
diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
index 68de0bcb7f..dc041d9fa2 100644
--- a/fftools/cmdutils.h
+++ b/fftools/cmdutils.h
@@ -112,23 +112,6 @@ int opt_timelimit(void *optctx, const char *opt, const char *arg);
 int parse_number(const char *context, const char *numstr, int type,
                            double min, double max, double *dst);
 
-/**
- * Parse a string specifying a time and return its corresponding
- * value as a number of microseconds. Exit from the application if
- * the string cannot be correctly parsed.
- *
- * @param context the context of the value to be set (e.g. the
- * corresponding command line option name)
- * @param timestr the string to be parsed
- * @param is_duration a flag which tells how to interpret timestr, if
- * not zero timestr is interpreted as a duration, otherwise as a
- * date
- *
- * @see av_parse_time()
- */
-int64_t parse_time_or_die(const char *context, const char *timestr,
-                          int is_duration);
-
 typedef struct SpecifierOpt {
     char *specifier;    /**< stream/chapter/program/... specifier */
     union {
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 02d71588ad..92d62c7b89 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -2410,11 +2410,11 @@ static int compare_int64(const void *a, const void *b)
     return FFDIFFSIGN(*(const int64_t *)a, *(const int64_t *)b);
 }
 
-static int parse_forced_key_frames(KeyframeForceCtx *kf, const Muxer *mux,
-                                   const char *spec)
+static int parse_forced_key_frames(void *log, KeyframeForceCtx *kf,
+                                   const Muxer *mux, const char *spec)
 {
     const char *p;
-    int n = 1, i, size, index = 0;
+    int n = 1, i, ret, size, index = 0;
     int64_t t, *pts;
 
     for (p = spec; *p; p++)
@@ -2441,7 +2441,16 @@ static int parse_forced_key_frames(KeyframeForceCtx *kf, const Muxer *mux,
                 !(pts = av_realloc_f(pts, size += nb_ch - 1,
                                      sizeof(*pts))))
                 return AVERROR(ENOMEM);
-            t = p[8] ? parse_time_or_die("force_key_frames", p + 8, 1) : 0;
+
+            if (p[8]) {
+                ret = av_parse_time(&t, p + 8, 1);
+                if (ret < 0) {
+                    av_log(log, AV_LOG_ERROR,
+                           "Invalid chapter time offset: %s\n", p + 8);
+                    goto fail;
+                }
+            } else
+                t = 0;
 
             for (j = 0; j < nb_ch; j++) {
                 const AVChapter *c = ch[j];
@@ -2452,7 +2461,13 @@ static int parse_forced_key_frames(KeyframeForceCtx *kf, const Muxer *mux,
 
         } else {
             av_assert1(index < size);
-            pts[index++] = parse_time_or_die("force_key_frames", p, 1);
+            ret = av_parse_time(&t, p, 1);
+            if (ret < 0) {
+                av_log(log, AV_LOG_ERROR, "Invalid keyframe time: %s\n", p);
+                goto fail;
+            }
+
+            pts[index++] = t;
         }
 
         p = next;
@@ -2464,6 +2479,9 @@ static int parse_forced_key_frames(KeyframeForceCtx *kf, const Muxer *mux,
     kf->pts    = pts;
 
     return 0;
+fail:
+    av_freep(&pts);
+    return ret;
 }
 
 static int process_forced_keyframes(Muxer *mux, const OptionsContext *o)
@@ -2498,7 +2516,7 @@ static int process_forced_keyframes(Muxer *mux, const OptionsContext *o)
         } else if (!strcmp(forced_keyframes, "source_no_drop")) {
             ost->kf.type = KF_FORCE_SOURCE_NO_DROP;
         } else {
-            int ret = parse_forced_key_frames(&ost->kf, mux, forced_keyframes);
+            int ret = parse_forced_key_frames(ost, &ost->kf, mux, forced_keyframes);
             if (ret < 0)
                 return ret;
         }
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index e1696cdd59..44a6b49ed7 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -326,7 +326,10 @@ static int opt_abort_on(void *optctx, const char *opt, const char *arg)
 
 static int opt_stats_period(void *optctx, const char *opt, const char *arg)
 {
-    int64_t user_stats_period = parse_time_or_die(opt, arg, 1);
+    int64_t user_stats_period;
+    int ret = av_parse_time(&user_stats_period, arg, 1);
+    if (ret < 0)
+        return ret;
 
     if (user_stats_period <= 0) {
         av_log(NULL, AV_LOG_ERROR, "stats_period %s must be positive.\n", arg);
@@ -636,8 +639,16 @@ static int opt_recording_timestamp(void *optctx, const char *opt, const char *ar
 {
     OptionsContext *o = optctx;
     char buf[128];
-    int64_t recording_timestamp = parse_time_or_die(opt, arg, 0) / 1E6;
-    struct tm time = *gmtime((time_t*)&recording_timestamp);
+    int64_t recording_timestamp;
+    int ret;
+    struct tm time;
+
+    ret = av_parse_time(&recording_timestamp, arg, 0);
+    if (ret < 0)
+        return ret;
+
+    recording_timestamp /= 1e6;
+    time = *gmtime((time_t*)&recording_timestamp);
     if (!strftime(buf, sizeof(buf), "creation_time=%Y-%m-%dT%H:%M:%S%z", &time))
         return -1;
     parse_option(o, "metadata", buf, options);
diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index 6ca1ad98bf..89cea4d876 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -3485,18 +3485,6 @@ static int opt_sync(void *optctx, const char *opt, const char *arg)
     return 0;
 }
 
-static int opt_seek(void *optctx, const char *opt, const char *arg)
-{
-    start_time = parse_time_or_die(opt, arg, 1);
-    return 0;
-}
-
-static int opt_duration(void *optctx, const char *opt, const char *arg)
-{
-    duration = parse_time_or_die(opt, arg, 1);
-    return 0;
-}
-
 static int opt_show_mode(void *optctx, const char *opt, const char *arg)
 {
     show_mode = !strcmp(arg, "video") ? SHOW_MODE_VIDEO :
@@ -3561,8 +3549,8 @@ static const OptionDef options[] = {
     { "ast", OPT_STRING | HAS_ARG | OPT_EXPERT, { &wanted_stream_spec[AVMEDIA_TYPE_AUDIO] }, "select desired audio stream", "stream_specifier" },
     { "vst", OPT_STRING | HAS_ARG | OPT_EXPERT, { &wanted_stream_spec[AVMEDIA_TYPE_VIDEO] }, "select desired video stream", "stream_specifier" },
     { "sst", OPT_STRING | HAS_ARG | OPT_EXPERT, { &wanted_stream_spec[AVMEDIA_TYPE_SUBTITLE] }, "select desired subtitle stream", "stream_specifier" },
-    { "ss", HAS_ARG, { .func_arg = opt_seek }, "seek to a given position in seconds", "pos" },
-    { "t", HAS_ARG, { .func_arg = opt_duration }, "play  \"duration\" seconds of audio/video", "duration" },
+    { "ss", HAS_ARG | OPT_TIME, { &start_time }, "seek to a given position in seconds", "pos" },
+    { "t",  HAS_ARG | OPT_TIME, { &duration }, "play  \"duration\" seconds of audio/video", "duration" },
     { "bytes", OPT_INT | HAS_ARG, { &seek_by_bytes }, "seek by bytes 0=off 1=on -1=auto", "val" },
     { "seek_interval", OPT_FLOAT | HAS_ARG, { &seek_interval }, "set seek interval for left/right keys, in seconds", "seconds" },
     { "nodisp", OPT_BOOL, { &display_disable }, "disable graphical display" },
-- 
2.40.1

_______________________________________________
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".