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

media/blink/webmediaplayer_impl.cc

 // Copyright 2013 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 "media/blink/webmediaplayer_impl.h"
 
 #include <algorithm>
 #include <limits>
 #include <string>
 #include <vector>
@@ skipping 284 matching lines @@
 
 void WebMediaPlayerImpl::pause() {
   DVLOG(1) << __FUNCTION__;
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   const bool was_already_paused = paused_ || playback_rate_ == 0;
   paused_ = true;
   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));
 
   if (!was_already_paused && delegate_)
     delegate_->DidPause(this);
 }
 
 bool WebMediaPlayerImpl::supportsSave() const {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   return supports_save_;
@@ skipping 14 matching lines @@
     pending_seek_ = true;
     pending_seek_seconds_ = seconds;
     if (chunk_demuxer_)
       chunk_demuxer_->CancelPendingSeek(seek_time);
     return;
   }
 
   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;
 
   if (chunk_demuxer_)
     chunk_demuxer_->StartWaitingForSeek(seek_time);
 
   // Kick off the asynchronous seek!
   pipeline_.Seek(
       seek_time,
       BIND_TO_RENDER_LOOP1(&WebMediaPlayerImpl::OnPipelineSeeked, true));
@@ skipping 674 matching lines @@
   compositor_task_runner_->PostTask(FROM_HERE,
                                     base::Bind(&GetCurrentFrameAndSignal,
                                                base::Unretained(compositor_),
                                                &video_frame,
                                                &event));
   event.Wait();
   return video_frame;
 }
 
 }  // namespace media