Make ChunkDemuxer error handling more consistent and robust.

media/filters/chunk_demuxer_unittest.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/base_paths.h"
 #include "base/bind.h"
 #include "base/file_util.h"
+#include "base/message_loop.h"
 #include "base/path_service.h"
 #include "media/base/media.h"
 #include "media/base/mock_callback.h"
 #include "media/base/mock_ffmpeg.h"
+#include "media/base/mock_filter_host.h"
 #include "media/filters/chunk_demuxer.h"
 #include "media/filters/chunk_demuxer_client.h"
 #include "media/webm/cluster_builder.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using ::testing::AnyNumber;
 using ::testing::InSequence;
 using ::testing::Return;
 using ::testing::SetArgumentPointee;
 using ::testing::NiceMock;
 using ::testing::_;
 
 namespace media {
 
 static const uint8 kTracksHeader[] = {
   0x16, 0x54, 0xAE, 0x6B, // Tracks ID
@@ skipping 38 matching lines @@
     codecs_[VIDEO].width = 320;
     codecs_[VIDEO].height = 240;
 
     codecs_[AUDIO].codec_type = AVMEDIA_TYPE_AUDIO;
     codecs_[AUDIO].codec_id = CODEC_ID_VORBIS;
     codecs_[AUDIO].channels = 2;
     codecs_[AUDIO].sample_rate = 44100;
   }
 
   virtual ~ChunkDemuxerTest() {
+    message_loop_.RunAllPending();

[Ami GONE FROM CHROMIUM, 2011/08/03 21:06:02]

This makes me worry.  Why do you have it?

[acolwell GONE FROM CHROMIUM, 2011/08/03 22:08:11]

It was just catching the error on EndOfStream() cases. I've removed this and
added calls at the appropriate locations.

     ShutdownDemuxer();
   }
 
   void ReadFile(const std::string& name, scoped_array<uint8>* buffer,
                 int* size) {
     FilePath file_path;
     EXPECT_TRUE(PathService::Get(base::DIR_SOURCE_ROOT, &file_path));
     file_path = file_path.Append(FILE_PATH_LITERAL("media"))
         .Append(FILE_PATH_LITERAL("test"))
         .Append(FILE_PATH_LITERAL("data"))
@@ skipping 57 matching lines @@
       memcpy(buf, audio_track_entry.get(), audio_track_entry_size);
       buf += audio_track_entry_size;
     }
 
     if (has_video) {
       memcpy(buf, video_track_entry.get(), video_track_entry_size);
       buf += video_track_entry_size;
     }
   }
 
