[FFmpeg-devel] [PATCH] ffprobe: simplify checks in writer_print_time() and writer_print_rational()

[Clément Bœsch]

On Mon, Sep 24, 2012 at 06:26:28PM +0200, Stefano Sabatini wrote:
> Reduce duplication, as _print_time() and _print_rational() will call
> _print_int() or _print_string() anyway.
> ---
>  ffprobe.c |   21 +++++++++------------
>  1 files changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/ffprobe.c b/ffprobe.c
> index 48d395b..0d393cb 100644
> --- a/ffprobe.c
> +++ b/ffprobe.c
> @@ -326,16 +326,6 @@ 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, 0, AV_BPRINT_SIZE_AUTOMATIC);
> -    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)
>  {
> @@ -347,12 +337,20 @@ static inline void writer_print_string(WriterContext *wctx,
>      }
>  }
>  
> +static inline void writer_print_rational(WriterContext *wctx,
> +                                         const char *key, AVRational q, char sep)
> +{
> +    AVBPrint buf;
> +    av_bprint_init(&buf, 0, AV_BPRINT_SIZE_AUTOMATIC);
> +    av_bprintf(&buf, "%d%c%d", q.num, sep, q.den);
> +    writer_print_string(wctx, key, buf.str, 0);
> +}
> +
>  static void writer_print_time(WriterContext *wctx, const char *key,
>                                int64_t ts, const AVRational *time_base, int is_duration)
>  {
>      char buf[128];
>  
> -    if (!wctx->is_fmt_chapter || !fmt_entries_to_show || av_dict_get(fmt_entries_to_show, key, NULL, 0)) {
>          if ((!is_duration && ts == AV_NOPTS_VALUE) || (is_duration && ts == 0)) {
>              writer_print_string(wctx, key, "N/A", 1);
>          } else {
> @@ -363,7 +361,6 @@ static void writer_print_time(WriterContext *wctx, const char *key,
>              value_string(buf, sizeof(buf), uv);
>              writer_print_string(wctx, key, buf, 0);
>          }
> -    }
>  }
>  
>  static void writer_print_ts(WriterContext *wctx, const char *key, int64_t ts, int is_duration)

AFAICT the commit message and description look a bit misleading:
 - there is now a check done in writer_print_rational()
 - it removes a duplicated check in writer_print_time() more than
   "simplifying" it

Apart from these nitpicks, LGTM

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120925/ffe13d28/attachment.asc>