Fix ChunkDemuxer end of stream handling.

BUG=86536
TEST=ChunkDemuxerTest.TestReadsAfterEndOfStream, ChunkDemuxerTest.TestEndOfStreamWithPendingReads

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@98880 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 163 insertions, 5 deletions

diff --git a/media/filters/chunk_demuxer.cc b/media/filters/chunk_demuxer.cc
index 9da955a..89efadb 100644
--- a/media/filters/chunk_demuxer.cc
+++ b/media/filters/chunk_demuxer.cc
@@ -91,6 +91,7 @@ class ChunkDemuxerStream : public DemuxerStream {
   ReadCBQueue read_cbs_;
   BufferQueue buffers_;
   bool shutdown_called_;
+  bool received_end_of_stream_;
 
   // Keeps track of the timestamp of the last buffer we have
   // added to |buffers_|. This is used to enforce buffers with strictly
@@ -104,6 +105,7 @@ ChunkDemuxerStream::ChunkDemuxerStream(Type type, AVStream* stream)
     : type_(type),
       av_stream_(stream),
       shutdown_called_(false),
+      received_end_of_stream_(false),
       last_buffer_timestamp_(kNoTimestamp) {
 }
 
@@ -113,6 +115,7 @@ void ChunkDemuxerStream::Flush() {
   VLOG(1) << "Flush()";
   base::AutoLock auto_lock(lock_);
   buffers_.clear();
+  received_end_of_stream_ = false;
   last_buffer_timestamp_ = kNoTimestamp;
 }
 
@@ -139,8 +142,21 @@ void ChunkDemuxerStream::AddBuffers(const BufferQueue& buffers) {
 
     for (BufferQueue::const_iterator itr = buffers.begin();
          itr != buffers.end(); itr++) {
-
-      if (!(*itr)->IsEndOfStream()) {
+      // Make sure we aren't trying to add a buffer after we have received and
+      // "end of stream" buffer.
+      DCHECK(!received_end_of_stream_);
+
+      if ((*itr)->IsEndOfStream()) {
+        received_end_of_stream_ = true;
+
+        // Push enough EOS buffers to satisfy outstanding Read() requests.
+        if (read_cbs_.size() > buffers_.size()) {
+          size_t pending_read_without_data = read_cbs_.size() - buffers_.size();
+          for (size_t i = 0; i <= pending_read_without_data; ++i) {
+            buffers_.push_back(*itr);
+          }
+        }
+      } else {
         base::TimeDelta current_ts = (*itr)->GetTimestamp();
         if (last_buffer_timestamp_ != kNoTimestamp) {
           DCHECK_GT(current_ts.ToInternalValue(),
@@ -148,9 +164,8 @@ void ChunkDemuxerStream::AddBuffers(const BufferQueue& buffers) {
         }
 
         last_buffer_timestamp_ = current_ts;
+        buffers_.push_back(*itr);
       }
-
-      buffers_.push_back(*itr);
     }
 
     while (!buffers_.empty() && !read_cbs_.empty()) {
@@ -230,7 +245,7 @@ void ChunkDemuxerStream::Read(const ReadCallback& read_callback) {
   {
     base::AutoLock auto_lock(lock_);
 
-    if (shutdown_called_) {
+    if (shutdown_called_ || (received_end_of_stream_ && buffers_.empty())) {
       buffer = CreateEOSBuffer();
     } else {
       if (buffers_.empty()) {
diff --git a/media/filters/chunk_demuxer_unittest.cc b/media/filters/chunk_demuxer_unittest.cc
index e6b3aba..8217514 100644
--- a/media/filters/chunk_demuxer_unittest.cc
+++ b/media/filters/chunk_demuxer_unittest.cc
@@ -464,4 +464,147 @@ TEST_F(ChunkDemuxerTest, TestNetworkErrorEndOfStream) {
   demuxer_->EndOfStream(PIPELINE_ERROR_NETWORK);
 }
 
+// Helper class to reduce duplicate code when testing end of stream
+// Read() behavior.
+class EndOfStreamHelper {
+ public:
+  EndOfStreamHelper(const scoped_refptr<Demuxer> demuxer)
+      : demuxer_(demuxer),
+        audio_read_done_(false),
+        video_read_done_(false) {
+  }
+
+  // Request a read on the audio and video streams.
+  void RequestReads() {
+    EXPECT_FALSE(audio_read_done_);
+    EXPECT_FALSE(video_read_done_);
+
+    scoped_refptr<DemuxerStream> audio =
+        demuxer_->GetStream(DemuxerStream::AUDIO);
+    scoped_refptr<DemuxerStream> video =
+        demuxer_->GetStream(DemuxerStream::VIDEO);
+
+    audio->Read(base::Bind(&OnEndOfStreamReadDone,
+                           &audio_read_done_));
+
+    video->Read(base::Bind(&OnEndOfStreamReadDone,
+                           &video_read_done_));
+  }
+
+  // Check to see if |audio_read_done_| and |video_read_done_| variables
+  // match |expected|.
+  void CheckIfReadDonesWereCalled(bool expected) {
+    EXPECT_EQ(expected, audio_read_done_);
+    EXPECT_EQ(expected, video_read_done_);
+  }
+
+ private:
+  static void OnEndOfStreamReadDone(bool* called, Buffer* buffer) {
+    EXPECT_TRUE(buffer->IsEndOfStream());
+    *called = true;
+  }
+
+  scoped_refptr<Demuxer> demuxer_;
+  bool audio_read_done_;
+  bool video_read_done_;
+
+  DISALLOW_COPY_AND_ASSIGN(EndOfStreamHelper);
+};
+
+// Make sure that all pending reads that we don't have media data for get an
+// "end of stream" buffer when EndOfStream() is called.
+TEST_F(ChunkDemuxerTest, TestEndOfStreamWithPendingReads) {
+  InitDemuxer(true, true);
+
+  scoped_refptr<DemuxerStream> audio =
+      demuxer_->GetStream(DemuxerStream::AUDIO);
+  scoped_refptr<DemuxerStream> video =
+      demuxer_->GetStream(DemuxerStream::VIDEO);
+
+  bool audio_read_done_1 = false;
+  bool video_read_done_1 = false;
+  EndOfStreamHelper end_of_stream_helper_1(demuxer_);
+  EndOfStreamHelper end_of_stream_helper_2(demuxer_);
+
+  audio->Read(base::Bind(&OnReadDone,
+                         base::TimeDelta::FromMilliseconds(32),
+                         &audio_read_done_1));
+
+  video->Read(base::Bind(&OnReadDone,
+                         base::TimeDelta::FromMilliseconds(123),
+                         &video_read_done_1));
+
+  end_of_stream_helper_1.RequestReads();
+  end_of_stream_helper_2.RequestReads();
+
+  ClusterBuilder cb;
+  cb.SetClusterTimecode(0);
+  AddSimpleBlock(&cb, kAudioTrackNum, 32);
+  AddSimpleBlock(&cb, kVideoTrackNum, 123);
+  scoped_ptr<Cluster> cluster(cb.Finish());
+
+  AppendData(cluster->data(), cluster->size());
+
+  EXPECT_TRUE(audio_read_done_1);
+  EXPECT_TRUE(video_read_done_1);
+  end_of_stream_helper_1.CheckIfReadDonesWereCalled(false);
+  end_of_stream_helper_2.CheckIfReadDonesWereCalled(false);
+
+  demuxer_->EndOfStream(PIPELINE_OK);
+
+  end_of_stream_helper_1.CheckIfReadDonesWereCalled(true);
+  end_of_stream_helper_2.CheckIfReadDonesWereCalled(true);
+}
+
+// Make sure that all Read() calls after we get an EndOfStream()
+// call return an "end of stream" buffer.
+TEST_F(ChunkDemuxerTest, TestReadsAfterEndOfStream) {
+  InitDemuxer(true, true);
+
+  scoped_refptr<DemuxerStream> audio =
+      demuxer_->GetStream(DemuxerStream::AUDIO);
+  scoped_refptr<DemuxerStream> video =
+      demuxer_->GetStream(DemuxerStream::VIDEO);
+
+  bool audio_read_done_1 = false;
+  bool video_read_done_1 = false;
+  EndOfStreamHelper end_of_stream_helper_1(demuxer_);
+  EndOfStreamHelper end_of_stream_helper_2(demuxer_);
+  EndOfStreamHelper end_of_stream_helper_3(demuxer_);
+
+  audio->Read(base::Bind(&OnReadDone,
+                         base::TimeDelta::FromMilliseconds(32),
+                         &audio_read_done_1));
+
+  video->Read(base::Bind(&OnReadDone,
+                         base::TimeDelta::FromMilliseconds(123),
+                         &video_read_done_1));
+
+  end_of_stream_helper_1.RequestReads();
+
+  ClusterBuilder cb;
+  cb.SetClusterTimecode(0);
+  AddSimpleBlock(&cb, kAudioTrackNum, 32);
+  AddSimpleBlock(&cb, kVideoTrackNum, 123);
+  scoped_ptr<Cluster> cluster(cb.Finish());
+
+  AppendData(cluster->data(), cluster->size());
+
+  EXPECT_TRUE(audio_read_done_1);
+  EXPECT_TRUE(video_read_done_1);
+  end_of_stream_helper_1.CheckIfReadDonesWereCalled(false);
+
+  demuxer_->EndOfStream(PIPELINE_OK);
+
+  end_of_stream_helper_1.CheckIfReadDonesWereCalled(true);
+
+  // Request a few more reads and make sure we immediately get
+  // end of stream buffers.
+  end_of_stream_helper_2.RequestReads();
+  end_of_stream_helper_2.CheckIfReadDonesWereCalled(true);
+
+  end_of_stream_helper_3.RequestReads();
+  end_of_stream_helper_3.CheckIfReadDonesWereCalled(true);
+}
+
 }  // namespace media