Skip to content
Snippets Groups Projects
Commit d1928b0e authored by Eloi Bail's avatar Eloi Bail Committed by Guillaume Roguez
Browse files

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
parent 4c93660e
Branches
Tags
No related merge requests found
...@@ -280,10 +280,11 @@ int MediaDecoder::setupFromVideoData() ...@@ -280,10 +280,11 @@ int MediaDecoder::setupFromVideoData()
} }
MediaDecoder::Status MediaDecoder::Status
MediaDecoder::decode(VideoFrame& result, video::VideoPacket& video_packet) MediaDecoder::decode(VideoFrame& result)
{ {
AVPacket *inpacket = video_packet.get(); AVPacket inpacket;
int ret = av_read_frame(inputCtx_, inpacket); av_init_packet(&inpacket);
int ret = av_read_frame(inputCtx_, &inpacket);
if (ret == AVERROR(EAGAIN)) { if (ret == AVERROR(EAGAIN)) {
return Status::Success; return Status::Success;
} else if (ret == AVERROR_EOF) { } else if (ret == AVERROR_EOF) {
...@@ -296,13 +297,18 @@ MediaDecoder::decode(VideoFrame& result, video::VideoPacket& video_packet) ...@@ -296,13 +297,18 @@ MediaDecoder::decode(VideoFrame& result, video::VideoPacket& video_packet)
} }
// is this a packet from the video stream? // 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; return Status::Success;
}
auto frame = result.pointer(); auto frame = result.pointer();
int frameFinished = 0; int frameFinished = 0;
int len = avcodec_decode_video2(decoderCtx_, frame, int len = avcodec_decode_video2(decoderCtx_, frame,
&frameFinished, inpacket); &frameFinished, &inpacket);
av_packet_unref(&inpacket);
if (len <= 0) if (len <= 0)
return Status::DecodeError; return Status::DecodeError;
...@@ -333,10 +339,7 @@ MediaDecoder::decode(const AudioFrame& decodedFrame) ...@@ -333,10 +339,7 @@ MediaDecoder::decode(const AudioFrame& decodedFrame)
const auto frame = decodedFrame.pointer(); const auto frame = decodedFrame.pointer();
AVPacket inpacket; AVPacket inpacket;
memset(&inpacket, 0, sizeof(inpacket));
av_init_packet(&inpacket); av_init_packet(&inpacket);
inpacket.data = NULL;
inpacket.size = 0;
int ret = av_read_frame(inputCtx_, &inpacket); int ret = av_read_frame(inputCtx_, &inpacket);
if (ret == AVERROR(EAGAIN)) { if (ret == AVERROR(EAGAIN)) {
...@@ -351,12 +354,16 @@ MediaDecoder::decode(const AudioFrame& decodedFrame) ...@@ -351,12 +354,16 @@ MediaDecoder::decode(const AudioFrame& decodedFrame)
} }
// is this a packet from the audio stream? // 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; return Status::Success;
}
int frameFinished = 0; int frameFinished = 0;
int len = avcodec_decode_audio4(decoderCtx_, frame, int len = avcodec_decode_audio4(decoderCtx_, frame,
&frameFinished, &inpacket); &frameFinished, &inpacket);
av_packet_unref(&inpacket);
if (len <= 0) { if (len <= 0) {
return Status::DecodeError; return Status::DecodeError;
} }
...@@ -385,14 +392,14 @@ MediaDecoder::Status ...@@ -385,14 +392,14 @@ MediaDecoder::Status
MediaDecoder::flush(VideoFrame& result) MediaDecoder::flush(VideoFrame& result)
{ {
AVPacket inpacket; AVPacket inpacket;
memset(&inpacket, 0, sizeof(inpacket));
av_init_packet(&inpacket); av_init_packet(&inpacket);
inpacket.data = NULL;
inpacket.size = 0;
int frameFinished = 0; int frameFinished = 0;
auto len = avcodec_decode_video2(decoderCtx_, result.pointer(), auto len = avcodec_decode_video2(decoderCtx_, result.pointer(),
&frameFinished, &inpacket); &frameFinished, &inpacket);
av_packet_unref(&inpacket);
if (len <= 0) if (len <= 0)
return Status::DecodeError; return Status::DecodeError;
......
...@@ -17,9 +17,7 @@ ...@@ -17,9 +17,7 @@
* along with this program; if not, write to the Free Software * along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/ */
#pragma once
#ifndef __MEDIA_DECODER_H__
#define __MEDIA_DECODER_H__
#include "config.h" #include "config.h"
...@@ -79,7 +77,7 @@ class MediaDecoder { ...@@ -79,7 +77,7 @@ class MediaDecoder {
void setIOContext(MediaIOHandle *ioctx); void setIOContext(MediaIOHandle *ioctx);
#ifdef RING_VIDEO #ifdef RING_VIDEO
int setupFromVideoData(); int setupFromVideoData();
Status decode(VideoFrame&, video::VideoPacket&); Status decode(VideoFrame&);
Status flush(VideoFrame&); Status flush(VideoFrame&);
#endif // RING_VIDEO #endif // RING_VIDEO
...@@ -125,5 +123,3 @@ class MediaDecoder { ...@@ -125,5 +123,3 @@ class MediaDecoder {
}; };
} // namespace ring } // namespace ring
#endif // __MEDIA_DECODER_H__
...@@ -29,15 +29,6 @@ ...@@ -29,15 +29,6 @@
namespace ring { namespace video { 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 =========================================================*/ /*=== VideoGenerator =========================================================*/
VideoFrame& VideoFrame&
......
...@@ -121,19 +121,6 @@ public: ...@@ -121,19 +121,6 @@ public:
struct VideoFrameActiveWriter: Observable<std::shared_ptr<VideoFrame>> {}; struct VideoFrameActiveWriter: Observable<std::shared_ptr<VideoFrame>> {};
struct VideoFramePassiveReader: Observer<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 =========================================================*/ /*=== VideoGenerator =========================================================*/
class VideoGenerator : public VideoFrameActiveWriter class VideoGenerator : public VideoFrameActiveWriter
......
...@@ -108,8 +108,7 @@ bool VideoInput::captureFrame() ...@@ -108,8 +108,7 @@ bool VideoInput::captureFrame()
if (not decoder_) if (not decoder_)
return false; return false;
VideoPacket pkt; const auto ret = decoder_->decode(getNewFrame());
const auto ret = decoder_->decode(getNewFrame(), pkt);
switch (ret) { switch (ret) {
case MediaDecoder::Status::ReadError: case MediaDecoder::Status::ReadError:
......
...@@ -155,8 +155,7 @@ void VideoReceiveThread::addIOContext(SocketPair &socketPair) ...@@ -155,8 +155,7 @@ void VideoReceiveThread::addIOContext(SocketPair &socketPair)
bool VideoReceiveThread::decodeFrame() bool VideoReceiveThread::decodeFrame()
{ {
VideoPacket pkt; const auto ret = videoDecoder_->decode(getNewFrame());
const auto ret = videoDecoder_->decode(getNewFrame(), pkt);
switch (ret) { switch (ret) {
case MediaDecoder::Status::FrameFinished: case MediaDecoder::Status::FrameFinished:
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment