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

7 files changed, 57 insertions, 19 deletions

diff --git a/media/base/mock_ffmpeg.cc b/media/base/mock_ffmpeg.cc
index 8a397de..054f996 100644
--- a/media/base/mock_ffmpeg.cc
+++ b/media/base/mock_ffmpeg.cc
@@ -57,6 +57,10 @@ int avcodec_open(AVCodecContext* avctx, AVCodec* 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) {
   return media::MockFFmpeg::get()->AVCodecThreadInit(avctx, threads);
 }
@@ -83,6 +87,10 @@ int av_open_input_file(AVFormatContext** format, const char* filename,
                                                    parameters);
 }
 
+void av_close_input_file(AVFormatContext* format) {
+  media::MockFFmpeg::get()->AVCloseInputFile(format);
+}
+
 int av_find_stream_info(AVFormatContext* format) {
   return media::MockFFmpeg::get()->AVFindStreamInfo(format);
 }
diff --git a/media/base/mock_ffmpeg.h b/media/base/mock_ffmpeg.h
index 70f34e3..3afdef9 100644
--- a/media/base/mock_ffmpeg.h
+++ b/media/base/mock_ffmpeg.h
@@ -18,6 +18,7 @@ class MockFFmpeg {
 
   MOCK_METHOD1(AVCodecFindDecoder, AVCodec*(enum CodecID id));
   MOCK_METHOD2(AVCodecOpen, int(AVCodecContext* avctx, AVCodec* codec));
+  MOCK_METHOD1(AVCodecClose, int(AVCodecContext* avctx));
   MOCK_METHOD2(AVCodecThreadInit, int(AVCodecContext* avctx, int threads));
   MOCK_METHOD1(AVCodecFlushBuffers, void(AVCodecContext* avctx));
   MOCK_METHOD0(AVCodecAllocFrame, AVFrame*());
@@ -30,6 +31,7 @@ class MockFFmpeg {
                                     AVInputFormat* input_format,
                                     int buffer_size,
                                     AVFormatParameters* parameters));
