[FFmpeg-devel] [PATCH][RFC] ffprobe: Consider the chosen separator when doing CSV escaping

[Alexander Strasser]

Alexander Strasser wrote:
> Stefano Sabatini wrote:
> > On date Tuesday 2012-09-18 23:26:42 +0200, Alexander Strasser encoded:
> > > Signed-off-by: Alexander Strasser <eclipse7 at gmx.net>
> > > ---
> > > 
> > 
> > >   Just a first step. More stuff like escaping key fields and checking the
> > > separator does not collide with other meta characters would be the ones I
> > > can think ATM.
> > 
> > which other meta characters?
> 
>   I think the output will be broken if the separator character is equal to
> the end-of-record or quote character. I do not yet know much of the ffprobe
> code so please forgive if I am ignorant.
> 
> > >   But I wanted comments on this ideas first. As the discussion might lead
> > > to develop things in a different direction.
> > > 
> > >   Also I am curious if the code will work with all compilers we currently
> > > support. AFAICT non-const aggregate initialization is not allowed by C89.
> > > But it is trivial to rewrite anyway.
> > > 
> > >  ffprobe.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/ffprobe.c b/ffprobe.c
> > > index aad5d90..09be445 100644
> > > --- a/ffprobe.c
> > > +++ b/ffprobe.c
> > > @@ -566,7 +566,8 @@ static const char *c_escape_str(AVBPrint *dst, const char *src, const char sep,
> > >   */
> > >  static const char *csv_escape_str(AVBPrint *dst, const char *src, const char sep, void *log_ctx)
> > >  {
> > > -    int needs_quoting = !!src[strcspn(src, "\",\n\r")];
> > > +    char meta_chars[] = { sep, '"', '\n', '\r', '\0' };
> > > +    int needs_quoting = !!src[strcspn(src, meta_chars)];
> > 
> > Should be good if tested, thanks.
> 
>   I tested it and will push soon if I hear no more comments. Especially
> if someone wants to compile-test: now would be the time. Else I will 
> monitor what we have on FATE.

  I think it is an improvement over the current situation. Pushed.

  When I get the time I might look a bit deeper into the compact/csv writer.

  Alexander