On May 29, 2013, at 10:05 AM, Anton Khirnov <[email protected]> wrote:
> > On Wed, 29 May 2013 08:57:13 +0200, Sebastien Zwickert <[email protected]> > wrote: >> >> On May 28, 2013, at 8:09 PM, Anton Khirnov <[email protected]> wrote: >> >>> >>> On Tue, 28 May 2013 09:15:36 +0200, Sebastien Zwickert <[email protected]> >>> wrote: >>>> From: Xidorn Quan <[email protected]> >>>> >>>> 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 | 27 +++++++++++++++++++++++++++ >>>> 2 files changed, 38 insertions(+) >>>> >>>> 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..a183afa 100644 >>>> --- a/libavcodec/vda_h264.c >>>> +++ b/libavcodec/vda_h264.c >>>> @@ -28,6 +28,10 @@ >>>> #include "h264.h" >>>> #include "vda.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, >>>> CFDictionaryRef user_info, >>>> @@ -107,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) >>>> @@ -123,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; >>> >>> Are the previous 3 entries always filled? I think the code currently assumes >>> the buffers are filled continuously. >> >> It depends on the applications get_buffer/get_buffer2 custom implementations. >> However, there is no reason to have previously filled these entries. > > Then this patch should check what buffers are set and append after the last > one. I've sent an updated patch that checks first free entry to append the vda extra buffer. There's a workaround here to bypass the ref counted buffer mechanism here to avoid memory leaks issue when flushing dropped frames (i.e.: while seeking). I've added some comments before creating the extra buffer to try to make clear this workaround. > Though I don't quite understand how does this work. If get_buffer2 is used, so > the frame data lives in user memory, then why is this additional buffer > needed. Core video buffers in vda_h264_end_frame may never be got by the user while seeking for example and this causes some memory leaks. _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
