[FFmpeg-devel] [PATCH 1/2] avcodec/vda_h264: use av_buffer to manage buffers

[Xidorn Quan]

On Tue, May 21, 2013 at 12:12 PM, Xidorn Quan <quanxunzhen at gmail.com> wrote:

> This patch fixes a leak of buffer when seeking occurs.
> It adds a flag in struct vda_context for compatibility with apps which
> currently use it. If the flag is not set, the hwaccel will behave like
> before.
> ---
>  libavcodec/vda.h      | 11 +++++++++++
>  libavcodec/vda_h264.c | 26 ++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+)
>
> diff --git a/libavcodec/vda.h b/libavcodec/vda.h
> index 281785f..b3d6399 100644
> --- a/libavcodec/vda.h
> +++ b/libavcodec/vda.h
> @@ -134,6 +134,17 @@ struct vda_context {
>       * - decoding: Set/Unset by libavcodec.
>       */
>      int                 priv_allocated_size;
> +
> +    /**
> +     * Use av_buffer to manage buffer.
> +     * When the flag is set, the CVPixelBuffers returned by the decoder
> will
> +     * be released automatically, so you have to retain them if necessary.
> +     * Not setting this flag may cause memory leak.
> +     *
> +     * encoding: unused
> +     * decoding: Set by user.
> +     */
> +    int                 use_ref_buffer;
>  };
>
>  /** Create the video decoder. */
> diff --git a/libavcodec/vda_h264.c b/libavcodec/vda_h264.c
> index d0237c2..c01a21a 100644
> --- a/libavcodec/vda_h264.c
> +++ b/libavcodec/vda_h264.c
> @@ -28,6 +28,9 @@
>  #include "libavutil/avutil.h"
>  #include "h264.h"
>
> +struct vda_buffer {
> +    CVPixelBufferRef cv_buffer;
> +};
>
>  /* Decoder callback that adds the vda frame to the queue in display
> order. */
>  static void vda_decoder_callback (void *vda_hw_ctx,
> @@ -108,11 +111,20 @@ static int vda_h264_decode_slice(AVCodecContext
> *avctx,
>      return 0;
>  }
>
> +static void vda_h264_release_buffer(void *opaque, uint8_t *data)
> +{
> +    struct vda_buffer *context = opaque;
> +    CVPixelBufferRelease(context->cv_buffer);
> +    av_free(context);
> +}
> +
>  static int vda_h264_end_frame(AVCodecContext *avctx)
>  {
>      H264Context *h                      = avctx->priv_data;
>      struct vda_context *vda_ctx         = avctx->hwaccel_context;
>      AVFrame *frame                      = &h->cur_pic_ptr->f;
> +    struct vda_buffer *context;
> +    AVBufferRef *buffer;
>      int status;
>
>      if (!vda_ctx->decoder || !vda_ctx->priv_bitstream)
> @@ -124,6 +136,20 @@ static int vda_h264_end_frame(AVCodecContext *avctx)
>      if (status)
>          av_log(avctx, AV_LOG_ERROR, "Failed to decode frame (%d)\n",
> status);
>
> +    if (!vda_ctx->use_ref_buffer || status)
> +        return status;
> +
> +    context = av_mallocz(sizeof(*context));
> +    buffer = av_buffer_create(NULL, 0, vda_h264_release_buffer, context,
> 0);
> +    if (!context || !buffer) {
> +        CVPixelBufferRelease(vda_ctx->cv_buffer);
> +        av_free(context);
> +        return -1;
> +    }
> +
> +    context->cv_buffer = vda_ctx->cv_buffer;
> +    frame->buf[3] = buffer;
> +
>      return status;
>  }
>
> --
> 1.8.2.3
>
>
As the maintainer of vda_h264 has said that this patch is fine, could
somebody have a review on or apply it?

-- 
Xidorn Quan
GnuPG fingerprint: 6F1E DF9A D250 7505 63E2  345E 7570 8D3F 7C9A 1209