Stop using VoEVideoSync in Call/VideoReceiveStream.

webrtc/video/rtp_streams_synchronizer.cc

Index: webrtc/video/rtp_streams_synchronizer.cc
diff --git a/webrtc/video/rtp_streams_synchronizer.cc b/webrtc/video/rtp_streams_synchronizer.cc
index 0d026b310a492d0463f6d9ac12ff0d4f46bec1d3..522647e39f4557eb6c76a182a29d33085f4e194b 100644
--- a/webrtc/video/rtp_streams_synchronizer.cc
+++ b/webrtc/video/rtp_streams_synchronizer.cc
@@ -14,13 +14,13 @@
 #include "webrtc/base/logging.h"
 #include "webrtc/base/timeutils.h"
 #include "webrtc/base/trace_event.h"
+#include "webrtc/call/syncable.h"
 #include "webrtc/modules/rtp_rtcp/include/rtp_receiver.h"
 #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h"
 #include "webrtc/modules/video_coding/video_coding_impl.h"
 #include "webrtc/system_wrappers/include/clock.h"
 #include "webrtc/video/stream_synchronization.h"
 #include "webrtc/video_frame.h"
-#include "webrtc/voice_engine/include/voe_video_sync.h"
 
 namespace webrtc {
 namespace {
@@ -57,8 +57,7 @@ RtpStreamsSynchronizer::RtpStreamsSynchronizer(
       video_receiver_(video_receiver),
       video_rtp_receiver_(rtp_stream_receiver->GetRtpReceiver()),
       video_rtp_rtcp_(rtp_stream_receiver->rtp_rtcp()),
-      voe_channel_id_(-1),
-      voe_sync_interface_(nullptr),
+      syncable_(nullptr),
       audio_rtp_receiver_(nullptr),
       audio_rtp_rtcp_(nullptr),
       sync_(),
@@ -66,31 +65,24 @@ RtpStreamsSynchronizer::RtpStreamsSynchronizer(
   process_thread_checker_.DetachFromThread();
 }
 
-void RtpStreamsSynchronizer::ConfigureSync(int voe_channel_id,
-                                           VoEVideoSync* voe_sync_interface) {
-  if (voe_channel_id != -1)
-    RTC_DCHECK(voe_sync_interface);
-
+void RtpStreamsSynchronizer::ConfigureSync(Syncable* syncable) {
   rtc::CritScope lock(&crit_);
-  if (voe_channel_id_ == voe_channel_id &&
-      voe_sync_interface_ == voe_sync_interface) {
+  if (syncable == syncable_) {
     // This prevents expensive no-ops.
     return;
   }
-  voe_channel_id_ = voe_channel_id;
-  voe_sync_interface_ = voe_sync_interface;
+  syncable_ = syncable;
 
   audio_rtp_rtcp_ = nullptr;
   audio_rtp_receiver_ = nullptr;
   sync_.reset(nullptr);
 
-  if (voe_channel_id_ != -1) {
-    voe_sync_interface_->GetRtpRtcp(voe_channel_id_, &audio_rtp_rtcp_,
-                                    &audio_rtp_receiver_);
+  if (syncable_) {
+    syncable_->GetRtpRtcp(&audio_rtp_rtcp_, &audio_rtp_receiver_);

[stefan-webrtc, 2017/01/13 16:21:04]

Maybe we take the opportunity to change the interface so that it doesn't have to
return a full RtpRtcp and an RtpReceiver. Instead we could return what is really
being used, which I think is the NTP/RTP timestamp pair from RTCP and the latest
played out RTP timestamp together with the last playout time. Would be cleaner,
IMO.

[the sun, 2017/01/19 11:45:29]

Yes, I agree it makes sense to not expose the RtpRtcp module for audio. See
latest PS.

However, the RtpStreamsSynchronizer still knows about the RtpRtcp module for the
video case. Would it make sense to use Syncable for video as well? One obstacle
could be that the methods on Syncable are provided for video by both
vcm::VideoReceiver and RtpStreamReceiver.

[stefan-webrtc, 2017/01/19 11:59:59]

Yes, I think it would make a lot of sense to use the same interface. All that
should be needed for something to be syncable is the ability to get the mapping
between rtp and ntp, the time and timestamp of the latest playout event and the
possibility to change the min buffer size to control the sync.

I think we should be able to make VideoReceiveStream syncable.

     RTC_DCHECK(audio_rtp_rtcp_);
     RTC_DCHECK(audio_rtp_receiver_);
     sync_.reset(new StreamSynchronization(video_rtp_rtcp_->SSRC(),
-                                          voe_channel_id_));
+                                          syncable_->id()));
   }
 }
 
@@ -108,19 +100,15 @@ void RtpStreamsSynchronizer::Process() {
   last_sync_time_ = rtc::TimeNanos();
 
   rtc::CritScope lock(&crit_);
-  if (voe_channel_id_ == -1) {
+  if (!syncable_) {
     return;
   }
-  RTC_DCHECK(voe_sync_interface_);
   RTC_DCHECK(sync_.get());
 
   int audio_jitter_buffer_delay_ms = 0;
   int playout_buffer_delay_ms = 0;
-  if (voe_sync_interface_->GetDelayEstimate(voe_channel_id_,
-                                            &audio_jitter_buffer_delay_ms,
-                                            &playout_buffer_delay_ms) != 0) {
-    return;
-  }
+  syncable_->GetDelayEstimate(&audio_jitter_buffer_delay_ms,
+                              &playout_buffer_delay_ms);
   const int current_audio_delay_ms = audio_jitter_buffer_delay_ms +
       playout_buffer_delay_ms;
 
@@ -161,10 +149,7 @@ void RtpStreamsSynchronizer::Process() {
     return;
   }
 
-  if (voe_sync_interface_->SetMinimumPlayoutDelay(
-      voe_channel_id_, target_audio_delay_ms) == -1) {
-    LOG(LS_ERROR) << "Error setting voice delay.";
-  }
+  syncable_->SetMinimumPlayoutDelay(target_audio_delay_ms);
   video_receiver_->SetMinimumPlayoutDelay(target_video_delay_ms);
 }
 
@@ -173,15 +158,12 @@ bool RtpStreamsSynchronizer::GetStreamSyncOffsetInMs(
     int64_t* stream_offset_ms,
     double* estimated_freq_khz) const {
   rtc::CritScope lock(&crit_);
-  if (voe_channel_id_ == -1)
-    return false;
-
-  uint32_t playout_timestamp = 0;
-  if (voe_sync_interface_->GetPlayoutTimestamp(voe_channel_id_,
-                                               playout_timestamp) != 0) {
+  if (!syncable_) {
     return false;
   }
 
+  uint32_t playout_timestamp = syncable_->GetPlayoutTimestamp();
+
   int64_t latest_audio_ntp;
   if (!audio_measurement_.rtp_to_ntp.Estimate(playout_timestamp,
                                               &latest_audio_ntp)) {