Propagating RTT from send-only channel to receive-only channel.

This is important for obtaining ntp time at receiver-only channel, which does not have RTT directly.

BUG=3978

TEST=chromium with hangout calls
R=henrika@webrtc.org, mflodman@webrtc.org, stefan@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/29989004

Cr-Commit-Position: refs/heads/master@{#9186}

talk/media/webrtc/fakewebrtcvoiceengine.h

@@ -215,6 +215,7 @@ class FakeWebRtcVoiceEngine
           receive_audio_level_ext_(-1),
           send_absolute_sender_time_ext_(-1),
           receive_absolute_sender_time_ext_(-1),
+          associate_send_channel(-1),
           neteq_capacity(-1) {
       memset(&send_codec, 0, sizeof(send_codec));
       memset(&rx_agc_config, 0, sizeof(rx_agc_config));
@@ -246,6 +247,7 @@ class FakeWebRtcVoiceEngine
     int receive_audio_level_ext_;
     int send_absolute_sender_time_ext_;
     int receive_absolute_sender_time_ext_;
+    int associate_send_channel;
     DtmfInfo dtmf_info;
     std::vector<webrtc::CodecInst> recv_codecs;
     webrtc::CodecInst send_codec;
@@ -431,6 +433,10 @@ class FakeWebRtcVoiceEngine
 
   int GetNumSetSendCodecs() const { return num_set_send_codecs_; }
 
+  int GetAssociateSendChannel(int channel) {
+    return channels_[channel]->associate_send_channel;
+  }
+
   WEBRTC_STUB(Release, ());
 
   // webrtc::VoEBase
@@ -461,6 +467,11 @@ class FakeWebRtcVoiceEngine
   }
   WEBRTC_FUNC(DeleteChannel, (int channel)) {
     WEBRTC_CHECK_CHANNEL(channel);
+    for (const auto& ch : channels_) {
+      if (ch.second->associate_send_channel == channel) {
+        ch.second->associate_send_channel = -1;
+      }
+    }
     delete channels_[channel];
     channels_.erase(channel);
     return 0;
@@ -503,6 +514,12 @@ class FakeWebRtcVoiceEngine
   WEBRTC_STUB(LastError, ());
   WEBRTC_STUB(SetOnHoldStatus, (int, bool, webrtc::OnHoldModes));
   WEBRTC_STUB(GetOnHoldStatus, (int, bool&, webrtc::OnHoldModes&));
+  WEBRTC_FUNC(AssociateSendChannel, (int channel,
+                                     int accociate_send_channel)) {
+    WEBRTC_CHECK_CHANNEL(channel);
+    channels_[channel]->associate_send_channel = accociate_send_channel;
+    return 0;
+  }
 
   // webrtc::VoECodec
   WEBRTC_FUNC(NumOfCodecs, ()) {

talk/media/webrtc/webrtcvoiceengine.cc

@@ -2809,6 +2809,13 @@ bool WebRtcVoiceMediaChannel::ConfigureRecvChannel(int channel) {
     return false;
   }
 
+  // Associate receive channel to default channel (so the receive channel can
+  // obtain RTT from the send channel)
+  engine()->voe()->base()->AssociateSendChannel(channel, voe_channel());
+  LOG(LS_INFO) << "VoiceEngine channel #"
+               << channel << " is associated with channel #"
+               << voe_channel() << ".";
+
   // Use the same recv payload types as our default channel.
   ResetRecvCodecs(channel);
   if (!recv_codecs_.empty()) {

talk/media/webrtc/webrtcvoiceengine_unittest.cc

@@ -3605,3 +3605,72 @@ TEST_F(WebRtcVoiceEngineTestFake, DeliverAudioPacket_Call) {
 
   media_channel->SetCall(nullptr);
 }
+
+// Associate channel should not set on 1:1 call, since the receive channel also
+// sends RTCP SR.
+TEST_F(WebRtcVoiceEngineTestFake, AssociateChannelUnset1On1) {
+  EXPECT_TRUE(SetupEngine());
+  EXPECT_TRUE(channel_->AddRecvStream(cricket::StreamParams::CreateLegacy(1)));
+  int recv_ch = voe_.GetLastChannel();
+  EXPECT_EQ(voe_.GetAssociateSendChannel(recv_ch), -1);
+}
+
+// This test is an extension of AssociateChannelUnset1On1. We create two receive
+// channels. The second should be associated with the default channel, since it
+// does not send RTCP SR.
+TEST_F(WebRtcVoiceEngineTestFake, AssociateDefaultChannelOnSecondRecvChannel) {
+  EXPECT_TRUE(SetupEngine());
+  cricket::WebRtcVoiceMediaChannel* media_channel =
+      static_cast<cricket::WebRtcVoiceMediaChannel*>(channel_);
+  int default_channel = media_channel->voe_channel();
+  EXPECT_TRUE(channel_->AddRecvStream(cricket::StreamParams::CreateLegacy(1)));
+  int recv_ch_1 = voe_.GetLastChannel();
+  EXPECT_EQ(recv_ch_1, default_channel);
+  EXPECT_TRUE(channel_->AddRecvStream(cricket::StreamParams::CreateLegacy(2)));
+  int recv_ch_2 = voe_.GetLastChannel();
+  EXPECT_EQ(voe_.GetAssociateSendChannel(recv_ch_1), -1);
+  EXPECT_EQ(voe_.GetAssociateSendChannel(recv_ch_2), default_channel);
+  // Add send stream, the association remains.
+  EXPECT_TRUE(channel_->AddSendStream(cricket::StreamParams::CreateLegacy(3)));
+  EXPECT_EQ(voe_.GetAssociateSendChannel(recv_ch_1), -1);
+  EXPECT_EQ(voe_.GetAssociateSendChannel(recv_ch_2), default_channel);
+}
+
+// In conference mode, all receive channels should be associated with the
+// default channel, since they do not send RTCP SR.
+TEST_F(WebRtcVoiceEngineTestFake, AssociateDefaultChannelOnConference) {
+  EXPECT_TRUE(SetupEngine());
+  EXPECT_TRUE(channel_->SetOptions(options_conference_));
+  cricket::WebRtcVoiceMediaChannel* media_channel =
+      static_cast<cricket::WebRtcVoiceMediaChannel*>(channel_);
+  int default_channel = media_channel->voe_channel();
+  EXPECT_TRUE(channel_->AddRecvStream(cricket::StreamParams::CreateLegacy(1)));
+  int recv_ch = voe_.GetLastChannel();
+  EXPECT_NE(recv_ch, default_channel);
+  EXPECT_EQ(voe_.GetAssociateSendChannel(recv_ch), default_channel);
+  EXPECT_TRUE(channel_->AddSendStream(cricket::StreamParams::CreateLegacy(2)));
+  EXPECT_EQ(voe_.GetAssociateSendChannel(recv_ch), default_channel);
+}
+
+TEST_F(WebRtcVoiceEngineTestFake, AssociateChannelResetUponDeleteChannnel) {
+  EXPECT_TRUE(SetupEngine());
+  EXPECT_TRUE(channel_->SetOptions(options_conference_));
+
+  EXPECT_TRUE(channel_->AddRecvStream(cricket::StreamParams::CreateLegacy(1)));
+  int recv_ch = voe_.GetLastChannel();
+
+  EXPECT_TRUE(channel_->AddSendStream(cricket::StreamParams::CreateLegacy(2)));
+  int send_ch = voe_.GetLastChannel();
+
+  // Manually associate |recv_ch| to |send_ch|. This test is to verify a
+  // deleting logic, i.e., deleting |send_ch| will reset the associate send
+  // channel of |recv_ch|.This is not a common case, since， normally, only the
+  // default channel can be associated. However, the default is not deletable.
+  // So we force the |recv_ch| to associate with a non-default channel.
+  EXPECT_EQ(0, voe_.AssociateSendChannel(recv_ch, send_ch));
+  EXPECT_EQ(voe_.GetAssociateSendChannel(recv_ch), send_ch);
+
+  EXPECT_TRUE(channel_->RemoveSendStream(2));
+  EXPECT_EQ(voe_.GetAssociateSendChannel(recv_ch), -1);
+}
+

webrtc/voice_engine/channel.cc

@@ -822,7 +822,9 @@ Channel::Channel(int32_t channelId,
     _rxNsIsEnabled(false),
     restored_packet_in_use_(false),
     rtcp_observer_(new VoERtcpObserver(this)),
-    network_predictor_(new NetworkPredictor(Clock::GetRealTimeClock()))
+    network_predictor_(new NetworkPredictor(Clock::GetRealTimeClock())),
+    assoc_send_channel_lock_(CriticalSectionWrapper::CreateCriticalSection()),
+    associate_send_channel_(ChannelOwner(nullptr))
 {
     WEBRTC_TRACE(kTraceMemory, kTraceVoice, VoEId(_instanceId,_channelId),
                  "Channel::Channel() - ctor");
@@ -1757,21 +1759,22 @@ int32_t Channel::ReceivedRTCPPacket(const int8_t* data, size_t length) {
         "Channel::IncomingRTPPacket() RTCP packet is invalid");
   }
 
+  int64_t rtt = GetRTT(true);
+  if (rtt == 0) {
+    // Waiting for valid RTT.
+    return 0;
+  }
+  uint32_t ntp_secs = 0;
+  uint32_t ntp_frac = 0;
+  uint32_t rtp_timestamp = 0;
+  if (0 != _rtpRtcpModule->RemoteNTP(&ntp_secs, &ntp_frac, NULL, NULL,
+                                     &rtp_timestamp)) {
+    // Waiting for RTCP.
+    return 0;
+  }
+
   {
     CriticalSectionScoped lock(ts_stats_lock_.get());
-    int64_t rtt = GetRTT();
-    if (rtt == 0) {
-      // Waiting for valid RTT.
-      return 0;
-    }
-    uint32_t ntp_secs = 0;
-    uint32_t ntp_frac = 0;
-    uint32_t rtp_timestamp = 0;
-    if (0 != _rtpRtcpModule->RemoteNTP(&ntp_secs, &ntp_frac, NULL, NULL,
-                                       &rtp_timestamp)) {
-      // Waiting for RTCP.
-      return 0;
-    }
     ntp_estimator_.UpdateRtcpTimestamp(rtt, ntp_secs, ntp_frac, rtp_timestamp);
   }
   return 0;
@@ -3226,7 +3229,7 @@ Channel::GetRTPStatistics(CallStatistics& stats)
                  stats.jitterSamples);
 
     // --- RTT
-    stats.rttMs = GetRTT();
+    stats.rttMs = GetRTT(true);
     if (stats.rttMs == 0) {
       WEBRTC_TRACE(kTraceWarning, kTraceVoice, VoEId(_instanceId, _channelId),
                    "GetRTPStatistics() failed to get RTT");
@@ -3564,6 +3567,17 @@ Channel::EncodeAndSend()
     return 0;
 }
 
+void Channel::DisassociateSendChannel(int channel_id) {
+  CriticalSectionScoped lock(assoc_send_channel_lock_.get());
+  Channel* channel = associate_send_channel_.channel();
+  if (channel && channel->ChannelId() == channel_id) {
+    // If this channel is associated with a send channel of the specified
+    // Channel ID, disassociate with it.
+    ChannelOwner ref(NULL);
+    associate_send_channel_ = ref;
+  }
+}
+
 int Channel::RegisterExternalMediaProcessing(
     ProcessingTypes type,
     VoEMediaProcess& processObject)
@@ -4197,15 +4211,29 @@ int32_t Channel::GetPlayoutFrequency() {
   return playout_frequency;
 }
 
-int64_t Channel::GetRTT() const {
+int64_t Channel::GetRTT(bool allow_associate_channel) const {
   RTCPMethod method = _rtpRtcpModule->RTCP();
   if (method == kRtcpOff) {
     return 0;
   }
   std::vector<RTCPReportBlock> report_blocks;
   _rtpRtcpModule->RemoteRTCPStat(&report_blocks);
+
+  int64_t rtt = 0;
   if (report_blocks.empty()) {
-    return 0;
+    if (allow_associate_channel) {
+      CriticalSectionScoped lock(assoc_send_channel_lock_.get());
+      Channel* channel = associate_send_channel_.channel();
+      // Tries to get RTT from an associated channel. This is important for
+      // receive-only channels.
+      if (channel) {
+        // To prevent infinite recursion and deadlock, calling GetRTT of
+        // associate channel should always use "false" for argument:
+        // |allow_associate_channel|.
+        rtt = channel->GetRTT(false);
+      }
+    }
+    return rtt;
   }
 
   uint32_t remoteSSRC = rtp_receiver_->SSRC();
@@ -4221,7 +4249,7 @@ int64_t Channel::GetRTT() const {
     // the SSRC of the other end.
     remoteSSRC = report_blocks[0].remoteSSRC;
   }
-  int64_t rtt = 0;
+
   int64_t avg_rtt = 0;
   int64_t max_rtt= 0;
   int64_t min_rtt = 0;

webrtc/voice_engine/channel.h

@@ -442,6 +442,17 @@ public:
     uint32_t PrepareEncodeAndSend(int mixingFrequency);
     uint32_t EncodeAndSend();
 
+    // Associate to a send channel.
+    // Used for obtaining RTT for a receive-only channel.
+    void set_associate_send_channel(const ChannelOwner& channel) {
+      assert(_channelId != channel.channel()->ChannelId());
+      CriticalSectionScoped lock(assoc_send_channel_lock_.get());
+      associate_send_channel_ = channel;
+    }
+
+    // Disassociate a send channel if it was associated.
+    void DisassociateSendChannel(int channel_id);
+
 protected:
     void OnIncomingFractionLoss(int fraction_lost);
 
@@ -467,7 +478,7 @@ private:
                                   unsigned char id);
 
     int32_t GetPlayoutFrequency();
-    int64_t GetRTT() const;
+    int64_t GetRTT(bool allow_associate_channel) const;
 
     CriticalSectionWrapper& _fileCritSect;
     CriticalSectionWrapper& _callbackCritSect;
@@ -572,6 +583,9 @@ private:
     // RtcpBandwidthObserver
     rtc::scoped_ptr<VoERtcpObserver> rtcp_observer_;
     rtc::scoped_ptr<NetworkPredictor> network_predictor_;
+    // An associated send channel.
+    rtc::scoped_ptr<CriticalSectionWrapper> assoc_send_channel_lock_;
+    ChannelOwner associate_send_channel_ GUARDED_BY(assoc_send_channel_lock_);
 };
 
 }  // namespace voe

webrtc/voice_engine/channel_manager.cc

@@ -8,9 +8,9 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
-#include "webrtc/common.h"
 #include "webrtc/voice_engine/channel_manager.h"
 
+#include "webrtc/common.h"
 #include "webrtc/voice_engine/channel.h"
 
 namespace webrtc {
@@ -94,16 +94,21 @@ void ChannelManager::DestroyChannel(int32_t channel_id) {
   ChannelOwner reference(NULL);
   {
     CriticalSectionScoped crit(lock_.get());
+    std::vector<ChannelOwner>::iterator to_delete = channels_.end();
+    for (auto it = channels_.begin(); it != channels_.end(); ++it) {
+      Channel* channel = it->channel();
+      // For channels associated with the channel to be deleted, disassociate
+      // with that channel.
+      channel->DisassociateSendChannel(channel_id);
 
-    for (std::vector<ChannelOwner>::iterator it = channels_.begin();
-         it != channels_.end();
-         ++it) {
-      if (it->channel()->ChannelId() == channel_id) {
-        reference = *it;
-        channels_.erase(it);
-        break;
+      if (channel->ChannelId() == channel_id) {
+        to_delete = it;
       }
     }
+    if (to_delete != channels_.end()) {
+      reference = *to_delete;
+      channels_.erase(to_delete);
+    }
   }
 }
 

webrtc/voice_engine/channel_manager.h

@@ -52,8 +52,9 @@ class ChannelOwner {
 
   ChannelOwner& operator=(const ChannelOwner& other);
 
-  Channel* channel() { return channel_ref_->channel.get(); }
+  Channel* channel() const { return channel_ref_->channel.get(); }
   bool IsValid() { return channel_ref_->channel.get() != NULL; }
+  int use_count() const { return channel_ref_->ref_count.Value(); }
  private:
   // Shared instance of a Channel. Copying ChannelOwners increase the reference
   // count and destroying ChannelOwners decrease references. Channels are

webrtc/voice_engine/include/voe_base.h

@@ -177,6 +177,13 @@ class WEBRTC_DLLEXPORT VoEBase {
   // implements the interface in its FakeWebRtcVoiceEngine.
   virtual AudioTransport* audio_transport() { return NULL; }
 
+  // Associate a send channel to a receive channel.
+  // Used for obtaining RTT for a receive-only channel.
+  // One should be careful not to crate a circular association, e.g.,
+  // 1 <- 2 <- 1.
+  virtual int AssociateSendChannel(int channel, int accociate_send_channel) =
+      0;
+
   // To be removed. Don't use.
   virtual int SetOnHoldStatus(int channel,
                               bool enable,

webrtc/voice_engine/voe_base_impl.cc

@@ -837,4 +837,33 @@ void VoEBaseImpl::GetPlayoutData(int sample_rate, int number_of_channels,
   *ntp_time_ms = audioFrame_.ntp_time_ms_;
 }
 
+int VoEBaseImpl::AssociateSendChannel(int channel,
+                                      int accociate_send_channel) {
+  CriticalSectionScoped cs(shared_->crit_sec());
+
+  if (!shared_->statistics().Initialized()) {
+      shared_->SetLastError(VE_NOT_INITED, kTraceError);
+      return -1;
+  }
+
+  voe::ChannelOwner ch = shared_->channel_manager().GetChannel(channel);
+  voe::Channel* channel_ptr = ch.channel();
+  if (channel_ptr == NULL) {
+    shared_->SetLastError(VE_CHANNEL_NOT_VALID, kTraceError,
+        "AssociateSendChannel() failed to locate channel");
+    return -1;
+  }
+
+  ch = shared_->channel_manager().GetChannel(accociate_send_channel);
+  voe::Channel* accociate_send_channel_ptr = ch.channel();
+  if (accociate_send_channel_ptr == NULL) {
+    shared_->SetLastError(VE_CHANNEL_NOT_VALID, kTraceError,
+        "AssociateSendChannel() failed to locate accociate_send_channel");
+    return -1;
+  }
+
+  channel_ptr->set_associate_send_channel(ch);
+  return 0;
+}
+
 }  // namespace webrtc

webrtc/voice_engine/voe_base_impl.h

@@ -51,6 +51,8 @@ class VoEBaseImpl : public VoEBase,
 
   AudioTransport* audio_transport() override { return this; }
 
+  int AssociateSendChannel(int channel, int accociate_send_channel) override;
+
   // AudioTransport
   int32_t RecordedDataIsAvailable(const void* audioSamples, uint32_t nSamples,
                                   uint8_t nBytesPerSample, uint8_t nChannels,

webrtc/voice_engine/voe_base_unittest.cc

@@ -12,7 +12,10 @@
 
 #include "testing/gtest/include/gtest/gtest.h"
 #include "webrtc/modules/audio_processing/include/audio_processing.h"
+#include "webrtc/voice_engine/channel_manager.h"
+#include "webrtc/voice_engine/shared_data.h"
 #include "webrtc/voice_engine/voice_engine_fixture.h"
+#include "webrtc/voice_engine/voice_engine_impl.h"
 
 namespace webrtc {
 
@@ -44,4 +47,35 @@ TEST_F(VoEBaseTest, CreateChannelAfterInit) {
   EXPECT_EQ(0, base_->DeleteChannel(channelID));
 }
 
+TEST_F(VoEBaseTest, AssociateSendChannel) {
+  AudioProcessing* audioproc = AudioProcessing::Create();
+  EXPECT_EQ(0, base_->Init(&adm_, audioproc));
+
+  const int channel_1 = base_->CreateChannel();
+
+  // Associating with a channel that does not exist should fail.
+  EXPECT_EQ(-1, base_->AssociateSendChannel(channel_1, channel_1 + 1));
+
+  const int channel_2 = base_->CreateChannel();
+
+  // Let the two channels associate with each other. This is not a normal use
+  // case. Actually, circular association should be avoided in practice. This
+  // is just to test that no crash is caused.
+  EXPECT_EQ(0, base_->AssociateSendChannel(channel_1, channel_2));
+  EXPECT_EQ(0, base_->AssociateSendChannel(channel_2, channel_1));
+
+  voe::SharedData* shared_data = static_cast<voe::SharedData*>(
+      static_cast<VoiceEngineImpl*>(voe_));
+  voe::ChannelOwner reference = shared_data->channel_manager()
+      .GetChannel(channel_1);
+  EXPECT_EQ(0, base_->DeleteChannel(channel_1));
+  // Make sure that the only use of the channel-to-delete is |reference|
+  // at this point.
+  EXPECT_EQ(1, reference.use_count());
+
+  reference = shared_data->channel_manager().GetChannel(channel_2);
+  EXPECT_EQ(0, base_->DeleteChannel(channel_2));
+  EXPECT_EQ(1, reference.use_count());
+}
+
 }  // namespace webrtc