[FFmpeg-devel] [PATCH] ffprobe: rework/fix flat writer

[Stefano Sabatini]

On date Friday 2012-09-28 23:54:43 +0200, Clément Bœsch encoded:
> On Wed, Sep 26, 2012 at 07:28:57PM +0200, Stefano Sabatini wrote:
> > Do not build from scratch the section header for each section, but build
> > using the previous level buffer, thus improving efficiency and fix some
> > few corner cases which are exposed by the pending disposition patch.
> > ---
> >  ffprobe.c |   37 +++++++++++++++++--------------------
> >  1 files changed, 17 insertions(+), 20 deletions(-)
> > 
> > diff --git a/ffprobe.c b/ffprobe.c
> > index 1b6bea7..8550c22 100644
> > --- a/ffprobe.c
> > +++ b/ffprobe.c
> > @@ -898,35 +898,32 @@ static void flat_print_section_header(WriterContext *wctx)
> >  {
> >      FlatContext *flat = wctx->priv;
> >      AVBPrint *buf = &flat->section_header[wctx->level];
> > -    int i;
> > +    const struct section *section = wctx->section[wctx->level];
> > +    const struct section *parent_section = wctx->level ?
> > +        wctx->section[wctx->level-1] : NULL;
> >  
> >      /* build section header */
> >      av_bprint_clear(buf);
> > -    for (i = 1; i <= wctx->level; i++) {
> > -        if (flat->hierarchical ||
> > -            !(wctx->section[i]->flags & (SECTION_FLAG_IS_ARRAY|SECTION_FLAG_IS_WRAPPER)))
> > -            av_bprintf(buf, "%s%s", wctx->section[i]->name, flat->sep_str);
> > -    }
> > -}
> > -
> > -static void flat_print_key_prefix(WriterContext *wctx)
> > -{
> > -    FlatContext *flat = wctx->priv;
> > -    const struct section *parent_section = wctx->section[wctx->level-1];
> > +    if (!wctx->level)
> > +        return;
> 
> Nit: maybe this could be better expressed as "if (parent_section)"
> 
> > +    av_bprintf(buf, "%s", flat->section_header[wctx->level-1].str);
> >  
> > -    printf("%s", flat->section_header[wctx->level].str);
> > +    if (flat->hierarchical ||
> > +        !(section->flags & (SECTION_FLAG_IS_ARRAY|SECTION_FLAG_IS_WRAPPER))) {
> > +        av_bprintf(buf, "%s%s", wctx->section[wctx->level]->name, flat->sep_str);
> >  
> > -    if (parent_section->flags & SECTION_FLAG_IS_ARRAY) {
> > -        int n = parent_section->id == SECTION_ID_PACKETS_AND_FRAMES ?
> > -            wctx->nb_section_packet_frame : wctx->nb_item[wctx->level-1];
> > -        printf("%d%s", n, flat->sep_str);
> > +        if (parent_section && parent_section->flags & SECTION_FLAG_IS_ARRAY) {
> 
> and so that parent_section check would become meaningless?

Good idea.

> 
> > +            int n = parent_section->id == SECTION_ID_PACKETS_AND_FRAMES ?
> > +                wctx->nb_section_packet_frame : wctx->nb_item[wctx->level-1];
> > +            av_bprintf(buf, "%d%s", n, flat->sep_str);
> > +        }
> >      }
> >  }
> >  
> >  static void flat_print_int(WriterContext *wctx, const char *key, long long int value)
> >  {
> > -    flat_print_key_prefix(wctx);
> > -    printf("%s=%lld\n", key, value);
> > +    FlatContext *flat = wctx->priv;
> > +    printf("%s%s=%lld\n", flat->section_header[wctx->level].str, key, value);
> >  }
> >  
> >  static void flat_print_str(WriterContext *wctx, const char *key, const char *value)
> > @@ -934,7 +931,7 @@ static void flat_print_str(WriterContext *wctx, const char *key, const char *val
> >      FlatContext *flat = wctx->priv;
> >      AVBPrint buf;
> >  
> > -    flat_print_key_prefix(wctx);
> > +    printf("%s", flat->section_header[wctx->level].str);
> >      av_bprint_init(&buf, 1, AV_BPRINT_SIZE_UNLIMITED);
> >      printf("%s=", flat_escape_key_str(&buf, key, flat->sep));
> >      av_bprint_clear(&buf);
> 
> Looks OK otherwise

Thanks, will push it soon.
-- 
FFmpeg = Foolish and Friendly Miracolous Pitiful Enhancing Generator