On Tue, 10 May 2011 09:29:47 +0200, Stefano Sabatini
<stefano.sabatini-l...@poste.it> wrote:
> On date Monday 2011-05-09 20:25:31 -0400, Ronald S. Bultje encoded:
> > Hi,
> >
> > On Mon, May 9, 2011 at 7:09 PM, Stefano Sabatini
> > <stefano.sabatini-l...@poste.it> wrote:
> > > On date Monday 2011-05-09 18:06:51 -0400, Ronald S. Bultje encoded:
> > >> Hi,
> > >>
> > >> On Mon, May 9, 2011 at 1:49 PM, Anton Khirnov <an...@khirnov.net> wrote:
> > >> > -static void opt_qscale(const char *arg)
> > >> > +static int opt_qscale(const char *opt, const char *arg)
> > >> > ??{
> > >> > - ?? ??video_qscale = atof(arg);
> > >> > - ?? ??if (video_qscale <= 0 ||
> > >> > - ?? ?? ?? ??video_qscale > 255) {
> > >> > + ?? ??video_qscale = parse_number_or_die(opt, arg, OPT_FLOAT, 0, 255);
> > >> > + ?? ??if (video_qscale <= 0 || video_qscale > 255) {
> > >> > ?? ?? ?? ?? fprintf(stderr, "qscale must be > 0.0 and <= 255\n");
> > >> > - ?? ?? ?? ??ffmpeg_exit(1);
> > >> > + ?? ?? ?? ??return AVERROR(EINVAL);
> > >> > ?? ?? }
> > >> > + ?? ??return 0;
> > >> > ??}
> > >>
> > >> Shouldn't it die already if outside the [0,255] range as per
> > >> parse_number_or_die?
> > >
> > > No parse_number_or_die(opt, arg, OPT_FLOAT, 0, 255) won't fail with
> > > 0.0, which is not a valid value. Alternatively maybe:
> > > parse_number_or_die(opt, arg, OPT_FLOAT, EPS, 255)
>
>
> > >
> > > but which is not very clear from the reporting point of view, or
> > > choose as minimum an arbitrary small number.
> >
> > Your check post parse_number_or_die() has the same issue, no?
>
> The post-check has the only objective of rejecting q == 0.
>
Then it should do exactly that and not more, as that's confusing.
>From 387e115ac1d6e2efda4ee61f2b90cdaee4b4f0c9 Mon Sep 17 00:00:00 2001
From: Stefano Sabatini <stefano.sabatini-l...@poste.it>
Date: Mon, 11 Apr 2011 13:16:07 +0200
Subject: [PATCH] ffmpeg: use parse_number_and_die() when it makes sense
Prefer parse_number_or_die() over atoi()/atol() parsing for the options:
-pass, -top, -vc, and -qscale.
Improve input validation.
Signed-off-by: Stefano Sabatini <stefano.sabatini-l...@poste.it>
Signed-off-by: Anton Khirnov <an...@khirnov.net>
---
ffmpeg.c | 39 ++++++++++++++++++---------------------
1 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/ffmpeg.c b/ffmpeg.c
index 3ae8672..68a9ed5 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -2822,19 +2822,20 @@ static int opt_metadata(const char *opt, const char
*arg)
return 0;
}
-static void opt_qscale(const char *arg)
+static int opt_qscale(const char *opt, const char *arg)
{
- video_qscale = atof(arg);
- if (video_qscale <= 0 ||
- video_qscale > 255) {
+ video_qscale = parse_number_or_die(opt, arg, OPT_FLOAT, 0, 255);
+ if (video_qscale == 0)
fprintf(stderr, "qscale must be > 0.0 and <= 255\n");
- ffmpeg_exit(1);
+ return AVERROR(EINVAL);
}
+ return 0;
}
-static void opt_top_field_first(const char *arg)
+static int opt_top_field_first(const char *opt, const char *arg)
{
- top_field_first= atoi(arg);
+ top_field_first = parse_number_or_die(opt, arg, OPT_INT, 0, 1);
+ return 0;
}
static int opt_thread_count(const char *opt, const char *arg)
@@ -2876,9 +2877,10 @@ static int opt_audio_channels(const char *opt, const
char *arg)
return 0;
}
-static void opt_video_channel(const char *arg)
+static int opt_video_channel(const char *opt, const char *arg)
{
- video_channel = strtol(arg, NULL, 0);
+ video_channel = parse_number_or_die(opt, arg, OPT_INT64, 0, INT_MAX);
+ return 0;
}
static void opt_video_standard(const char *arg)
@@ -3860,15 +3862,10 @@ static void opt_output_file(const char *filename)
}
/* same option as mencoder */
-static void opt_pass(const char *pass_str)
+static int opt_pass(const char *opt, const char *arg)
{
- int pass;
- pass = atoi(pass_str);
- if (pass != 1 && pass != 2) {
- fprintf(stderr, "pass number can be only 1 or 2\n");
- ffmpeg_exit(1);
- }
- do_pass = pass;
+ do_pass = parse_number_or_die(opt, arg, OPT_INT, 1, 2);
+ return 0;
}
static int64_t getutime(void)
@@ -4290,13 +4287,13 @@ static const OptionDef options[] = {
{ "intra", OPT_BOOL | OPT_EXPERT | OPT_VIDEO, {(void*)&intra_only}, "use
only intra frames"},
{ "vn", OPT_BOOL | OPT_VIDEO, {(void*)&video_disable}, "disable video" },
{ "vdt", OPT_INT | HAS_ARG | OPT_EXPERT | OPT_VIDEO,
{(void*)&video_discard}, "discard threshold", "n" },
- { "qscale", HAS_ARG | OPT_EXPERT | OPT_VIDEO, {(void*)opt_qscale}, "use
fixed video quantizer scale (VBR)", "q" },
+ { "qscale", HAS_ARG | OPT_FUNC2 | OPT_EXPERT | OPT_VIDEO,
{(void*)opt_qscale}, "use fixed video quantizer scale (VBR)", "q" },
{ "rc_override", HAS_ARG | OPT_EXPERT | OPT_VIDEO,
{(void*)opt_video_rc_override_string}, "rate control override for specific
intervals", "override" },
{ "vcodec", HAS_ARG | OPT_VIDEO, {(void*)opt_video_codec}, "force video
codec ('copy' to copy stream)", "codec" },
{ "me_threshold", HAS_ARG | OPT_FUNC2 | OPT_EXPERT | OPT_VIDEO,
{(void*)opt_me_threshold}, "motion estimaton threshold", "threshold" },
{ "sameq", OPT_BOOL | OPT_VIDEO, {(void*)&same_quality},
"use same quantizer as source (implies VBR)" },
- { "pass", HAS_ARG | OPT_VIDEO, {(void*)&opt_pass}, "select the pass number
(1 or 2)", "n" },
+ { "pass", HAS_ARG | OPT_FUNC2 | OPT_VIDEO, {(void*)opt_pass}, "select the
pass number (1 or 2)", "n" },
{ "passlogfile", HAS_ARG | OPT_STRING | OPT_VIDEO,
{(void*)&pass_logfilename_prefix}, "select two pass log file name prefix",
"prefix" },
{ "deinterlace", OPT_BOOL | OPT_EXPERT | OPT_VIDEO,
{(void*)&do_deinterlace},
"deinterlace pictures" },
@@ -4308,7 +4305,7 @@ static const OptionDef options[] = {
#endif
{ "intra_matrix", HAS_ARG | OPT_EXPERT | OPT_VIDEO,
{(void*)opt_intra_matrix}, "specify intra matrix coeffs", "matrix" },
{ "inter_matrix", HAS_ARG | OPT_EXPERT | OPT_VIDEO,
{(void*)opt_inter_matrix}, "specify inter matrix coeffs", "matrix" },
- { "top", HAS_ARG | OPT_EXPERT | OPT_VIDEO, {(void*)opt_top_field_first},
"top=1/bottom=0/auto=-1 field first", "" },
+ { "top", HAS_ARG | OPT_FUNC2 | OPT_EXPERT | OPT_VIDEO,
{(void*)opt_top_field_first}, "top=1/bottom=0/auto=-1 field first", "" },
{ "dc", OPT_INT | HAS_ARG | OPT_EXPERT | OPT_VIDEO,
{(void*)&intra_dc_precision}, "intra_dc_precision", "precision" },
{ "vtag", OPT_FUNC2 | HAS_ARG | OPT_EXPERT | OPT_VIDEO,
{(void*)opt_codec_tag}, "force video tag/fourcc", "fourcc/tag" },
{ "newvideo", OPT_VIDEO | OPT_FUNC2, {(void*)opt_new_stream}, "add a new
video stream to the current output stream" },
@@ -4340,7 +4337,7 @@ static const OptionDef options[] = {
{ "stag", OPT_FUNC2 | HAS_ARG | OPT_EXPERT | OPT_SUBTITLE,
{(void*)opt_codec_tag}, "force subtitle tag/fourcc", "fourcc/tag" },
/* grab options */
- { "vc", HAS_ARG | OPT_EXPERT | OPT_VIDEO | OPT_GRAB,
{(void*)opt_video_channel}, "set video grab channel (DV1394 only)", "channel" },
+ { "vc", HAS_ARG | OPT_FUNC2 | OPT_EXPERT | OPT_VIDEO | OPT_GRAB,
{(void*)opt_video_channel}, "set video grab channel (DV1394 only)", "channel" },
{ "tvstd", HAS_ARG | OPT_EXPERT | OPT_VIDEO | OPT_GRAB,
{(void*)opt_video_standard}, "set television standard (NTSC, PAL (SECAM))",
"standard" },
{ "isync", OPT_BOOL | OPT_EXPERT | OPT_GRAB, {(void*)&input_sync}, "sync
read on input", "" },
--
1.7.5.1
--
Anton Khirnov
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
