Cast: Using a min filter to compute time offset

media/cast/video_receiver/video_receiver.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/cast/video_receiver/video_receiver.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "media/cast/cast_defines.h"
 #include "media/cast/framer/framer.h"
 #include "media/cast/rtcp/receiver_rtcp_event_subscriber.h"
 #include "media/cast/rtcp/rtcp_sender.h"
 #include "media/cast/video_receiver/video_decoder.h"
 
 namespace {
 
 static const int64 kMinSchedulingDelayMs = 1;
-static const int64 kMinTimeBetweenOffsetUpdatesMs = 2000;
-static const int kTimeOffsetFilter = 8;
+static const int64 kMinTimeBetweenOffsetUpdatesMs = 1000;
+static const int kTimeOffsetMaxCounter = 10;
 static const int64_t kMinProcessIntervalMs = 5;
 
 }  // namespace
 
 namespace media {
 namespace cast {
 
 // Local implementation of RtpData (defined in rtp_rtcp_defines.h).
 // Used to pass payload data into the video receiver.
 class LocalRtpVideoData : public RtpData {
@@ skipping 67 matching lines @@
       frame_delay_(base::TimeDelta::FromMilliseconds(
           1000 / video_config.max_frame_rate)),
       incoming_payload_callback_(new LocalRtpVideoData(this)),
       incoming_payload_feedback_(new LocalRtpVideoFeedback(this)),
       rtp_receiver_(cast_environment_->Clock(),
                     NULL,
                     &video_config,
                     incoming_payload_callback_.get()),
       rtp_video_receiver_statistics_(
           new LocalRtpReceiverStatistics(&rtp_receiver_)),
+      time_offset_counter_(0),

[pwestin, 2014/03/04 00:10:54]

we should initialize time_offset_

[mikhal1, 2014/03/04 00:28:45]

will be initialized by the default const'
On 2014/03/04 00:10:54, pwestin wrote:

       decryptor_(),
       time_incoming_packet_updated_(false),
       incoming_rtp_timestamp_(0),
       weak_factory_(this) {
   int max_unacked_frames =
       video_config.rtp_max_delay_ms * video_config.max_frame_rate / 1000;
   DCHECK(max_unacked_frames) << "Invalid argument";
 
   decryptor_.Initialize(video_config.aes_key, video_config.aes_iv_mask);
   framer_.reset(new Framer(cast_environment->Clock(),
@@ skipping 229 matching lines @@
 }
 
 base::TimeTicks VideoReceiver::GetRenderTime(base::TimeTicks now,
                                              uint32 rtp_timestamp) {
   DCHECK(cast_environment_->CurrentlyOn(CastEnvironment::MAIN));
   // Senders time in ms when this frame was captured.
   // Note: the senders clock and our local clock might not be synced.
   base::TimeTicks rtp_timestamp_in_ticks;
 
   // Compute the time offset_in_ticks based on the incoming_rtp_timestamp_.
-  if (time_offset_.InMilliseconds() == 0) {
+  if (time_offset_counter_ == 0) {
     if (!rtcp_->RtpTimestampInSenderTime(kVideoFrequency,
                                          incoming_rtp_timestamp_,
                                          &rtp_timestamp_in_ticks)) {
       // We have not received any RTCP to sync the stream play it out as soon as
       // possible.
       return now;
     }
-    time_offset_ = time_incoming_packet_ - rtp_timestamp_in_ticks;
+    ++time_offset_counter_;

[pwestin, 2014/03/04 00:10:54]

This will result in a bad first value; since we have no valid  time_offset_; I
think we should return now

[mikhal1, 2014/03/04 00:28:45]

true. fixed.
On 2014/03/04 00:10:54, pwestin wrote:

   } else if (time_incoming_packet_updated_) {
     if (rtcp_->RtpTimestampInSenderTime(kVideoFrequency,
                                         incoming_rtp_timestamp_,
                                         &rtp_timestamp_in_ticks)) {
       // Time to update the time_offset.
       base::TimeDelta time_offset =
           time_incoming_packet_ - rtp_timestamp_in_ticks;
-      time_offset_ = ((kTimeOffsetFilter - 1) * time_offset_ + time_offset) /
-                     kTimeOffsetFilter;
+      if (time_offset_counter_  < kTimeOffsetMaxCounter &&
+          time_offset < time_offset_)

[pwestin, 2014/03/04 00:10:54]

we should do 
if (time_offset_counter_ == 1)
  time_offset_ = time_offset; 
else if (time_offset_counter_  < kTimeOffsetMaxCounter)
  time_offset_ = std::min(time_offset_, time_offset);

[mikhal1, 2014/03/04 00:28:45]

updated.
On 2014/03/04 00:10:54, pwestin wrote:

+        time_offset_ = time_offset;

[hguihot1, 2014/03/04 00:06:40]

This will select the minimum value within the first 10 samples (9 actually since
the first one is ignored). Is that the intent? There is still no guarantee we'll
get a 'good' time_offset_ during that time, so wouldn't it make more sense the
select the minimum within a moving window?

[mikhal1, 2014/03/04 00:28:45]

That indeed is the intention, the underlying assumption is that the drift is
insignificant (for the duration of a reasonable session), and therefore can be
ignored. Added a comment.
On 2014/03/04 00:06:40, hguihot1 wrote:

+      ++time_offset_counter_;
     }
   }
   // Reset |time_incoming_packet_updated_| to enable a future measurement.
   time_incoming_packet_updated_ = false;
   // Compute the actual rtp_timestamp_in_ticks based on the current timestamp.
   if (!rtcp_->RtpTimestampInSenderTime(
            kVideoFrequency, rtp_timestamp, &rtp_timestamp_in_ticks)) {
     // This can fail if we have not received any RTCP packets in a long time.
     return now;
   }
@@ skipping 128 matching lines @@
 }
 
 void VideoReceiver::SendNextRtcpReport() {
   DCHECK(cast_environment_->CurrentlyOn(CastEnvironment::MAIN));
   rtcp_->SendRtcpFromRtpReceiver(NULL, NULL);
   ScheduleNextRtcpReport();
 }
 
 }  // namespace cast
 }  // namespace media