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

[Benoit Fouet]

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)

-- 
Benoit Fouet