[FFmpeg-devel] [PATCH] 5 of 5 Add Speex Encoding

[Art Clarke]

On Tue, Jul 7, 2009 at 9:37 PM, Art Clarke <aclarke at xuggle.com> wrote:

>
> Maybe I'm being stupid here, but why does a muxer or demuxer ever need to
> know about this?  Users of av_decode_audio already have to handle multiple
> frames of audio in one packet with the API.
>
> So for now since we only know of one use-case for an encoder actually
> putting multiple frames into a packet, just restrict it to the AVCodeContext
> frames_per_packet field, and only the speex encoder knows about it
> currently?  That seems the simplest thing to do?
>
>
>
One other idea (since I hate adding MORE fields to AVCodecContext) is to
implement a general way for codecs to implement their own options without
having to pollute the AVCodecContext space, yet still make all settable
optiosn be discoverable by using opt.h.

Here's my thought on how to do that:

1) Have one new member on AVCodecContext called "const AVClass
**codec_options" that AVCodec implementations can set to non-zero if they
support options that can be set via opt.h methods
2) Add a new callback to AVCodec called "configure" or something like that
that will be executed when someone uses the avcodec_set_codec(...) method
(i.e. before avcodec_open() is called, since that's when most people set
options).
3) Add an avcodec_set_codec method that people can use to bind a
AVCodecContext to a AVCodec and get options support
4) In codec like libspeexenc.c, in their configure callback, set
AVCodecContext->codec_options to a chunk of memory with attached AVOptions.
5) We can free the memory on AVCodec->close(...)

I've attached some psuedo code for the patches to avcodec.h/utils.c to make
most of this happen (not meant as an actual patch -- just to illustrate an
idea), and then a libav user would do:

AVCodecContext context = avcodec_alloc_context();
AVCodec codec = avcodec_find_encoder(...); // get the speex encoder

// bind the speex encoder to our context;
avcodec_set_codec(context, codec);

// the codec->configure(...) method would set this
if (context->codec_options) {

  // and now we can set any custom option for that codec on the
AVCodecContext
  av_set_string3(context->codec_options, "custom-option", "value", 0, 0);

}

People who set codec_id and codec_type directly on AVCodecContext wouldn't
be able to set custom options, but it would otherwise be 100% backwards
compatible, and code that care about custom options could slowly migrate.

And if this approach was a good idea, a similar approach can be used with
AVFormatContext to make custom AVInputFormat and AVOutputFormat options.

Thoughts?

- Art

-- 
http://www.xuggle.com/
xu?ggle (z?' gl) v. To freely encode, decode, and experience audio and
video.

Use Xuggle to get the power of FFmpeg in Java.
-------------- next part --------------
Index: libavcodec/avcodec.h
===================================================================
--- libavcodec/avcodec.h	(revision 19369)
+++ libavcodec/avcodec.h	(working copy)
@@ -2518,6 +2518,21 @@
      * - decoding: Set by libavcodec
      */
      enum AVChromaLocation chroma_sample_location;
+
+     /**
+      * Users must call avcodec_set_codec() in order to
+      * let AVCodecs that support codec_options initialize
+      * their options.  Not all AVCodec implementation support
+      * custom options.
+      *
+      * If non-zero, av_set_string3 (see opt.h) can
+      * be used to get and set codec-specific options.
+      * - encoding : User can use opt.h functions to get and set options
+      *              on this item, and the specific codec will use it.
+      * - decoding : User can use opt.h functions to get and set options
+      *              on this item, and the specific codec will use it.
+      */
+     const AVClass ** codec_options;
 } AVCodecContext;
 
 /**
@@ -2559,6 +2574,17 @@
     const int *supported_samplerates;       ///< array of supported audio samplerates, or NULL if unknown, array is terminated by 0
     const enum SampleFormat *sample_fmts;   ///< array of supported sample formats, or NULL if unknown, array is terminated by -1
     const int64_t *channel_layouts;         ///< array of support channel layouts, or NULL if unknown. array is terminated by 0
+
+    /**
+     * Called by avcodec_set_codec
+     *
+     * This method should be used to set AVCodecContext->codec_options
+     * to a private data structure if you want to support custom codec
+     * options.
+     *
+     * Memory allocated in configure should be deallocated in close
+     */
+    int (*configure)(AVCodecContext *); 
 } AVCodec;
 
 /**
@@ -3110,6 +3136,21 @@
  *  we WILL change its arguments and name a few times! */
 AVCodecContext *avcodec_alloc_context2(enum CodecType);
 
+/** THIS FUNCTION IS NOT YET PART OF THE PBULIC API!
+ * we WILL change its arguments and name a few times!
+ *
+ * Sets the codec_id and codec_type of avctx, and calls
+ * codec->configure to let the codec do any custom (pre-init)
+ * configuration such as settup up custom avctx->codec_options
+ * structures.
+ *
+ * @param avctx The codec context
+ * @param codec The codec to set
+ *
+ * @return 0 on success; <0 on error.
+ */
+int avcodec_set_codec(AVCodecContext *avctx, AVCodec* codec);
+
 /**
  * Sets the fields of the given AVFrame to default values.
  *
Index: libavcodec/utils.c
===================================================================
--- libavcodec/utils.c	(revision 19369)
+++ libavcodec/utils.c	(working copy)
@@ -420,6 +420,19 @@
     return fmt[0];
 }
 
+int avcodec_set_codec(AVCodecContext *avctx, AVCodec *codec)
+{
+    if (!avctx || !codec)
+        return -1;
+
+    avctx->codec_id = codec->id;
+    avctx->codec_type = codec->type;
+    if (codec->configure)
+        return codec->configure(avctx);
+    else
+        return 0;
+}
+
 void avcodec_get_frame_defaults(AVFrame *pic){
     memset(pic, 0, sizeof(AVFrame));