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

[Jason Garrett-Glaser]

2009/9/29 M?ns Rullg?rd <mans at mansr.com>:
> 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?

Each codec will have a flag saying whether or not it supports the new
system.  If "vcodec" is set to a codec which supports that flag,
ffmpeg will use the new system, else, it will use the old system.

>> 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?

In the meantime, we'll need to support both systems, so it controls
whether or not per-codec defaults are used.

>> ?#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.

We could do that.

>> ?//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.

Obviously yes.  Fixed locally.

Dark Shikari