On Sun, 9 Jun 2013 11:17:43 +0200, Sebastien Zwickert <[email protected]> wrote: > From: Xidorn Quan <[email protected]> > > This patch fixes a buffer leak 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 | 36 +++++++++++++++++++++++++++++++++++- > 2 files changed, 46 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/vda.h b/libavcodec/vda.h > index 987b94f..13e4fc5 100644 > --- a/libavcodec/vda.h > +++ b/libavcodec/vda.h > @@ -125,6 +125,17 @@ struct vda_context { > * The reference size used for fast reallocation. > */ > 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 6c1845a..aa6a3a7 100644 > --- a/libavcodec/vda_h264.c > +++ b/libavcodec/vda_h264.c > @@ -107,12 +107,19 @@ static int vda_h264_decode_slice(AVCodecContext *avctx, > return 0; > } > > +static void vda_h264_release_buffer(void *opaque, uint8_t *data) > +{ > + CVPixelBufferRef cv_buffer = opaque; > + CVPixelBufferRelease(cv_buffer); > +} > + > 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; > - int status; > + AVBufferRef *buffer; > + int status, i; > > if (!vda_ctx->decoder || !vda_ctx->priv_bitstream) > return -1; > @@ -123,6 +130,33 @@ 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; > + > + buffer = NULL; > + > + /* VDA workaround to release properly each core video buffer: > + * we need to create an extra av_buffer with a custom freeing callback. > This extra buffer > + * should not be reference-counted to avoid potential memory leaks. > That's why we put it > + * after the first free entry of AVFrame.buf to not reference this extra > buffer in > + * AVFrame.av_frame_ref(). */ > + for (i = 1; i < AV_NUM_DATA_POINTERS; i++) { > + if (!frame->buf[i] && !frame->buf[i-1]) { > + buffer = av_buffer_create(NULL, 0, vda_h264_release_buffer, > vda_ctx->cv_buffer, 0); > + if (!buffer) { > + CVPixelBufferRelease(vda_ctx->cv_buffer); > + return -1; > + } > + frame->buf[i] = buffer; > + break; > + } > + }
I must say this looks quite hacky to me. If this isn't meant to propagate to other references, why is this stored in the frame at all? Why don't you store it in the private context? -- Anton Khirnov _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
