On date Saturday 2011-05-28 19:28:17 +0200, Stefano Sabatini encoded: > On date Saturday 2011-05-28 06:57:10 -0400, Ronald S. Bultje encoded: > > Hi, > > > > On Sat, May 28, 2011 at 5:43 AM, Stefano Sabatini > > <[email protected]> wrote: > > > * the function code knows exactly why the failure happened, and can > > > provide useful hints for spotting the problem, which can't be done > > > in the calling code. > > > > We can log this under NULL, AV_LOG_DEBUG or even as a av_dlog() message... > > > > The problem goes both ways. The function doesn't know the context in > > which the parsing happened (was it the UI using the API for some > > preference box? > > The calling function can provide such a context, with a further log > message. > > > Or libavfilter internal setup? Or a developer testing > > a new feature he's developing and screwing up a test? Or a user typing > > an invalid ffmpeg -x parameter?). But the caller doesn't know the > > detailed reason why it failed. > > > > > Generally the user cares more about the context (and then can figure > > out why 320c240 wasn't a valid size, oh, it should be s/c/x/ - typo). > > With number parsing it is more complicated, the failure may depend > on an invalid range, on a typo, or on the parsed number not being an > integer, discovering the reason of the failure requires duplicating > the logic in the calling code, if you don't report the failure > reason you have a poor feedback (and I don't want to duplicate the > error reporting every time I use that function, check ffmpeg for an > example). > > > Developers can use av_dlog() messages with DEBUG defined. So there is > > something to say for it. Whatever you guys prefer is fine with me, I'd > > have a slight preference for the part where you have a simpler (so 4- > > instead of 6-argument API), but don't care much. > > I believe the two parameters are a modest price for the added > flexibility/functionality.
Ping and updated patch (Note for the committer: update libavutil minor number and APIchanges number and date).
>From 6df4ad4bc40cd5dab6831d8b553718a810e07393 Mon Sep 17 00:00:00 2001 From: Stefano Sabatini <[email protected]> Date: Sat, 28 May 2011 01:20:27 +0200 Subject: [PATCH] parseutils: add av_parse_number() function and children, and use them in the ff* tools --- cmdutils.c | 44 ++++++++++------------------- cmdutils.h | 15 ---------- doc/APIchanges | 9 ++++++ ffmpeg.c | 33 ++++++++------------- ffplay.c | 15 +++------ libavutil/parseutils.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++ libavutil/parseutils.h | 26 +++++++++++++++++ 7 files changed, 141 insertions(+), 74 deletions(-) diff --git a/cmdutils.c b/cmdutils.c index b9a5d1b..163c615 100644 --- a/cmdutils.c +++ b/cmdutils.c @@ -97,25 +97,6 @@ void log_callback_help(void* ptr, int level, const char* fmt, va_list vl) vfprintf(stdout, fmt, vl); } -double parse_number_or_die(const char *context, const char *numstr, int type, double min, double max) -{ - char *tail; - const char *error; - double d = av_strtod(numstr, &tail); - if (*tail) - error= "Expected number for %s but found: %s\n"; - else if (d < min || d > max) - error= "The value for %s was %s which is not within %f - %f\n"; - else if(type == OPT_INT64 && (int64_t)d != d) - error= "Expected int64 for %s but found %s\n"; - else if (type == OPT_INT && (int)d != d) - error= "Expected int for %s but found %s\n"; - else - return d; - fprintf(stderr, error, context, numstr, min, max); - exit(1); -} - int64_t parse_time_or_die(const char *context, const char *timestr, int is_duration) { int64_t us; @@ -224,7 +205,7 @@ void parse_options(int argc, char **argv, const OptionDef *options, void (* parse_arg_function)(const char*)) { const char *opt, *arg; - int optindex, handleoptions=1; + int ret = 0, optindex, handleoptions=1; const OptionDef *po; /* perform system-dependent conversions for arguments list */ @@ -272,16 +253,18 @@ unknown_opt: } else if (po->flags & OPT_BOOL) { *po->u.int_arg = bool_val; } else if (po->flags & OPT_INT) { - *po->u.int_arg = parse_number_or_die(opt, arg, OPT_INT64, INT_MIN, INT_MAX); + ret = av_parse_int(po->u.int_arg, arg, INT_MIN, INT_MAX, 0, NULL); } else if (po->flags & OPT_INT64) { - *po->u.int64_arg = parse_number_or_die(opt, arg, OPT_INT64, INT64_MIN, INT64_MAX); + ret = av_parse_int64(po->u.int64_arg, arg, INT64_MIN, INT64_MAX, 0, NULL); } else if (po->flags & OPT_FLOAT) { - *po->u.float_arg = parse_number_or_die(opt, arg, OPT_FLOAT, -INFINITY, INFINITY); + ret = av_parse_float(po->u.float_arg, arg, -INFINITY, INFINITY, 0, NULL); } else { - if (po->u.func_arg(opt, arg) < 0) { - fprintf(stderr, "%s: failed to set value '%s' for option '%s'\n", argv[0], arg, opt); - exit(1); - } + ret = po->u.func_arg(opt, arg); + } + + if (ret < 0) { + fprintf(stderr, "%s: failed to set value '%s' for option '%s'\n", argv[0], arg, opt); + exit(1); } if(po->flags & OPT_EXIT) exit(0); @@ -391,8 +374,11 @@ int opt_loglevel(const char *opt, const char *arg) int opt_timelimit(const char *opt, const char *arg) { #if HAVE_SETRLIMIT - int lim = parse_number_or_die(opt, arg, OPT_INT64, 0, INT_MAX); - struct rlimit rl = { lim, lim + 1 }; + int lim; + struct rlimit rl; + if (av_parse_int(&lim, arg, 0, INT_MAX, 0, NULL) < 0) + return AVERROR(EINVAL); + rl = (struct rlimit){ lim, lim + 1 }; if (setrlimit(RLIMIT_CPU, &rl)) perror("setrlimit"); #else diff --git a/cmdutils.h b/cmdutils.h index e231b1f..f960683 100644 --- a/cmdutils.h +++ b/cmdutils.h @@ -78,21 +78,6 @@ int opt_loglevel(const char *opt, const char *arg); int opt_timelimit(const char *opt, const char *arg); /** - * Parse a string and return its corresponding value as a double. - * Exit from the application if the string cannot be correctly - * parsed or the corresponding value is invalid. - * - * @param context the context of the value to be set (e.g. the - * corresponding commandline option name) - * @param numstr the string to be parsed - * @param type the type (OPT_INT64 or OPT_FLOAT) as which the - * string should be parsed - * @param min the minimum valid accepted value - * @param max the maximum valid accepted value - */ -double parse_number_or_die(const char *context, const char *numstr, int type, double min, double max); - -/** * 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. diff --git a/doc/APIchanges b/doc/APIchanges index a55b152..6e5677e 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -13,6 +13,15 @@ libavutil: 2011-04-18 API changes, most recent first: +2011-06-XX - xxxxxx - lavu 51.X.Y - parseutils.h + Add functions: + av_parse_number() + av_parse_int() + av_parse_int64() + av_parse_float() + av_parse_double() + to libavutil/parseutils.h. + 2011-05-28 - 0420bd7 - lavu 51.2.0 - pixdesc.h Add av_get_pix_fmt_name() in libavutil/pixdesc.h, and deprecate avcodec_get_pix_fmt_name() in libavcodec/avcodec.h in its favor. diff --git a/ffmpeg.c b/ffmpeg.c index 1c4c4b1..51648f5 100644 --- a/ffmpeg.c +++ b/ffmpeg.c @@ -2717,14 +2717,12 @@ static int opt_video_rc_override_string(const char *opt, const char *arg) static int opt_me_threshold(const char *opt, const char *arg) { - me_threshold = parse_number_or_die(opt, arg, OPT_INT64, INT_MIN, INT_MAX); - return 0; + return av_parse_int(&me_threshold, arg, INT_MIN, INT_MAX, 0, NULL); } static int opt_verbose(const char *opt, const char *arg) { - verbose = parse_number_or_die(opt, arg, OPT_INT64, -10, 10); - return 0; + return av_parse_int(&verbose, arg, -10, 10, 0, NULL); } static int opt_frame_rate(const char *opt, const char *arg) @@ -2825,7 +2823,8 @@ static int opt_metadata(const char *opt, const char *arg) static int opt_qscale(const char *opt, const char *arg) { - video_qscale = parse_number_or_die(opt, arg, OPT_FLOAT, 0, 255); + if (av_parse_float(&video_qscale, arg, 0, 255, 0, NULL) < 0) + return AVERROR(EINVAL); if (video_qscale == 0) { fprintf(stderr, "qscale must be > 0.0 and <= 255\n"); return AVERROR(EINVAL); @@ -2835,18 +2834,16 @@ static int opt_qscale(const char *opt, const char *arg) static int opt_top_field_first(const char *opt, const char *arg) { - top_field_first = parse_number_or_die(opt, arg, OPT_INT, 0, 1); - return 0; + return av_parse_int(&top_field_first, arg, 0, 1, 0, NULL); } static int opt_thread_count(const char *opt, const char *arg) { - thread_count= parse_number_or_die(opt, arg, OPT_INT64, 0, INT_MAX); #if !HAVE_THREADS if (verbose >= 0) fprintf(stderr, "Warning: not compiled with thread support, using thread emulation\n"); #endif - return 0; + return av_parse_int(&thread_count, arg, 0, INT_MAX, 0, NULL); } static int opt_audio_sample_fmt(const char *opt, const char *arg) @@ -2869,20 +2866,17 @@ static int opt_audio_sample_fmt(const char *opt, const char *arg) static int opt_audio_rate(const char *opt, const char *arg) { - audio_sample_rate = parse_number_or_die(opt, arg, OPT_INT64, 0, INT_MAX); - return 0; + return av_parse_int(&audio_sample_rate, arg, 0, INT_MAX, 0, NULL); } static int opt_audio_channels(const char *opt, const char *arg) { - audio_channels = parse_number_or_die(opt, arg, OPT_INT64, 0, INT_MAX); - return 0; + return av_parse_int(&audio_channels, arg, 0, INT_MAX, 0, NULL); } static int opt_video_channel(const char *opt, const char *arg) { - video_channel = parse_number_or_die(opt, arg, OPT_INT64, 0, INT_MAX); - return 0; + return av_parse_int(&video_channel, arg, 0, INT_MAX, 0, NULL); } static int opt_video_standard(const char *opt, const char *arg) @@ -3742,10 +3736,10 @@ static int opt_streamid(const char *opt, const char *arg) ffmpeg_exit(1); } *p++ = '\0'; - idx = parse_number_or_die(opt, idx_str, OPT_INT, 0, MAX_STREAMS-1); + if (av_parse_int(&idx, idx_str, 0, MAX_STREAMS-1, 0, NULL) < 0) + return AVERROR(EINVAL); streamid_map = grow_array(streamid_map, sizeof(*streamid_map), &nb_streamid_map, idx+1); - streamid_map[idx] = parse_number_or_die(opt, p, OPT_INT, 0, INT_MAX); - return 0; + return av_parse_int(&streamid_map[idx], p, 0, INT_MAX, 0, NULL); } static void opt_output_file(const char *filename) @@ -3895,8 +3889,7 @@ static void opt_output_file(const char *filename) /* same option as mencoder */ static int opt_pass(const char *opt, const char *arg) { - do_pass = parse_number_or_die(opt, arg, OPT_INT, 1, 2); - return 0; + return av_parse_int(&do_pass, arg, 1, 2, 0, NULL); } static int64_t getutime(void) diff --git a/ffplay.c b/ffplay.c index e036bbd..83faf97 100644 --- a/ffplay.c +++ b/ffplay.c @@ -2852,14 +2852,12 @@ static int opt_frame_size(const char *opt, const char *arg) static int opt_width(const char *opt, const char *arg) { - screen_width = parse_number_or_die(opt, arg, OPT_INT64, 1, INT_MAX); - return 0; + return av_parse_int(&screen_width, arg, 1, INT_MAX, 0, NULL); } static int opt_height(const char *opt, const char *arg) { - screen_height = parse_number_or_die(opt, arg, OPT_INT64, 1, INT_MAX); - return 0; + return av_parse_int(&screen_height, arg, 1, INT_MAX, 0, NULL); } static int opt_format(const char *opt, const char *arg) @@ -2908,23 +2906,20 @@ static int opt_duration(const char *opt, const char *arg) static int opt_debug(const char *opt, const char *arg) { av_log_set_level(99); - debug = parse_number_or_die(opt, arg, OPT_INT64, 0, INT_MAX); - return 0; + return av_parse_int(&debug, arg, 0, INT_MAX, 0, NULL); } static int opt_vismv(const char *opt, const char *arg) { - debug_mv = parse_number_or_die(opt, arg, OPT_INT64, INT_MIN, INT_MAX); - return 0; + return av_parse_int(&debug_mv, arg, INT_MIN, INT_MAX, 0, NULL); } static int opt_thread_count(const char *opt, const char *arg) { - thread_count= parse_number_or_die(opt, arg, OPT_INT64, 0, INT_MAX); #if !HAVE_THREADS fprintf(stderr, "Warning: not compiled with thread support, using thread emulation\n"); #endif - return 0; + return av_parse_int(&thread_count, arg, 0, INT_MAX, 0, NULL); } static const OptionDef options[] = { diff --git a/libavutil/parseutils.c b/libavutil/parseutils.c index cc90131..8e16e05 100644 --- a/libavutil/parseutils.c +++ b/libavutil/parseutils.c @@ -30,6 +30,79 @@ #include "libavutil/avstring.h" #include "libavutil/random_seed.h" +typedef struct ParseUtils { + const AVClass *class; + int log_offset; + void *log_ctx; +} ParseUtils; + +static const AVClass parseutils_class = { + "PARSEUTILS", av_default_item_name, NULL, LIBAVUTIL_VERSION_INT, + offsetof(ParseUtils, log_offset), offsetof(ParseUtils, log_ctx) +}; + +int av_parse_number(double *res, const char *numstr, enum AVParseNumberType type, + double min, double max, + int log_offset, void *log_ctx) +{ + ParseUtils parseutils = { &parseutils_class, log_offset, log_ctx }; + + char *tail; + double d = av_strtod(numstr, &tail); + if ((unsigned)type >= AV_PARSE_NUM_TYPE_NB) { + av_log(&parseutils, AV_LOG_ERROR, "Unknown parse number type '%d'\n", type); + } else if (*tail) { + av_log(&parseutils, AV_LOG_ERROR, "Expected number but found '%s'\n", numstr); + } else if (d < min || d > max) { + av_log(&parseutils, AV_LOG_ERROR, + "The numeric value for '%s' is %f, which is not within the inclusive interval %f - %f\n", + numstr, d, min, max); + } else if (type == AV_PARSE_NUM_TYPE_INT64 && (int64_t)d != d) { + av_log(&parseutils, AV_LOG_ERROR, + "Expected int64 but found %s\n", numstr); + } else if (type == AV_PARSE_NUM_TYPE_INT && (int)d != d) { + av_log(&parseutils, AV_LOG_ERROR, + "Expected int but found %s\n", numstr); + } else { /* float and double */ + *res = d; + return 0; + } + + return AVERROR(EINVAL); +} + +int av_parse_int(int *res, const char *numstr, int min, int max, int log_offset, void *log_ctx) +{ + double d; + int ret = av_parse_number(&d, numstr, AV_PARSE_NUM_TYPE_INT, min, max, log_offset, log_ctx); + if (ret < 0) + *res = d; + return ret; +} + +int av_parse_int64(int64_t *res, const char *numstr, int64_t min, int64_t max, int log_offset, void *log_ctx) +{ + double d; + int ret = av_parse_number(&d, numstr, AV_PARSE_NUM_TYPE_INT64, min, max, log_offset, log_ctx); + if (ret < 0) + *res = d; + return ret; +} + +int av_parse_float(float *res, const char *numstr, float min, float max, int log_offset, void *log_ctx) +{ + double d; + int ret = av_parse_number(&d, numstr, AV_PARSE_NUM_TYPE_FLOAT, min, max,log_offset, log_ctx); + if (ret < 0) + *res = d; + return ret; +} + +int av_parse_double(double *res, const char *numstr, double min, double max, int log_offset, void *log_ctx) +{ + return av_parse_number(res, numstr, AV_PARSE_NUM_TYPE_DOUBLE, min, max, log_offset, log_ctx); +} + typedef struct { const char *abbr; int width, height; diff --git a/libavutil/parseutils.h b/libavutil/parseutils.h index c0f9aec..a072735 100644 --- a/libavutil/parseutils.h +++ b/libavutil/parseutils.h @@ -26,6 +26,32 @@ * misc parsing utilities */ +enum AVParseNumberType { + AV_PARSE_NUM_TYPE_INT, + AV_PARSE_NUM_TYPE_INT64, + AV_PARSE_NUM_TYPE_FLOAT, + AV_PARSE_NUM_TYPE_DOUBLE, + AV_PARSE_NUM_TYPE_NB +}; + +/** + * Parse a string and return its corresponding value as a double. + * + * @param res pointer to where is put the resulting parsed value + * @param numstr the string to be parsed + * @param type the type of number to get + * @param min the minimum valid accepted value + * @param max the maximum valid accepted value + * @return >=0 in case of success, a negative AVERROR code in case of failure + */ +int av_parse_number(double *res, const char *numstr, enum AVParseNumberType type, + double min, double max, int log_offset, void *log_ctx); + +int av_parse_int (int *res, const char *numstr, int min, int max, int log_offset, void *log_ctx); +int av_parse_int64 (int64_t *res, const char *numstr, int64_t min, int64_t max, int log_offset, void *log_ctx); +int av_parse_float (float *res, const char *numstr, float min, float max, int log_offset, void *log_ctx); +int av_parse_double(double *res, const char *numstr, double min, double max, int log_offset, void *log_ctx); + /** * Parse str and put in width_ptr and height_ptr the detected values. * -- 1.7.2.3
_______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