+  void AppendData(const uint8* data, unsigned length) {
+    EXPECT_CALL(mock_filter_host_, SetBufferedBytes(_)).Times(AnyNumber());
+    EXPECT_CALL(mock_filter_host_, SetBufferedTime(_)).Times(AnyNumber());
+    EXPECT_CALL(mock_filter_host_, SetNetworkActivity(true)).Times(AnyNumber());

[Ami GONE FROM CHROMIUM, 2011/08/03 21:06:02]

Ugh to the above three lines, and the other green EXPECT_CALLs in this change. 
Can you say "brittle"?

[acolwell GONE FROM CHROMIUM, 2011/08/03 22:08:11]

Yes. Yes I can...These are on the must die list along with other FilterHost
behavior.

+    EXPECT_TRUE(demuxer_->AppendData(data, length));
+    message_loop_.RunAllPending();
+  }
+
   void AppendInfoTracks(bool has_audio, bool has_video) {
     EXPECT_CALL(mock_ffmpeg_, AVOpenInputFile(_, _, NULL, 0, NULL))
         .WillOnce(DoAll(SetArgumentPointee<0>(&format_context_),
                         Return(0)));
 
     EXPECT_CALL(mock_ffmpeg_, AVFindStreamInfo(&format_context_))
         .WillOnce(Return(0));
 
     EXPECT_CALL(mock_ffmpeg_, AVCloseInputFile(&format_context_));
 
     EXPECT_CALL(mock_ffmpeg_, AVRegisterLockManager(_))
         .WillRepeatedly(Return(0));
 
     scoped_array<uint8> info_tracks;
     int info_tracks_size = 0;
     CreateInfoTracks(has_audio, has_video, &info_tracks, &info_tracks_size);
 
     SetupAVFormatContext(has_audio, has_video);
 
-    demuxer_->AppendData(info_tracks.get(), info_tracks_size);
+    AppendData(info_tracks.get(), info_tracks_size);
   }
 
   static void InitDoneCalled(bool* was_called, PipelineStatus expectedStatus,
                              PipelineStatus status) {
     EXPECT_EQ(status, expectedStatus);
     *was_called = true;
   }
 
   void InitDemuxer(bool has_audio, bool has_video) {
     bool init_done_called = false;
     PipelineStatus expectedStatus =
         (has_audio || has_video) ? PIPELINE_OK : DEMUXER_ERROR_COULD_NOT_OPEN;
 
     EXPECT_CALL(*client_, DemuxerOpened(_));
     demuxer_->Init(base::Bind(&ChunkDemuxerTest::InitDoneCalled,
                               &init_done_called,
                               expectedStatus));
 
     EXPECT_FALSE(init_done_called);
 
     AppendInfoTracks(has_audio, has_video);
 
     EXPECT_TRUE(init_done_called);
+    EXPECT_CALL(mock_filter_host_, SetDuration(_));
+    EXPECT_CALL(mock_filter_host_, SetCurrentReadPosition(_));
+    demuxer_->set_host(&mock_filter_host_);
   }
 
   void ShutdownDemuxer() {
     if (demuxer_) {
       EXPECT_CALL(*client_, DemuxerClosed());
       demuxer_->Shutdown();
     }
 
     if (format_context_.streams) {
       delete[] format_context_.streams;
       format_context_.streams = NULL;
       format_context_.nb_streams = 0;
     }
   }
 
   void AddSimpleBlock(ClusterBuilder* cb, int track_num, int64 timecode) {
     uint8 data[] = { 0x00 };
     cb->AddSimpleBlock(track_num, timecode, 0, data, sizeof(data));
   }
 
   MOCK_METHOD1(Checkpoint, void(int id));
 
+  MessageLoop message_loop_;

[Ami GONE FROM CHROMIUM, 2011/08/03 21:06:02]

I don't get where this is populated, only drained.  What?

[acolwell GONE FROM CHROMIUM, 2011/08/03 22:08:11]

The constructor is magical and initializes the thread local storage. This
declaration causes MessageLoop::current() to no longer be null.

I can add a message_loop_ member to ChunkDemuxer & pass this in if you'd prefer
that to just using MessageLoop::current() in the one place I need it.

[Ami GONE FROM CHROMIUM, 2011/08/03 23:41:35]

Per offline convo, I think you can avoid ML entirely.

[acolwell GONE FROM CHROMIUM, 2011/08/04 00:22:16]

Removed

   MockFFmpeg mock_ffmpeg_;
