Fix seeking when the start time is non-zero.

media/filters/ffmpeg_demuxer_unittest.cc

Index: media/filters/ffmpeg_demuxer_unittest.cc
diff --git a/media/filters/ffmpeg_demuxer_unittest.cc b/media/filters/ffmpeg_demuxer_unittest.cc
index a1614a421297fb83681baa506cbe683d8dd4de22..b70138e942b4942ac04d575ce383b8360abdd1aa 100644
--- a/media/filters/ffmpeg_demuxer_unittest.cc
+++ b/media/filters/ffmpeg_demuxer_unittest.cc
@@ -103,13 +103,27 @@ class FFmpegDemuxerTest : public testing::Test {
     InitializeDemuxerText(false);
   }
 
+  void InitializeDemuxerWithTimelineOffset(const char* utc_date) {
+    EXPECT_CALL(host_, SetDuration(_));
+    WaitableMessageLoopEvent event;
+    demuxer_->Initialize(&host_, event.GetPipelineStatusCB(), false);
+    // Inject the timeline offset into the FFmpeg metadata dictionary.
+    av_dict_set(&demuxer_->glue_->format_context()->metadata,

[acolwell GONE FROM CHROMIUM, 2014/06/13 18:12:13]

This seems brittle. Why not just add an xxForTesting() method and set the value
directly? The demuxer could just avoid updating timeline_offset_ if it is
already set.

[DaleCurtis, 2014/06/13 20:40:02]

The timeline_offset_ value isn't set until OnFindStreamInfoDone is run, which is
PostTask'd during Initialize().  The code which adjusts the timeline_offset_ by
start_time_ is done there, so we need to set this value before that call
happens.  A SetTimelineOffsetForTesting() function wouldn't accomplish that.

At best I could move this code into FFmpegDemuxer, but I don't think that's any
less fragile.  Using av_dict_set() instead of overriding timeline_offset_ also
gives us better testing of the complete timeline_offset_ pipeline.

+                "creation_time",
+                utc_date,
+                0);
+    event.RunAndWaitForStatus(PIPELINE_OK);
+  }
+
   MOCK_METHOD2(OnReadDoneCalled, void(int, int64));
 
   // Verifies that |buffer| has a specific |size| and |timestamp|.
   // |location| simply indicates where the call to this function was made.
   // This makes it easier to track down where test failures occur.
   void OnReadDone(const tracked_objects::Location& location,
-                  int size, int64 timestampInMicroseconds,
+                  int size,
+                  int64 timestamp_us,
+                  base::TimeDelta discard_front_padding,
                   DemuxerStream::Status status,
                   const scoped_refptr<DecoderBuffer>& buffer) {
     std::string location_str;
@@ -117,21 +131,39 @@ class FFmpegDemuxerTest : public testing::Test {
     location_str += "\n";
     SCOPED_TRACE(location_str);
     EXPECT_EQ(status, DemuxerStream::kOk);
-    OnReadDoneCalled(size, timestampInMicroseconds);
+    OnReadDoneCalled(size, timestamp_us);
     EXPECT_TRUE(buffer.get() != NULL);
     EXPECT_EQ(size, buffer->data_size());
-    EXPECT_EQ(base::TimeDelta::FromMicroseconds(timestampInMicroseconds),
-              buffer->timestamp());
-
+    EXPECT_EQ(timestamp_us, buffer->timestamp().InMicroseconds());
+    EXPECT_EQ(discard_front_padding, buffer->discard_padding().first);
     DCHECK_EQ(&message_loop_, base::MessageLoop::current());
     message_loop_.PostTask(FROM_HERE, base::MessageLoop::QuitWhenIdleClosure());
   }
 
   DemuxerStream::ReadCB NewReadCB(const tracked_objects::Location& location,
-                                  int size, int64 timestampInMicroseconds) {
-    EXPECT_CALL(*this, OnReadDoneCalled(size, timestampInMicroseconds));
-    return base::Bind(&FFmpegDemuxerTest::OnReadDone, base::Unretained(this),
-                      location, size, timestampInMicroseconds);
+                                  int size,
+                                  int64 timestamp_us) {
+    EXPECT_CALL(*this, OnReadDoneCalled(size, timestamp_us));
+    return base::Bind(&FFmpegDemuxerTest::OnReadDone,
+                      base::Unretained(this),
+                      location,
+                      size,
+                      timestamp_us,
+                      base::TimeDelta());
+  }
+
+  DemuxerStream::ReadCB NewReadCBWithCheckedDiscard(
+      const tracked_objects::Location& location,
+      int size,
+      int64 timestamp_us,
+      base::TimeDelta discard_front_padding) {
+    EXPECT_CALL(*this, OnReadDoneCalled(size, timestamp_us));
+    return base::Bind(&FFmpegDemuxerTest::OnReadDone,
+                      base::Unretained(this),
+                      location,
+                      size,
+                      timestamp_us,
+                      discard_front_padding);
   }
 
   // TODO(xhwang): This is a workaround of the issue that move-only parameters
@@ -386,25 +418,103 @@ TEST_F(FFmpegDemuxerTest, Read_Text) {
   message_loop_.Run();
 }
 
-TEST_F(FFmpegDemuxerTest, Read_VideoNonZeroStart) {
+TEST_F(FFmpegDemuxerTest, Read_VideoPositiveStartTime) {
+  const char kTimelineOffset[] = "2012-11-10 12:34:56";
+  const int64 kTimelineOffsetMs = 1352550896000LL;
+
   // Test the start time is the first timestamp of the video and audio stream.
   CreateDemuxer("nonzero-start-time.webm");
-  InitializeDemuxer();
+  InitializeDemuxerWithTimelineOffset(kTimelineOffset);
 
   // Attempt a read from the video stream and run the message loop until done.
   DemuxerStream* video = demuxer_->GetStream(DemuxerStream::VIDEO);
   DemuxerStream* audio = demuxer_->GetStream(DemuxerStream::AUDIO);
 
-  // Check first buffer in video stream.
-  video->Read(NewReadCB(FROM_HERE, 5636, 400000));
-  message_loop_.Run();
+  const base::TimeDelta video_start_time =
+      base::TimeDelta::FromMicroseconds(400000);
+  const base::TimeDelta audio_start_time =
+      base::TimeDelta::FromMicroseconds(396000);
+
+  // Run the test twice with a seek in between.
+  for (int i = 0; i < 2; ++i) {
+    // Check first buffer in video stream.  It should have been adjusted such
+    // that
+    // it starts 400ms after the first audio buffer.
+    video->Read(
+        NewReadCB(FROM_HERE,
+                  5636,
+                  (video_start_time - audio_start_time).InMicroseconds()));
+    message_loop_.Run();
 
-  // Check first buffer in audio stream.
-  audio->Read(NewReadCB(FROM_HERE, 165, 396000));
-  message_loop_.Run();
+    // Since the audio buffer has a lower first timestamp, it should become
+    // zero.
+    audio->Read(NewReadCB(FROM_HERE, 165, 0));
+    message_loop_.Run();
+
+    // Verify that the start time is equal to the lowest timestamp (ie the
+    // audio).
+    EXPECT_EQ(audio_start_time, demuxer_->start_time());
+
+    // Verify that the timeline offset has been adjusted by the start time.
+    EXPECT_EQ(kTimelineOffsetMs + audio_start_time.InMilliseconds(),
+              demuxer_->GetTimelineOffset().ToJavaTime());
+
+    // Seek back to the beginning and repeat the test.
+    WaitableMessageLoopEvent event;
+    demuxer_->Seek(base::TimeDelta(), event.GetPipelineStatusCB());
+    event.RunAndWaitForStatus(PIPELINE_OK);
+  }
+}
 
-  // Verify that the start time is equal to the lowest timestamp (ie the audio).
-  EXPECT_EQ(demuxer_->GetStartTime().InMicroseconds(), 396000);
+TEST_F(FFmpegDemuxerTest, Read_AudioNoStartTime) {
+  // FFmpeg does not set timestamps when demuxing wave files.  Ensure that the
+  // demuxer sets a start time of zero in this case.
+  CreateDemuxer("sfx_s24le.wav");
+  InitializeDemuxer();
+
+  // Run the test twice with a seek in between.
+  for (int i = 0; i < 2; ++i) {
+    demuxer_->GetStream(DemuxerStream::AUDIO)
+        ->Read(NewReadCB(FROM_HERE, 4095, 0));
+    message_loop_.Run();
+    EXPECT_EQ(base::TimeDelta(), demuxer_->start_time());
+
+    // Seek back to the beginning and repeat the test.
+    WaitableMessageLoopEvent event;
+    demuxer_->Seek(base::TimeDelta(), event.GetPipelineStatusCB());
+    event.RunAndWaitForStatus(PIPELINE_OK);
+  }
+}
+
+TEST_F(FFmpegDemuxerTest, Read_AudioNegativeStartTimeAndOggDiscard) {
+  // Many ogg files have negative starting timestamps, so ensure demuxing and
+  // seeking work correctly with a negative start time.
+  CreateDemuxer("bear.ogv");
+  InitializeDemuxer();
+
+  // Run the test twice with a seek in between.
+  for (int i = 0; i < 2; ++i) {
+    demuxer_->GetStream(DemuxerStream::AUDIO)->Read(
+        NewReadCBWithCheckedDiscard(FROM_HERE, 40, 0, kInfiniteDuration()));
+    message_loop_.Run();
+    demuxer_->GetStream(DemuxerStream::AUDIO)->Read(
+        NewReadCBWithCheckedDiscard(FROM_HERE, 41, 2903, kInfiniteDuration()));
+    message_loop_.Run();
+    demuxer_->GetStream(DemuxerStream::AUDIO)->Read(NewReadCBWithCheckedDiscard(
+        FROM_HERE, 173, 5805, base::TimeDelta::FromMicroseconds(10159)));
+    message_loop_.Run();
+
+    demuxer_->GetStream(DemuxerStream::AUDIO)
+        ->Read(NewReadCB(FROM_HERE, 148, 18866));
+    message_loop_.Run();
+    EXPECT_EQ(base::TimeDelta::FromMicroseconds(-15964),
+              demuxer_->start_time());
+
+    // Seek back to the beginning and repeat the test.
+    WaitableMessageLoopEvent event;
+    demuxer_->Seek(base::TimeDelta(), event.GetPipelineStatusCB());
+    event.RunAndWaitForStatus(PIPELINE_OK);
+  }
 }
 
 TEST_F(FFmpegDemuxerTest, Read_EndOfStream) {