Initialize media timeline correctly for positive start times.

The previous fix for positive start times http://crrev.com/93c37f1b is
incomplete. It fixed seeking, but did not fix rendering. As such, the
media timeline will start playback from 0 and advance in real time to
any positive start time... even if the actual start is hours away.

When testing I used a test clip with a short positive start time so
did not notice the issue.  In attempting to write a layout test, I
retested with a larger start time which revealed the deficiency in my
first solution.

This change reverts the removal of Demuxer::StartTime() with the added
restriction that it must always be positive.  Pipeline uses this time
like it did before, during initialization and as a minimum bound for
seeking.

BUG=413292
TEST=four new tests! layout test forthcoming.

Review URL: https://codereview.chromium.org/575643002

Cr-Commit-Position: refs/heads/master@{#295159}

media/base/demuxer.h

@@ -73,6 +73,9 @@ class MEDIA_EXPORT Demuxer : public DemuxerStreamProvider {
   // method (including Stop()) after a demuxer has stopped.
   virtual void Stop() = 0;
 
+  // Returns the starting time for the media file; it's always positive.
+  virtual base::TimeDelta GetStartTime() const = 0;
+
   // Returns Time represented by presentation timestamp 0.
   // If the timstamps are not associated with a Time, then
   // a null Time is returned.

media/base/mock_filters.h

@@ -39,6 +39,7 @@ class MockDemuxer : public Demuxer {
   MOCK_METHOD0(Stop, void());
   MOCK_METHOD0(OnAudioRendererDisabled, void());
   MOCK_METHOD1(GetStream, DemuxerStream*(DemuxerStream::Type));
+  MOCK_CONST_METHOD0(GetStartTime, base::TimeDelta());
   MOCK_CONST_METHOD0(GetTimelineOffset, base::Time());
   MOCK_CONST_METHOD0(GetLiveness, Liveness());
 

media/base/pipeline.cc

@@ -343,10 +343,12 @@ void Pipeline::StateTransitionTask(PipelineStatus status) {
       if (!is_initialized_) {
         is_initialized_ = true;
         ReportMetadata();
+        start_timestamp_ = demuxer_->GetStartTime();
       }
 
       base::ResetAndReturn(&seek_cb_).Run(PIPELINE_OK);
 
+      DCHECK(start_timestamp_ >= base::TimeDelta());
       renderer_->StartPlayingFrom(start_timestamp_);
 
       if (text_renderer_)
@@ -584,13 +586,16 @@ void Pipeline::SeekTask(TimeDelta time, const PipelineStatusCB& seek_cb) {
 
   DCHECK(seek_cb_.is_null());
 
+  const base::TimeDelta seek_timestamp =
+      std::max(time, demuxer_->GetStartTime());
+
   SetState(kSeeking);
   seek_cb_ = seek_cb;
   renderer_ended_ = false;
   text_renderer_ended_ = false;
-  start_timestamp_ = time;
+  start_timestamp_ = seek_timestamp;
 
-  DoSeek(time,
+  DoSeek(seek_timestamp,
          base::Bind(&Pipeline::OnStateTransition, weak_factory_.GetWeakPtr()));
 }
 

media/base/pipeline_unittest.cc

@@ -103,6 +103,8 @@ class PipelineTest : public ::testing::Test {
 
     EXPECT_CALL(*renderer_, GetMediaTime())
         .WillRepeatedly(Return(base::TimeDelta()));
+
+    EXPECT_CALL(*demuxer_, GetStartTime()).WillRepeatedly(Return(start_time_));
   }
 
   virtual ~PipelineTest() {
@@ -203,7 +205,7 @@ class PipelineTest : public ::testing::Test {
       EXPECT_CALL(callbacks_, OnMetadata(_)).WillOnce(SaveArg<0>(&metadata_));
       EXPECT_CALL(*renderer_, SetPlaybackRate(0.0f));
       EXPECT_CALL(*renderer_, SetVolume(1.0f));
-      EXPECT_CALL(*renderer_, StartPlayingFrom(base::TimeDelta()))
+      EXPECT_CALL(*renderer_, StartPlayingFrom(start_time_))
           .WillOnce(SetBufferingState(&buffering_state_cb_,
                                       BUFFERING_HAVE_ENOUGH));
       EXPECT_CALL(callbacks_, OnBufferingStateChange(BUFFERING_HAVE_ENOUGH));
@@ -313,6 +315,7 @@ class PipelineTest : public ::testing::Test {
   base::Closure ended_cb_;
   VideoDecoderConfig video_decoder_config_;
   PipelineMetadata metadata_;
+  base::TimeDelta start_time_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(PipelineTest);
@@ -742,6 +745,22 @@ TEST_F(PipelineTest, Underflow) {
   DoSeek(expected);
 }
 
+TEST_F(PipelineTest, PositiveStartTime) {
+  start_time_ = base::TimeDelta::FromSeconds(1);
+  EXPECT_CALL(*demuxer_, GetStartTime()).WillRepeatedly(Return(start_time_));
+  CreateAudioStream();
+  MockDemuxerStreamVector streams;
+  streams.push_back(audio_stream());
+  SetDemuxerExpectations(&streams);
+  SetRendererExpectations();
+  StartPipelineAndExpect(PIPELINE_OK);
+  ExpectDemuxerStop();
+  ExpectPipelineStopAndDestroyPipeline();
+  pipeline_->Stop(
+      base::Bind(&CallbackHelper::OnStop, base::Unretained(&callbacks_)));
+  message_loop_.RunUntilIdle();
+}
+
 class PipelineTeardownTest : public PipelineTest {
  public:
   enum TeardownState {

media/filters/chunk_demuxer.cc

@@ -1141,6 +1141,10 @@ DemuxerStream* ChunkDemuxer::GetStream(DemuxerStream::Type type) {
   return NULL;
 }
 
+TimeDelta ChunkDemuxer::GetStartTime() const {
+  return TimeDelta();
+}
+
 Demuxer::Liveness ChunkDemuxer::GetLiveness() const {
   return liveness_;
 }
@@ -1642,7 +1646,7 @@ void ChunkDemuxer::OnSourceInitDone(
     return;
   }
 
-  SeekAllSources(base::TimeDelta());
+  SeekAllSources(GetStartTime());
   StartReturningData();
 
   if (duration_ == kNoTimestamp())

media/filters/chunk_demuxer.h

@@ -164,6 +164,7 @@ class MEDIA_EXPORT ChunkDemuxer : public Demuxer {
   virtual void Seek(base::TimeDelta time, const PipelineStatusCB&  cb) OVERRIDE;
   virtual base::Time GetTimelineOffset() const OVERRIDE;
   virtual DemuxerStream* GetStream(DemuxerStream::Type type) OVERRIDE;
+  virtual base::TimeDelta GetStartTime() const OVERRIDE;
   virtual Liveness GetLiveness() const OVERRIDE;
 
   // Methods used by an external object to control this demuxer.

media/filters/ffmpeg_demuxer.cc

@@ -686,6 +686,10 @@ FFmpegDemuxerStream* FFmpegDemuxer::GetFFmpegStream(
   return NULL;
 }
 
+base::TimeDelta FFmpegDemuxer::GetStartTime() const {
+  return std::max(start_time_, base::TimeDelta());
+}
+
 Demuxer::Liveness FFmpegDemuxer::GetLiveness() const {
   DCHECK(task_runner_->BelongsToCurrentThread());
   return liveness_;
@@ -959,7 +963,7 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
 
   // Since we're shifting the externally visible start time to zero, we need to
   // adjust the timeline offset to compensate.
-  if (!timeline_offset_.is_null())
+  if (!timeline_offset_.is_null() && start_time_ < base::TimeDelta())
     timeline_offset_ += start_time_;
 
   if (max_duration == kInfiniteDuration() && !timeline_offset_.is_null()) {

media/filters/ffmpeg_demuxer.h

@@ -168,6 +168,7 @@ class MEDIA_EXPORT FFmpegDemuxer : public Demuxer {
   virtual void Seek(base::TimeDelta time, const PipelineStatusCB& cb) OVERRIDE;
   virtual base::Time GetTimelineOffset() const OVERRIDE;
   virtual DemuxerStream* GetStream(DemuxerStream::Type type) OVERRIDE;
+  virtual base::TimeDelta GetStartTime() const OVERRIDE;
   virtual Liveness GetLiveness() const OVERRIDE;
 
   // Calls |need_key_cb_| with the initialization data encountered in the file.
@@ -179,8 +180,9 @@ class MEDIA_EXPORT FFmpegDemuxer : public Demuxer {
   void NotifyCapacityAvailable();
   void NotifyBufferingChanged();
 
-  // The lowest demuxed timestamp.  DemuxerStream's must use this to adjust
-  // packet timestamps such that external clients see a zero-based timeline.
+  // The lowest demuxed timestamp.  If negative, DemuxerStreams must use this to
+  // adjust packet timestamps such that external clients see a zero-based
+  // timeline.
   base::TimeDelta start_time() const { return start_time_; }
 
  private:

media/filters/ffmpeg_demuxer_unittest.cc

@@ -447,9 +447,8 @@ TEST_F(FFmpegDemuxerTest, Read_VideoPositiveStartTime) {
     // 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());
+    // Verify that the timeline offset has not been adjusted by the start time.
+    EXPECT_EQ(kTimelineOffsetMs, demuxer_->GetTimelineOffset().ToJavaTime());
 
     // Seek back to the beginning and repeat the test.
     WaitableMessageLoopEvent event;
@@ -549,6 +548,10 @@ TEST_F(FFmpegDemuxerTest, Read_AudioNegativeStartTimeAndOggDiscard_Sync) {
     EXPECT_EQ(base::TimeDelta::FromMicroseconds(-2902),
               demuxer_->start_time());
 
+    // Though the internal start time may be below zero, the exposed media time
+    // must always be greater than zero.
+    EXPECT_EQ(base::TimeDelta(), demuxer_->GetStartTime());
+
     video->Read(NewReadCB(FROM_HERE, 9997, 0));
     message_loop_.Run();
 

media/filters/pipeline_integration_test.cc

@@ -1563,6 +1563,7 @@ TEST_F(PipelineIntegrationTest, BasicPlaybackChainedOgg) {
   ASSERT_TRUE(Start(GetTestDataFilePath("double-sfx.ogg"), PIPELINE_OK));
   Play();
   ASSERT_TRUE(WaitUntilOnEnded());
+  ASSERT_EQ(base::TimeDelta(), demuxer_->GetStartTime());
 }
 
 // Ensures audio-video playback with missing or negative timestamps fails softly
@@ -1571,6 +1572,7 @@ TEST_F(PipelineIntegrationTest, BasicPlaybackChainedOggVideo) {
   ASSERT_TRUE(Start(GetTestDataFilePath("double-bear.ogv"), PIPELINE_OK));
   Play();
   EXPECT_EQ(PIPELINE_ERROR_DECODE, WaitUntilEndedOrError());
+  ASSERT_EQ(base::TimeDelta(), demuxer_->GetStartTime());
 }
 
 // Tests that we signal ended even when audio runs longer than video track.
@@ -1595,4 +1597,13 @@ TEST_F(PipelineIntegrationTest, BasicPlaybackAudioShorterThanVideo) {
   ASSERT_TRUE(WaitUntilOnEnded());
 }
 
+TEST_F(PipelineIntegrationTest, BasicPlaybackPositiveStartTime) {
+  ASSERT_TRUE(
+      Start(GetTestDataFilePath("nonzero-start-time.webm"), PIPELINE_OK));
+  Play();
+  ASSERT_TRUE(WaitUntilOnEnded());
+  ASSERT_EQ(base::TimeDelta::FromMicroseconds(396000),
+            demuxer_->GetStartTime());
+}
+
 }  // namespace media