[FFmpeg-devel] [PATCH/RFC] Per-codec option system

[Måns Rullgård]

Jason Garrett-Glaser <darkshikari at gmail.com> writes:

> A very simple implementation of a string option system, with x264
> completed as an example.  Not implemented in ffmpeg.c yet.
>
> Eventual goal: eliminate [nearly] all options in AVCodecContext and
> revert entirely to codec-specific options.
>
> Potential improvements: add a .help() command as well to return help
> information on the options for a specific codec.
>
> I think practically everyone agrees that this needs to be done at some
> point.  If bikeshedding results in some variant of this not being
> committed within the next week, I will commit without discussion.  At
> a minimum, I think the (or a similar) API should go in as soon as
> possible so that we can start writing interfaces to it and converting
> more codecs to use it.

How do you intend this to be used from ffmpeg.c and the command line?

> Index: libavcodec/options.c
> ===================================================================
> --- libavcodec/options.c	(revision 20083)
> +++ libavcodec/options.c	(working copy)
> @@ -402,6 +402,7 @@
>  {"color_range", NULL, OFFSET(color_range), FF_OPT_TYPE_INT, AVCOL_RANGE_UNSPECIFIED, 0, AVCOL_RANGE_NB-1, V|E|D},
>  {"chroma_sample_location", NULL, OFFSET(chroma_sample_location), FF_OPT_TYPE_INT, AVCHROMA_LOC_UNSPECIFIED, 0, AVCHROMA_LOC_NB-1, V|E|D},
>  {NULL},
> +{"use_string_options", NULL, OFFSET(use_string_options), FF_OPT_TYPE_INT, DEFAULT, INT_MIN, INT_MAX},
>  };

What is the purpose of this?

>  #undef A
> Index: libavcodec/avcodec.h
> ===================================================================
> --- libavcodec/avcodec.h	(revision 20083)
> +++ libavcodec/avcodec.h	(working copy)
> @@ -609,6 +609,10 @@
>   * Codec can output multiple frames per AVPacket
>   */
>  #define CODEC_CAP_SUBFRAMES        0x0100
> +/** Codec supports the string option system.
> + *
> + */
> +#define CODEC_CAP_STRING_OPTIONS   0x0200

Isn't .set_option() being non-null enough?  It's hard to tell without
seeing the interface actually being used.

>  //The following defines may change, don't expect compatibility if you use them.
>  #define MB_TYPE_INTRA4x4   0x0001
> @@ -2526,6 +2530,13 @@
>       * - decoding: Set by libavcodec
>       */
>      enum AVChromaLocation chroma_sample_location;
> +
> +    /**
> +     * Use the string option instead of the options in AVCodecContext.
> +     * - encoding: Set by user
> +     * - decoding: Set by user
> +     */
> +    int use_string_options;
>  } AVCodecContext;
>  
>  /**
> @@ -2547,6 +2558,20 @@
>      int (*close)(AVCodecContext *);
>      int (*decode)(AVCodecContext *, void *outdata, int *outdata_size, AVPacket *avpkt);
>      /**
> +     * Apply a codec-specific option using the option string system.
> +     * Only works if use_string_options is set in the AVCodecContext.
> +     * Only supported for codecs with CODEC_CAP_STRING_OPTIONS in capabilities.
> +     * Must be called before init().
> +     * Returns: negative if failure, zero if success.
> +     */
> +    int (*set_option)(AVCodecContext *ctx, const char *name, const char *value);
> +    /**
> +     * Apply the default options for the codec-specific options system.
> +     * Only supported for codecs with CODEC_CAP_STRING_OPTIONS in capabilities.
> +     * Must be called before set_option().
> +     */
> +    int (*defaults)(AVCodecContext *ctx);
> +    /**
>       * Codec capabilities.
>       * see CODEC_CAP_*
>       */
> Index: libavcodec/libx264.c
> ===================================================================
> --- libavcodec/libx264.c	(revision 20083)
> +++ libavcodec/libx264.c	(working copy)
> @@ -145,16 +145,25 @@
>      return 0;
>  }
>  
> +static av_cold void X264_default_options(AVCodecContext *avctx)
> +{
> +    x264_param_default(&x4->params);
> +    x4->params.pf_log = X264_log;
> +    x4->params.p_log_private = avctx;
> +}
> +
> +static av_cold int X264_set_option(AVCodecContext *avctx, const char *name, const char *value)
> +{
> +    X264Context *x4 = avctx->priv_data;
> +    return x264_param_default(&x4->params, name, value);

I think you mean x264_param_parse() there.

-- 
M?ns Rullg?rd
mans at mansr.com