Interpolate media time for mojo rendering pipeline.

media/mojo/services/mojo_renderer_service.cc

 // Copyright 2014 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/mojo/services/mojo_renderer_service.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "media/base/audio_renderer_sink.h"
@@ skipping 11 matching lines @@
 
 MojoRendererService::MojoRendererService(
     base::WeakPtr<MojoCdmServiceContext> mojo_cdm_service_context,
     scoped_refptr<AudioRendererSink> audio_sink,
     std::unique_ptr<VideoRendererSink> video_sink,
     std::unique_ptr<media::Renderer> renderer,
     mojo::InterfaceRequest<mojom::Renderer> request)
     : 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)),
       weak_factory_(this) {
   DVLOG(1) << __FUNCTION__;
   DCHECK(renderer_);
 
   weak_this_ = weak_factory_.GetWeakPtr();
 }
 
@@ skipping 38 matching lines @@
 
 void MojoRendererService::StartPlayingFrom(base::TimeDelta time_delta) {
   DVLOG(2) << __FUNCTION__ << ": " << time_delta;
   renderer_->StartPlayingFrom(time_delta);
   SchedulePeriodicMediaTimeUpdates();
 }
 
 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);
 }
 
 void MojoRendererService::SetVolume(float volume) {
   renderer_->SetVolume(volume);
 }
 
 void MojoRendererService::SetCdm(int32_t cdm_id,
                                  const SetCdmCallback& callback) {
   if (!mojo_cdm_service_context_) {
@@ skipping 81 matching lines @@
     state_ = STATE_ERROR;
     callback.Run(false);
     return;
   }
 
   state_ = STATE_PLAYING;
   callback.Run(true);
 }
 
 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__;
+
+  time_update_timer_.Stop();
   UpdateMediaTime(false);
-  time_update_timer_.Stop();
 }
 
 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));
 }
 
 void MojoRendererService::OnFlushCompleted(const FlushCallback& callback) {
   DVLOG(1) << __FUNCTION__;
   DCHECK_EQ(state_, STATE_FLUSHING);
   state_ = STATE_PLAYING;
   callback.Run();
 }
 
 void MojoRendererService::OnCdmAttached(
     scoped_refptr<MediaKeys> cdm,
     const base::Callback<void(bool)>& callback,
     bool success) {
   DVLOG(1) << __FUNCTION__ << "(" << success << ")";
 
   if (success)
     cdm_ = cdm;
 
   callback.Run(success);
 }
 
 }  // namespace media