[FFmpeg-devel] [PATCH] lavu: introduce av_parse_ratio() and use it in ffmpeg and lavfi/aspect

[Michael Niedermayer]

On Fri, Jan 27, 2012 at 12:53:44PM +0100, Stefano Sabatini wrote:
> On date Wednesday 2012-01-25 20:35:32 +0100, Stefano Sabatini encoded:
> > On date Wednesday 2012-01-25 19:31:36 +0100, Reimar Döffinger encoded:
> > > On Tue, Jan 24, 2012 at 11:46:57PM +0100, Stefano Sabatini wrote:
> > > > On date Saturday 2012-01-21 15:24:23 +0100, Reimar Döffinger encoded:
> > > > > On Sat, Jan 21, 2012 at 02:59:59PM +0100, Stefano Sabatini wrote:
> > > > > > av_parse_ratio_l(AVRational *q, const char *str, int max, void *log_ctx, int log_level_off)
> > > > > 
> > > > > As long as you write out "log". Saving 2 characters is hardly worth the
> > > > > obfuscation.
> > > > 
> > > > The problem is that then it sounds "av_parse_ratio_log()" which reads
> > > > as "parse the ratio log" why "_l" just denotes a variant of the
> > > > function (like _e, _d, _s, used in other APIs).
> > > 
> > > Well, I'd say and just as much only acting as confusion instead of real
> > > help :-)
> > > I mean it is getting a bit long, but what about _logged?
> > 
> > > Or maybe giving the define instead the name av_parse_ratio_quiet?
> > 
> > Works for me, especially considering that the logging variant is "the
> > real thing" and the other is just an humble macro.
> > 
> > > None of those sound good enough to me that I'd be mad at you for
> > > not changing it, but I'd really prefer something better than
> > > a single letter that could really mean anything.
> > 
> > I'll let other people bikes^H^H^H^H^H propose their favorite names,
> > then I'll update the patch in a few days if I read no more proposals.
> 
> Upped, please review, I'll commit in a few days/a week if I read no
> comments.
> -- 
> FFmpeg = Forgiving and Fanciful Mortal Proud EnGraver

>  ffmpeg.c                |   35 +++++++++--------------------------
>  libavfilter/vf_aspect.c |   25 ++++++-------------------
>  libavutil/log.h         |    1 +
>  libavutil/parseutils.c  |   31 +++++++++++++++++++++++++++----
>  libavutil/parseutils.h  |   24 ++++++++++++++++++++++++
>  5 files changed, 67 insertions(+), 49 deletions(-)
> 4197e6acbcb1e3e4772503a8b5bc072a8f3d1d3b  0001-lavu-introduce-av_parse_ratio-and-use-it-in-ffmpeg-a.patch
> From f4bf294b84cf618523acaa06c2750ddb529868f1 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefasab at gmail.com>
> Date: Tue, 17 Jan 2012 15:25:14 +0100
> Subject: [PATCH] lavu: introduce av_parse_ratio() and use it in ffmpeg and
>  lavfi/aspect
> 
> Factorize code and provide ratio parsing consistency.
> ---
>  ffmpeg.c                |   35 +++++++++--------------------------
>  libavfilter/vf_aspect.c |   25 ++++++-------------------
>  libavutil/log.h         |    1 +
>  libavutil/parseutils.c  |   31 +++++++++++++++++++++++++++----
>  libavutil/parseutils.h  |   24 ++++++++++++++++++++++++
>  5 files changed, 67 insertions(+), 49 deletions(-)
> 
> diff --git a/ffmpeg.c b/ffmpeg.c
> index 463e1f4..0fdfd6c 100644
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -3145,30 +3145,6 @@ static int opt_pad(const char *opt, const char *arg)
>      return -1;
>  }
>  
> -static double parse_frame_aspect_ratio(const char *arg)
> -{
> -    int x = 0, y = 0;
> -    double ar = 0;
> -    const char *p;
> -    char *end;
> -
> -    p = strchr(arg, ':');
> -    if (p) {
> -        x = strtol(arg, &end, 10);
> -        if (end == p)
> -            y = strtol(end + 1, &end, 10);
> -        if (x > 0 && y > 0)
> -            ar = (double)x / (double)y;
> -    } else
> -        ar = strtod(arg, NULL);
> -
> -    if (!ar) {
> -        av_log(NULL, AV_LOG_FATAL, "Incorrect aspect ratio specification.\n");
> -        exit_program(1);
> -    }
> -    return ar;
> -}
> -
>  static int opt_video_channel(const char *opt, const char *arg)
>  {
>      av_log(NULL, AV_LOG_WARNING, "This option is deprecated, use -channel.\n");
> @@ -3998,8 +3974,15 @@ static OutputStream *new_video_stream(OptionsContext *o, AVFormatContext *oc)
>          }
>  
>          MATCH_PER_STREAM_OPT(frame_aspect_ratios, str, frame_aspect_ratio, oc, st);
> -        if (frame_aspect_ratio)
> -            ost->frame_aspect_ratio = parse_frame_aspect_ratio(frame_aspect_ratio);
> +        if (frame_aspect_ratio) {
> +            AVRational q;
> +            if (av_parse_ratio(&q, frame_aspect_ratio, 255, 0, NULL) < 0 ||
> +                q.num <= 0 || q.den <= 0) {
> +                av_log(NULL, AV_LOG_FATAL, "Invalid aspect ratio: %s\n", frame_aspect_ratio);
> +                exit_program(1);
> +            }
> +            ost->frame_aspect_ratio = av_q2d(q);
> +        }

how can a undefined aspect be specified ?
(use case would be a few files with wrong but unknown aspect as input)

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120127/9f23fb92/attachment.asc>