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

[Sebastien Zwickert]

2013/5/24 Xidorn Quan <quanxunzhen at gmail.com>

> 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?
>  <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
>

Looks good to me.
I'll apply it in a few days if no objection.