Interpolate media time for mojo rendering pipeline.

media/mojo/services/mojo_renderer_service.cc

Index: media/mojo/services/mojo_renderer_service.cc
diff --git a/media/mojo/services/mojo_renderer_service.cc b/media/mojo/services/mojo_renderer_service.cc
index 40c7006c1f10d6dfa8373197958cd537d86c6ec2..851c04d3f2f35ee8153564147d43e7f24a05a673 100644
--- a/media/mojo/services/mojo_renderer_service.cc
+++ b/media/mojo/services/mojo_renderer_service.cc
@@ -29,6 +29,7 @@ MojoRendererService::MojoRendererService(
     : binding_(this, std::move(request)),
       mojo_cdm_service_context_(mojo_cdm_service_context),
       state_(STATE_UNINITIALIZED),
+      playback_rate_(0.0),

[DaleCurtis, 2016/09/06 19:21:25]

Can just use zero.

[alokp, 2016/09/07 16:46:21]

Done.

       audio_sink_(std::move(audio_sink)),
       video_sink_(std::move(video_sink)),
       renderer_(std::move(renderer)),
@@ -87,6 +88,7 @@ void MojoRendererService::StartPlayingFrom(base::TimeDelta time_delta) {
 void MojoRendererService::SetPlaybackRate(double playback_rate) {
   DVLOG(2) << __FUNCTION__ << ": " << playback_rate;
   DCHECK(state_ == STATE_PLAYING || state_ == STATE_ERROR);
+  playback_rate_ = playback_rate;
   renderer_->SetPlaybackRate(playback_rate);
 }
 
@@ -188,26 +190,33 @@ void MojoRendererService::OnRendererInitializeDone(
 }
 
 void MojoRendererService::UpdateMediaTime(bool force) {
-  base::TimeDelta media_time = renderer_->GetMediaTime();
+  const base::TimeDelta media_time = renderer_->GetMediaTime();
   if (!force && media_time == last_media_time_)
     return;
 
-  client_->OnTimeUpdate(media_time, media_time);
+  base::TimeDelta max_time = media_time;
+  if (time_update_timer_.IsRunning() && (playback_rate_ > 0)) {

[DaleCurtis, 2016/09/06 19:21:25]

Drop unnecessary parens.

[alokp, 2016/09/07 16:46:21]

Done.

+    // Allow some slop to account for delays in scheduling time update tasks.
+    max_time += base::TimeDelta::FromMilliseconds(2 * kTimeUpdateIntervalMs);

[DaleCurtis, 2016/09/06 19:21:25]

Hmm, seems like something that may be implementation specific? Add some note
about how the value was chosen.

[alokp, 2016/09/07 16:46:21]

Are you referring to kTimeUpdateIntervalMs? This patch does not change that. It
is currently hardcoded to 50ms.

[DaleCurtis, 2016/09/08 16:58:23]

No I meant the choice of 2*interval as the allowed wiggle room.

[alokp, 2016/09/08 17:01:42]

I just used whatever was being used for the cma pipeline. Did you have any
concerns/suggestions?

[DaleCurtis, 2016/09/08 17:09:01]

100ms just seems like a lot of wiggle room for a timer that is running every
50ms so I was looking for an explanation of how that was chosen. That means if I
send a timeupdate of 0,50 the max time can be 0,150.

We don't allow currenttime to go backwards so if there's a rendering hitch this
means time may desync significantly IIUC. I don't recall if this is used for
captions or not.

[alokp, 2016/09/08 17:36:28]

This is not accurate. We always send a time-update of [current_time,
current_time + 100ms]. So if the updates are firing at expected rate, we will
send [0, 100], [50, 150], and so on.

The amount of allowance should be >= kTimeUpdateIntervalMs. If you think
2*kTimeUpdateIntervalMs is too much, we can experiment with something smaller.
Ideally we should check how much decoded data is buffered by the underlying
renderer, but am not sure if it worth the trouble.
I agree about the concern for currenttime going backward irrespective of the
amount of allowance. I added some code on the client to prevent that.

+  }
+
+  client_->OnTimeUpdate(media_time, max_time, base::TimeTicks::Now());
   last_media_time_ = media_time;
 }
 
 void MojoRendererService::CancelPeriodicMediaTimeUpdates() {
   DVLOG(2) << __FUNCTION__;
-  UpdateMediaTime(false);
+
   time_update_timer_.Stop();
+  UpdateMediaTime(false);
 }
 
 void MojoRendererService::SchedulePeriodicMediaTimeUpdates() {
   DVLOG(2) << __FUNCTION__;
+
   UpdateMediaTime(true);
   time_update_timer_.Start(
-      FROM_HERE,
-      base::TimeDelta::FromMilliseconds(kTimeUpdateIntervalMs),
+      FROM_HERE, base::TimeDelta::FromMilliseconds(kTimeUpdateIntervalMs),
       base::Bind(&MojoRendererService::UpdateMediaTime, weak_this_, false));
 }