[FFmpeg-devel] [PATCH] Define cmdline_program_name and use it in show_help

Stefano Sabatini stefano.sabatini-lala
Tue Oct 28 23:14:08 CET 2008


On date Tuesday 2008-10-28 22:41:09 +0100, Benoit Fouet encoded:
> Hi,
> 
> Stefano Sabatini a ?crit :
> > On date Tuesday 2008-10-28 13:40:48 +0100, Aurelien Jacobs encoded:
> >   
> >> Stefano Sabatini wrote:
> >>
> >>     
> >>> On date Monday 2008-10-27 13:46:24 +0100, Luca Abeni encoded:
> >>>       
> >>>> Hi all,
> >>>>
> >>>> Aurelien Jacobs wrote:
> >>>> [...]
> >>>>         
> >>>>>>> Index: ffmpeg.c
> >>>>>>> ===================================================================
> >>>>>>> --- ffmpeg.c	(revision 15712)
> >>>>>>> +++ ffmpeg.c	(working copy)
> >>>>>>> @@ -71,6 +71,7 @@
> >>>>>>>  #undef exit
> >>>>>>>  
> >>>>>>>  const char program_name[] = "FFmpeg";
> >>>>>>> +const char* cmdline_program_name = NULL;
> >>>>>>>   
> >>>>>>>               
> >>>>>> why is it not static ? btw, I'd prefer char *foo to char* foo, but maybe
> >>>>>> that's just me...
> >>>>>>             
> >>>>> I also prefer char *foo.
> >>>>> And no need to initialize it to NULL.
> >>>>>           
> >>> OK.
> >>>  
> >>>       
> >>>> Maybe this is a stupid question, but... Why using a global variable instead
> >>>> of passing argv[0] as an argument to show_help() (in ffplay.c and ffmpeg.c)?
> >>>>         
> >>> For consistency with program_name, no strong opinion on this.
> >>>       
> >> This don't give more consistency. program_name is used in other .c files,
> >> while cmdline_program_name is not.
> >>     
> >
> > Yes.
> >
> >   
> >> If show_help() is only called from main, then passing argv[0] is definitely
> >> the way to go.
> >>     
> >
> > The problem is that show_help is called by cmdline.c:parse_options,
> > where the OPT_EXIT callback is not meant to take an argument.
> >
> > We could change the OPT_EXIT callback to take an opaque argument, but
> > this would look overkill (well, maybe this patch too is overkill...).
> >
> > So making the cmdline_program_name vars globals looks to me the better
> > compromise.
> >
> > Regards.
> >   
> > ------------------------------------------------------------------------
> >
> > Index: ffmpeg.c
> > ===================================================================
> > --- ffmpeg.c	(revision 15731)
> > +++ ffmpeg.c	(working copy)
> > @@ -91,6 +91,8 @@
> >  
> >  #define MAX_FILES 20
> >  
> > +static const char *cmdline_program_name;
> > +
> >  static AVFormatContext *input_files[MAX_FILES];
> >  static int64_t input_files_ts_offset[MAX_FILES];
> >  static double input_files_ts_scale[MAX_FILES][MAX_STREAMS];
> > @@ -3447,8 +3449,8 @@
> >  static void show_help(void)
> >  {
> >      av_log_set_callback(log_callback_help);
> > -    printf("usage: ffmpeg [[infile options] -i infile]... {[outfile options] outfile}...\n"
> > -           "Hyper fast Audio and Video encoder\n");
> > +    printf("usage: %s [[infile options] -i infile]... {[outfile options] outfile}...\n"
> > +           "Hyper fast Audio and Video encoder\n", cmdline_program_name);
> >      printf("\n");
> >      show_help_options(options, "Main options:\n",
> >                        OPT_EXPERT | OPT_AUDIO | OPT_VIDEO | OPT_SUBTITLE | OPT_GRAB, 0);
> > @@ -3858,6 +3860,7 @@
> >  {
> >      int i;
> >      int64_t ti;
> > +    cmdline_program_name = argv[0];
> >  
> >   
> 
> sorry for the late notice, but it could be good to break a line between
> declaration and assignment too...
> (same goes for the two other ones)

New patch, regards.
-- 
FFmpeg = Frenzy and Fostering Martial Patchable Ecstatic God
-------------- next part --------------
A non-text attachment was scrubbed...
Name: define-cmdline-program-name-03.patch
Type: text/x-diff
Size: 3021 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081028/6b2eca74/attachment.patch>



More information about the ffmpeg-devel mailing list