[FFmpeg-devel] [PATCH] avcodec_copy_context()

[Stefano Sabatini]

On date Wednesday 2010-03-31 11:19:18 -0400, Ronald S. Bultje encoded:
> Hi,
> 
> On Wed, Mar 31, 2010 at 10:53 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Wed, Mar 31, 2010 at 10:11:11AM -0400, Ronald S. Bultje wrote:
> >> On Wed, Mar 31, 2010 at 9:59 AM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
> >> > Hi Michael,
> >> >
> >> > On Wed, Mar 31, 2010 at 6:00 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >> >> On Tue, Mar 30, 2010 at 02:23:03PM -0400, Ronald S. Bultje wrote:
> >> >>> + * @param dest target codec context, should be allocated with
> >> >>> + * ? ? ? ? ? ? #avcodec_alloc_context(), but otherwise uninitialized
> >> >>
> >> >> this should be checked if(->codec) or so maybe
> >> >
> >> > Coding error, so should be assert(), will add.
> >
> > avcodec* is public api and public api should not call abort() for invalid
> > user arguments imho
> 
> You're right, now that I think about it. AVERROR(EFAULT) OK then?
> 
> Ronald

> Index: ffmpeg-svn/libavcodec/avcodec.h
> ===================================================================
> --- ffmpeg-svn.orig/libavcodec/avcodec.h	2010-03-29 13:37:31.000000000 -0400
> +++ ffmpeg-svn/libavcodec/avcodec.h	2010-03-30 12:00:24.000000000 -0400
> @@ -3256,6 +3256,19 @@
>  AVCodecContext *avcodec_alloc_context2(enum CodecType);
>  
>  /**
> + * Copy the settings of the source AVCodecContext into the destination
      ^^^^
This continues to hurt my eyes ;-)...

> + * AVCodecContext. The resulting destination codec context will be
> + * unopened, i.e. you are required to call avcodec_open() before you
> + * can use this AVCodecContext to decode/encode video/audio data.
> + *
> + * @param dest target codec context, should be initialized with
> + *             #avcodec_alloc_context(), but otherwise uninitialized

Please don't use '#', is ugly, not required and inconsitent with the
most part of the doxy.

[...]
> +    /* reallocate values that should be allocated separately */
> +    dest->rc_eq           = NULL;
> +    dest->extradata       = NULL;
> +    dest->intra_matrix    = NULL;
> +    dest->inter_matrix    = NULL;
> +    dest->rc_override     = NULL;
> +    if (src->rc_eq) {
> +        dest->rc_eq = av_strdup(src->rc_eq);
> +        if (!dest->rc_eq)
> +            return AVERROR(ENOMEM);
> +    }
> +
> +#define alloc_and_cpy_or_fail(obj, size, pad) \

Please use "copy" instead of "cpy", just one character more, so much
more clear, also vocals are cheaper these days.

Regards.
-- 
FFmpeg = Furious and Faithful Mortal Patchable Ecumenical Game