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

[Stefano Sabatini]

On date Tuesday 2009-09-29 11:21:10 -0700, Jason Garrett-Glaser encoded:
> 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.

That's similar to what happens in lavfi: we have a private context for
each filter, which is filled using an ad-hoc options array.

For codecs / mu-demuxers / devices we have an common context shared by
all of them, so an idea could be to put somewhere a private context
(e.g. in priv_data), and use that for setting private options.

In the case of an external library, the private context would be an
opaque struct, in this case we need a function to set the private
options used by the library.

The first case can be reduced to the second one using for example:

int mucodec_set_private_option(AVCodecContext *avctx, const char *name, const char *value)
{
    MuCodecContext *muctx = avctx->priv_data;
    return av_set_option(muctx, name, value);
}
 
> 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.

I don't think this is something can be set fast and furiously,
especially as it affects the whole framework so extra care should be
taken.

[...]
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

 //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;

As Mans noted maybe this is not necessary.

 /**
@@ -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.

Please use third person in doxies.

+     * 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);
+    /**

I would prefer something more explicit like "set_private_option()" /
"set_private_options_defaults()", also "defaults" is not even a verb.

Regards.
-- 
FFmpeg = Faboulous and Fostering MultiPurpose Extravagant Ghost