[FFmpeg-devel] [PATCH] Implement common show_banner and show_version functions

[Benoit Fouet]

Hi,

Stefano Sabatini wrote:
> On date Monday 2007-09-24 08:58:40 +0200, Benoit Fouet encoded:
>   
>> Hi,
>>
>> Stefano Sabatini wrote:
>>     
>>> On date Friday 2007-09-14 16:44:53 +0200, Stefano Sabatini encoded:
>>>   
>>>       
>>>> On date Wednesday 2007-09-12 20:34:30 +0200, Michael Niedermayer encoded:
>>>>     
>>>>         
>> [...]
>>
>>     
>>>>> so why not
>>>>> static const char *application_name= "my program";
>>>>> ...
>>>>>
>>>>> show_banner(application_name, ...);
>>>>>       
>>>>>           
>>>> Because in this way you can still define at the top of the file:
>>>>
>>>> static const char *application_name= "my program";
>>>> static const int application_birth_year= 2000;
>>>>
>>>> but then use show_banner like this:
>>>> show_banner("another program", 1000);
>>>>
>>>> Furthermore, requiring the definition of program_name and
>>>> program_birth_name in the program main file when including cmdutils.h
>>>> isn't necessarily a bad thing, because in this way you're saying:
>>>> "this is a command util, you *have* to define the program_name and
>>>> program_birth_year blurb for it or it won't compile".
>>>>
>>>> On the other hand if a file doesn't define the main for a command
>>>> line util, then you have no reasons to include cmdutils.h.
>>>>     
>>>>         
>>> [...]
>>>
>>> I'm attaching the patches corresponding to the two approaches: I'm OK
>>> with both of them, with a preference for the first one proposed
>>> (program_name and program_birth_year declared as extern in cmdutils.c)
>>> for the abovementioned reasons.
>>>
>>>   
>>>       
>> i really prefer the one with static variables...
>> IMHO, you cannot force an application to define globals just to be able
>> to use one or two functions out of the bunch a header file proposes.
>>     
>
> OK, as I said I'm OK with both, and this solution has also its
> advantages.
>   

[...]

>  
>   
> Index: cmdutils.h
> ===================================================================
> --- cmdutils.h	(revision 10573)
> +++ cmdutils.h	(working copy)
> @@ -65,6 +65,23 @@
>  void print_error(const char *filename, int err);
>  
>  /**
> + * Prints the banner of the program on stderr. The banner message
> + * depends on the current versions of the repository and of the libav*
> + * libraries.
> + * @param program_name The name of the program.
> + * @param program_birth_year The year of birth of the program.
>   

i would neither capitalize the "T" nor use period.
english/doc experts ?

> + */
> +void show_banner(const char *program_name, int program_birth_year);
> +
> +/**
> + * Prints the version of the program on stdout. The version message
> + * depends on the current versions of the repository and of the libav*
> + * libraries.
> + * @param program_name The name of the program.
>   

ditto

> + */
> +void show_version(const char *program_name);
> +
> +/**
>   * Prints on stdout the license of the program, which depends on the license of
>   * the compiled libav* libraries.
>   */
> Index: ffmpeg.c
> ===================================================================
> --- ffmpeg.c	(revision 10573)
> +++ ffmpeg.c	(working copy)
> @@ -63,6 +63,9 @@
>  
>  #undef exit
>  
> +static const char *program_name = "FFmpeg";
>   

i'd say static const char program_name[]

> +static const int program_birth_year = 2000;
> +
>  /* select an input stream for an output stream */
>  typedef struct AVStreamMap {
>      int file_index;
> Index: ffserver.c
> ===================================================================
> --- ffserver.c	(revision 10573)
> +++ ffserver.c	(working copy)
> @@ -53,6 +53,9 @@
>  
>  #undef exit
>  
> +static const char *program_name = "FFserver";
>   

ditto

> +static const int program_birth_year = 2000;
> +
>  /* maximum number of simultaneous HTTP connections */
>  #define HTTP_MAX_CONNECTIONS 2000
>  
>   

-- 
Ben