Relanding 'Ignore seek operations to the current time in pause state' patch

media/blink/webmediaplayer_impl.cc

Index: media/blink/webmediaplayer_impl.cc
diff --git a/media/blink/webmediaplayer_impl.cc b/media/blink/webmediaplayer_impl.cc
index b1744bd1c3d4893f8ed38aa8933ab804430bea90..af5c2002b3434005f47a6d6d371d16d91ba55fa6 100644
--- a/media/blink/webmediaplayer_impl.cc
+++ b/media/blink/webmediaplayer_impl.cc
@@ -302,7 +302,13 @@ void WebMediaPlayerImpl::pause() {
   pipeline_.SetPlaybackRate(0.0f);
   if (data_source_)
     data_source_->MediaIsPaused();
-  paused_time_ = pipeline_.GetMediaTime();
+
+  // FIXME: In some cases GetMediaTime() returns a value less than duration

[philipj_slow, 2014/11/20 15:27:21]

Would it also fix the problem to always clamp currentTime to duration or
alternatively to report a new duration when currentTime exceeds it? It seems
like inconsistency could be observable before ended_ is true.

[DaleCurtis, 2014/11/20 19:02:27]

Instead, can you try modifying GetMediaTime() in media/base/pipeline.cc to
return duration_ if renderer_ended_ is true?

[philipj_slow, 2014/11/20 19:10:43]

Wouldn't that have the same problem, that currentTime can exceed duration until
the real end is reached? That'll be timing sensitive, so any bugs related to the
inconsistency may become harder to track down.

[DaleCurtis, 2014/11/20 19:14:39]

Pipeline already clamps currentTime to duration if it's over.

[philipj_slow, 2014/11/21 00:26:38]

D'oh! If I had read the comment carefully, I would have seen that this is about
when currentTime stops short of duration, not when it goes beyond.

Then the options are claiming that currentTime is duration, or reporting a new
duration. If there is no limit to how wrong the previously reported duration can
be, I think reporting a new duration seems like the sane thing.

[Srirama, 2014/11/21 04:05:44]

But here we are trying to correct/manipulate the current time (paused_time_) and
not the duration right. And it is the specific case of "Pipeline End", so
duration would have been finalized and reported even though we have reported
wrong duration before.
Did i misunderstood something?

+  // even though playback has ended.
+  if (ended_)
+    paused_time_ = pipeline_.GetMediaDuration();
+  else
+    paused_time_ = pipeline_.GetMediaTime();
 
   media_log_->AddEvent(media_log_->CreateEvent(MediaLogEvent::PAUSE));
 
@@ -337,8 +343,20 @@ void WebMediaPlayerImpl::seek(double seconds) {
   media_log_->AddEvent(media_log_->CreateSeekEvent(seconds));
 
   // Update our paused time.
-  if (paused_)
-    paused_time_ = seek_time;
+  // In paused state ignore the seek operations to current time and generate
+  // buffer state change event to eventually fire seeking and seeked events
+  if (paused_) {
+    if (paused_time_ != seek_time) {
+      paused_time_ = seek_time;
+    } else {
+      main_task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(&WebMediaPlayerImpl::OnPipelineBufferingStateChanged,
+                     AsWeakPtr(),
+                     BUFFERING_HAVE_ENOUGH));
+      return;
+    }
+  }
 
   seeking_ = true;