[FFmpeg-devel] [PATCH] show-banner and show-license as cmdutils.c functions
Stefano Sabatini
stefano.sabatini-lala
Tue Aug 7 12:13:25 CEST 2007
On date Sunday 2007-08-05 16:25:37 +0200, Michael Niedermayer encoded:
[...]
> On Mon, Jul 16, 2007 at 01:46:50PM +0200, Stefano Sabatini wrote:
> > Hi all,
> >
> > this patch implements the various show-banner and show-license
> > functions in ffmpeg.c, ffserver.c and ffplay.c as calls to
> > corresponding cmdutils.c:show_ffmpeg_license() and
> > cmdutils.c:show_ffmpeg_banner() functions.
> >
> > Reduce code duplication in the command line utils programs.
> >
> > Note that it also changes the exit code of ffserver -L and ffmpeg -L
> > >from 1 to 0, which seems to me the correct behaviour.
>
> agree but this should be in a seperate patch
OK.
[...]
> [...]
> > Index: ffplay.c
> > ===================================================================
> > --- ffplay.c (revision 9682)
> > +++ ffplay.c (working copy)
> > @@ -2478,8 +2478,8 @@
> >
> > void show_help(void)
> > {
> > - printf("ffplay version " FFMPEG_VERSION ", Copyright (c) 2003-2007 Fabrice Bellard, et al.\n"
> > - "usage: ffplay [options] input_file\n"
> > + show_ffmpeg_banner(stderr);
> > + printf("usage: ffplay [options] input_file\n"
> > "Simple media player\n");
>
> this changes stdout to stderr in half the code
See below.
> [...]
> > @@ -3780,6 +3781,11 @@
> > }
> > #endif
> >
> > +/* required for the inclusion of cmdutils.h */
> > +void parse_arg_file(const char *filename)
> > +{
> > +}
> > +
>
> ugly hack
Agree, but I couldn't find a better way to manage it.
What about to make cmdutils.c:parse_options like this:
void parse_options(int argc, char **argv, const OptionDef *options,(void (*)(const char *))parse_arg_function)
when parse_arg_function specifyies which function has to be executed
on a bare arg, and manage the case when is parse_arg_function ==
NULL (as would be in the ffserver case)?
> > static int parse_ffconfig(const char *filename)
> > {
> > FILE *f;
> > @@ -4441,14 +4447,9 @@
> > return 0;
> > }
> >
> > -static void show_banner(void)
> > -{
> > - printf("ffserver version " FFMPEG_VERSION ", Copyright (c) 2000-2006 Fabrice Bellard, et al.\n");
> > -}
> > -
> > static void show_help(void)
> > {
> > - show_banner();
> > + show_ffmpeg_banner(stderr);
>
> stdout->stderr change
>
> also ffmpeg != ffserver, the code does not display the same thing after this
[...]
In all the ff* applications the banner shows FFMPEG_VERSION, in the
case of ffmpeg it also displays the libav[cfu] versions, so it seems
safe to me to have just a common banner function that shows them all: or
eventually we can have an argument inside ffmpeg_show_banner that
takes the name of the application as argument, like this:
void show_ffmpeg_banner(FILE *stream, const char* program_name)
{
fprintf(stream, "%s version " FFMPEG_VERSION ", Copyright (c) 2000-2007 Fabrice Bellard, et al.\n", program_name);
fprintf(stream, " configuration: " FFMPEG_CONFIGURATION "\n");
fprintf(stream, " libavutil version: " AV_STRINGIFY(LIBAVUTIL_VERSION) "\n");
fprintf(stream, " libavcodec version: " AV_STRINGIFY(LIBAVCODEC_VERSION) "\n");
fprintf(stream, " libavformat version: " AV_STRINGIFY(LIBAVFORMAT_VERSION) "\n");
fprintf(stream, " built on " __DATE__ " " __TIME__);
#ifdef __GNUC__
fprintf(stream, ", gcc: " __VERSION__ "\n");
#else
fprintf(stream, ", using a non-gcc compiler\n");
#endif
}
Where to print the banner?
Current situation:
application | banner/version output destination |
=================================================
ffplay | stdout
ffserver | stdout
ffmpeg | stderr
I prefer to print it on stderr, but on stdout when it is explicitly
requested the version of the program (for example by a "-version"
option), anyway the behaviour should be consistent across the various
ff* tools.
For what regards the license, it also seems safe to me to have just
one common function (which changes according to --enable-gpl), since the
license of the various ff* depends on the compiled library license
(but maybe I'm wrong here).
BTW: why to bother at all about this stuff?
I'd like to add to FFmpeg a simple diagnostic tool (named ffprobe) and
I'd like to avoid to rewrite again the various banner and license functions.
Cheers.
--
Stefano Sabatini
Linux user number 337176 (see http://counter.li.org)
More information about the ffmpeg-devel
mailing list