From 3df6b990ae4e137ae4069961131ebb39e1dbdf06 Mon Sep 17 00:00:00 2001 From: initramfs Date: Mon, 25 Apr 2016 04:11:56 -0400 Subject: [PATCH] fix(video): fix memory leak caused by unfreed buffers in CoreVideoSource Fixes a memory leak caused by old code within CoreVideoSource in the way AVFrame buffers are allocated. --- src/video/corevideosource.cpp | 22 +++++++++------------- src/video/videoframe.cpp | 33 ++++++++++++++++++++------------- src/video/videoframe.h | 8 +++++--- 3 files changed, 34 insertions(+), 29 deletions(-) diff --git a/src/video/corevideosource.cpp b/src/video/corevideosource.cpp index 25e32c54b..d22ed17fd 100644 --- a/src/video/corevideosource.cpp +++ b/src/video/corevideosource.cpp @@ -70,22 +70,19 @@ void CoreVideoSource::pushFrame(const vpx_image_t* vpxframe) AVFrame* avframe = av_frame_alloc(); if (!avframe) return; + avframe->width = width; avframe->height = height; avframe->format = AV_PIX_FMT_YUV420P; - int imgBufferSize = av_image_get_buffer_size(AV_PIX_FMT_YUV420P, width, height, 1); - uint8_t* buf = (uint8_t*)av_malloc(imgBufferSize); - if (!buf) - { + int bufSize = av_image_alloc(avframe->data, avframe->linesize, + width, height, + static_cast(AV_PIX_FMT_YUV420P), VideoFrame::frameAlignment); + + if(bufSize < 0){ av_frame_free(&avframe); return; } - avframe->opaque = buf; - - uint8_t** data = avframe->data; - int* linesize = avframe->linesize; - av_image_fill_arrays(data, linesize, buf, AV_PIX_FMT_YUV420P, width, height, 1); for (int i = 0; i < 3; i++) { @@ -96,14 +93,13 @@ void CoreVideoSource::pushFrame(const vpx_image_t* vpxframe) for (int j = 0; j < size; j++) { - uint8_t *dst = avframe->data[i] + dstStride * j; - uint8_t *src = vpxframe->planes[i] + srcStride * j; + uint8_t* dst = avframe->data[i] + dstStride * j; + uint8_t* src = vpxframe->planes[i] + srcStride * j; memcpy(dst, src, minStride); } } - vframe = std::make_shared(avframe); - + vframe = std::make_shared(avframe, true); emit frameAvailable(vframe); } diff --git a/src/video/videoframe.cpp b/src/video/videoframe.cpp index 547dcf3b1..25c0ebce2 100644 --- a/src/video/videoframe.cpp +++ b/src/video/videoframe.cpp @@ -24,19 +24,20 @@ extern "C"{ #include } -VideoFrame::VideoFrame(AVFrame* sourceFrame, QRect dimensions, int pixFmt, std::function destructCallback) +VideoFrame::VideoFrame(AVFrame* sourceFrame, QRect dimensions, int pixFmt, std::function destructCallback, bool freeSourceFrame) : sourceDimensions(dimensions), sourcePixelFormat(pixFmt), + freeSourceFrame(freeSourceFrame), destructCallback(destructCallback) { frameBuffer[pixFmt][dimensionsToKey(dimensions.size())] = sourceFrame; } -VideoFrame::VideoFrame(AVFrame* sourceFrame, std::function destructCallback) - : VideoFrame(sourceFrame, QRect {0, 0, sourceFrame->width, sourceFrame->height}, sourceFrame->format, destructCallback){} +VideoFrame::VideoFrame(AVFrame* sourceFrame, std::function destructCallback, bool freeSourceFrame) + : VideoFrame(sourceFrame, QRect {0, 0, sourceFrame->width, sourceFrame->height}, sourceFrame->format, destructCallback, freeSourceFrame){} -VideoFrame::VideoFrame(AVFrame* sourceFrame) - : VideoFrame(sourceFrame, QRect {0, 0, sourceFrame->width, sourceFrame->height}, sourceFrame->format, nullptr){} +VideoFrame::VideoFrame(AVFrame* sourceFrame, bool freeSourceFrame) + : VideoFrame(sourceFrame, QRect {0, 0, sourceFrame->width, sourceFrame->height}, sourceFrame->format, nullptr, freeSourceFrame){} VideoFrame::~VideoFrame() { @@ -248,6 +249,11 @@ AVFrame* VideoFrame::generateAVFrame(const QSize& dimensions, const int pixelFor return nullptr; } + // Populate AVFrame fields + ret->width = dimensions.width(); + ret->height = dimensions.height(); + ret->format = pixelFormat; + int bufSize = av_image_alloc(ret->data, ret->linesize, dimensions.width(), dimensions.height(), static_cast(pixelFormat), frameAlignment); @@ -267,6 +273,8 @@ AVFrame* VideoFrame::generateAVFrame(const QSize& dimensions, const int pixelFor resizeAlgo, nullptr, nullptr, nullptr); if(!swsCtx){ + av_freep(&ret->data[0]); + av_frame_unref(ret); av_frame_free(&ret); return nullptr; } @@ -274,13 +282,8 @@ AVFrame* VideoFrame::generateAVFrame(const QSize& dimensions, const int pixelFor AVFrame* source = frameBuffer[sourcePixelFormat][dimensionsToKey(sourceDimensions.size())]; sws_scale(swsCtx, source->data, source->linesize, 0, sourceDimensions.height(), ret->data, ret->linesize); - - // Populate AVFrame fields - ret->width = dimensions.width(); - ret->height = dimensions.height(); - ret->format = pixelFormat; - sws_freeContext(swsCtx); + return ret; } @@ -292,7 +295,7 @@ void VideoFrame::storeAVFrame(AVFrame* frame, const QSize& dimensions, const int AVFrame* old_ret = frameBuffer[pixelFormat][dimensionsToKey(dimensions)]; // Free old frame - av_freep(&frame->data[0]); + av_freep(&old_ret->data[0]); av_frame_unref(old_ret); av_frame_free(&old_ret); } @@ -312,9 +315,13 @@ void VideoFrame::deleteFrameBuffer() { AVFrame* frame = dimKeyIter.value(); - // We only need to free allocated buffers for derived frames and not the original source + // Treat source frame and derived frames separately if(pixFmtIter.key() == sourcePixelFormat && dimKeyIter.key() == sourceKey) { + if(freeSourceFrame) + { + av_freep(&frame->data[0]); + } av_frame_unref(frame); av_frame_free(&frame); } diff --git a/src/video/videoframe.h b/src/video/videoframe.h index 3c185aa0c..043d5761e 100644 --- a/src/video/videoframe.h +++ b/src/video/videoframe.h @@ -56,10 +56,11 @@ public: * @param destructCallback callback function to run upon destruction of the VideoFrame * this callback is only run when destroying a valid VideoFrame (e.g. a VideoFrame instance in * which releaseFrame() was called upon it will not call the callback). + * @param freeSourceFrame whether to free the source frame buffers or not. */ - VideoFrame(AVFrame* sourceFrame, QRect dimensions, int pixFmt, std::function destructCallback); - VideoFrame(AVFrame* sourceFrame, std::function destructCallback); - VideoFrame(AVFrame* sourceFrame); + VideoFrame(AVFrame* sourceFrame, QRect dimensions, int pixFmt, std::function destructCallback, bool freeSourceFrame = false); + VideoFrame(AVFrame* sourceFrame, std::function destructCallback, bool freeSourceFrame = false); + VideoFrame(AVFrame* sourceFrame, bool freeSourceFrame = false); /** * Destructor for VideoFrame. @@ -218,6 +219,7 @@ private: // Source frame const QRect sourceDimensions; const int sourcePixelFormat; + const bool freeSourceFrame; // Destructor callback const std::function destructCallback;