From d1928b0e596ad9e46f4f77e4ab863ad221e68596 Mon Sep 17 00:00:00 2001
From: Eloi BAIL <eloi.bail@savoirfairelinux.com>
Date: Thu, 5 Nov 2015 11:48:24 -0500
Subject: [PATCH] media: fix memory leak in audio decoder
AVPacket are not freed after use. The solution deals with
allocating AVPacket on the stack and ensuring to free it
calling av_packet_unref.
This fix has also widly deployed on Video for having a unique
method other the code.
Change-Id: I049b29effb4e6748ee2dd12b4c52536c99fef498
Tuleap: #91
---
src/media/media_decoder.cpp | 31 +++++++++++++++---------
src/media/media_decoder.h | 8 ++----
src/media/video/video_base.cpp | 9 -------
src/media/video/video_base.h | 13 ----------
src/media/video/video_input.cpp | 3 +--
src/media/video/video_receive_thread.cpp | 3 +--
6 files changed, 23 insertions(+), 44 deletions(-)
diff --git a/src/media/media_decoder.cpp b/src/media/media_decoder.cpp
index 2cd8a06f22..cadbb23002 100644
--- a/src/media/media_decoder.cpp
+++ b/src/media/media_decoder.cpp
@@ -280,10 +280,11 @@ int MediaDecoder::setupFromVideoData()
}
MediaDecoder::Status
-MediaDecoder::decode(VideoFrame& result, video::VideoPacket& video_packet)
+MediaDecoder::decode(VideoFrame& result)
{
- AVPacket *inpacket = video_packet.get();
- int ret = av_read_frame(inputCtx_, inpacket);
+ AVPacket inpacket;
+ av_init_packet(&inpacket);
+ int ret = av_read_frame(inputCtx_, &inpacket);
if (ret == AVERROR(EAGAIN)) {
return Status::Success;
} else if (ret == AVERROR_EOF) {
@@ -296,13 +297,18 @@ MediaDecoder::decode(VideoFrame& result, video::VideoPacket& video_packet)
}
// is this a packet from the video stream?
- if (inpacket->stream_index != streamIndex_)
+ if (inpacket.stream_index != streamIndex_) {
+ av_packet_unref(&inpacket);
return Status::Success;
+ }
auto frame = result.pointer();
int frameFinished = 0;
int len = avcodec_decode_video2(decoderCtx_, frame,
- &frameFinished, inpacket);
+ &frameFinished, &inpacket);
+
+ av_packet_unref(&inpacket);
+
if (len <= 0)
return Status::DecodeError;
@@ -333,10 +339,7 @@ MediaDecoder::decode(const AudioFrame& decodedFrame)
const auto frame = decodedFrame.pointer();
AVPacket inpacket;
- memset(&inpacket, 0, sizeof(inpacket));
av_init_packet(&inpacket);
- inpacket.data = NULL;
- inpacket.size = 0;
int ret = av_read_frame(inputCtx_, &inpacket);
if (ret == AVERROR(EAGAIN)) {
@@ -351,12 +354,16 @@ MediaDecoder::decode(const AudioFrame& decodedFrame)
}
// is this a packet from the audio stream?
- if (inpacket.stream_index != streamIndex_)
+ if (inpacket.stream_index != streamIndex_) {
+ av_packet_unref(&inpacket);
return Status::Success;
+ }
int frameFinished = 0;
int len = avcodec_decode_audio4(decoderCtx_, frame,
&frameFinished, &inpacket);
+ av_packet_unref(&inpacket);
+
if (len <= 0) {
return Status::DecodeError;
}
@@ -385,14 +392,14 @@ MediaDecoder::Status
MediaDecoder::flush(VideoFrame& result)
{
AVPacket inpacket;
- memset(&inpacket, 0, sizeof(inpacket));
av_init_packet(&inpacket);
- inpacket.data = NULL;
- inpacket.size = 0;
int frameFinished = 0;
auto len = avcodec_decode_video2(decoderCtx_, result.pointer(),
&frameFinished, &inpacket);
+
+ av_packet_unref(&inpacket);
+
if (len <= 0)
return Status::DecodeError;
diff --git a/src/media/media_decoder.h b/src/media/media_decoder.h
index d473d1f0d3..16dad8250f 100644
--- a/src/media/media_decoder.h
+++ b/src/media/media_decoder.h
@@ -17,9 +17,7 @@
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/
-
-#ifndef __MEDIA_DECODER_H__
-#define __MEDIA_DECODER_H__
+#pragma once
#include "config.h"
@@ -79,7 +77,7 @@ class MediaDecoder {
void setIOContext(MediaIOHandle *ioctx);
#ifdef RING_VIDEO
int setupFromVideoData();
- Status decode(VideoFrame&, video::VideoPacket&);
+ Status decode(VideoFrame&);
Status flush(VideoFrame&);
#endif // RING_VIDEO
@@ -125,5 +123,3 @@ class MediaDecoder {
};
} // namespace ring
-
-#endif // __MEDIA_DECODER_H__
diff --git a/src/media/video/video_base.cpp b/src/media/video/video_base.cpp
index 835deb8211..9345bce08d 100644
--- a/src/media/video/video_base.cpp
+++ b/src/media/video/video_base.cpp
@@ -29,15 +29,6 @@
namespace ring { namespace video {
-/*=== VideoPacket ===========================================================*/
-
-VideoPacket::VideoPacket() : packet_(static_cast<AVPacket *>(av_mallocz(sizeof(AVPacket))))
-{
- av_init_packet(packet_);
-}
-
-VideoPacket::~VideoPacket() { av_free_packet(packet_); av_free(packet_); }
-
/*=== VideoGenerator =========================================================*/
VideoFrame&
diff --git a/src/media/video/video_base.h b/src/media/video/video_base.h
index 8d694d438f..71c0813286 100644
--- a/src/media/video/video_base.h
+++ b/src/media/video/video_base.h
@@ -121,19 +121,6 @@ public:
struct VideoFrameActiveWriter: Observable<std::shared_ptr<VideoFrame>> {};
struct VideoFramePassiveReader: Observer<std::shared_ptr<VideoFrame>> {};
-/*=== VideoPacket ===========================================================*/
-
-class VideoPacket {
- public:
- VideoPacket();
- ~VideoPacket();
- AVPacket* get() { return packet_; };
-
- private:
- NON_COPYABLE(VideoPacket);
- AVPacket *packet_;
-};
-
/*=== VideoGenerator =========================================================*/
class VideoGenerator : public VideoFrameActiveWriter
diff --git a/src/media/video/video_input.cpp b/src/media/video/video_input.cpp
index 19355e1301..550cb4a34e 100644
--- a/src/media/video/video_input.cpp
+++ b/src/media/video/video_input.cpp
@@ -108,8 +108,7 @@ bool VideoInput::captureFrame()
if (not decoder_)
return false;
- VideoPacket pkt;
- const auto ret = decoder_->decode(getNewFrame(), pkt);
+ const auto ret = decoder_->decode(getNewFrame());
switch (ret) {
case MediaDecoder::Status::ReadError:
diff --git a/src/media/video/video_receive_thread.cpp b/src/media/video/video_receive_thread.cpp
index 10d0d54e59..6fe210455e 100644
--- a/src/media/video/video_receive_thread.cpp
+++ b/src/media/video/video_receive_thread.cpp
@@ -155,8 +155,7 @@ void VideoReceiveThread::addIOContext(SocketPair &socketPair)
bool VideoReceiveThread::decodeFrame()
{
- VideoPacket pkt;
- const auto ret = videoDecoder_->decode(getNewFrame(), pkt);
+ const auto ret = videoDecoder_->decode(getNewFrame());
switch (ret) {
case MediaDecoder::Status::FrameFinished:
--
GitLab