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

[Stefano Sabatini]

On date Sunday 2012-09-02 11:16:54 +0200, Alexander Strasser encoded:
> Hi Stefano,
> 
>   there is a CVS vs CSV typo in the subject.
> 
>   NIT: Following might be slightly easier to grasp for the human reader:
> 
>   add escaping for filename field of the CSV list file 

Changed.

> 
> 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.

> NIT: avio_print_csv_escaped_str ing sounds better to me, don't know
> currently how this relates the names we have already given though.

I choose it to be consistent with avio_put_str().

[...] 
>   Otherwise  patch looks fine to me.

I'll push it soon, thanks for the review.
-- 
FFmpeg = Frightening and Faithless Mortal Peaceless Enchanting Gem
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-lavf-segment-add-escaping-for-filename-field-of-the-.patch
Type: text/x-diff
Size: 2497 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120902/b6cfad17/attachment.bin>