[FFmpeg-devel] [PATCH] show-banner and show-license as cmdutils.c functions

[Michael Niedermayer]

Hi

On Tue, Aug 07, 2007 at 09:45:31PM +0200, Stefano Sabatini wrote:
> On date Tuesday 2007-08-07 18:02:39 +0200, Michael Niedermayer encoded:
> > On Tue, Aug 07, 2007 at 12:13:25PM +0200, Stefano Sabatini wrote:
> > [...]
> > > > [...]
> > > > > @@ -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)?
> > 
> > ok
> > 
> > 
> > > 
> > > > >  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.
> > 
> > ok
> [...]
> 
> Second attempt, regression test passed.
> 
> Suggested log: implement show_version and show_license as cmdutils.c functions.
> 
> Note that the ff* tools behaviour results still inconsistent, further
> patches should "normalize" ffplay and ffserver (that is: version
> printed on stderr at startup, no version displayed when showing help
> and license, version and license outputted on stdout when explicitly
> requested).
> 
> Cheers.
> -- 

[...]
> @@ -3575,9 +3576,9 @@
>  
>  const OptionDef options[] = {
>      /* main options */
> -    { "L", 0, {(void*)show_license}, "show license" },
> +    { "L", 0, {(void*)opt_show_license}, "show license" },
>      { "h", 0, {(void*)show_help}, "show help" },
> -    { "version", 0, {(void*)show_version}, "show version" },
> +    { "version", 0, {(void*)opt_show_version}, "show version" },
>      { "formats", 0, {(void*)show_formats}, "show available formats, codecs, protocols, ..." },

these changes do NOT belong in this patch, they are not needed for
moving the code to cmdutils


[...]
>  int main(int argc, char **argv)
>  {
>      int i;
> @@ -3810,12 +3753,12 @@
>      avformat_opts = av_alloc_format_context();
>      sws_opts = sws_getContext(16,16,0, 16,16,0, sws_flags, NULL,NULL,NULL);
>  
> -    show_banner();
> +    show_version(stderr, "ffmpeg");

this changes FFmpeg to ffmpeg


>      if (argc <= 1)
>          show_help();
>  
>      /* parse options */
> -    parse_options(argc, argv, options);
> +    parse_options(argc, argv, options, opt_output_file);
>  

this belongs into a seperate patch


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070807/2dd24d9f/attachment.pgp>