Issues with video looping

Some video streams are badly muxed and have video packets
with timestamps greater than the duration. This causes the
logic in the video renderer to sleep forever and never read
the end-of-stream packet and thus the stream is never ended.

This change adjusted the condition on when we should sleep
and handle timestamp greater than duration as a special case.

TEST=video loops
BUG=41579

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

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

10 files changed, 43 insertions, 20 deletions

diff --git a/media/base/filter_host.h b/media/base/filter_host.h
index 4ac939b..0d9af0f 100644
--- a/media/base/filter_host.h
+++ b/media/base/filter_host.h
@@ -32,6 +32,9 @@ class FilterHost {
   // Gets the current time in microseconds.
   virtual base::TimeDelta GetTime() const = 0;
 
+  // Gets the duration in microseconds.
+  virtual base::TimeDelta GetDuration() const = 0;
+
   // Updates the current time.  Other filters should poll to examine the updated
   // time.
   virtual void SetTime(base::TimeDelta time) = 0;
diff --git a/media/base/mock_filter_host.h b/media/base/mock_filter_host.h
index 0ec6912..a697226 100644
--- a/media/base/mock_filter_host.h
+++ b/media/base/mock_filter_host.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.  Use of this
+// Copyright (c) 2010 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.
 //
@@ -29,6 +29,7 @@ class MockFilterHost : public FilterHost {
   // FilterHost implementation.
   MOCK_METHOD0(InitializationComplete, void());
   MOCK_METHOD1(SetError, void(PipelineError error));
+  MOCK_CONST_METHOD0(GetDuration, base::TimeDelta());
   MOCK_CONST_METHOD0(GetTime, base::TimeDelta());
   MOCK_METHOD1(SetTime, void(base::TimeDelta time));
   MOCK_METHOD1(SetDuration, void(base::TimeDelta duration));
diff --git a/media/base/pipeline.h b/media/base/pipeline.h
index f474df7..82c551c 100644
--- a/media/base/pipeline.h
+++ b/media/base/pipeline.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -133,7 +133,7 @@ class Pipeline : public base::RefCountedThreadSafe<Pipeline> {
 
   // Get the duration of the media in microseconds.  If the duration has not
   // been determined yet, then returns 0.
-  virtual base::TimeDelta GetDuration() const = 0;
+  virtual base::TimeDelta GetMediaDuration() const = 0;
 
   // Get the total number of bytes that are buffered on the client and ready to
   // be played.
diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc
index 0331e5c..cbc1061b 100644
--- a/media/base/pipeline_impl.cc
+++ b/media/base/pipeline_impl.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2008-2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 //
@@ -240,7 +240,7 @@ base::TimeDelta PipelineImpl::GetBufferedTime() const {
       static_cast<int64>(duration_.InMilliseconds() * ratio));
 }
 
-base::TimeDelta PipelineImpl::GetDuration() const {
+base::TimeDelta PipelineImpl::GetMediaDuration() const {
   AutoLock auto_lock(lock_);
   return duration_;
 }
@@ -363,6 +363,11 @@ base::TimeDelta PipelineImpl::GetTime() const {
   return GetCurrentTime();
 }
 
+base::TimeDelta PipelineImpl::GetDuration() const {
+  DCHECK(IsRunning());
+  return GetMediaDuration();
+}
+
 void PipelineImpl::SetTime(base::TimeDelta time) {
   DCHECK(IsRunning());
   AutoLock auto_lock(lock_);
diff --git a/media/base/pipeline_impl.h b/media/base/pipeline_impl.h
index 92eceaa..a1ca141 100644
--- a/media/base/pipeline_impl.h
+++ b/media/base/pipeline_impl.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2008-2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -80,7 +80,7 @@ class PipelineImpl : public Pipeline, public FilterHost {
   virtual void SetVolume(float volume);
   virtual base::TimeDelta GetCurrentTime() const;
   virtual base::TimeDelta GetBufferedTime() const;
-  virtual base::TimeDelta GetDuration() const;
+  virtual base::TimeDelta GetMediaDuration() const;
   virtual int64 GetBufferedBytes() const;
   virtual int64 GetTotalBytes() const;
   virtual void GetVideoSize(size_t* width_out, size_t* height_out) const;
@@ -141,6 +141,7 @@ class PipelineImpl : public Pipeline, public FilterHost {
   // FilterHost implementation.
   virtual void SetError(PipelineError error);
   virtual base::TimeDelta GetTime() const;
+  virtual base::TimeDelta GetDuration() const;
   virtual void SetTime(base::TimeDelta time);
   virtual void SetDuration(base::TimeDelta duration);
   virtual void SetBufferedTime(base::TimeDelta buffered_time);
diff --git a/media/base/pipeline_impl_unittest.cc b/media/base/pipeline_impl_unittest.cc
index f442e0b..9edae50 100644
--- a/media/base/pipeline_impl_unittest.cc
+++ b/media/base/pipeline_impl_unittest.cc
@@ -270,7 +270,7 @@ TEST_F(PipelineImplTest, NotStarted) {
 
   EXPECT_TRUE(kZero == pipeline_->GetCurrentTime());
   EXPECT_TRUE(kZero == pipeline_->GetBufferedTime());
-  EXPECT_TRUE(kZero == pipeline_->GetDuration());
+  EXPECT_TRUE(kZero == pipeline_->GetMediaDuration());
 
   EXPECT_EQ(0, pipeline_->GetBufferedBytes());
   EXPECT_EQ(0, pipeline_->GetTotalBytes());
@@ -485,7 +485,7 @@ TEST_F(PipelineImplTest, Properties) {
   EXPECT_TRUE(pipeline_->IsInitialized());
   EXPECT_EQ(PIPELINE_OK, pipeline_->GetError());
   EXPECT_EQ(kDuration.ToInternalValue(),
-            pipeline_->GetDuration().ToInternalValue());
+            pipeline_->GetMediaDuration().ToInternalValue());
   EXPECT_EQ(kTotalBytes, pipeline_->GetTotalBytes());
   EXPECT_EQ(kBufferedBytes, pipeline_->GetBufferedBytes());
   EXPECT_EQ(kDuration.ToInternalValue(),
diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index 2a205b8..5cc06d5 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -452,12 +452,13 @@ void FFmpegDemuxer::InitializeTask(DataSource* data_source,
     callback->Run();
     return;
   }
-  if (max_duration.InMicroseconds() == 0 &&
-      format_context_->duration != static_cast<int64_t>(AV_NOPTS_VALUE)) {
-    // If we could not obtain duration from an A/V stream, and file duration
-    // has been set: use the file duration as |max_duration|.
+  if (format_context_->duration != static_cast<int64_t>(AV_NOPTS_VALUE)) {
+    // If there is a duration value in the container use that to find the
+    // maximum between it and the duration from A/V streams.
     const AVRational av_time_base = {1, AV_TIME_BASE};
-    max_duration = ConvertTimestamp(av_time_base, format_context_->duration);
+    max_duration =
+        std::max(max_duration,
+                 ConvertTimestamp(av_time_base, format_context_->duration));
   }
   // Good to go: set the duration and notify we're done initializing.
   host()->SetDuration(max_duration);
diff --git a/media/filters/video_renderer_base.cc b/media/filters/video_renderer_base.cc
index e58868d..46906c06e 100644
--- a/media/filters/video_renderer_base.cc
+++ b/media/filters/video_renderer_base.cc
@@ -199,10 +199,18 @@ void VideoRendererBase::ThreadMain() {
       state = state_;
       playback_rate = playback_rate_;
 
-      // Calculate how long until we should advance the frame, which is
-      // typically negative but for playback rates < 1.0f may be long enough
-      // that it makes more sense to idle and check again.
-      remaining_time = current_frame_->GetTimestamp() - host()->GetTime();
+      if (current_frame_->GetTimestamp() > host()->GetDuration()) {
+        // This is a special case when the stream is badly formatted that
+        // we get video frame with timestamp greater than the duration.
+        // In this case we should proceed anyway and try to obtain the
+        // end-of-stream packet.
+        remaining_time = base::TimeDelta();
+      } else {
+        // Calculate how long until we should advance the frame, which is
+        // typically negative but for playback rates < 1.0f may be long enough
+        // that it makes more sense to idle and check again.
+        remaining_time = current_frame_->GetTimestamp() - host()->GetTime();
+      }
     }
     if (state == kStopped) {
       return;
diff --git a/media/filters/video_renderer_base_unittest.cc b/media/filters/video_renderer_base_unittest.cc
index d5eaf99..2cdde22 100644
--- a/media/filters/video_renderer_base_unittest.cc
+++ b/media/filters/video_renderer_base_unittest.cc
@@ -146,6 +146,10 @@ TEST_F(VideoRendererBaseTest, Initialize_Successful) {
   // VideoRendererBase... it makes mocking much harder.
   EXPECT_CALL(host_, GetTime()).WillRepeatedly(Return(base::TimeDelta()));
 
+  // Expects the video renderer to get duration from the host.
+  EXPECT_CALL(host_, GetDuration())
+      .WillRepeatedly(Return(base::TimeDelta()));
+
   InSequence s;
 
   // We expect the video size to be set.
diff --git a/media/tools/player_wtl/movie.cc b/media/tools/player_wtl/movie.cc
index c65f35a..7070138 100644
--- a/media/tools/player_wtl/movie.cc
+++ b/media/tools/player_wtl/movie.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.  Use of this
+// Copyright (c) 2010 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.
 
@@ -103,7 +103,7 @@ float Movie::GetPlayRate() {
 float Movie::GetDuration() {
   float duration = 0.f;
   if (pipeline_)
-    duration = (pipeline_->GetDuration()).InMicroseconds() / 1000000.0f;
+    duration = (pipeline_->GetMediaDuration()).InMicroseconds() / 1000000.0f;
   return duration;
 }