+  MockFilterHost mock_filter_host_;
 
   AVFormatContext format_context_;
   AVCodecContext codecs_[MAX_CODECS_INDEX];
   AVStream streams_[MAX_CODECS_INDEX];
 
   scoped_ptr<MockChunkDemuxerClient> client_;
   scoped_refptr<ChunkDemuxer> demuxer_;
 
  private:
   void SetupAVFormatContext(bool has_audio, bool has_video) {
@@ skipping 64 matching lines @@
 
   EXPECT_CALL(*this, Checkpoint(2));
 
   ClusterBuilder cb;
   cb.SetClusterTimecode(0);
   AddSimpleBlock(&cb, kVideoTrackNum, 0);
   scoped_ptr<Cluster> cluster(cb.Finish());
 
   Checkpoint(1);
 
-  EXPECT_TRUE(demuxer_->AppendData(cluster->data(), cluster->size()));
+  AppendData(cluster->data(), cluster->size());
 
   Checkpoint(2);
 }
 
 // Test the case where AppendData() is called before Init().
 TEST_F(ChunkDemuxerTest, TestAppendDataBeforeInit) {
   scoped_array<uint8> info_tracks;
   int info_tracks_size = 0;
   CreateInfoTracks(true, true, &info_tracks, &info_tracks_size);
 
@@ skipping 25 matching lines @@
   video->Read(base::Bind(&OnReadDone,
                          base::TimeDelta::FromMilliseconds(123),
                          &video_read_done));
 
   ClusterBuilder cb;
   cb.SetClusterTimecode(0);
   AddSimpleBlock(&cb, kAudioTrackNum, 32);
   AddSimpleBlock(&cb, kVideoTrackNum, 123);
   scoped_ptr<Cluster> cluster(cb.Finish());
 
-  EXPECT_TRUE(demuxer_->AppendData(cluster->data(), cluster->size()));
+  AppendData(cluster->data(), cluster->size());
 
   EXPECT_TRUE(audio_read_done);
   EXPECT_TRUE(video_read_done);
 }
 
 TEST_F(ChunkDemuxerTest, TestOutOfOrderClusters) {
   InitDemuxer(true, true);
 
   ClusterBuilder cb;
 
   cb.SetClusterTimecode(10);
   AddSimpleBlock(&cb, kAudioTrackNum, 10);
   AddSimpleBlock(&cb, kVideoTrackNum, 10);
   AddSimpleBlock(&cb, kAudioTrackNum, 33);
   AddSimpleBlock(&cb, kVideoTrackNum, 43);
   scoped_ptr<Cluster> clusterA(cb.Finish());
 
-  EXPECT_TRUE(demuxer_->AppendData(clusterA->data(), clusterA->size()));
+  AppendData(clusterA->data(), clusterA->size());
 
   // Cluster B starts before clusterA and has data
   // that overlaps.
   cb.SetClusterTimecode(5);
   AddSimpleBlock(&cb, kAudioTrackNum, 5);
   AddSimpleBlock(&cb, kVideoTrackNum, 7);
   AddSimpleBlock(&cb, kAudioTrackNum, 28);
   AddSimpleBlock(&cb, kVideoTrackNum, 40);
   scoped_ptr<Cluster> clusterB(cb.Finish());
 
   // Make sure that AppendData() fails because this cluster data
   // is before previous data.
-  EXPECT_FALSE(demuxer_->AppendData(clusterB->data(), clusterB->size()));
+  EXPECT_CALL(mock_filter_host_, SetError(PIPELINE_ERROR_DECODE));
+  AppendData(clusterB->data(), clusterB->size());
 
-  // Cluster C starts after clusterA.
-  cb.SetClusterTimecode(56);
-  AddSimpleBlock(&cb, kAudioTrackNum, 56);
-  AddSimpleBlock(&cb, kVideoTrackNum, 76);
-  AddSimpleBlock(&cb, kAudioTrackNum, 79);
-  AddSimpleBlock(&cb, kVideoTrackNum, 109);
+  // Verify that AppendData() doesn't accept more data now.
+  cb.SetClusterTimecode(45);
+  AddSimpleBlock(&cb, kAudioTrackNum, 45);
+  AddSimpleBlock(&cb, kVideoTrackNum, 45);
   scoped_ptr<Cluster> clusterC(cb.Finish());
-
-  // Verify that clusterC is accepted.
-  EXPECT_TRUE(demuxer_->AppendData(clusterC->data(), clusterC->size()));
-
-  // Flush and try clusterB again.
-  demuxer_->FlushData();
-  EXPECT_TRUE(demuxer_->AppendData(clusterB->data(), clusterB->size()));
-
-  // Following that with clusterC should work too since it doesn't
-  // overlap with clusterB.
-  EXPECT_TRUE(demuxer_->AppendData(clusterC->data(), clusterC->size()));
+  EXPECT_FALSE(demuxer_->AppendData(clusterC->data(), clusterC->size()));
 }
 
