[FFmpeg-devel] [PATCH] List supported video size and rate abbreviations

[Benoit Fouet]

Hi,

Stefano Sabatini wrote:
> Index: ffmpeg.c
> ===================================================================
> --- ffmpeg.c	(revision 9165)
> +++ ffmpeg.c	(working copy)
> @@ -2065,11 +2065,26 @@
>      av_log_level = atoi(arg);
>  }
>  
> +static void list_video_rate_abvs(void)
> +{
> +    int i;
> +    char video_rate_abv_str[128];
> +    for (i=-1; i < video_size_rate_abv_nb; i++) {
> +        if (avcodec_video_rate_abv_string (video_rate_abv_str, sizeof(video_rate_abv_str), i) == 0)
> +            fprintf(stdout, "%s\n", video_rate_abv_str);
> +    }
> +}
> +
>  static void opt_frame_rate(const char *arg)
>  {
> -    if (parse_frame_rate(&frame_rate, &frame_rate_base, arg) < 0) {
> -        fprintf(stderr, "Incorrect frame rate\n");
> -        exit(1);
> +    if (strcmp (arg, "list") == 0) {
> +        list_video_rate_abvs();
> +        exit(0);
> +    } else {
> +        if (parse_frame_rate(&frame_rate, &frame_rate_base, arg) < 0) {
> +            fprintf(stderr, "Incorrect frame rate\n");
> +            exit(1);
> +        }
>   

what i mean is that you should not reindent the part:

    if (parse_frame_rate(&frame_rate, &frame_rate_base, arg) < 0) {
        fprintf(stderr, "Incorrect frame rate\n");
        exit(1);

but do it in a second patch, after the first one is applied

>      }
>  }
>  
> @@ -2145,16 +2160,31 @@
>      frame_width -= frame_rightBand;
>  }
>  
> +static void list_video_size_abvs(void)
> +{
> +    int i;
> +    char video_size_abv_str[128];
> +    for (i=-1; i < video_size_rate_abv_nb; i++) {
> +        if (avcodec_video_size_abv_string (video_size_abv_str, sizeof(video_size_abv_str), i) == 0)
> +            fprintf(stdout, "%s\n", video_size_abv_str);
> +    }
> +}
> +
>  static void opt_frame_size(const char *arg)
>  {
> -    if (parse_image_size(&frame_width, &frame_height, arg) < 0) {
> -        fprintf(stderr, "Incorrect frame size\n");
> -        exit(1);
> +    if (strcmp (arg, "list") == 0) {
> +        list_video_size_abvs();
> +        exit(0);
> +    } else {
> +        if (parse_image_size(&frame_width, &frame_height, arg) < 0) {
> +            fprintf(stderr, "Incorrect frame size\n");
> +            exit(1);
> +        }
> +        if ((frame_width % 2) != 0 || (frame_height % 2) != 0) {
> +            fprintf(stderr, "Frame size must be a multiple of 2\n");
> +            exit(1);
> +        }
>      }
> -    if ((frame_width % 2) != 0 || (frame_height % 2) != 0) {
> -        fprintf(stderr, "Frame size must be a multiple of 2\n");
> -        exit(1);
> -    }
>  }
>   

and same thing here

[snip]

> Index: libavformat/avformat.h
> ===================================================================
> --- libavformat/avformat.h	(revision 9165)
> +++ libavformat/avformat.h	(working copy)
> @@ -883,6 +883,51 @@
>  
>  int match_ext(const char *filename, const char *extensions);
>  
> +
> +/**
> + * Number of supported video size and rate abbreviations supported.
> + */
> +extern int const video_size_rate_abv_nb;
> +
> +/**
> + * Prints in buf the string corresponding to the video size
> + * abbreviation with number video_size_abv.  FFmpeg supports
> + * video_size_rate_abv_nb different video size abbreviations, so the
> + * argument may vary between 0 and video_size_rate_abv_nb -1. If
> + * video_size_abv is negative it will print a header, if
> + * video_size_abv is greater or equal to video_size_rate_abv_nb it
> + * will do nothing and return -1.
> + *
> + * @return 0 if video_size_abv is negative or it corresponds to a valid
> + * video size abbreviation, -1 otherwise.
> + * @param buf the buffer where to write the string
> + * @param buf_size the size of buf
> + * @param video_size_abv the number of the video size abbreviation
> + * whose information you want to get, or a negative number to print a
> + * header
>   

well, this is still redundant information

> + */
> +int avcodec_video_size_abv_string (char *buf, int buf_size, int video_size_abv);
> +
> +/**
> + * Prints in buf the string corresponding to the video rate
> + * abbreviation with number video_rate_abv.  FFmpeg supports up to
> + * video_size_rate_abv_nb different video rate abbreviations, so the
> + * argument may vary between 0 and video_size_rate_abv_nb -1, but not
> + * every one of these numbers will correspond to a video framerate. If
> + * video_rate_abv is negative it will print a header, if=
> + * video_rate_abv is greater or equal to video_size_rate_abv_nb it
> + * will do nothing and return -1.
> + *
> + * @return 0 if video_rate_abv is negative or it corresponds to a valid
> + * video rate abbreviation, -1 otherwise.
> + * @param buf the buffer where to write the string
> + * @param buf_size the size of buf
> + * @param video_rate_abv the number of the video rate abbreviation
> + * whose information you want to get, or a negative number to print a
> + * header
> + */
>   

same here

> +int avcodec_video_rate_abv_string (char *buf, int buf_size, int video_rate_abv);
> +
>  #endif /* HAVE_AV_CONFIG_H */
>  
>  #endif /* AVFORMAT_H */
> Index: libavformat/utils.c
> ===================================================================
> --- libavformat/utils.c	(revision 9165)
> +++ libavformat/utils.c	(working copy)
>   

[snip]

> +int avcodec_video_rate_abv_string (char *buf, int buf_size, int video_rate_abv)
> +{
> +    AbvEntry info= frame_abvs[video_rate_abv];
> +
> +    /* print header */
> +    if (video_rate_abv < 0) {
> +        snprintf (buf, buf_size,
> +                  "name      "  " frame_rate" " frame_rate_base"
> +            );
> +    } else if (video_rate_abv < video_size_rate_abv_nb && info.frame_rate != 0) {
>   

"!= 0" is unneeded

Ben
-- 
Purple Labs S.A.
www.purplelabs.com