[FFmpeg-devel] [PATCH 4/4] ffprobe: add flat output format.

[Clément Bœsch]

On Fri, Jun 01, 2012 at 10:13:04AM +0200, Stefano Sabatini wrote:
> On date Thursday 2012-05-31 20:56:50 +0200, Clément Bœsch encoded:
> > On Thu, May 31, 2012 at 01:14:17AM +0200, Stefano Sabatini wrote:
> [...]
> > > > +
> > > > +    if (flat->hierarchical && wctx->multiple_sections)
> > > > +        printf("%s%c", flat->chapter, flat->chr_sep);
> > > > +    printf("%s%c", flat->section, flat->chr_sep);
> > > > +    if (wctx->multiple_sections)
> > > > +        printf("%d%c", n, flat->chr_sep);
> > > > +}
> > > 
> > > Would it make sense to use '_' like default separator character? This
> > > way you have sh code output by default.
> > > 
> > 
> > A lot of keys have a '_' (codec_time_base, display_aspect_ratio,
> > bits_per_sample, …), so I'm a bit reluctant to provide an unreadable
> > output by default.
> > 
> > > Also: you could cache the section name in the private context, like it
> > > is done in the INI writer.
> > > 
> > 
> 
> > I admit I didn't quite understood the point of the copy in the INI writer;
> > I keep a pointer to the current section and chapter names in the private
> > context already, what would be the point of the copy?
> 
> Just avoiding to build the field key prefix every time you need to
> print a field, admittedly this is largely a matter of subjective
> preference.
> 

Mmh ok...

> [...]
> > From 5deb33ad48c58409f844977be1823d9f87775805 Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
> > Date: Tue, 29 May 2012 23:22:39 +0200
> > Subject: [PATCH 3/3] ffprobe: add flat output format.
> > 
> > ---
> >  doc/ffprobe.texi |   29 ++++++++++
> >  ffprobe.c        |  161 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 189 insertions(+), 1 deletion(-)
> > 
> > diff --git a/doc/ffprobe.texi b/doc/ffprobe.texi
> > index a725c37..1778dab 100644
> > --- a/doc/ffprobe.texi
> > +++ b/doc/ffprobe.texi
> > @@ -269,6 +269,35 @@ CSV format.
> >  This writer is equivalent to
> >  @code{compact=item_sep=,:nokey=1:escape=csv}.
> >  
> > + at section flat
> > +Flat format.
> > +
> > +A free-form output where each line contains an explicit key=value, such as
> > +"streams.stream.3.tags.foo=bar". The output is shell escaped, so it can be
> > +directly embedded in sh scripts as long as a valid separator is selected (see
> > +option @var{sep_char}).
> 
> More specifically: as long as a the separator character is an
> alphanumeric character or an underscore ...
> 
> See:
> http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap03.html#tag_03_230
> 

Ah, thanks, added.

> [...]
> > +static void flat_print_section(WriterContext *wctx)
> > +{
> > +    FlatContext *flat = wctx->priv;
> > +    int n = wctx->is_packets_and_frames ? wctx->nb_section_packet_frame
> > +                                        : wctx->nb_section;
> > +
> > +    if (flat->hierarchical && wctx->multiple_sections)
> > +        printf("%s%c", flat->chapter, flat->sep);
> > +    printf("%s%c", flat->section, flat->sep);
> > +    if (wctx->multiple_sections)
> > +        printf("%d%c", n, flat->sep);
> > +}
> 
> Again, the prefix could be cached in an AVBprint, this code executed
> in flat_print_section_header(), and save some CPU cycles, but feel
> free to keep it as long as you don't want to change it.
> 

I'll keep it that way for now, feel free to change it; I prefer to push a
simple approved version first.

Thanks for the review, pushed with the rest of the patchset.

-- 
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/20120601/17ccb1fc/attachment.asc>