Re: [FFmpeg-devel] [PATCH v0 02/14] avcodec: move AVCodecInternal allocation to avcodec_alloc_context3

[Anton Khirnov <anton@khirnov.net>]

Quoting Jan Ekström (2023-03-21 00:33:56)
> This allows for private values to be stored before the {de,en}coder
> has been opened and initialized.
> 
> Add a new unsigned boolean entry to specifically note that a
> context has been opened instead of just depending on the internal
> pointer.
> ---
>  libavcodec/avcodec.c              | 18 +++++++++++-------
>  libavcodec/frame_thread_encoder.c |  7 ++++---
>  libavcodec/internal.h             |  5 +++++
>  libavcodec/options.c              | 20 +++++++++++++++++++-
>  4 files changed, 39 insertions(+), 11 deletions(-)
> 
> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
> index fb1362290f..c110b19e08 100644
> --- a/libavcodec/avcodec.c
> +++ b/libavcodec/avcodec.c
> @@ -115,7 +115,7 @@ static int64_t get_bit_rate(AVCodecContext *ctx)
>  int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *codec, AVDictionary **options)
>  {
>      int ret = 0;
> -    AVCodecInternal *avci;
> +    AVCodecInternal *avci = NULL;

Just initialize it to avctx->avci.

>      const FFCodec *codec2;
>  
>      if (avcodec_is_open(avctx))
> @@ -147,12 +147,13 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code
>      if (avctx->extradata_size < 0 || avctx->extradata_size >= FF_MAX_EXTRADATA_SIZE)
>          return AVERROR(EINVAL);
>  
> -    avci = av_mallocz(sizeof(*avci));
> +    avci = avctx->internal;
>      if (!avci) {
> -        ret = AVERROR(ENOMEM);
> -        goto end;
> +        av_log(avctx, AV_LOG_ERROR,
> +               "This AVCodecContext was not properly allocated! Please utilize "
> +               "avcodec_alloc_context3!\n");
> +        return AVERROR(EINVAL);

This should be an assert.

> @@ -147,7 +149,21 @@ FF_ENABLE_DEPRECATION_WARNINGS
>              d++;
>          }
>      }
> +
> +    avci = av_mallocz(sizeof(*avci));
> +    if (!avci)
> +        goto alloc_fail;
> +
> +    s->internal = avci;
> +
>      return 0;
> +
> +alloc_fail:
> +    av_freep(&s->internal);
> +
> +    av_freep(&s->priv_data);
> +
> +    return AVERROR(ENOMEM);

A bit overdoing it with empty lines.

Otherwise looks ok.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".