-TEST_F(ChunkDemuxerTest, TestInvalidBlockSequences) {
+TEST_F(ChunkDemuxerTest, TestNonMonotonicButAboveClusterTimecode) {
   InitDemuxer(true, true);
 
   ClusterBuilder cb;
 
-  // Test the case where timecode is not monotonically
-  // increasing but stays above the cluster timecode.
+  // Test the case where block timecodes are not monotonically
+  // increasing but stay above the cluster timecode.
   cb.SetClusterTimecode(5);
   AddSimpleBlock(&cb, kAudioTrackNum, 5);
   AddSimpleBlock(&cb, kVideoTrackNum, 10);
   AddSimpleBlock(&cb, kAudioTrackNum, 7);
   AddSimpleBlock(&cb, kVideoTrackNum, 15);
   scoped_ptr<Cluster> clusterA(cb.Finish());
 
-  EXPECT_FALSE(demuxer_->AppendData(clusterA->data(), clusterA->size()));
+  EXPECT_CALL(mock_filter_host_, SetError(PIPELINE_ERROR_DECODE));
+  AppendData(clusterA->data(), clusterA->size());
 
-  // Test timecodes going backwards before cluster timecode.
+  // Verify that AppendData() doesn't accept more data now.
+  cb.SetClusterTimecode(20);
+  AddSimpleBlock(&cb, kAudioTrackNum, 20);
+  AddSimpleBlock(&cb, kVideoTrackNum, 20);
+  scoped_ptr<Cluster> clusterB(cb.Finish());
+  EXPECT_FALSE(demuxer_->AppendData(clusterB->data(), clusterB->size()));
+}
+
+TEST_F(ChunkDemuxerTest, TestBackwardsAndBeforeClusterTimecode) {
+  InitDemuxer(true, true);
+
+  ClusterBuilder cb;
+
+  // Test timecodes going backwards and including values less than the cluster
+  // timecode.
   cb.SetClusterTimecode(5);
   AddSimpleBlock(&cb, kAudioTrackNum, 5);
   AddSimpleBlock(&cb, kVideoTrackNum, 5);
   AddSimpleBlock(&cb, kAudioTrackNum, 3);
   AddSimpleBlock(&cb, kVideoTrackNum, 3);
+  scoped_ptr<Cluster> clusterA(cb.Finish());
+
+  EXPECT_CALL(mock_filter_host_, SetError(PIPELINE_ERROR_DECODE));
+  AppendData(clusterA->data(), clusterA->size());
+
+  // Verify that AppendData() doesn't accept more data now.
+  cb.SetClusterTimecode(6);
+  AddSimpleBlock(&cb, kAudioTrackNum, 6);
+  AddSimpleBlock(&cb, kVideoTrackNum, 6);
   scoped_ptr<Cluster> clusterB(cb.Finish());
+  EXPECT_FALSE(demuxer_->AppendData(clusterB->data(), clusterB->size()));
+}
 
-  EXPECT_FALSE(demuxer_->AppendData(clusterB->data(), clusterB->size()));
+
+TEST_F(ChunkDemuxerTest, TestPerStreamMonotonicallyIncreasingTimestamps) {
+  InitDemuxer(true, true);
+
+  ClusterBuilder cb;
 
   // Test strict monotonic increasing timestamps on a per stream
   // basis.
   cb.SetClusterTimecode(5);
   AddSimpleBlock(&cb, kAudioTrackNum, 5);
   AddSimpleBlock(&cb, kVideoTrackNum, 5);
   AddSimpleBlock(&cb, kAudioTrackNum, 5);
   AddSimpleBlock(&cb, kVideoTrackNum, 7);
-  scoped_ptr<Cluster> clusterC(cb.Finish());
+  scoped_ptr<Cluster> cluster(cb.Finish());
 
-  EXPECT_FALSE(demuxer_->AppendData(clusterC->data(), clusterC->size()));
+  EXPECT_CALL(mock_filter_host_, SetError(PIPELINE_ERROR_DECODE));
+  AppendData(cluster->data(), cluster->size());
+}
+
+TEST_F(ChunkDemuxerTest, TestMonotonicallyIncreasingTimestampsAcrossClusters) {
+  InitDemuxer(true, true);
+
+  ClusterBuilder cb;
 
   // Test strict monotonic increasing timestamps on a per stream
   // basis across clusters.
   cb.SetClusterTimecode(5);
   AddSimpleBlock(&cb, kAudioTrackNum, 5);
   AddSimpleBlock(&cb, kVideoTrackNum, 5);
-  scoped_ptr<Cluster> clusterD(cb.Finish());
+  scoped_ptr<Cluster> clusterA(cb.Finish());
 
-  EXPECT_TRUE(demuxer_->AppendData(clusterD->data(), clusterD->size()));
+  AppendData(clusterA->data(), clusterA->size());
 
   cb.SetClusterTimecode(5);
   AddSimpleBlock(&cb, kAudioTrackNum, 5);
   AddSimpleBlock(&cb, kVideoTrackNum, 7);
-  scoped_ptr<Cluster> clusterE(cb.Finish());
+  scoped_ptr<Cluster> clusterB(cb.Finish());
 
-  EXPECT_FALSE(demuxer_->AppendData(clusterE->data(), clusterE->size()));
+  EXPECT_CALL(mock_filter_host_, SetError(PIPELINE_ERROR_DECODE));
+  AppendData(clusterB->data(), clusterB->size());
+
+  // Verify that AppendData() doesn't accept more data now.
+  cb.SetClusterTimecode(10);
+  AddSimpleBlock(&cb, kAudioTrackNum, 10);
+  AddSimpleBlock(&cb, kVideoTrackNum, 10);
+  scoped_ptr<Cluster> clusterC(cb.Finish());
+  EXPECT_FALSE(demuxer_->AppendData(clusterC->data(), clusterC->size()));
 }
 
 // Test the case where a cluster is passed to AppendData() before
 // INFO & TRACKS data.
 TEST_F(ChunkDemuxerTest, TestClusterBeforeInfoTracks) {
   EXPECT_CALL(*client_, DemuxerOpened(_));
   demuxer_->Init(NewExpectedStatusCB(DEMUXER_ERROR_COULD_NOT_OPEN));
 
   ClusterBuilder cb;
   cb.SetClusterTimecode(0);
   AddSimpleBlock(&cb, kVideoTrackNum, 0);
   scoped_ptr<Cluster> cluster(cb.Finish());
 
-  EXPECT_FALSE(demuxer_->AppendData(cluster->data(), cluster->size()));
+  AppendData(cluster->data(), cluster->size());
 }
 
-
 // Test cases where we get an EndOfStream() call during initialization.
 TEST_F(ChunkDemuxerTest, TestEOSDuringInit) {
   EXPECT_CALL(*client_, DemuxerOpened(_));
   demuxer_->Init(NewExpectedStatusCB(DEMUXER_ERROR_COULD_NOT_OPEN));
   demuxer_->EndOfStream(PIPELINE_OK);
 }
+
+TEST_F(ChunkDemuxerTest, TestDecodeErrorEndOfStream) {
+  InitDemuxer(true, true);
+
+  ClusterBuilder cb;
+  cb.SetClusterTimecode(0);
+  AddSimpleBlock(&cb, kAudioTrackNum, 0);
+  AddSimpleBlock(&cb, kVideoTrackNum, 0);
+  AddSimpleBlock(&cb, kAudioTrackNum, 23);
+  AddSimpleBlock(&cb, kVideoTrackNum, 33);
+  scoped_ptr<Cluster> cluster(cb.Finish());
+  AppendData(cluster->data(), cluster->size());
+
+  EXPECT_CALL(mock_filter_host_, SetError(PIPELINE_ERROR_DECODE));
+  demuxer_->EndOfStream(PIPELINE_ERROR_DECODE);
+}
+
+TEST_F(ChunkDemuxerTest, TestNetworkErrorEndOfStream) {
+  InitDemuxer(true, true);
+
+  ClusterBuilder cb;
+  cb.SetClusterTimecode(0);
+  AddSimpleBlock(&cb, kAudioTrackNum, 0);
+  AddSimpleBlock(&cb, kVideoTrackNum, 0);
+  AddSimpleBlock(&cb, kAudioTrackNum, 23);
+  AddSimpleBlock(&cb, kVideoTrackNum, 33);
+  scoped_ptr<Cluster> cluster(cb.Finish());
+  AppendData(cluster->data(), cluster->size());
+
+  EXPECT_CALL(mock_filter_host_, SetError(PIPELINE_ERROR_NETWORK));
+  demuxer_->EndOfStream(PIPELINE_ERROR_NETWORK);
+}
+
 }  // namespace media