diff --git a/talk/media/webrtc/fakewebrtcvoiceengine.h b/talk/media/webrtc/fakewebrtcvoiceengine.h index 328ef2b6cc..dba7d634ad 100644 --- a/talk/media/webrtc/fakewebrtcvoiceengine.h +++ b/talk/media/webrtc/fakewebrtcvoiceengine.h @@ -41,7 +41,6 @@ #include "webrtc/base/gunit.h" #include "webrtc/base/stringutils.h" #include "webrtc/modules/audio_processing/include/audio_processing.h" -#include "webrtc/video_engine/include/vie_network.h" namespace cricket { @@ -203,8 +202,6 @@ class FakeWebRtcVoiceEngine dtmf_type(106), red_type(117), nack_max_packets(0), - vie_network(NULL), - video_channel(-1), send_ssrc(0), send_audio_level_ext_(-1), receive_audio_level_ext_(-1), @@ -235,8 +232,6 @@ class FakeWebRtcVoiceEngine int dtmf_type; int red_type; int nack_max_packets; - webrtc::ViENetwork* vie_network; - int video_channel; uint32 send_ssrc; int send_audio_level_ext_; int receive_audio_level_ext_; @@ -332,16 +327,6 @@ class FakeWebRtcVoiceEngine int GetNACKMaxPackets(int channel) { return channels_[channel]->nack_max_packets; } - webrtc::ViENetwork* GetViENetwork(int channel) { - WEBRTC_ASSERT_CHANNEL(channel); - // WARNING: This pointer is for verification purposes only. Calling - // functions on it may result in undefined behavior! - return channels_[channel]->vie_network; - } - int GetVideoChannel(int channel) { - WEBRTC_ASSERT_CHANNEL(channel); - return channels_[channel]->video_channel; - } const webrtc::PacketTime& GetLastRtpPacketTime(int channel) { WEBRTC_ASSERT_CHANNEL(channel); return channels_[channel]->last_rtp_packet_time; @@ -1028,19 +1013,9 @@ class FakeWebRtcVoiceEngine unsigned short payloadSize)); WEBRTC_STUB(GetLastRemoteTimeStamp, (int channel, uint32_t* lastRemoteTimeStamp)); - WEBRTC_FUNC(SetVideoEngineBWETarget, (int channel, + WEBRTC_STUB(SetVideoEngineBWETarget, (int channel, webrtc::ViENetwork* vie_network, - int video_channel)) { - WEBRTC_CHECK_CHANNEL(channel); - channels_[channel]->vie_network = vie_network; - channels_[channel]->video_channel = video_channel; - if (vie_network) { - // The interface is released here to avoid leaks. A test should not - // attempt to call functions on the interface stored in the channel. - vie_network->Release(); - } - return 0; - } + int video_channel)); // webrtc::VoEVideoSync WEBRTC_STUB(GetPlayoutBufferSize, (int& bufferMs)); diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc index ac36ed5ace..5bf54d442e 100644 --- a/talk/media/webrtc/webrtcvoiceengine.cc +++ b/talk/media/webrtc/webrtcvoiceengine.cc @@ -1887,8 +1887,6 @@ WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel(WebRtcVoiceEngine *engine) typing_noise_detected_(false), desired_send_(SEND_NOTHING), send_(SEND_NOTHING), - shared_bwe_vie_(NULL), - shared_bwe_vie_channel_(-1), call_(nullptr), default_receive_ssrc_(0) { engine->RegisterChannel(this); @@ -1900,7 +1898,6 @@ WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel(WebRtcVoiceEngine *engine) WebRtcVoiceMediaChannel::~WebRtcVoiceMediaChannel() { LOG(LS_VERBOSE) << "WebRtcVoiceMediaChannel::~WebRtcVoiceMediaChannel " << voe_channel(); - SetupSharedBandwidthEstimation(NULL, -1); DCHECK(receive_streams_.empty() || call_); // Remove any remaining send streams, the default channel will be deleted @@ -2003,11 +2000,6 @@ bool WebRtcVoiceMediaChannel::SetOptions(const AudioOptions& options) { } } - // Force update of Video Engine BWE forwarding to reflect experiment setting. - if (!SetupSharedBandwidthEstimation(shared_bwe_vie_, - shared_bwe_vie_channel_)) { - return false; - } SetCall(call_); LOG(LS_INFO) << "Set voice channel options. Current options: " @@ -2767,9 +2759,6 @@ bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) { receive_channels_.insert(std::make_pair( default_receive_ssrc_, new WebRtcVoiceChannelRenderer(voe_channel(), audio_transport))); - if (!SetupSharedBweOnChannel(voe_channel())) { - return false; - } return SetPlayout(voe_channel(), playout_); } @@ -2857,11 +2846,6 @@ bool WebRtcVoiceMediaChannel::ConfigureRecvChannel(int channel) { return false; } - // Set up channel to be able to forward incoming packets to video engine BWE. - if (!SetupSharedBweOnChannel(channel)) { - return false; - } - return SetPlayout(channel, playout_); } @@ -3639,34 +3623,12 @@ int WebRtcVoiceMediaChannel::GetSendChannelNum(uint32 ssrc) { return -1; } -bool WebRtcVoiceMediaChannel::SetupSharedBandwidthEstimation( - webrtc::VideoEngine* vie, int vie_channel) { - shared_bwe_vie_ = vie; - shared_bwe_vie_channel_ = vie_channel; - - if (!SetupSharedBweOnChannel(voe_channel())) { - return false; - } - for (ChannelMap::iterator it = receive_channels_.begin(); - it != receive_channels_.end(); ++it) { - if (!SetupSharedBweOnChannel(it->second->channel())) { - return false; - } - } - return true; -} - void WebRtcVoiceMediaChannel::SetCall(webrtc::Call* call) { DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(!call || !shared_bwe_vie_); - DCHECK(!call || shared_bwe_vie_channel_ == -1); - for (const auto& it : receive_channels_) { TryRemoveAudioRecvStream(it.first); } - call_ = call; - for (const auto& it : receive_channels_) { TryAddAudioRecvStream(it.first); } @@ -3821,25 +3783,6 @@ bool WebRtcVoiceMediaChannel::SetHeaderExtension(ExtensionSetterFunction setter, return true; } -bool WebRtcVoiceMediaChannel::SetupSharedBweOnChannel(int voe_channel) { - webrtc::ViENetwork* vie_network = NULL; - int vie_channel = -1; - if (options_.combined_audio_video_bwe.GetWithDefaultIfUnset(false) && - shared_bwe_vie_ != NULL && shared_bwe_vie_channel_ != -1) { - vie_network = webrtc::ViENetwork::GetInterface(shared_bwe_vie_); - vie_channel = shared_bwe_vie_channel_; - } - if (engine()->voe()->rtp()->SetVideoEngineBWETarget(voe_channel, vie_network, - vie_channel) == -1) { - LOG_RTCERR3(SetVideoEngineBWETarget, voe_channel, vie_network, vie_channel); - if (vie_network != NULL) { - // Don't fail if we're tearing down. - return false; - } - } - return true; -} - void WebRtcVoiceMediaChannel::TryAddAudioRecvStream(uint32 ssrc) { DCHECK(thread_checker_.CalledOnValidThread()); // If we are hooked up to a webrtc::Call, create an AudioReceiveStream too. diff --git a/talk/media/webrtc/webrtcvoiceengine.h b/talk/media/webrtc/webrtcvoiceengine.h index 29a565688d..30f57174af 100644 --- a/talk/media/webrtc/webrtcvoiceengine.h +++ b/talk/media/webrtc/webrtcvoiceengine.h @@ -394,8 +394,6 @@ class WebRtcVoiceMediaChannel int GetReceiveChannelNum(uint32 ssrc); int GetSendChannelNum(uint32 ssrc); - bool SetupSharedBandwidthEstimation(webrtc::VideoEngine* vie, - int vie_channel); void SetCall(webrtc::Call* call); protected: int GetLastEngineError() { return engine()->GetLastEngineError(); } @@ -439,8 +437,6 @@ class WebRtcVoiceMediaChannel bool SetHeaderExtension(ExtensionSetterFunction setter, int channel_id, const RtpHeaderExtension* extension); - bool SetupSharedBweOnChannel(int voe_channel); - void TryAddAudioRecvStream(uint32 ssrc); void TryRemoveAudioRecvStream(uint32 ssrc); @@ -468,11 +464,6 @@ class WebRtcVoiceMediaChannel bool typing_noise_detected_; SendFlags desired_send_; SendFlags send_; - // shared_bwe_vie_ and shared_bwe_vie_channel_ together identifies a WebRTC - // VideoEngine channel that this voice channel should forward incoming packets - // to for Bandwidth Estimation purposes. - webrtc::VideoEngine* shared_bwe_vie_; - int shared_bwe_vie_channel_; webrtc::Call* call_; // send_channels_ contains the channels which are being used for sending. diff --git a/talk/media/webrtc/webrtcvoiceengine_unittest.cc b/talk/media/webrtc/webrtcvoiceengine_unittest.cc index 0a3e91cdc0..0ea260c092 100644 --- a/talk/media/webrtc/webrtcvoiceengine_unittest.cc +++ b/talk/media/webrtc/webrtcvoiceengine_unittest.cc @@ -3410,116 +3410,6 @@ TEST(WebRtcVoiceEngineTest, CoInitialize) { } #endif -TEST_F(WebRtcVoiceEngineTestFake, ChangeCombinedAudioVideoBweOption) { - // Test that changing the combined_audio_video_bwe option results in the - // expected state changes in VoiceEngine. - cricket::ViEWrapper vie; - const int kVieCh = 667; - - EXPECT_TRUE(SetupEngine()); - cricket::WebRtcVoiceMediaChannel* media_channel = - static_cast(channel_); - EXPECT_TRUE(media_channel->SetupSharedBandwidthEstimation(vie.engine(), - kVieCh)); - EXPECT_TRUE(media_channel->AddRecvStream( - cricket::StreamParams::CreateLegacy(2))); - int recv_ch = voe_.GetLastChannel(); - - // Combined BWE should not be set up yet. - EXPECT_EQ(NULL, voe_.GetViENetwork(recv_ch)); - EXPECT_EQ(-1, voe_.GetVideoChannel(recv_ch)); - - // Enable combined BWE option - now it should be set up. - cricket::AudioOptions options; - options.combined_audio_video_bwe.Set(true); - EXPECT_TRUE(media_channel->SetOptions(options)); - EXPECT_EQ(vie.network(), voe_.GetViENetwork(recv_ch)); - EXPECT_EQ(kVieCh, voe_.GetVideoChannel(recv_ch)); - - // Disable combined BWE option - should be disabled again. - options.combined_audio_video_bwe.Set(false); - EXPECT_TRUE(media_channel->SetOptions(options)); - EXPECT_EQ(NULL, voe_.GetViENetwork(recv_ch)); - EXPECT_EQ(-1, voe_.GetVideoChannel(recv_ch)); - - EXPECT_TRUE(media_channel->SetupSharedBandwidthEstimation(NULL, -1)); -} - -TEST_F(WebRtcVoiceEngineTestFake, SetupSharedBandwidthEstimation) { - // Test that calling SetupSharedBandwidthEstimation() on the voice media - // channel results in the expected state changes in VoiceEngine. - cricket::ViEWrapper vie1; - cricket::ViEWrapper vie2; - const int kVieCh1 = 667; - const int kVieCh2 = 70; - - EXPECT_TRUE(SetupEngine()); - cricket::WebRtcVoiceMediaChannel* media_channel = - static_cast(channel_); - cricket::AudioOptions options; - options.combined_audio_video_bwe.Set(true); - EXPECT_TRUE(media_channel->SetOptions(options)); - EXPECT_TRUE(media_channel->AddRecvStream( - cricket::StreamParams::CreateLegacy(2))); - int recv_ch = voe_.GetLastChannel(); - - // Combined BWE should not be set up yet. - EXPECT_EQ(NULL, voe_.GetViENetwork(recv_ch)); - EXPECT_EQ(-1, voe_.GetVideoChannel(recv_ch)); - - // Register - should be enabled. - EXPECT_TRUE(media_channel->SetupSharedBandwidthEstimation(vie1.engine(), - kVieCh1)); - EXPECT_EQ(vie1.network(), voe_.GetViENetwork(recv_ch)); - EXPECT_EQ(kVieCh1, voe_.GetVideoChannel(recv_ch)); - - // Re-register - should still be enabled. - EXPECT_TRUE(media_channel->SetupSharedBandwidthEstimation(vie2.engine(), - kVieCh2)); - EXPECT_EQ(vie2.network(), voe_.GetViENetwork(recv_ch)); - EXPECT_EQ(kVieCh2, voe_.GetVideoChannel(recv_ch)); - - // Unregister - should be disabled again. - EXPECT_TRUE(media_channel->SetupSharedBandwidthEstimation(NULL, -1)); - EXPECT_EQ(NULL, voe_.GetViENetwork(recv_ch)); - EXPECT_EQ(-1, voe_.GetVideoChannel(recv_ch)); -} - -TEST_F(WebRtcVoiceEngineTestFake, ConfigureCombinedBweForNewRecvStreams) { - // Test that adding receive streams after enabling combined bandwidth - // estimation will correctly configure each channel. - cricket::ViEWrapper vie; - const int kVieCh = 667; - - EXPECT_TRUE(SetupEngine()); - cricket::WebRtcVoiceMediaChannel* media_channel = - static_cast(channel_); - EXPECT_TRUE(media_channel->SetupSharedBandwidthEstimation(vie.engine(), - kVieCh)); - cricket::AudioOptions options; - options.combined_audio_video_bwe.Set(true); - EXPECT_TRUE(media_channel->SetOptions(options)); - - static const uint32 kSsrcs[] = {1, 2, 3, 4}; - int voe_channels[ARRAY_SIZE(kSsrcs)] = {0}; - for (unsigned int i = 0; i < ARRAY_SIZE(kSsrcs); ++i) { - EXPECT_TRUE(media_channel->AddRecvStream( - cricket::StreamParams::CreateLegacy(kSsrcs[i]))); - int recv_ch = media_channel->GetReceiveChannelNum(kSsrcs[i]); - EXPECT_NE(-1, recv_ch); - voe_channels[i] = recv_ch; - EXPECT_EQ(vie.network(), voe_.GetViENetwork(recv_ch)); - EXPECT_EQ(kVieCh, voe_.GetVideoChannel(recv_ch)); - } - - EXPECT_TRUE(media_channel->SetupSharedBandwidthEstimation(NULL, -1)); - - for (unsigned int i = 0; i < ARRAY_SIZE(voe_channels); ++i) { - EXPECT_EQ(NULL, voe_.GetViENetwork(voe_channels[i])); - EXPECT_EQ(-1, voe_.GetVideoChannel(voe_channels[i])); - } -} - TEST_F(WebRtcVoiceEngineTestFake, ChangeCombinedBweOption_Call) { // Test that changing the combined_audio_video_bwe option results in the // expected state changes on an associated Call.