Commit 1dbba251 authored by hclam@chromium.org's avatar hclam@chromium.org

Cleanup resources allocated by FFmpeg to avoid memory and threads leaks

Calls avcodec_free and avcodec_thread_free appropriately.

Review URL: http://codereview.chromium.org/151192

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@20039 0039d316-1c4b-4281-b951-d872f2087c98
parent 8931c41a
...@@ -57,6 +57,10 @@ int avcodec_open(AVCodecContext* avctx, AVCodec* codec) { ...@@ -57,6 +57,10 @@ int avcodec_open(AVCodecContext* avctx, AVCodec* codec) {
return media::MockFFmpeg::get()->AVCodecOpen(avctx, codec); return media::MockFFmpeg::get()->AVCodecOpen(avctx, codec);
} }
int avcodec_close(AVCodecContext* avctx) {
return media::MockFFmpeg::get()->AVCodecClose(avctx);
}
int avcodec_thread_init(AVCodecContext* avctx, int threads) { int avcodec_thread_init(AVCodecContext* avctx, int threads) {
return media::MockFFmpeg::get()->AVCodecThreadInit(avctx, threads); return media::MockFFmpeg::get()->AVCodecThreadInit(avctx, threads);
} }
...@@ -83,6 +87,10 @@ int av_open_input_file(AVFormatContext** format, const char* filename, ...@@ -83,6 +87,10 @@ int av_open_input_file(AVFormatContext** format, const char* filename,
parameters); parameters);
} }
void av_close_input_file(AVFormatContext* format) {
media::MockFFmpeg::get()->AVCloseInputFile(format);
}
int av_find_stream_info(AVFormatContext* format) { int av_find_stream_info(AVFormatContext* format) {
return media::MockFFmpeg::get()->AVFindStreamInfo(format); return media::MockFFmpeg::get()->AVFindStreamInfo(format);
} }
......
...@@ -18,6 +18,7 @@ class MockFFmpeg { ...@@ -18,6 +18,7 @@ class MockFFmpeg {
MOCK_METHOD1(AVCodecFindDecoder, AVCodec*(enum CodecID id)); MOCK_METHOD1(AVCodecFindDecoder, AVCodec*(enum CodecID id));
MOCK_METHOD2(AVCodecOpen, int(AVCodecContext* avctx, AVCodec* codec)); MOCK_METHOD2(AVCodecOpen, int(AVCodecContext* avctx, AVCodec* codec));
MOCK_METHOD1(AVCodecClose, int(AVCodecContext* avctx));
MOCK_METHOD2(AVCodecThreadInit, int(AVCodecContext* avctx, int threads)); MOCK_METHOD2(AVCodecThreadInit, int(AVCodecContext* avctx, int threads));
MOCK_METHOD1(AVCodecFlushBuffers, void(AVCodecContext* avctx)); MOCK_METHOD1(AVCodecFlushBuffers, void(AVCodecContext* avctx));
MOCK_METHOD0(AVCodecAllocFrame, AVFrame*()); MOCK_METHOD0(AVCodecAllocFrame, AVFrame*());
...@@ -30,6 +31,7 @@ class MockFFmpeg { ...@@ -30,6 +31,7 @@ class MockFFmpeg {
AVInputFormat* input_format, AVInputFormat* input_format,
int buffer_size, int buffer_size,
AVFormatParameters* parameters)); AVFormatParameters* parameters));
MOCK_METHOD1(AVCloseInputFile, void(AVFormatContext* format));
MOCK_METHOD1(AVFindStreamInfo, int(AVFormatContext* format)); MOCK_METHOD1(AVFindStreamInfo, int(AVFormatContext* format));
MOCK_METHOD2(AVReadFrame, int(AVFormatContext* format, AVPacket* packet)); MOCK_METHOD2(AVReadFrame, int(AVFormatContext* format, AVPacket* packet));
MOCK_METHOD4(AVSeekFrame, int(AVFormatContext *format, MOCK_METHOD4(AVSeekFrame, int(AVFormatContext *format,
......
...@@ -215,16 +215,40 @@ base::TimeDelta FFmpegDemuxerStream::ConvertTimestamp(int64 timestamp) { ...@@ -215,16 +215,40 @@ base::TimeDelta FFmpegDemuxerStream::ConvertTimestamp(int64 timestamp) {
// FFmpegDemuxer // FFmpegDemuxer
// //
FFmpegDemuxer::FFmpegDemuxer() FFmpegDemuxer::FFmpegDemuxer()
: thread_id_(NULL) { : format_context_(NULL),
thread_id_(NULL) {
} }
FFmpegDemuxer::~FFmpegDemuxer() { FFmpegDemuxer::~FFmpegDemuxer() {
DCHECK(!format_context_.get()); // In this destructor, we clean up resources held by FFmpeg. It is ugly to
// TODO(scherkus): I believe we need to use av_close_input_file() here // close the codec contexts here because the corresponding codecs are opened
// instead of scoped_ptr_malloc calling av_free(). // in the decoder filters. By reaching this point, all filters should have
// // stopped, so this is the only safe place to do the global clean up.
// Note that av_close_input_file() doesn't close the codecs so we need to // TODO(hclam): close the codecs in the corresponding decoders.
// figure out who's responsible for closing the them. AutoLock auto_lock(FFmpegLock::get()->lock());
if (!format_context_)
return;
// Iterate each stream and destroy each one of them.
int streams = format_context_->nb_streams;
for (int i = 0; i < streams; ++i) {
AVStream* stream = format_context_->streams[i];
// The conditions for calling avcodec_close():
// 1. AVStream is alive.
// 2. AVCodecContext in AVStream is alive.
// 3. AVCodec in AVCodecContext is alive.
// Notice that closing a codec context without prior avcodec_open() will
// result in a crash in FFmpeg.
if (stream && stream->codec && stream->codec->codec) {
stream->discard = AVDISCARD_ALL;
avcodec_close(stream->codec);
}
}
// Then finally cleanup the format context.
av_close_input_file(format_context_);
format_context_ = NULL;
} }
void FFmpegDemuxer::PostDemuxTask() { void FFmpegDemuxer::PostDemuxTask() {
...@@ -281,7 +305,7 @@ void FFmpegDemuxer::InititalizeTask(DataSource* data_source) { ...@@ -281,7 +305,7 @@ void FFmpegDemuxer::InititalizeTask(DataSource* data_source) {
std::string key = FFmpegGlue::get()->AddDataSource(data_source); std::string key = FFmpegGlue::get()->AddDataSource(data_source);
// Open FFmpeg AVFormatContext. // Open FFmpeg AVFormatContext.
DCHECK(!format_context_.get()); DCHECK(!format_context_);
AVFormatContext* context = NULL; AVFormatContext* context = NULL;
int result = av_open_input_file(&context, key.c_str(), NULL, 0, NULL); int result = av_open_input_file(&context, key.c_str(), NULL, 0, NULL);
...@@ -293,16 +317,15 @@ void FFmpegDemuxer::InititalizeTask(DataSource* data_source) { ...@@ -293,16 +317,15 @@ void FFmpegDemuxer::InititalizeTask(DataSource* data_source) {
return; return;
} }
// Assign to our scoped_ptr_malloc.
DCHECK(context); DCHECK(context);
format_context_.reset(context); format_context_ = context;
// Serialize calls to av_find_stream_info(). // Serialize calls to av_find_stream_info().
{ {
AutoLock auto_lock(FFmpegLock::get()->lock()); AutoLock auto_lock(FFmpegLock::get()->lock());
// Fully initialize AVFormatContext by parsing the stream a little. // Fully initialize AVFormatContext by parsing the stream a little.
result = av_find_stream_info(format_context_.get()); result = av_find_stream_info(format_context_);
if (result < 0) { if (result < 0) {
host_->Error(DEMUXER_ERROR_COULD_NOT_PARSE); host_->Error(DEMUXER_ERROR_COULD_NOT_PARSE);
return; return;
...@@ -350,7 +373,7 @@ void FFmpegDemuxer::SeekTask(base::TimeDelta time) { ...@@ -350,7 +373,7 @@ void FFmpegDemuxer::SeekTask(base::TimeDelta time) {
flags |= AVSEEK_FLAG_BACKWARD; flags |= AVSEEK_FLAG_BACKWARD;
} }
if (av_seek_frame(format_context_.get(), -1, time.InMicroseconds(), if (av_seek_frame(format_context_, -1, time.InMicroseconds(),
flags) < 0) { flags) < 0) {
// TODO(scherkus): signal error. // TODO(scherkus): signal error.
NOTIMPLEMENTED(); NOTIMPLEMENTED();
...@@ -367,7 +390,7 @@ void FFmpegDemuxer::DemuxTask() { ...@@ -367,7 +390,7 @@ void FFmpegDemuxer::DemuxTask() {
// Allocate and read an AVPacket from the media. // Allocate and read an AVPacket from the media.
scoped_ptr<AVPacket> packet(new AVPacket()); scoped_ptr<AVPacket> packet(new AVPacket());
int result = av_read_frame(format_context_.get(), packet.get()); int result = av_read_frame(format_context_, packet.get());
if (result < 0) { if (result < 0) {
// If we have reached the end of stream, tell the downstream filters about // If we have reached the end of stream, tell the downstream filters about
// the event. // the event.
...@@ -420,9 +443,6 @@ void FFmpegDemuxer::StopTask() { ...@@ -420,9 +443,6 @@ void FFmpegDemuxer::StopTask() {
for (iter = streams_.begin(); iter != streams_.end(); ++iter) { for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
(*iter)->Stop(); (*iter)->Stop();
} }
// Free our AVFormatContext.
format_context_.reset();
} }
bool FFmpegDemuxer::StreamsHavePendingReads() { bool FFmpegDemuxer::StreamsHavePendingReads() {
......
...@@ -161,7 +161,7 @@ class FFmpegDemuxer : public Demuxer { ...@@ -161,7 +161,7 @@ class FFmpegDemuxer : public Demuxer {
void StreamHasEnded(); void StreamHasEnded();
// FFmpeg context handle. // FFmpeg context handle.
scoped_ptr_malloc<AVFormatContext, ScopedPtrAVFree> format_context_; AVFormatContext* format_context_;
// Latest timestamp read on the demuxer thread. // Latest timestamp read on the demuxer thread.
base::TimeDelta current_timestamp_; base::TimeDelta current_timestamp_;
......
...@@ -115,6 +115,12 @@ class FFmpegDemuxerTest : public testing::Test { ...@@ -115,6 +115,12 @@ class FFmpegDemuxerTest : public testing::Test {
// Finish up any remaining tasks. // Finish up any remaining tasks.
message_loop_.RunAllPending(); message_loop_.RunAllPending();
// Release the reference to the demuxer.
demuxer_ = NULL;
// Filter host also holds a reference to demuxer, destroy it.
filter_host_.reset();
// Reset MockFFmpeg. // Reset MockFFmpeg.
MockFFmpeg::set(NULL); MockFFmpeg::set(NULL);
} }
...@@ -125,7 +131,7 @@ class FFmpegDemuxerTest : public testing::Test { ...@@ -125,7 +131,7 @@ class FFmpegDemuxerTest : public testing::Test {
.WillOnce(DoAll(SetArgumentPointee<0>(&format_context_), Return(0))); .WillOnce(DoAll(SetArgumentPointee<0>(&format_context_), Return(0)));
EXPECT_CALL(*MockFFmpeg::get(), AVFindStreamInfo(&format_context_)) EXPECT_CALL(*MockFFmpeg::get(), AVFindStreamInfo(&format_context_))
.WillOnce(Return(0)); .WillOnce(Return(0));
EXPECT_CALL(*MockFFmpeg::get(), AVFree(&format_context_)); EXPECT_CALL(*MockFFmpeg::get(), AVCloseInputFile(&format_context_));
} }
// Initializes both MockFFmpeg and FFmpegDemuxer. // Initializes both MockFFmpeg and FFmpegDemuxer.
...@@ -202,7 +208,7 @@ TEST_F(FFmpegDemuxerTest, Initialize_ParseFails) { ...@@ -202,7 +208,7 @@ TEST_F(FFmpegDemuxerTest, Initialize_ParseFails) {
.WillOnce(DoAll(SetArgumentPointee<0>(&format_context_), Return(0))); .WillOnce(DoAll(SetArgumentPointee<0>(&format_context_), Return(0)));
EXPECT_CALL(*MockFFmpeg::get(), AVFindStreamInfo(&format_context_)) EXPECT_CALL(*MockFFmpeg::get(), AVFindStreamInfo(&format_context_))
.WillOnce(Return(AVERROR_IO)); .WillOnce(Return(AVERROR_IO));
EXPECT_CALL(*MockFFmpeg::get(), AVFree(&format_context_)); EXPECT_CALL(*MockFFmpeg::get(), AVCloseInputFile(&format_context_));
EXPECT_TRUE(demuxer_->Initialize(data_source_.get())); EXPECT_TRUE(demuxer_->Initialize(data_source_.get()));
message_loop_.RunAllPending(); message_loop_.RunAllPending();
......
...@@ -11,6 +11,7 @@ int av_new_packet(AVPacket *pkt, int size); ...@@ -11,6 +11,7 @@ int av_new_packet(AVPacket *pkt, int size);
int avcodec_decode_audio3(AVCodecContext *avctx, int16_t *samples, int *frame_size_ptr, AVPacket *avpkt); int avcodec_decode_audio3(AVCodecContext *avctx, int16_t *samples, int *frame_size_ptr, AVPacket *avpkt);
int avcodec_decode_video2(AVCodecContext *avctx, AVFrame *picture, int *got_picture_ptr, AVPacket *avpkt); int avcodec_decode_video2(AVCodecContext *avctx, AVFrame *picture, int *got_picture_ptr, AVPacket *avpkt);
int avcodec_open(AVCodecContext *avctx, AVCodec *codec); int avcodec_open(AVCodecContext *avctx, AVCodec *codec);
int avcodec_close(AVCodecContext *avctx);
int avcodec_thread_init(AVCodecContext *s, int thread_count); int avcodec_thread_init(AVCodecContext *s, int thread_count);
void av_free_packet(AVPacket *pkt); void av_free_packet(AVPacket *pkt);
void av_init_packet(AVPacket *pkt); void av_init_packet(AVPacket *pkt);
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
int av_find_stream_info(AVFormatContext *ic); int av_find_stream_info(AVFormatContext *ic);
int av_open_input_file(AVFormatContext **ic_ptr, const char *filename, AVInputFormat *fmt, int buf_size, AVFormatParameters *ap); int av_open_input_file(AVFormatContext **ic_ptr, const char *filename, AVInputFormat *fmt, int buf_size, AVFormatParameters *ap);
void av_close_input_file(AVFormatContext *s);
int av_read_frame(AVFormatContext *s, AVPacket *pkt); int av_read_frame(AVFormatContext *s, AVPacket *pkt);
int av_register_protocol(URLProtocol *protocol); int av_register_protocol(URLProtocol *protocol);
int av_seek_frame(AVFormatContext *s, int stream_index, int64_t timestamp, int flags); int av_seek_frame(AVFormatContext *s, int stream_index, int64_t timestamp, int flags);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment