[FFmpeg-devel] [PATCH] tools: add ffescape utility
Stefano Sabatini
stefasab at gmail.com
Tue Oct 23 01:28:31 CEST 2012
On date Monday 2012-10-22 17:47:26 +0200, Nicolas George encoded:
> Le primidi 1er brumaire, an CCXXI, Stefano Sabatini a écrit :
> > >From 9c8a31416465147edf2c433084eae4a649a1dac3 Mon Sep 17 00:00:00 2001
> > From: Stefano Sabatini <stefasab at gmail.com>
> > Date: Mon, 15 Oct 2012 12:17:15 +0200
> > Subject: [PATCH] tools: add ffescape utility
> >
> > ---
> > .gitignore | 1 +
> > libavutil/Makefile | 2 +-
> > tools/ffescape.c | 181 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 183 insertions(+), 1 deletions(-)
> > create mode 100644 tools/ffescape.c
> >
> > diff --git a/.gitignore b/.gitignore
> > index 0b00f07..2c8cd17 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -56,6 +56,7 @@
> > /tools/bisect.need
> > /tools/cws2fws
> > /tools/ffeval
> > +/tools/ffescape
> > /tools/graph2dot
> > /tools/ismindex
> > /tools/pktdumper
> > diff --git a/libavutil/Makefile b/libavutil/Makefile
> > index 63a48be..e0dd816 100644
> > --- a/libavutil/Makefile
> > +++ b/libavutil/Makefile
> > @@ -130,6 +130,6 @@ TESTPROGS = adler32 \
> >
> > TESTPROGS-$(HAVE_LZO1X_999_COMPRESS) += lzo
> >
> > -TOOLS = ffeval
> > +TOOLS = ffeval ffescape
> >
> > $(SUBDIR)lzo-test$(EXESUF): ELIBS = -llzo2
> > diff --git a/tools/ffescape.c b/tools/ffescape.c
> > new file mode 100644
> > index 0000000..cb189a9
> > --- /dev/null
> > +++ b/tools/ffescape.c
> > @@ -0,0 +1,181 @@
> > +/*
> > + * Copyright (c) 2012 Stefano Sabatini
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > + */
> > +
> > +#include "config.h"
> > +#if HAVE_UNISTD_H
> > +#include <unistd.h> /* getopt */
> > +#endif
> > +
> > +#include "libavutil/log.h"
> > +#include "libavutil/bprint.h"
> > +
> > +#if !HAVE_GETOPT
> > +#include "compat/getopt.c"
> > +#endif
> > +
> > +/**
> > + * @file
> > + * escaping utility
> > + */
> > +
> > +static void usage(void)
> > +{
> > + printf("Escape an input string, adopting the av_get_token() escaping logic\n");
> > + printf("usage: ffescape [OPTIONS]\n");
> > + printf("\n"
> > + "Options:\n"
> > + "-e echo each input line on output\n"
> > + "-h print this help\n"
> > + "-i INFILE set INFILE as input file, stdin if omitted\n"
> > + "-l LEVEL set the number of escaping levels, 1 if omitted\n"
> > + "-o OUTFILE set OUTFILE as output file, stdout if omitted\n"
> > + "-p PROMPT set output prompt, is '=> ' by default\n"
> > + "-s SPECIAL_CHARS set the list of special characters\n");
> > +}
> > +
> > +#define WHITESPACES " \n\t"
>
> We really need to unify that.
Suggestions?
> > +
> > +static char *escape(const char *src, const char *special_chars)
>
> I believe you could make the program simpler if this function would take the
> bprint buffer as argument: it would just have to add to it, and all errors
> checks and finalization would go outside. Without that, it looks like you
> need to check for out-of-memory twice: once before returning from escape,
> and a second time after the call to escape.
My first idea was to implement a library function, but I preferred to
delay this since I'm not yet sure about which features to implement
(e.g. \ lazy escaping \ or 'quoting'), and I wanted to avoid an
header dependency on bprint.h.
> > +{
> > + AVBPrint dst;
> > + char *ret;
> > +
> > + av_bprint_init(&dst, 1, AV_BPRINT_SIZE_UNLIMITED);
> > + for (; *src; src++) {
> > + if ((special_chars && strchr(special_chars, *src)) ||
> > + strchr("'\\" WHITESPACES, *src))
> > + av_bprintf(&dst, "\\%c", *src);
> > + else
> > + av_bprint_chars(&dst, *src, 1);
>
> I believe the escaping could be made more elegant: count the number of
> special chars S and the number of single quotes Q, if S > kQ, escape using
> single quotes (the single quotes themselves being escaped as '\'', therefore
> k=3 is a good guess), else escape using backslashes.
Yes this is a good idea. I wonder if there are some circumstances
where the user may want to favor one type of escaping or the other
(and thus make it configurable).
> > + }
> > +
> > + if (!av_bprint_is_complete(&dst)) {
> > + av_bprint_finalize(&dst, NULL);
> > + return NULL;
> > + } else {
> > + av_bprint_finalize(&dst, &ret);
> > + return ret;
> > + }
> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > + AVBPrint src;
> > + char *src_buf, *dst_buf;
> > + const char *outfilename = NULL, *infilename = NULL;
> > + FILE *outfile = NULL, *infile = NULL;
> > + const char *prompt = "=> ";
> > + int level = 1;
> > + int echo = 0;
> > + char *special_chars = NULL;
> > + int c;
> > +
> > + while ((c = getopt(argc, argv, "ehi:l:o:p:s:")) != -1) {
> > + switch (c) {
> > + case 'e':
> > + echo = 1;
> > + break;
> > + case 'h':
> > + usage();
> > + return 0;
> > + case 'i':
> > + infilename = optarg;
> > + break;
>
> Do we really need to duplicate the shell's features?
pipes are not always available, e.g. in cmd.exe, thus helps
portability.
> > + case 'l':
> > + {
> > + char *tail;
> > + long int li = strtol(optarg, &tail, 10);
> > + if (*tail || li > INT_MAX || li < 0) {
> > + av_log(NULL, AV_LOG_ERROR,
> > + "Invalid value '%s' for option -l, argument must be a non negative integer\n",
> > + optarg);
> > + return 1;
> > + }
> > + level = li;
> > + break;
> > + }
> > + case 'o':
> > + outfilename = optarg;
> > + break;
> > + case 'p':
> > + prompt = optarg;
> > + break;
> > + case 's':
> > + special_chars = optarg;
> > + break;
>
> > + case '?':
> > + return 1;
>
> usage?
Uhm maybe, in this case ffeval should be changed accordingly.
[...]
--
FFmpeg = Fiendish and Freak Multipurpose Portentous Evil Genius
More information about the ffmpeg-devel
mailing list