diff --git a/talk/media/webrtc/fakewebrtcvoiceengine.h b/talk/media/webrtc/fakewebrtcvoiceengine.h index 18999ba3d0..c6d39bbb22 100644 --- a/talk/media/webrtc/fakewebrtcvoiceengine.h +++ b/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 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, ()) { diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc index 4f2c38a1cc..62404db9f2 100644 --- a/talk/media/webrtc/webrtcvoiceengine.cc +++ b/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()) { diff --git a/talk/media/webrtc/webrtcvoiceengine_unittest.cc b/talk/media/webrtc/webrtcvoiceengine_unittest.cc index 4d43cd29d8..bc14770ea4 100644 --- a/talk/media/webrtc/webrtcvoiceengine_unittest.cc +++ b/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(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(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); +} + diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index def39887e3..9038d6bf56 100644 --- a/webrtc/voice_engine/channel.cc +++ b/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 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; diff --git a/webrtc/voice_engine/channel.h b/webrtc/voice_engine/channel.h index d3d6d89ad9..ea8e4fdc5d 100644 --- a/webrtc/voice_engine/channel.h +++ b/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 rtcp_observer_; rtc::scoped_ptr network_predictor_; + // An associated send channel. + rtc::scoped_ptr assoc_send_channel_lock_; + ChannelOwner associate_send_channel_ GUARDED_BY(assoc_send_channel_lock_); }; } // namespace voe diff --git a/webrtc/voice_engine/channel_manager.cc b/webrtc/voice_engine/channel_manager.cc index 9386257476..76664d4b12 100644 --- a/webrtc/voice_engine/channel_manager.cc +++ b/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::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::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); + } } } diff --git a/webrtc/voice_engine/channel_manager.h b/webrtc/voice_engine/channel_manager.h index eae014ed09..07cebb0327 100644 --- a/webrtc/voice_engine/channel_manager.h +++ b/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 diff --git a/webrtc/voice_engine/include/voe_base.h b/webrtc/voice_engine/include/voe_base.h index 7666a57553..1d8c7f48dd 100644 --- a/webrtc/voice_engine/include/voe_base.h +++ b/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, diff --git a/webrtc/voice_engine/voe_base_impl.cc b/webrtc/voice_engine/voe_base_impl.cc index ab5ebcd12f..226b858593 100644 --- a/webrtc/voice_engine/voe_base_impl.cc +++ b/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 diff --git a/webrtc/voice_engine/voe_base_impl.h b/webrtc/voice_engine/voe_base_impl.h index 5ab2773946..dfb2e04749 100644 --- a/webrtc/voice_engine/voe_base_impl.h +++ b/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, diff --git a/webrtc/voice_engine/voe_base_unittest.cc b/webrtc/voice_engine/voe_base_unittest.cc index 08b935805f..5c71784b4f 100644 --- a/webrtc/voice_engine/voe_base_unittest.cc +++ b/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( + static_cast(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