[FFmpeg-devel] [PATCH] bintext: use private options now that AVFormatParameters has been removed

[Stefano Sabatini]

On date Saturday 2012-01-28 20:58:48 +1100, Peter Ross encoded:
> ---
>  libavformat/bintext.c |   63 ++++++++++++++++++++++++++++++++++++++-----------
>  1 files changed, 49 insertions(+), 14 deletions(-)
> 
> diff --git a/libavformat/bintext.c b/libavformat/bintext.c
> index d9395f0..a658225 100644
> --- a/libavformat/bintext.c
> +++ b/libavformat/bintext.c
> @@ -31,15 +31,18 @@
>   */
>  
>  #include "libavutil/intreadwrite.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/parseutils.h"
>  #include "avformat.h"
>  #include "internal.h"
>  #include "sauce.h"
>  #include "libavcodec/bintext.h"
>  
> -#define LINE_RATE 6000 /** characters per second */
> -
>  typedef struct {
> -    int chars_per_frame;
> +    AVClass *class;

const?

> +    int chars_per_frame; /**< Set by private option */
> +    char *video_size;    /**< Set by private option */
> +    char *framerate;     /**< Set by private option */

nit: add some more specific information, just telling that it is set
by a private option is not very helpful

>      uint64_t fsize;  /**< file size less metadata buffer */
>  } BinDemuxContext;
>  
> @@ -108,17 +111,30 @@ static AVStream * init_stream(AVFormatContext *s)
>      st->codec->codec_tag   = 0;
>      st->codec->codec_type  = AVMEDIA_TYPE_VIDEO;
>  
> -//     if (!ap->time_base.num) {

> +    if (bin->video_size) {
> +        if (av_parse_video_size(&st->codec->width, &st->codec->height, bin->video_size) < 0) {
> +            av_log(s, AV_LOG_ERROR, "Could not parse video size.\n");
> +            return NULL;

nit: provide error context, i.e. tell "Could not parse video size '%s'\n", video_size_str

> +        }
> +    } else {
> +        st->codec->width  = (80<<3);
> +        st->codec->height = (25<<4);
> +    }
> +

> +    if (bin->framerate) {
> +        AVRational framerate;
> +        if (av_parse_video_rate(&framerate, bin->framerate) < 0) {
> +            av_log(s, AV_LOG_ERROR, "Could not parse framerate.\n");
> +            return NULL;

ditto

> +        }
> +        avpriv_set_pts_info(st, 60, framerate.den, framerate.num);
> +    } else {
>          avpriv_set_pts_info(st, 60, 1, 25);
> -//     } else {
> -//         avpriv_set_pts_info(st, 60, ap->time_base.num, ap->time_base.den);
> -//     }
> +    }
>  
>      /* simulate tty display speed */
> -    bin->chars_per_frame = FFMAX(av_q2d(st->time_base) * (/*ap->sample_rate ? ap->sample_rate :*/ LINE_RATE), 1);
> +    bin->chars_per_frame = FFMAX(av_q2d(st->time_base) * bin->chars_per_frame, 1);
>  
> -    st->codec->width  = /*ap->width  ? ap->width  :*/ (80<<3);
> -    st->codec->height = /*ap->height ? ap->height :*/ (25<<4);
>      return st;
>  }
>  
> @@ -144,10 +160,10 @@ static int bintext_read_header(AVFormatContext *s)
>          bin->fsize = avio_size(pb);
>          if (ff_sauce_read(s, &bin->fsize, &got_width, 0) < 0)
>              next_tag_read(s, &bin->fsize);
> -//         if (!ap->width)
> +        if (!bin->video_size) {
>              predict_width(st->codec, bin->fsize, got_width);
> -//         if (!ap->height)
>              calculate_height(st->codec, bin->fsize);
> +        }
>          avio_seek(pb, 0, SEEK_SET);
>      }
>      return 0;
> @@ -243,7 +259,7 @@ static int adf_read_header(AVFormatContext *s)
>          bin->fsize = avio_size(pb) - 1 - 192 - 4096;
>          st->codec->width = 80<<3;
>          ff_sauce_read(s, &bin->fsize, &got_width, 0);
> -//         if (!ap->height)
> +        if (!bin->video_size)
>              calculate_height(st->codec, bin->fsize);
>          avio_seek(pb, 1 + 192 + 4096, SEEK_SET);
>      }
> @@ -296,7 +312,7 @@ static int idf_read_header(AVFormatContext *s)
>  
>      bin->fsize = avio_size(pb) - 12 - 4096 - 48;
>      ff_sauce_read(s, &bin->fsize, &got_width, 0);
> -//     if (!ap->height)
> +    if (!bin->video_size)
>          calculate_height(st->codec, bin->fsize);
>      avio_seek(pb, 12, SEEK_SET);
>      return 0;
> @@ -325,6 +341,21 @@ static int read_packet(AVFormatContext *s,
>      return 0;
>  }
>  
> +#define OFFSET(x) offsetof(BinDemuxContext, x)

> +static const AVOption options[] = {

> +    { "linespeed", "Line speed (bytes per second)", OFFSET(chars_per_frame), AV_OPT_TYPE_INT, {.dbl = 6000}, 1, INT_MAX, AV_OPT_FLAG_DECODING_PARAM},
> +    { "video_size", "A string describing frame size, such as 640x480 or hd720.", OFFSET(video_size), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, AV_OPT_FLAG_DECODING_PARAM },

please use a form "set line speed ...", "set frame size ..."

option.help represents what the option does, rather than what the
argument is (yes I know this is inconsistent).

> +    { "framerate", "", OFFSET(framerate), AV_OPT_TYPE_STRING, {.str = "25"}, 0, 0, AV_OPT_FLAG_DECODING_PARAM },

missing description.

[...]

A description in demuxers.texi would be nice, but of course it is not
required by this patch.
-- 
FFmpeg = Fancy & Forgiving Marvellous Problematic Enlightening Guru