+  MOCK_METHOD1(AVCloseInputFile, void(AVFormatContext* format));
   MOCK_METHOD1(AVFindStreamInfo, int(AVFormatContext* format));
   MOCK_METHOD2(AVReadFrame, int(AVFormatContext* format, AVPacket* packet));
   MOCK_METHOD4(AVSeekFrame, int(AVFormatContext *format,
diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index fb39f10..00a2fbd 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -215,16 +215,40 @@ base::TimeDelta FFmpegDemuxerStream::ConvertTimestamp(int64 timestamp) {
 // FFmpegDemuxer
 //
 FFmpegDemuxer::FFmpegDemuxer()
-    : thread_id_(NULL) {
+    : format_context_(NULL),
+      thread_id_(NULL) {
 }
 
 FFmpegDemuxer::~FFmpegDemuxer() {
-  DCHECK(!format_context_.get());
-  // TODO(scherkus): I believe we need to use av_close_input_file() here
-  // instead of scoped_ptr_malloc calling av_free().
-  //
-  // Note that av_close_input_file() doesn't close the codecs so we need to
-  // figure out who's responsible for closing the them.
+  // In this destructor, we clean up resources held by FFmpeg. It is ugly to
+  // close the codec contexts here because the corresponding codecs are opened
+  // 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.
+  // TODO(hclam): close the codecs in the corresponding decoders.
+  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() {
@@ -281,7 +305,7 @@ void FFmpegDemuxer::InititalizeTask(DataSource* data_source) {
   std::string key = FFmpegGlue::get()->AddDataSource(data_source);
 
   // Open FFmpeg AVFormatContext.
-  DCHECK(!format_context_.get());
+  DCHECK(!format_context_);
   AVFormatContext* context = NULL;
   int result = av_open_input_file(&context, key.c_str(), NULL, 0, NULL);
 
@@ -293,16 +317,15 @@ void FFmpegDemuxer::InititalizeTask(DataSource* data_source) {
     return;
   }
 
-  // Assign to our scoped_ptr_malloc.
   DCHECK(context);
-  format_context_.reset(context);
+  format_context_ = context;
 
   // Serialize calls to av_find_stream_info().
   {
     AutoLock auto_lock(FFmpegLock::get()->lock());
 
     // 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) {
       host_->Error(DEMUXER_ERROR_COULD_NOT_PARSE);
       return;
@@ -350,7 +373,7 @@ void FFmpegDemuxer::SeekTask(base::TimeDelta time) {
     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) {
     // TODO(scherkus): signal error.
     NOTIMPLEMENTED();
@@ -367,7 +390,7 @@ void FFmpegDemuxer::DemuxTask() {
 
   // Allocate and read an AVPacket from the media.
   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 we have reached the end of stream, tell the downstream filters about
     // the event.
@@ -420,9 +443,6 @@ void FFmpegDemuxer::StopTask() {
   for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
     (*iter)->Stop();
   }
-
-  // Free our AVFormatContext.
-  format_context_.reset();
 }
 
 bool FFmpegDemuxer::StreamsHavePendingReads() {
diff --git a/media/filters/ffmpeg_demuxer.h b/media/filters/ffmpeg_demuxer.h
index bb5c285..8c4e128 100644
--- a/media/filters/ffmpeg_demuxer.h
+++ b/media/filters/ffmpeg_demuxer.h
@@ -161,7 +161,7 @@ class FFmpegDemuxer : public Demuxer {
   void StreamHasEnded();
 
   // FFmpeg context handle.
-  scoped_ptr_malloc<AVFormatContext, ScopedPtrAVFree> format_context_;
+  AVFormatContext* format_context_;
 
   // Latest timestamp read on the demuxer thread.
   base::TimeDelta current_timestamp_;
diff --git a/media/filters/ffmpeg_demuxer_unittest.cc b/media/filters/ffmpeg_demuxer_unittest.cc
index cdacf12..96e1d85 100644
--- a/media/filters/ffmpeg_demuxer_unittest.cc
+++ b/media/filters/ffmpeg_demuxer_unittest.cc
@@ -115,6 +115,12 @@ class FFmpegDemuxerTest : public testing::Test {
     // Finish up any remaining tasks.
     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.
     MockFFmpeg::set(NULL);
   }
@@ -125,7 +131,7 @@ class FFmpegDemuxerTest : public testing::Test {
         .WillOnce(DoAll(SetArgumentPointee<0>(&format_context_), Return(0)));
     EXPECT_CALL(*MockFFmpeg::get(), AVFindStreamInfo(&format_context_))
         .WillOnce(Return(0));
-    EXPECT_CALL(*MockFFmpeg::get(), AVFree(&format_context_));
+    EXPECT_CALL(*MockFFmpeg::get(), AVCloseInputFile(&format_context_));
   }
 
   // Initializes both MockFFmpeg and FFmpegDemuxer.
@@ -202,7 +208,7 @@ TEST_F(FFmpegDemuxerTest, Initialize_ParseFails) {
       .WillOnce(DoAll(SetArgumentPointee<0>(&format_context_), Return(0)));
   EXPECT_CALL(*MockFFmpeg::get(), AVFindStreamInfo(&format_context_))
       .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()));
   message_loop_.RunAllPending();
diff --git a/third_party/ffmpeg/avcodec-52.sigs b/third_party/ffmpeg/avcodec-52.sigs
index 27b8ef9..0e4daf2 100644
--- a/third_party/ffmpeg/avcodec-52.sigs
+++ b/third_party/ffmpeg/avcodec-52.sigs
@@ -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_video2(AVCodecContext *avctx, AVFrame *picture, int *got_picture_ptr, AVPacket *avpkt);
 int avcodec_open(AVCodecContext *avctx, AVCodec *codec);
+int avcodec_close(AVCodecContext *avctx);
 int avcodec_thread_init(AVCodecContext *s, int thread_count);
 void av_free_packet(AVPacket *pkt);
 void av_init_packet(AVPacket *pkt);
diff --git a/third_party/ffmpeg/avformat-52.sigs b/third_party/ffmpeg/avformat-52.sigs
index 5f7b193..a4b643e 100644
--- a/third_party/ffmpeg/avformat-52.sigs
+++ b/third_party/ffmpeg/avformat-52.sigs
@@ -6,6 +6,7 @@
 
 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);
+void av_close_input_file(AVFormatContext *s);
 int av_read_frame(AVFormatContext *s, AVPacket *pkt);
 int av_register_protocol(URLProtocol *protocol);
 int av_seek_frame(AVFormatContext *s, int stream_index, int64_t timestamp, int flags);