[FFmpeg-devel] [PATCH] ffprobe: add writer_print_rational()
Stefano Sabatini
stefasab at gmail.com
Sat Jun 16 00:37:27 CEST 2012
On date Friday 2012-06-15 00:05:57 +0200, Clément Bœsch encoded:
> On Thu, Jun 14, 2012 at 11:47:03PM +0200, Stefano Sabatini wrote:
> > On date Thursday 2012-06-14 21:59:01 +0200, Clément Bœsch encoded:
> > > On Thu, Jun 14, 2012 at 02:06:46AM +0200, Stefano Sabatini wrote:
> > > > Improve overall consistency, allow some factorization.
> > > >
> > > > Also consistently prefer '/' over ':' for separating numerator and
> > > > denominator. This unfortunately breaks parsing of ratio strings.
> > >
> > > I'd prefer to use an argument for the separator. A "16:9 ratio" makes
> > > sense, and a "30000/1001 framerate" as well. And I don't like the idea of
> > > breaking the output...
> >
> > Allright...
>
> Thank you :)
>
> > --
> > FFmpeg = Forgiving and Formidable Mysterious Prodigious Earthshaking Gangster
>
> > From 3370016417ca0174663378980b75262389c1c201 Mon Sep 17 00:00:00 2001
> > From: Stefano Sabatini <stefasab at gmail.com>
> > Date: Wed, 13 Jun 2012 14:33:32 +0200
> > Subject: [PATCH] ffprobe: add writer_print_rational()
> >
> > Improve overall consistency, allow some factorization.
> > ---
> > ffprobe.c | 32 +++++++++++++++++++-------------
> > 1 files changed, 19 insertions(+), 13 deletions(-)
> >
> > diff --git a/ffprobe.c b/ffprobe.c
> > index b87059e..b4783ba 100644
> > --- a/ffprobe.c
> > +++ b/ffprobe.c
> > @@ -165,6 +165,7 @@ typedef struct Writer {
> > void (*print_section_header)(WriterContext *wctx, const char *);
> > void (*print_section_footer)(WriterContext *wctx, const char *);
> > void (*print_integer) (WriterContext *wctx, const char *, long long int);
> > + void (*print_rational) (WriterContext *wctx, AVRational *q, char *sep);
> > void (*print_string) (WriterContext *wctx, const char *, const char *);
> > void (*show_tags) (WriterContext *wctx, AVDictionary *dict);
> > int flags; ///< a combination or WRITER_FLAG_*
> > @@ -309,6 +310,16 @@ static inline void writer_print_integer(WriterContext *wctx,
> > }
> > }
> >
> > +static inline void writer_print_rational(WriterContext *wctx,
> > + const char *key, AVRational q, char sep)
> > +{
> > + AVBPrint buf;
> > + av_bprint_init(&buf, 1, AV_BPRINT_SIZE_UNLIMITED);
>
> av_bprint_init(&buf, 0, AV_BPRINT_SIZE_AUTOMATIC);
>
> A 64B stack buffer might be enough here though.
Fixed.
> > + av_bprintf(&buf, "%d%c%d", q.num, sep, q.den);
> > + wctx->writer->print_string(wctx, key, buf.str);
> > + wctx->nb_item++;
> > +}
> > +
> > static inline void writer_print_string(WriterContext *wctx,
> > const char *key, const char *val, int opt)
> > {
> > @@ -1508,6 +1519,7 @@ static void writer_register_all(void)
> > } while (0)
> >
> > #define print_int(k, v) writer_print_integer(w, k, v)
> > +#define print_q(k, v, s) writer_print_rational(w, k, v, s)
> > #define print_str(k, v) writer_print_string(w, k, v, 0)
> > #define print_str_opt(k, v) writer_print_string(w, k, v, 1)
> > #define print_time(k, v, tb) writer_print_time(w, k, v, tb, 0)
> > @@ -1580,9 +1592,7 @@ static void show_frame(WriterContext *w, AVFrame *frame, AVStream *stream)
> > if (s) print_str ("pix_fmt", s);
> > else print_str_opt("pix_fmt", "unknown");
> > if (frame->sample_aspect_ratio.num) {
> > - print_fmt("sample_aspect_ratio", "%d:%d",
> > - frame->sample_aspect_ratio.num,
> > - frame->sample_aspect_ratio.den);
> > + print_q("sample_aspect_ratio", frame->sample_aspect_ratio, ':');
>
> Not related but since I like to parasite threads: is there any reason not
> to use av_guess_sample_aspect_ratio() here? Derek recently complained
> about that information not being raised properly with a MOV.
Patch is welcome (and, is there a ticket for it?).
>
> > } else {
> > print_str_opt("sample_aspect_ratio", "N/A");
> > }
> > @@ -1710,7 +1720,7 @@ static void show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_i
> > s = av_get_media_type_string(dec_ctx->codec_type);
> > if (s) print_str ("codec_type", s);
> > else print_str_opt("codec_type", "unknown");
> > - print_fmt("codec_time_base", "%d/%d", dec_ctx->time_base.num, dec_ctx->time_base.den);
> > + print_q("codec_time_base", dec_ctx->time_base, '/');
> >
> > /* print AVI/FourCC tag */
> > av_get_codec_tag_string(val_str, sizeof(val_str), dec_ctx->codec_tag);
> > @@ -1723,16 +1733,12 @@ static void show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_i
> > print_int("height", dec_ctx->height);
> > print_int("has_b_frames", dec_ctx->has_b_frames);
> > if (dec_ctx->sample_aspect_ratio.num) {
> > - print_fmt("sample_aspect_ratio", "%d:%d",
> > - dec_ctx->sample_aspect_ratio.num,
> > - dec_ctx->sample_aspect_ratio.den);
> > + print_q("sample_aspect_ratio", dec_ctx->sample_aspect_ratio, ':');
> > av_reduce(&display_aspect_ratio.num, &display_aspect_ratio.den,
> > dec_ctx->width * dec_ctx->sample_aspect_ratio.num,
> > dec_ctx->height * dec_ctx->sample_aspect_ratio.den,
> > 1024*1024);
> > - print_fmt("display_aspect_ratio", "%d:%d",
> > - display_aspect_ratio.num,
> > - display_aspect_ratio.den);
> > + print_q("display_aspect_ratio", display_aspect_ratio, ':');
> > } else {
> > print_str_opt("sample_aspect_ratio", "N/A");
> > print_str_opt("display_aspect_ratio", "N/A");
> > @@ -1776,9 +1782,9 @@ static void show_stream(WriterContext *w, AVFormatContext *fmt_ctx, int stream_i
> >
> > if (fmt_ctx->iformat->flags & AVFMT_SHOW_IDS) print_fmt ("id", "0x%x", stream->id);
> > else print_str_opt("id", "N/A");
> > - print_fmt("r_frame_rate", "%d/%d", stream->r_frame_rate.num, stream->r_frame_rate.den);
> > - print_fmt("avg_frame_rate", "%d/%d", stream->avg_frame_rate.num, stream->avg_frame_rate.den);
> > - print_fmt("time_base", "%d/%d", stream->time_base.num, stream->time_base.den);
> > + print_q("r_frame_rate", stream->r_frame_rate, '/');
> > + print_q("avg_frame_rate", stream->avg_frame_rate, '/');
> > + print_q("time_base", stream->time_base, '/');
>
> nit+++: align the '/' to make it beautiful©
Done.
>
> Otherwise LGTM.
Pushed, thanks for the review.
--
FFmpeg = Faithful and Frenzy Mournful Portable Elitarian Gangster
More information about the ffmpeg-devel
mailing list