[FFmpeg-devel] [PATCH] lavf/segment: add CVS escaping for CSV list file filename field

[Stefano Sabatini]

On date Monday 2012-09-03 09:05:50 +0200, Alexander Strasser encoded:
> Hi,
> 
> Stefano Sabatini wrote:
> > On date Sunday 2012-09-02 11:16:54 +0200, Alexander Strasser encoded:
> [...]
> > > Stefano Sabatini wrote:
> > > > ---
> > > >  doc/muxers.texi       |    4 ++--
> > > >  libavformat/segment.c |   25 ++++++++++++++++++++++++-
> > > >  2 files changed, 26 insertions(+), 3 deletions(-)
> > > > 
> > > [...]
> > > > --- a/libavformat/segment.c
> > > > +++ b/libavformat/segment.c
> > > > @@ -67,6 +67,28 @@ typedef struct {
> > > >      double start_time, end_time;
> > > >  } SegmentContext;
> > > >  
> > 
> > > > +static void avio_print_csv_escaped_str(AVIOContext *ctx, const char *str)
> > > 
> > >   While technically correct I do not think it is a good idea to have
> > > a static routine called avio_print_csv_escaped_str, IMHO either it
> > > should be added to avio context methods or it should drop the prefix.
> > > ATM I think the first is better.
> > 
> > I don't want to bloat the API with a specific function. We may want to
> > add an escaping/de-escaping API for that, such for example:
> > 
> > int av_escape(AVBprint *dst, const char *str, enum escaping_type)
> > or
> > int avio_print_escaped_str(...)
> > 
> > And since I want to get the work done, I'd go with the simpler
> > route and have a local implementation rather than refantoning the
> > whole universe.
> 
>   OK, no problem. If you consider it not suited for public API
> keeping it local without the prefix is fine.
> 
> [...]
> > --- a/doc/muxers.texi
> > +++ b/doc/muxers.texi
> > @@ -498,8 +498,8 @@ each line matching the format:
> >  @end example
> >  
> >  @var{segment_filename} is the name of the output file generated by the
> > -muxer according to the provided pattern, and should not contain the
> > -"," character for simplifying parsing operations.
> > +muxer according to the provided pattern. CSV escaping (according to
> > +RFC4180) is applied if required.
> >  
> >  @var{segment_start_time} and @var{segment_end_time} specify
> >  the segment start and end time expressed in seconds.
> > diff --git a/libavformat/segment.c b/libavformat/segment.c
> > index 1dda310..2312c5d 100644
> > --- a/libavformat/segment.c
> > +++ b/libavformat/segment.c
> > @@ -67,6 +67,28 @@ typedef struct {
> >      double start_time, end_time;
> >  } SegmentContext;
> >  
> > +static void print_csv_escaped_str(AVIOContext *ctx, const char *str)
> > +{
> > +    const char *p;
> > +    int quote = 0;
> > +
> > +    /* check if input needs quoting */
> > +    for (p = str; *p; p++)
> > +        if (strchr("\",\n\r", *p))
> > +            quote = 1;
> 
>   I just woke up, so I really might be missing something.
> But do you really still need the loop?

No, added a break when quote is set.

[...]

Patch applied.
-- 
FFmpeg = Freak Free Mastering Philosofic Energized Gymnast