[FFmpeg-devel] [PATCH] ffprobe: implement generic reindent logic in the JSON writer

[Clément Bœsch]

On Sun, Jan 08, 2012 at 04:28:59PM +0100, Stefano Sabatini wrote:
> On date Sunday 2012-01-08 13:28:11 +0100, Clément Bœsch encoded:
> > On Sun, Jan 08, 2012 at 11:26:03AM +0100, Stefano Sabatini wrote:
> > > On date Sunday 2012-01-08 02:35:00 +0100, Clément Bœsch encoded:
> > > > On Sun, Jan 08, 2012 at 02:27:20AM +0100, Stefano Sabatini wrote:
> > > > > Clarify/generalize indent logic, possibly slower.
> > > > > ---
> > > > >  ffprobe.c |   54 ++++++++++++++++++++++++++++++++++++++----------------
> > > > >  1 files changed, 38 insertions(+), 16 deletions(-)
> > > [...]
> > > > > +#define INDENT "    "
> > > > > +
> > > > > +#define JSON_INDENT() { int i; for (i = 0; i < json->indent_level; i++) printf(INDENT); }
> > > > > +
> > > > 
> > > > I think I already suggested in a "very" old patch that this could be done
> > > > with "%*c", json->indent_level * 4, ' '. It might be faster (and cleaner).
> > > > 
> > > > Are you against this?
> > > 
> > > No, I was not aware of the syntax and failed to find reference to it
> > > the first time. Updated.
> > 
> > Before:
> > 
> > {
> >   "streams": [{
> >     "index": 0,
> >     "codec_name": "mpeg1video",
> >     ...
> >   },{
> >     "index": 1,
> >     "codec_name": "mp2",
> >     ...
> >   }],
> >   "format": {
> >     "filename": "foobar.mpg",
> >     "nb_streams": 2,
> >     ...
> >   }
> > }
> > 
> > After:
> > 
> 
> > {
> >     "streams": [{
> >             "index": 0,
> >             "codec_name": "mpeg1video",
> >             ...
> >         },{
> >             "index": 1,
> >             "codec_name": "mp2",
> >             ...
> >         }],
> >     "format": {
> >         "filename": "foobar.mpg",
> >         "nb_streams": 2,
> >         ...
> >     }
> > }
> > 
> > 
> 
> > Is this the wanted behaviour?
> 
> Code is changing the indent level two times for the nested stream
> section, so this is consistent with the implemented logic.
> 
> > I'm fine with the second one if it helps
> > improving the code, but the change of behaviour should be notified in the
> > commit message IMO.
> 
> Patch is supposed to be applied together with the next one, when the
> output becomes:
> 
> {
>     "streams": [
>         {
>             "index": 0,
>             "codec_name": "mpeg1video",
>             ...
>         },
>         {
>             "index": 1,
>             "codec_name": "mp2",
>             ...
>         }
>     ],
>     "format": {
>         "filename": "foobar.mpg",
>         "nb_streams": 2,
>         ...
>     }
> }
> 
> which I find more readable (as it is more clear the relation between
> the chapter and nested sections).

Fine with me.

-- 
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/20120108/8e154c4d/attachment.asc>