diff --git a/media/base/fake_media_engine.h b/media/base/fake_media_engine.h index d983300c1b..7e62877f74 100644 --- a/media/base/fake_media_engine.h +++ b/media/base/fake_media_engine.h @@ -408,10 +408,6 @@ class FakeVoiceMediaChannel : public RtpHelper { std::unique_ptr sink) override; std::vector GetSources(uint32_t ssrc) const override; - bool SenderNackEnabled() const override { return false; } - bool SenderNonSenderRttEnabled() const override { return false; } - void SetReceiveNackEnabled(bool enabled) {} - void SetReceiveNonSenderRttEnabled(bool enabled) {} private: class VoiceChannelAudioSink : public AudioSource::Sink { diff --git a/media/base/media_channel.h b/media/base/media_channel.h index 657257947c..1191986a84 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -888,8 +888,6 @@ class VoiceMediaSendChannelInterface : public MediaSendChannelInterface { // DTMF event 0-9, *, #, A-D. virtual bool InsertDtmf(uint32_t ssrc, int event, int duration) = 0; virtual bool GetStats(VoiceMediaSendInfo* stats) = 0; - virtual bool SenderNackEnabled() const = 0; - virtual bool SenderNonSenderRttEnabled() const = 0; }; class VoiceMediaReceiveChannelInterface : public MediaReceiveChannelInterface { @@ -914,8 +912,6 @@ class VoiceMediaReceiveChannelInterface : public MediaReceiveChannelInterface { virtual void SetDefaultRawAudioSink( std::unique_ptr sink) = 0; virtual bool GetStats(VoiceMediaReceiveInfo* stats, bool reset_legacy) = 0; - virtual void SetReceiveNackEnabled(bool enabled) = 0; - virtual void SetReceiveNonSenderRttEnabled(bool enabled) = 0; }; // TODO(deadbeef): Rename to VideoSenderParameters, since they're intended to diff --git a/media/base/media_channel_impl.h b/media/base/media_channel_impl.h index e37116624c..f18670f825 100644 --- a/media/base/media_channel_impl.h +++ b/media/base/media_channel_impl.h @@ -77,8 +77,7 @@ class MediaChannel : public MediaSendChannelInterface, enum class Role { kSend, kReceive, - kBoth // Temporary value for non-converted test and downstream code - // TODO(bugs.webrtc.org/13931): Remove kBoth when usage is removed. + kBoth // Temporary value for non-converted test code }; explicit MediaChannel(Role role, @@ -424,10 +423,6 @@ class VoiceMediaSendChannel : public VoiceMediaSendChannelInterface { bool GetStats(VoiceMediaSendInfo* info) override { return impl_->GetSendStats(info); } - bool SenderNackEnabled() const override { return impl_->SenderNackEnabled(); } - bool SenderNonSenderRttEnabled() const override { - return impl_->SenderNonSenderRttEnabled(); - } MediaChannel* ImplForTesting() override { return impl_; } private: @@ -544,12 +539,6 @@ class VoiceMediaReceiveChannel : public VoiceMediaReceiveChannelInterface { bool GetStats(VoiceMediaReceiveInfo* info, bool reset_legacy) override { return impl_->GetReceiveStats(info, reset_legacy); } - void SetReceiveNackEnabled(bool enabled) override { - impl_->SetReceiveNackEnabled(enabled); - } - void SetReceiveNonSenderRttEnabled(bool enabled) override { - impl_->SetReceiveNonSenderRttEnabled(enabled); - } MediaChannel* ImplForTesting() override { return impl_; } private: diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index ab9a3fc7b8..7bc6a8b470 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -417,7 +417,7 @@ VoiceMediaChannel* WebRtcVoiceEngine::CreateMediaChannel( webrtc::AudioCodecPairId codec_pair_id) { RTC_DCHECK_RUN_ON(call->worker_thread()); return new WebRtcVoiceMediaChannel(role, this, config, options, - crypto_options, call, codec_pair_id); + crypto_options, call); } void WebRtcVoiceEngine::ApplyOptions(const AudioOptions& options_in) { @@ -1256,14 +1256,12 @@ WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel( const MediaConfig& config, const AudioOptions& options, const webrtc::CryptoOptions& crypto_options, - webrtc::Call* call, - webrtc::AudioCodecPairId codec_pair_id) + webrtc::Call* call) : VoiceMediaChannel(role, call->network_thread(), config.enable_dscp), worker_thread_(call->worker_thread()), engine_(engine), call_(call), audio_config_(config.audio), - codec_pair_id_(codec_pair_id), crypto_options_(crypto_options) { network_thread_checker_.Detach(); RTC_LOG(LS_VERBOSE) << "WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel"; @@ -1737,40 +1735,28 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs( } call_->GetTransportControllerSend()->SetSdpBitrateParameters(bitrate_config); - // In legacy kBoth mode, the MediaChannel sets the NACK status. - // In other modes, this is done externally. - - if (role() == MediaChannel::Role::kBoth) { - SetReceiveNackEnabled(send_codec_spec_->nack_enabled); - SetReceiveNonSenderRttEnabled(send_codec_spec_->enable_non_sender_rtt); - } - - send_codecs_ = codecs; - return true; -} - -void WebRtcVoiceMediaChannel::SetReceiveNackEnabled(bool enabled) { // Check if the NACK status has changed on the // preferred send codec, and in that case reconfigure all receive streams. - if (recv_nack_enabled_ != enabled) { + if (recv_nack_enabled_ != send_codec_spec_->nack_enabled) { RTC_LOG(LS_INFO) << "Changing NACK status on receive streams."; - recv_nack_enabled_ = enabled; + recv_nack_enabled_ = send_codec_spec_->nack_enabled; for (auto& kv : recv_streams_) { kv.second->SetUseNack(recv_nack_enabled_); } } -} -void WebRtcVoiceMediaChannel::SetReceiveNonSenderRttEnabled(bool enabled) { // Check if the receive-side RTT status has changed on the preferred send // codec, in that case reconfigure all receive streams. - if (enable_non_sender_rtt_ != enabled) { + if (enable_non_sender_rtt_ != send_codec_spec_->enable_non_sender_rtt) { RTC_LOG(LS_INFO) << "Changing receive-side RTT status on receive streams."; - enable_non_sender_rtt_ = enabled; + enable_non_sender_rtt_ = send_codec_spec_->enable_non_sender_rtt; for (auto& kv : recv_streams_) { kv.second->SetNonSenderRttMeasurement(enable_non_sender_rtt_); } } + + send_codecs_ = codecs; + return true; } void WebRtcVoiceMediaChannel::SetPlayout(bool playout) { diff --git a/media/engine/webrtc_voice_engine.h b/media/engine/webrtc_voice_engine.h index 6d25397e68..5f9eb5d730 100644 --- a/media/engine/webrtc_voice_engine.h +++ b/media/engine/webrtc_voice_engine.h @@ -16,7 +16,6 @@ #include #include -#include "api/audio_codecs/audio_codec_pair_id.h" #include "api/audio_codecs/audio_encoder_factory.h" #include "api/field_trials_view.h" #include "api/scoped_refptr.h" @@ -75,7 +74,8 @@ class WebRtcVoiceEngine final : public VoiceEngineInterface { const MediaConfig& config, const AudioOptions& options, const webrtc::CryptoOptions& crypto_options, - webrtc::AudioCodecPairId codec_pair_id) override; + webrtc::AudioCodecPairId codec_pair_id = + webrtc::AudioCodecPairId::Create()) override; const std::vector& send_codecs() const override; const std::vector& recv_codecs() const override; @@ -147,8 +147,7 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, const MediaConfig& config, const AudioOptions& options, const webrtc::CryptoOptions& crypto_options, - webrtc::Call* call, - webrtc::AudioCodecPairId codec_pair_id); + webrtc::Call* call); WebRtcVoiceMediaChannel() = delete; WebRtcVoiceMediaChannel(const WebRtcVoiceMediaChannel&) = delete; @@ -249,22 +248,6 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, bool SendRtcp(const uint8_t* data, size_t len) override; - bool SenderNackEnabled() const override { - if (!send_codec_spec_) { - return false; - } - return send_codec_spec_->nack_enabled; - } - bool SenderNonSenderRttEnabled() const override { - if (!send_codec_spec_) { - return false; - } - return send_codec_spec_->enable_non_sender_rtt; - } - - void SetReceiveNackEnabled(bool enabled) override; - void SetReceiveNonSenderRttEnabled(bool enabled) override; - private: bool SetOptions(const AudioOptions& options); bool SetRecvCodecs(const std::vector& codecs); @@ -344,7 +327,8 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, send_codec_spec_; // TODO(kwiberg): Per-SSRC codec pair IDs? - const webrtc::AudioCodecPairId codec_pair_id_; + const webrtc::AudioCodecPairId codec_pair_id_ = + webrtc::AudioCodecPairId::Create(); // Per peer connection crypto options that last for the lifetime of the peer // connection. diff --git a/media/engine/webrtc_voice_engine_unittest.cc b/media/engine/webrtc_voice_engine_unittest.cc index 1393d0b9af..70cc46159b 100644 --- a/media/engine/webrtc_voice_engine_unittest.cc +++ b/media/engine/webrtc_voice_engine_unittest.cc @@ -232,8 +232,7 @@ class WebRtcVoiceEngineTestFake : public ::testing::TestWithParam { bool SetupChannel() { channel_ = engine_->CreateMediaChannel( cricket::MediaChannel::Role::kBoth, &call_, cricket::MediaConfig(), - cricket::AudioOptions(), webrtc::CryptoOptions(), - webrtc::AudioCodecPairId::Create()); + cricket::AudioOptions(), webrtc::CryptoOptions()); send_channel_ = std::make_unique(channel_); receive_channel_ = std::make_unique(channel_); @@ -3032,8 +3031,7 @@ TEST_P(WebRtcVoiceEngineTestFake, InitRecordingOnSend) { std::unique_ptr channel( engine_->CreateMediaChannel( cricket::MediaChannel::Role::kBoth, &call_, cricket::MediaConfig(), - cricket::AudioOptions(), webrtc::CryptoOptions(), - webrtc::AudioCodecPairId::Create())); + cricket::AudioOptions(), webrtc::CryptoOptions())); channel->SetSend(true); } @@ -3049,8 +3047,7 @@ TEST_P(WebRtcVoiceEngineTestFake, SkipInitRecordingOnSend) { std::unique_ptr channel( engine_->CreateMediaChannel(cricket::MediaChannel::Role::kBoth, &call_, cricket::MediaConfig(), options, - webrtc::CryptoOptions(), - webrtc::AudioCodecPairId::Create())); + webrtc::CryptoOptions())); channel->SetSend(true); } @@ -3075,16 +3072,16 @@ TEST_P(WebRtcVoiceEngineTestFake, SetOptionOverridesViaChannels) { std::unique_ptr channel1( static_cast( - engine_->CreateMediaChannel( - cricket::MediaChannel::Role::kBoth, &call_, - cricket::MediaConfig(), cricket::AudioOptions(), - webrtc::CryptoOptions(), webrtc::AudioCodecPairId::Create()))); + engine_->CreateMediaChannel(cricket::MediaChannel::Role::kBoth, + &call_, cricket::MediaConfig(), + cricket::AudioOptions(), + webrtc::CryptoOptions()))); std::unique_ptr channel2( static_cast( - engine_->CreateMediaChannel( - cricket::MediaChannel::Role::kBoth, &call_, - cricket::MediaConfig(), cricket::AudioOptions(), - webrtc::CryptoOptions(), webrtc::AudioCodecPairId::Create()))); + engine_->CreateMediaChannel(cricket::MediaChannel::Role::kBoth, + &call_, cricket::MediaConfig(), + cricket::AudioOptions(), + webrtc::CryptoOptions()))); // Have to add a stream to make SetSend work. cricket::StreamParams stream1; @@ -3196,8 +3193,7 @@ TEST_P(WebRtcVoiceEngineTestFake, TestSetDscpOptions) { channel.reset(static_cast( engine_->CreateMediaChannel(cricket::MediaChannel::Role::kBoth, &call_, config, cricket::AudioOptions(), - webrtc::CryptoOptions(), - webrtc::AudioCodecPairId::Create()))); + webrtc::CryptoOptions()))); channel->SetInterface(&network_interface); // Default value when DSCP is disabled should be DSCP_DEFAULT. EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface.dscp()); @@ -3207,8 +3203,7 @@ TEST_P(WebRtcVoiceEngineTestFake, TestSetDscpOptions) { channel.reset(static_cast( engine_->CreateMediaChannel(cricket::MediaChannel::Role::kBoth, &call_, config, cricket::AudioOptions(), - webrtc::CryptoOptions(), - webrtc::AudioCodecPairId::Create()))); + webrtc::CryptoOptions()))); channel->SetInterface(&network_interface); EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface.dscp()); @@ -3238,8 +3233,7 @@ TEST_P(WebRtcVoiceEngineTestFake, TestSetDscpOptions) { channel.reset(static_cast( engine_->CreateMediaChannel(cricket::MediaChannel::Role::kBoth, &call_, config, cricket::AudioOptions(), - webrtc::CryptoOptions(), - webrtc::AudioCodecPairId::Create()))); + webrtc::CryptoOptions()))); channel->SetInterface(&network_interface); // Default value when DSCP is disabled should be DSCP_DEFAULT. EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface.dscp()); @@ -3650,8 +3644,7 @@ TEST(WebRtcVoiceEngineTest, StartupShutdown) { auto call = absl::WrapUnique(webrtc::Call::Create(call_config)); cricket::VoiceMediaChannel* channel = engine.CreateMediaChannel( cricket::MediaChannel::Role::kBoth, call.get(), cricket::MediaConfig(), - cricket::AudioOptions(), webrtc::CryptoOptions(), - webrtc::AudioCodecPairId::Create()); + cricket::AudioOptions(), webrtc::CryptoOptions()); EXPECT_TRUE(channel != nullptr); delete channel; } @@ -3683,7 +3676,7 @@ TEST(WebRtcVoiceEngineTest, StartupShutdownWithExternalADM) { cricket::VoiceMediaChannel* channel = engine.CreateMediaChannel( cricket::MediaChannel::Role::kBoth, call.get(), cricket::MediaConfig(), cricket::AudioOptions(), - webrtc::CryptoOptions(), webrtc::AudioCodecPairId::Create()); + webrtc::CryptoOptions()); EXPECT_TRUE(channel != nullptr); delete channel; } @@ -3773,7 +3766,7 @@ TEST(WebRtcVoiceEngineTest, Has32Channels) { cricket::VoiceMediaChannel* channel = engine.CreateMediaChannel( cricket::MediaChannel::Role::kBoth, call.get(), cricket::MediaConfig(), cricket::AudioOptions(), - webrtc::CryptoOptions(), webrtc::AudioCodecPairId::Create()); + webrtc::CryptoOptions()); if (!channel) break; channels[num_channels++] = channel; @@ -3819,8 +3812,7 @@ TEST(WebRtcVoiceEngineTest, SetRecvCodecs) { auto call = absl::WrapUnique(webrtc::Call::Create(call_config)); cricket::WebRtcVoiceMediaChannel channel( cricket::MediaChannel::Role::kReceive, &engine, cricket::MediaConfig(), - cricket::AudioOptions(), webrtc::CryptoOptions(), call.get(), - webrtc::AudioCodecPairId::Create()); + cricket::AudioOptions(), webrtc::CryptoOptions(), call.get()); cricket::AudioRecvParameters parameters; parameters.codecs = engine.recv_codecs(); EXPECT_TRUE(channel.SetRecvParameters(parameters)); @@ -3854,8 +3846,7 @@ TEST(WebRtcVoiceEngineTest, SetRtpSendParametersMaxBitrate) { auto call = absl::WrapUnique(webrtc::Call::Create(call_config)); cricket::WebRtcVoiceMediaChannel channel( cricket::MediaChannel::Role::kSend, &engine, cricket::MediaConfig(), - cricket::AudioOptions(), webrtc::CryptoOptions(), call.get(), - webrtc::AudioCodecPairId::Create()); + cricket::AudioOptions(), webrtc::CryptoOptions(), call.get()); { cricket::AudioSendParameters params; params.codecs.push_back(cricket::AudioCodec(1, "opus", 48000, 32000, 2)); diff --git a/pc/channel.cc b/pc/channel.cc index 6c457fc2b4..6d261ece98 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -110,19 +110,15 @@ void RtpSendParametersFromMediaDescription( send_params->extmap_allow_mixed = desc->extmap_allow_mixed(); } -BaseChannel::BaseChannel( - rtc::Thread* worker_thread, - rtc::Thread* network_thread, - rtc::Thread* signaling_thread, - std::unique_ptr send_media_channel_impl, - std::unique_ptr receive_media_channel_impl, - absl::string_view mid, - bool srtp_required, - webrtc::CryptoOptions crypto_options, - UniqueRandomIdGenerator* ssrc_generator) - : media_send_channel_impl_(std::move(send_media_channel_impl)), - media_receive_channel_impl_(std::move(receive_media_channel_impl)), - worker_thread_(worker_thread), +BaseChannel::BaseChannel(rtc::Thread* worker_thread, + rtc::Thread* network_thread, + rtc::Thread* signaling_thread, + std::unique_ptr media_channel, + absl::string_view mid, + bool srtp_required, + webrtc::CryptoOptions crypto_options, + UniqueRandomIdGenerator* ssrc_generator) + : worker_thread_(worker_thread), network_thread_(network_thread), signaling_thread_(signaling_thread), alive_(PendingTaskSafetyFlag::Create()), @@ -131,11 +127,11 @@ BaseChannel::BaseChannel( crypto_options.srtp.enable_encrypted_rtp_header_extensions ? webrtc::RtpExtension::kPreferEncryptedExtension : webrtc::RtpExtension::kDiscardEncryptedExtension), + media_channel_(std::move(media_channel)), demuxer_criteria_(mid), ssrc_generator_(ssrc_generator) { RTC_DCHECK_RUN_ON(worker_thread_); - RTC_DCHECK(media_send_channel_impl_); - RTC_DCHECK(media_receive_channel_impl_); + RTC_DCHECK(media_channel_); RTC_DCHECK(ssrc_generator_); RTC_DLOG(LS_INFO) << "Created channel: " << ToString(); } @@ -152,9 +148,8 @@ BaseChannel::~BaseChannel() { } std::string BaseChannel::ToString() const { - return StringFormat( - "{mid: %s, media_type: %s}", mid().c_str(), - MediaTypeToString(media_send_channel_impl_->media_type()).c_str()); + return StringFormat("{mid: %s, media_type: %s}", mid().c_str(), + MediaTypeToString(media_channel_->media_type()).c_str()); } bool BaseChannel::ConnectToRtpTransport_n() { @@ -186,8 +181,7 @@ void BaseChannel::DisconnectFromRtpTransport_n() { rtp_transport_->SignalWritableState.disconnect(this); rtp_transport_->SignalSentPacket.disconnect(this); rtp_transport_ = nullptr; - media_send_channel()->SetInterface(nullptr); - media_receive_channel()->SetInterface(nullptr); + media_channel_->SetInterface(nullptr); } bool BaseChannel::SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport) { @@ -212,12 +206,10 @@ bool BaseChannel::SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport) { return false; } - RTC_DCHECK(!media_send_channel()->HasNetworkInterface()); - RTC_DCHECK(!media_receive_channel()->HasNetworkInterface()); - media_send_channel()->SetInterface(this); - media_receive_channel()->SetInterface(this); + RTC_DCHECK(!media_channel_->HasNetworkInterface()); + media_channel_->SetInterface(this); - media_send_channel()->OnReadyToSend(rtp_transport_->IsReadyToSend()); + media_channel_->OnReadyToSend(rtp_transport_->IsReadyToSend()); UpdateWritableState_n(); // Set the cached socket options. @@ -346,8 +338,7 @@ void BaseChannel::OnNetworkRouteChanged( // use the same transport name and MediaChannel::OnNetworkRouteChanged cannot // work correctly. Intentionally leave it broken to simplify the code and // encourage the users to stop using non-muxing RTCP. - media_send_channel()->OnNetworkRouteChanged(transport_name(), new_route); - media_receive_channel()->OnNetworkRouteChanged(transport_name(), new_route); + media_channel_->OnNetworkRouteChanged(transport_name(), new_route); } void BaseChannel::SetFirstPacketReceivedCallback( @@ -367,7 +358,7 @@ void BaseChannel::SetFirstPacketReceivedCallback( void BaseChannel::OnTransportReadyToSend(bool ready) { RTC_DCHECK_RUN_ON(network_thread()); RTC_DCHECK(network_initialized()); - media_send_channel()->OnReadyToSend(ready); + media_channel_->OnReadyToSend(ready); } bool BaseChannel::SendPacket(bool rtcp, @@ -442,7 +433,7 @@ void BaseChannel::OnRtpPacket(const webrtc::RtpPacketReceived& parsed_packet) { << ToString(); return; } - media_receive_channel()->OnPacketReceived(parsed_packet); + media_channel_->OnPacketReceived(parsed_packet); } bool BaseChannel::MaybeUpdateDemuxerAndRtpExtensions_w( @@ -492,7 +483,7 @@ bool BaseChannel::MaybeUpdateDemuxerAndRtpExtensions_w( } bool BaseChannel::RegisterRtpDemuxerSink_w() { - media_receive_channel()->OnDemuxerCriteriaUpdatePending(); + media_channel_->OnDemuxerCriteriaUpdatePending(); // Copy demuxer criteria, since they're a worker-thread variable // and we want to pass them to the network thread bool ret = network_thread_->BlockingCall( @@ -510,7 +501,7 @@ bool BaseChannel::RegisterRtpDemuxerSink_w() { return rtp_transport_->RegisterRtpDemuxerSink(demuxer_criteria, this); }); - media_receive_channel()->OnDemuxerCriteriaUpdateComplete(); + media_channel_->OnDemuxerCriteriaUpdateComplete(); return ret; } @@ -680,16 +671,6 @@ bool BaseChannel::UpdateLocalStreams_w(const std::vector& streams, if (media_send_channel()->AddSendStream(new_stream)) { RTC_LOG(LS_INFO) << "Add send stream ssrc: " << new_stream.ssrcs[0] << " into " << ToString(); - // Must also tell the corresponding receive stream to listen for - // RRs coming in on the new stream's SSRC - if (all_streams.size() == 1) { - if (!media_receive_channel()->SetLocalSsrc(new_stream)) { - error_desc = StringFormat( - "Failed to set local ssrc: %u into m-section with mid='%s'", - new_stream.first_ssrc(), mid().c_str()); - ret = false; - } - } } else { error_desc = StringFormat( "Failed to add send stream ssrc: %u into m-section with mid='%s'", @@ -826,27 +807,24 @@ void BaseChannel::SignalSentPacket_n(const rtc::SentPacket& sent_packet) { media_send_channel()->OnPacketSent(sent_packet); } -VoiceChannel::VoiceChannel( - rtc::Thread* worker_thread, - rtc::Thread* network_thread, - rtc::Thread* signaling_thread, - std::unique_ptr media_send_channel_impl, - std::unique_ptr media_receive_channel_impl, - absl::string_view mid, - bool srtp_required, - webrtc::CryptoOptions crypto_options, - UniqueRandomIdGenerator* ssrc_generator) +VoiceChannel::VoiceChannel(rtc::Thread* worker_thread, + rtc::Thread* network_thread, + rtc::Thread* signaling_thread, + std::unique_ptr media_channel, + absl::string_view mid, + bool srtp_required, + webrtc::CryptoOptions crypto_options, + UniqueRandomIdGenerator* ssrc_generator) : BaseChannel(worker_thread, network_thread, signaling_thread, - std::move(media_send_channel_impl), - std::move(media_receive_channel_impl), + std::move(media_channel), mid, srtp_required, crypto_options, ssrc_generator), - send_channel_(media_send_channel_impl_->AsVoiceChannel()), - receive_channel_(media_receive_channel_impl_->AsVoiceChannel()) {} + send_channel_(this->media_channel()->AsVoiceChannel()), + receive_channel_(this->media_channel()->AsVoiceChannel()) {} VoiceChannel::~VoiceChannel() { TRACE_EVENT0("webrtc", "VoiceChannel::~VoiceChannel"); @@ -950,38 +928,29 @@ bool VoiceChannel::SetRemoteContent_w(const MediaContentDescription* content, mid().c_str()); return false; } - // Update Receive channel based on Send channel's codec information. - // TODO(bugs.webrtc.org/14911): This is silly. Stop doing it. - media_receive_channel()->SetReceiveNackEnabled( - media_send_channel()->SenderNackEnabled()); - media_receive_channel()->SetReceiveNonSenderRttEnabled( - media_send_channel()->SenderNonSenderRttEnabled()); last_send_params_ = send_params; return UpdateRemoteStreams_w(content, type, error_desc); } -VideoChannel::VideoChannel( - rtc::Thread* worker_thread, - rtc::Thread* network_thread, - rtc::Thread* signaling_thread, - std::unique_ptr media_send_channel_impl, - std::unique_ptr media_receive_channel_impl, - absl::string_view mid, - bool srtp_required, - webrtc::CryptoOptions crypto_options, - UniqueRandomIdGenerator* ssrc_generator) +VideoChannel::VideoChannel(rtc::Thread* worker_thread, + rtc::Thread* network_thread, + rtc::Thread* signaling_thread, + std::unique_ptr media_channel, + absl::string_view mid, + bool srtp_required, + webrtc::CryptoOptions crypto_options, + UniqueRandomIdGenerator* ssrc_generator) : BaseChannel(worker_thread, network_thread, signaling_thread, - std::move(media_send_channel_impl), - std::move(media_receive_channel_impl), + std::move(media_channel), mid, srtp_required, crypto_options, ssrc_generator), - send_channel_(media_send_channel_impl_->AsVideoChannel()), - receive_channel_(media_receive_channel_impl_->AsVideoChannel()) {} + send_channel_(this->media_channel()->AsVideoChannel()), + receive_channel_(this->media_channel()->AsVideoChannel()) {} VideoChannel::~VideoChannel() { TRACE_EVENT0("webrtc", "VideoChannel::~VideoChannel"); diff --git a/pc/channel.h b/pc/channel.h index fb671c2dc7..b3020e377c 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -85,8 +85,7 @@ class BaseChannel : public ChannelInterface, BaseChannel(rtc::Thread* worker_thread, rtc::Thread* network_thread, rtc::Thread* signaling_thread, - std::unique_ptr media_send_channel_impl, - std::unique_ptr media_receive_channel_impl, + std::unique_ptr media_channel, absl::string_view mid, bool srtp_required, webrtc::CryptoOptions crypto_options, @@ -159,6 +158,8 @@ class BaseChannel : public ChannelInterface, // RtpPacketSinkInterface overrides. void OnRtpPacket(const webrtc::RtpPacketReceived& packet) override; + MediaChannel* media_channel() override { return media_channel_.get(); } + VideoMediaSendChannelInterface* video_media_send_channel() override { RTC_CHECK(false) << "Attempt to fetch video channel from non-video"; return nullptr; @@ -202,7 +203,7 @@ class BaseChannel : public ChannelInterface, } bool network_initialized() RTC_RUN_ON(network_thread()) { - return media_send_channel()->HasNetworkInterface(); + return media_channel_->HasNetworkInterface(); } bool enabled() const RTC_RUN_ON(worker_thread()) { return enabled_; } @@ -303,12 +304,6 @@ class BaseChannel : public ChannelInterface, // Return description of media channel to facilitate logging std::string ToString() const; - // MediaChannel related members that should be accessed from the worker - // thread. These are used in initializing the subclasses and deleting - // the channels when exiting; they have no accessors. - const std::unique_ptr media_send_channel_impl_; - const std::unique_ptr media_receive_channel_impl_; - private: bool ConnectToRtpTransport_n() RTC_RUN_ON(network_thread()); void DisconnectFromRtpTransport_n() RTC_RUN_ON(network_thread()); @@ -338,6 +333,9 @@ class BaseChannel : public ChannelInterface, // based on the supplied CryptoOptions. const webrtc::RtpExtension::Filter extensions_filter_; + // MediaChannel related members that should be accessed from the worker + // thread. + const std::unique_ptr media_channel_; // Currently the `enabled_` flag is accessed from the signaling thread as // well, but it can be changed only when signaling thread does a synchronous // call to the worker thread, so it should be safe. @@ -372,8 +370,7 @@ class VoiceChannel : public BaseChannel { VoiceChannel(rtc::Thread* worker_thread, rtc::Thread* network_thread, rtc::Thread* signaling_thread, - std::unique_ptr send_channel_impl, - std::unique_ptr receive_channel_impl, + std::unique_ptr channel, absl::string_view mid, bool srtp_required, webrtc::CryptoOptions crypto_options, @@ -434,8 +431,7 @@ class VideoChannel : public BaseChannel { VideoChannel(rtc::Thread* worker_thread, rtc::Thread* network_thread, rtc::Thread* signaling_thread, - std::unique_ptr media_send_channel_impl, - std::unique_ptr media_receive_channel_impl, + std::unique_ptr media_channel, absl::string_view mid, bool srtp_required, webrtc::CryptoOptions crypto_options, diff --git a/pc/channel_interface.h b/pc/channel_interface.h index 7495ad8931..783755e6fe 100644 --- a/pc/channel_interface.h +++ b/pc/channel_interface.h @@ -53,6 +53,8 @@ class ChannelInterface { virtual VideoChannel* AsVideoChannel() = 0; virtual VoiceChannel* AsVoiceChannel() = 0; + // Temporary fix while MediaChannel is being reconstructed + virtual MediaChannel* media_channel() = 0; virtual MediaSendChannelInterface* media_send_channel() = 0; // Typecasts of media_channel(). Will cause an exception if the // channel is of the wrong type. diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index c6ad9e896a..1635ea5135 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -151,23 +151,15 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { void CreateChannels(int flags1, int flags2) { CreateChannels(std::make_unique( - cricket::MediaChannel::Role::kSend, nullptr, + cricket::MediaChannel::Role::kBoth, nullptr, typename T::Options(), network_thread_), std::make_unique( - cricket::MediaChannel::Role::kReceive, nullptr, - typename T::Options(), network_thread_), - std::make_unique( - cricket::MediaChannel::Role::kSend, nullptr, - typename T::Options(), network_thread_), - std::make_unique( - cricket::MediaChannel::Role::kReceive, nullptr, + cricket::MediaChannel::Role::kBoth, nullptr, typename T::Options(), network_thread_), flags1, flags2); } - void CreateChannels(std::unique_ptr ch1s, - std::unique_ptr ch1r, - std::unique_ptr ch2s, - std::unique_ptr ch2r, + void CreateChannels(std::unique_ptr ch1, + std::unique_ptr ch2, int flags1, int flags2) { RTC_DCHECK(!channel1_); @@ -245,10 +237,10 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { fake_rtp_dtls_transport2_.get(), fake_rtcp_dtls_transport2_.get(), flags2); - channel1_ = CreateChannel(worker_thread, network_thread_, std::move(ch1s), - std::move(ch1r), rtp_transport1_.get(), flags1); - channel2_ = CreateChannel(worker_thread, network_thread_, std::move(ch2s), - std::move(ch2r), rtp_transport2_.get(), flags2); + channel1_ = CreateChannel(worker_thread, network_thread_, std::move(ch1), + rtp_transport1_.get(), flags1); + channel2_ = CreateChannel(worker_thread, network_thread_, std::move(ch2), + rtp_transport2_.get(), flags2); CreateContent(flags1, kPcmuCodec, kH264Codec, &local_media_content1_); CreateContent(flags2, kPcmuCodec, kH264Codec, &local_media_content2_); CopyContent(local_media_content1_, &remote_media_content1_); @@ -271,8 +263,7 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { std::unique_ptr CreateChannel( rtc::Thread* worker_thread, rtc::Thread* network_thread, - std::unique_ptr ch_send, - std::unique_ptr ch_receive, + std::unique_ptr ch, webrtc::RtpTransportInternal* rtp_transport, int flags); @@ -448,7 +439,7 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { } void SendRtp1(rtc::Buffer data) { - SendRtp(media_send_channel1_impl(), std::move(data)); + SendRtp(media_channel1(), std::move(data)); } void SendRtp2() { @@ -456,7 +447,7 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { } void SendRtp2(rtc::Buffer data) { - SendRtp(media_send_channel2_impl(), std::move(data)); + SendRtp(media_channel2(), std::move(data)); } // Methods to send custom data. @@ -468,21 +459,19 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { } bool CheckRtp1() { - return media_receive_channel1_impl()->CheckRtp(rtp_packet_.data(), - rtp_packet_.size()); + return media_channel1()->CheckRtp(rtp_packet_.data(), rtp_packet_.size()); } bool CheckRtp2() { - return media_receive_channel2_impl()->CheckRtp(rtp_packet_.data(), - rtp_packet_.size()); + return media_channel2()->CheckRtp(rtp_packet_.data(), rtp_packet_.size()); } // Methods to check custom data. bool CheckCustomRtp1(uint32_t ssrc, int sequence_number, int pl_type = -1) { rtc::Buffer data = CreateRtpData(ssrc, sequence_number, pl_type); - return media_receive_channel1_impl()->CheckRtp(data.data(), data.size()); + return media_channel1()->CheckRtp(data.data(), data.size()); } bool CheckCustomRtp2(uint32_t ssrc, int sequence_number, int pl_type = -1) { rtc::Buffer data = CreateRtpData(ssrc, sequence_number, pl_type); - return media_receive_channel2_impl()->CheckRtp(data.data(), data.size()); + return media_channel2()->CheckRtp(data.data(), data.size()); } rtc::Buffer CreateRtpData(uint32_t ssrc, int sequence_number, int pl_type) { rtc::Buffer data(rtp_packet_.data(), rtp_packet_.size()); @@ -495,8 +484,8 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { return data; } - bool CheckNoRtp1() { return media_send_channel1_impl()->CheckNoRtp(); } - bool CheckNoRtp2() { return media_send_channel2_impl()->CheckNoRtp(); } + bool CheckNoRtp1() { return media_channel1()->CheckNoRtp(); } + bool CheckNoRtp2() { return media_channel2()->CheckNoRtp(); } void CreateContent(int flags, const cricket::AudioCodec& audio_codec, @@ -578,20 +567,18 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { void TestInit() { CreateChannels(0, 0); EXPECT_FALSE(IsSrtpActive(channel1_)); - EXPECT_FALSE(media_send_channel1_impl()->sending()); + EXPECT_FALSE(media_channel1()->sending()); if (verify_playout_) { - EXPECT_FALSE(media_receive_channel1_impl()->playout()); + EXPECT_FALSE(media_channel1()->playout()); } - EXPECT_TRUE(media_send_channel1_impl()->codecs().empty()); - EXPECT_TRUE(media_receive_channel1_impl()->recv_streams().empty()); - EXPECT_TRUE(media_send_channel1_impl()->rtp_packets().empty()); + EXPECT_TRUE(media_channel1()->codecs().empty()); + EXPECT_TRUE(media_channel1()->recv_streams().empty()); + EXPECT_TRUE(media_channel1()->rtp_packets().empty()); // Basic sanity test for send and receive channel objects EXPECT_EQ(channel1_->media_send_channel()->media_type(), - media_send_channel1_impl()->media_type()); + media_channel1()->media_type()); EXPECT_EQ(channel1_->media_receive_channel()->media_type(), - media_receive_channel1_impl()->media_type()); - EXPECT_EQ(channel1_->media_send_channel()->media_type(), - channel1_->media_receive_channel()->media_type()); + media_channel1()->media_type()); } // Test that SetLocalContent and SetRemoteContent properly configure @@ -602,11 +589,11 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { CreateContent(0, kPcmuCodec, kH264Codec, &content); std::string err; EXPECT_TRUE(channel1_->SetLocalContent(&content, SdpType::kOffer, err)); - EXPECT_EQ(0U, media_send_channel1_impl()->codecs().size()); + EXPECT_EQ(0U, media_channel1()->codecs().size()); EXPECT_TRUE(channel1_->SetRemoteContent(&content, SdpType::kAnswer, err)); - ASSERT_EQ(1U, media_send_channel1_impl()->codecs().size()); - EXPECT_TRUE(CodecMatches(content.codecs()[0], - media_send_channel1_impl()->codecs()[0])); + ASSERT_EQ(1U, media_channel1()->codecs().size()); + EXPECT_TRUE( + CodecMatches(content.codecs()[0], media_channel1()->codecs()[0])); } // Test that SetLocalContent and SetRemoteContent properly configure @@ -624,7 +611,7 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { EXPECT_TRUE(channel1_->SetLocalContent(&content, SdpType::kOffer, err)); content.set_extmap_allow_mixed_enum(answer_enum); EXPECT_TRUE(channel1_->SetRemoteContent(&content, SdpType::kAnswer, err)); - EXPECT_EQ(answer, media_send_channel1_impl()->ExtmapAllowMixed()); + EXPECT_EQ(answer, media_send_channel1()->ExtmapAllowMixed()); } void TestSetContentsExtmapAllowMixedCallee(bool offer, bool answer) { // For a callee, SetRemoteContent() is called first with an offer and next @@ -650,11 +637,11 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { std::string err; EXPECT_TRUE(channel1_->SetLocalContent(&content, SdpType::kOffer, err)); CreateContent(0, kPcmuCodec, kH264Codec, &content); - EXPECT_EQ(0U, media_send_channel1_impl()->codecs().size()); + EXPECT_EQ(0U, media_channel1()->codecs().size()); EXPECT_TRUE(channel1_->SetRemoteContent(&content, SdpType::kAnswer, err)); - ASSERT_EQ(1U, media_send_channel1_impl()->codecs().size()); - EXPECT_TRUE(CodecMatches(content.codecs()[0], - media_send_channel1_impl()->codecs()[0])); + ASSERT_EQ(1U, media_channel1()->codecs().size()); + EXPECT_TRUE( + CodecMatches(content.codecs()[0], media_channel1()->codecs()[0])); } // Test that SetLocalContent and SetRemoteContent properly set RTCP @@ -696,20 +683,20 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { std::string err; EXPECT_TRUE(channel1_->SetLocalContent(&content1, SdpType::kOffer, err)); channel1_->Enable(true); - EXPECT_EQ(1u, media_send_channel1_impl()->send_streams().size()); + EXPECT_EQ(1u, media_channel1()->send_streams().size()); EXPECT_TRUE(channel2_->SetRemoteContent(&content1, SdpType::kOffer, err)); - EXPECT_EQ(1u, media_receive_channel2_impl()->recv_streams().size()); + EXPECT_EQ(1u, media_channel2()->recv_streams().size()); ConnectFakeTransports(); // Channel 2 do not send anything. typename T::Content content2; CreateContent(0, kPcmuCodec, kH264Codec, &content2); EXPECT_TRUE(channel1_->SetRemoteContent(&content2, SdpType::kAnswer, err)); - EXPECT_EQ(0u, media_receive_channel1_impl()->recv_streams().size()); + EXPECT_EQ(0u, media_channel1()->recv_streams().size()); EXPECT_TRUE(channel2_->SetLocalContent(&content2, SdpType::kAnswer, err)); channel2_->Enable(true); - EXPECT_EQ(0u, media_send_channel2_impl()->send_streams().size()); + EXPECT_EQ(0u, media_channel2()->send_streams().size()); SendCustomRtp1(kSsrc1, 0); WaitForThreads(); @@ -720,21 +707,21 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { CreateContent(0, kPcmuCodec, kH264Codec, &content3); content3.AddStream(stream2); EXPECT_TRUE(channel2_->SetLocalContent(&content3, SdpType::kOffer, err)); - ASSERT_EQ(1u, media_send_channel2_impl()->send_streams().size()); - EXPECT_EQ(stream2, media_send_channel2_impl()->send_streams()[0]); + ASSERT_EQ(1u, media_channel2()->send_streams().size()); + EXPECT_EQ(stream2, media_channel2()->send_streams()[0]); EXPECT_TRUE(channel1_->SetRemoteContent(&content3, SdpType::kOffer, err)); - ASSERT_EQ(1u, media_receive_channel1_impl()->recv_streams().size()); - EXPECT_EQ(stream2, media_receive_channel1_impl()->recv_streams()[0]); + ASSERT_EQ(1u, media_channel1()->recv_streams().size()); + EXPECT_EQ(stream2, media_channel1()->recv_streams()[0]); // Channel 1 replies but stop sending stream1. typename T::Content content4; CreateContent(0, kPcmuCodec, kH264Codec, &content4); EXPECT_TRUE(channel1_->SetLocalContent(&content4, SdpType::kAnswer, err)); - EXPECT_EQ(0u, media_send_channel1_impl()->send_streams().size()); + EXPECT_EQ(0u, media_channel1()->send_streams().size()); EXPECT_TRUE(channel2_->SetRemoteContent(&content4, SdpType::kAnswer, err)); - EXPECT_EQ(0u, media_receive_channel2_impl()->recv_streams().size()); + EXPECT_EQ(0u, media_channel2()->recv_streams().size()); SendCustomRtp2(kSsrc2, 0); WaitForThreads(); @@ -745,59 +732,59 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { void TestPlayoutAndSendingStates() { CreateChannels(0, 0); if (verify_playout_) { - EXPECT_FALSE(media_receive_channel1_impl()->playout()); + EXPECT_FALSE(media_channel1()->playout()); } - EXPECT_FALSE(media_send_channel1_impl()->sending()); + EXPECT_FALSE(media_channel1()->sending()); if (verify_playout_) { - EXPECT_FALSE(media_receive_channel2_impl()->playout()); + EXPECT_FALSE(media_channel2()->playout()); } - EXPECT_FALSE(media_send_channel2_impl()->sending()); + EXPECT_FALSE(media_channel2()->sending()); channel1_->Enable(true); FlushCurrentThread(); if (verify_playout_) { - EXPECT_FALSE(media_receive_channel1_impl()->playout()); + EXPECT_FALSE(media_channel1()->playout()); } - EXPECT_FALSE(media_send_channel1_impl()->sending()); + EXPECT_FALSE(media_channel1()->sending()); std::string err; EXPECT_TRUE(channel1_->SetLocalContent(&local_media_content1_, SdpType::kOffer, err)); if (verify_playout_) { - EXPECT_TRUE(media_receive_channel1_impl()->playout()); + EXPECT_TRUE(media_channel1()->playout()); } - EXPECT_FALSE(media_send_channel1_impl()->sending()); + EXPECT_FALSE(media_channel1()->sending()); EXPECT_TRUE(channel2_->SetRemoteContent(&local_media_content1_, SdpType::kOffer, err)); if (verify_playout_) { - EXPECT_FALSE(media_receive_channel2_impl()->playout()); + EXPECT_FALSE(media_channel2()->playout()); } - EXPECT_FALSE(media_send_channel2_impl()->sending()); + EXPECT_FALSE(media_channel2()->sending()); EXPECT_TRUE(channel2_->SetLocalContent(&local_media_content2_, SdpType::kAnswer, err)); if (verify_playout_) { - EXPECT_FALSE(media_receive_channel2_impl()->playout()); + EXPECT_FALSE(media_channel2()->playout()); } - EXPECT_FALSE(media_send_channel2_impl()->sending()); + EXPECT_FALSE(media_channel2()->sending()); ConnectFakeTransports(); if (verify_playout_) { - EXPECT_TRUE(media_receive_channel1_impl()->playout()); + EXPECT_TRUE(media_channel1()->playout()); } - EXPECT_FALSE(media_send_channel1_impl()->sending()); + EXPECT_FALSE(media_channel1()->sending()); if (verify_playout_) { - EXPECT_FALSE(media_receive_channel2_impl()->playout()); + EXPECT_FALSE(media_channel2()->playout()); } - EXPECT_FALSE(media_send_channel2_impl()->sending()); + EXPECT_FALSE(media_channel2()->sending()); channel2_->Enable(true); FlushCurrentThread(); if (verify_playout_) { - EXPECT_TRUE(media_receive_channel2_impl()->playout()); + EXPECT_TRUE(media_channel2()->playout()); } - EXPECT_TRUE(media_send_channel2_impl()->sending()); + EXPECT_TRUE(media_channel2()->sending()); EXPECT_TRUE(channel1_->SetRemoteContent(&local_media_content2_, SdpType::kAnswer, err)); if (verify_playout_) { - EXPECT_TRUE(media_receive_channel1_impl()->playout()); + EXPECT_TRUE(media_channel1()->playout()); } - EXPECT_TRUE(media_send_channel1_impl()->sending()); + EXPECT_TRUE(media_channel1()->sending()); } // Test that changing the MediaContentDirection in the local and remote @@ -815,13 +802,13 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { channel2_->Enable(true); FlushCurrentThread(); if (verify_playout_) { - EXPECT_FALSE(media_receive_channel1_impl()->playout()); + EXPECT_FALSE(media_channel1()->playout()); } - EXPECT_FALSE(media_send_channel1_impl()->sending()); + EXPECT_FALSE(media_channel1()->sending()); if (verify_playout_) { - EXPECT_FALSE(media_receive_channel2_impl()->playout()); + EXPECT_FALSE(media_channel2()->playout()); } - EXPECT_FALSE(media_send_channel2_impl()->sending()); + EXPECT_FALSE(media_channel2()->sending()); std::string err; EXPECT_TRUE(channel1_->SetLocalContent(&content1, SdpType::kOffer, err)); @@ -832,13 +819,13 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { ConnectFakeTransports(); if (verify_playout_) { - EXPECT_TRUE(media_receive_channel1_impl()->playout()); + EXPECT_TRUE(media_channel1()->playout()); } - EXPECT_FALSE(media_send_channel1_impl()->sending()); // remote InActive + EXPECT_FALSE(media_channel1()->sending()); // remote InActive if (verify_playout_) { - EXPECT_FALSE(media_receive_channel2_impl()->playout()); // local InActive + EXPECT_FALSE(media_channel2()->playout()); // local InActive } - EXPECT_FALSE(media_send_channel2_impl()->sending()); // local InActive + EXPECT_FALSE(media_channel2()->sending()); // local InActive // Update `content2` to be RecvOnly. content2.set_direction(RtpTransceiverDirection::kRecvOnly); @@ -847,13 +834,13 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { channel1_->SetRemoteContent(&content2, SdpType::kPrAnswer, err)); if (verify_playout_) { - EXPECT_TRUE(media_receive_channel1_impl()->playout()); + EXPECT_TRUE(media_channel1()->playout()); } - EXPECT_TRUE(media_send_channel1_impl()->sending()); + EXPECT_TRUE(media_channel1()->sending()); if (verify_playout_) { - EXPECT_TRUE(media_receive_channel2_impl()->playout()); // local RecvOnly + EXPECT_TRUE(media_channel2()->playout()); // local RecvOnly } - EXPECT_FALSE(media_send_channel2_impl()->sending()); // local RecvOnly + EXPECT_FALSE(media_channel2()->sending()); // local RecvOnly // Update `content2` to be SendRecv. content2.set_direction(RtpTransceiverDirection::kSendRecv); @@ -861,13 +848,13 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { EXPECT_TRUE(channel1_->SetRemoteContent(&content2, SdpType::kAnswer, err)); if (verify_playout_) { - EXPECT_TRUE(media_receive_channel1_impl()->playout()); + EXPECT_TRUE(media_channel1()->playout()); } - EXPECT_TRUE(media_send_channel1_impl()->sending()); + EXPECT_TRUE(media_channel1()->sending()); if (verify_playout_) { - EXPECT_TRUE(media_receive_channel2_impl()->playout()); + EXPECT_TRUE(media_channel2()->playout()); } - EXPECT_TRUE(media_send_channel2_impl()->sending()); + EXPECT_TRUE(media_channel2()->sending()); } // Tests that when the transport channel signals a candidate pair change @@ -883,15 +870,14 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { CreateChannels(DTLS, DTLS); SendInitiate(); - typename T::MediaChannel* media_send_channel1_impl = - this->media_send_channel1_impl(); - ASSERT_TRUE(media_send_channel1_impl); + typename T::MediaChannel* media_channel1 = this->media_channel1(); + ASSERT_TRUE(media_channel1); // Need to wait for the threads before calling // `set_num_network_route_changes` because the network route would be set // when creating the channel. WaitForThreads(); - media_send_channel1_impl->set_num_network_route_changes(0); + media_channel1->set_num_network_route_changes(0); SendTask(network_thread_, [this] { rtc::NetworkRoute network_route; // The transport channel becomes disconnected. @@ -899,9 +885,9 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { absl::optional(network_route)); }); WaitForThreads(); - EXPECT_EQ(1, media_send_channel1_impl->num_network_route_changes()); - EXPECT_FALSE(media_send_channel1_impl->last_network_route().connected); - media_send_channel1_impl->set_num_network_route_changes(0); + EXPECT_EQ(1, media_channel1->num_network_route_changes()); + EXPECT_FALSE(media_channel1->last_network_route().connected); + media_channel1->set_num_network_route_changes(0); SendTask(network_thread_, [this] { rtc::NetworkRoute network_route; @@ -918,19 +904,16 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { absl::optional(network_route)); }); WaitForThreads(); - EXPECT_EQ(1, media_send_channel1_impl->num_network_route_changes()); - EXPECT_TRUE(media_send_channel1_impl->last_network_route().connected); - EXPECT_EQ( - kLocalNetId, - media_send_channel1_impl->last_network_route().local.network_id()); - EXPECT_EQ( - kRemoteNetId, - media_send_channel1_impl->last_network_route().remote.network_id()); - EXPECT_EQ( - kLastPacketId, - media_send_channel1_impl->last_network_route().last_sent_packet_id); + EXPECT_EQ(1, media_channel1->num_network_route_changes()); + EXPECT_TRUE(media_channel1->last_network_route().connected); + EXPECT_EQ(kLocalNetId, + media_channel1->last_network_route().local.network_id()); + EXPECT_EQ(kRemoteNetId, + media_channel1->last_network_route().remote.network_id()); + EXPECT_EQ(kLastPacketId, + media_channel1->last_network_route().last_sent_packet_id); EXPECT_EQ(kTransportOverheadPerPacket + kSrtpOverheadPerPacket, - media_send_channel1_impl->transport_overhead_per_packet()); + media_channel1->transport_overhead_per_packet()); } // Test setting up a call. @@ -939,18 +922,18 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { EXPECT_FALSE(IsSrtpActive(channel1_)); EXPECT_TRUE(SendInitiate()); if (verify_playout_) { - EXPECT_TRUE(media_receive_channel1_impl()->playout()); + EXPECT_TRUE(media_channel1()->playout()); } - EXPECT_FALSE(media_send_channel1_impl()->sending()); + EXPECT_FALSE(media_channel1()->sending()); EXPECT_TRUE(SendAccept()); EXPECT_FALSE(IsSrtpActive(channel1_)); - EXPECT_TRUE(media_send_channel1_impl()->sending()); - EXPECT_EQ(1U, media_send_channel1_impl()->codecs().size()); + EXPECT_TRUE(media_channel1()->sending()); + EXPECT_EQ(1U, media_channel1()->codecs().size()); if (verify_playout_) { - EXPECT_TRUE(media_receive_channel2_impl()->playout()); + EXPECT_TRUE(media_channel2()->playout()); } - EXPECT_TRUE(media_send_channel2_impl()->sending()); - EXPECT_EQ(1U, media_send_channel2_impl()->codecs().size()); + EXPECT_TRUE(media_channel2()->sending()); + EXPECT_EQ(1U, media_channel2()->codecs().size()); } // Send voice RTP data to the other side and ensure it gets there. @@ -1077,7 +1060,7 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { // Regain writability SendTask(network_thread_, [this] { fake_rtp_dtls_transport1_->SetWritable(true); }); - EXPECT_TRUE(media_send_channel1_impl()->sending()); + EXPECT_TRUE(media_channel1()->sending()); SendRtp1(); SendRtp2(); WaitForThreads(); @@ -1091,7 +1074,7 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { bool asymmetric = true; fake_rtp_dtls_transport1_->SetDestination(nullptr, asymmetric); }); - EXPECT_TRUE(media_send_channel1_impl()->sending()); + EXPECT_TRUE(media_channel1()->sending()); // Should fail also. SendRtp1(); @@ -1107,7 +1090,7 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { fake_rtp_dtls_transport1_->SetDestination(fake_rtp_dtls_transport2_.get(), asymmetric); }); - EXPECT_TRUE(media_send_channel1_impl()->sending()); + EXPECT_TRUE(media_channel1()->sending()); SendRtp1(); SendRtp2(); WaitForThreads(); @@ -1160,17 +1143,17 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { std::unique_ptr content( CreateMediaContentWithStream(1)); - media_receive_channel1_impl()->set_fail_set_recv_codecs(true); + media_channel1()->set_fail_set_recv_codecs(true); EXPECT_FALSE( channel1_->SetLocalContent(content.get(), SdpType::kOffer, err)); EXPECT_FALSE( channel1_->SetLocalContent(content.get(), SdpType::kAnswer, err)); - media_send_channel1_impl()->set_fail_set_send_codecs(true); + media_channel1()->set_fail_set_send_codecs(true); EXPECT_FALSE( channel1_->SetRemoteContent(content.get(), SdpType::kOffer, err)); - media_send_channel1_impl()->set_fail_set_send_codecs(true); + media_channel1()->set_fail_set_send_codecs(true); EXPECT_FALSE( channel1_->SetRemoteContent(content.get(), SdpType::kAnswer, err)); } @@ -1183,14 +1166,14 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { CreateMediaContentWithStream(1)); EXPECT_TRUE( channel1_->SetLocalContent(content1.get(), SdpType::kOffer, err)); - EXPECT_TRUE(media_send_channel1_impl()->HasSendStream(1)); + EXPECT_TRUE(media_channel1()->HasSendStream(1)); std::unique_ptr content2( CreateMediaContentWithStream(2)); EXPECT_TRUE( channel1_->SetLocalContent(content2.get(), SdpType::kOffer, err)); - EXPECT_FALSE(media_send_channel1_impl()->HasSendStream(1)); - EXPECT_TRUE(media_send_channel1_impl()->HasSendStream(2)); + EXPECT_FALSE(media_channel1()->HasSendStream(1)); + EXPECT_TRUE(media_channel1()->HasSendStream(2)); } void TestReceiveTwoOffers() { @@ -1201,14 +1184,14 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { CreateMediaContentWithStream(1)); EXPECT_TRUE( channel1_->SetRemoteContent(content1.get(), SdpType::kOffer, err)); - EXPECT_TRUE(media_receive_channel1_impl()->HasRecvStream(1)); + EXPECT_TRUE(media_channel1()->HasRecvStream(1)); std::unique_ptr content2( CreateMediaContentWithStream(2)); EXPECT_TRUE( channel1_->SetRemoteContent(content2.get(), SdpType::kOffer, err)); - EXPECT_FALSE(media_receive_channel1_impl()->HasRecvStream(1)); - EXPECT_TRUE(media_receive_channel1_impl()->HasRecvStream(2)); + EXPECT_FALSE(media_channel1()->HasRecvStream(1)); + EXPECT_TRUE(media_channel1()->HasRecvStream(2)); } void TestSendPrAnswer() { @@ -1220,24 +1203,24 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { CreateMediaContentWithStream(1)); EXPECT_TRUE( channel1_->SetRemoteContent(content1.get(), SdpType::kOffer, err)); - EXPECT_TRUE(media_receive_channel1_impl()->HasRecvStream(1)); + EXPECT_TRUE(media_channel1()->HasRecvStream(1)); // Send PR answer std::unique_ptr content2( CreateMediaContentWithStream(2)); EXPECT_TRUE( channel1_->SetLocalContent(content2.get(), SdpType::kPrAnswer, err)); - EXPECT_TRUE(media_receive_channel1_impl()->HasRecvStream(1)); - EXPECT_TRUE(media_send_channel1_impl()->HasSendStream(2)); + EXPECT_TRUE(media_channel1()->HasRecvStream(1)); + EXPECT_TRUE(media_channel1()->HasSendStream(2)); // Send answer std::unique_ptr content3( CreateMediaContentWithStream(3)); EXPECT_TRUE( channel1_->SetLocalContent(content3.get(), SdpType::kAnswer, err)); - EXPECT_TRUE(media_receive_channel1_impl()->HasRecvStream(1)); - EXPECT_FALSE(media_send_channel1_impl()->HasSendStream(2)); - EXPECT_TRUE(media_send_channel1_impl()->HasSendStream(3)); + EXPECT_TRUE(media_channel1()->HasRecvStream(1)); + EXPECT_FALSE(media_channel1()->HasSendStream(2)); + EXPECT_TRUE(media_channel1()->HasSendStream(3)); } void TestReceivePrAnswer() { @@ -1249,39 +1232,39 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { CreateMediaContentWithStream(1)); EXPECT_TRUE( channel1_->SetLocalContent(content1.get(), SdpType::kOffer, err)); - EXPECT_TRUE(media_send_channel1_impl()->HasSendStream(1)); + EXPECT_TRUE(media_channel1()->HasSendStream(1)); // Receive PR answer std::unique_ptr content2( CreateMediaContentWithStream(2)); EXPECT_TRUE( channel1_->SetRemoteContent(content2.get(), SdpType::kPrAnswer, err)); - EXPECT_TRUE(media_send_channel1_impl()->HasSendStream(1)); - EXPECT_TRUE(media_receive_channel1_impl()->HasRecvStream(2)); + EXPECT_TRUE(media_channel1()->HasSendStream(1)); + EXPECT_TRUE(media_channel1()->HasRecvStream(2)); // Receive answer std::unique_ptr content3( CreateMediaContentWithStream(3)); EXPECT_TRUE( channel1_->SetRemoteContent(content3.get(), SdpType::kAnswer, err)); - EXPECT_TRUE(media_send_channel1_impl()->HasSendStream(1)); - EXPECT_FALSE(media_receive_channel1_impl()->HasRecvStream(2)); - EXPECT_TRUE(media_receive_channel1_impl()->HasRecvStream(3)); + EXPECT_TRUE(media_channel1()->HasSendStream(1)); + EXPECT_FALSE(media_channel1()->HasRecvStream(2)); + EXPECT_TRUE(media_channel1()->HasRecvStream(3)); } void TestOnTransportReadyToSend() { CreateChannels(0, 0); - EXPECT_FALSE(media_send_channel1_impl()->ready_to_send()); + EXPECT_FALSE(media_channel1()->ready_to_send()); network_thread_->PostTask( [this] { channel1_->OnTransportReadyToSend(true); }); WaitForThreads(); - EXPECT_TRUE(media_send_channel1_impl()->ready_to_send()); + EXPECT_TRUE(media_channel1()->ready_to_send()); network_thread_->PostTask( [this] { channel1_->OnTransportReadyToSend(false); }); WaitForThreads(); - EXPECT_FALSE(media_send_channel1_impl()->ready_to_send()); + EXPECT_FALSE(media_channel1()->ready_to_send()); } bool SetRemoteContentWithBitrateLimit(int remote_limit) { @@ -1310,7 +1293,7 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { std::string err; EXPECT_TRUE(channel1_->SetLocalContent(&local_media_content1_, SdpType::kOffer, err)); - EXPECT_EQ(media_send_channel1_impl()->max_bps(), -1); + EXPECT_EQ(media_channel1()->max_bps(), -1); VerifyMaxBitrate(media_send_channel1()->GetRtpSendParameters(kSsrc1), absl::nullopt); } @@ -1428,49 +1411,31 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { ProcessThreadQueue(rtc::Thread::Current()); } - // Accessors that return the standard VideoMedia{Send|Receive}ChannelInterface - typename T::MediaSendChannel* media_send_channel1() { - return channel1_->media_send_channel(); - } - typename T::MediaSendChannel* media_send_channel2() { - return channel2_->media_send_channel(); - } - typename T::MediaReceiveChannel* media_receive_channel1() { - return channel1_->media_receive_channel(); - } - typename T::MediaReceiveChannel* media_receive_channel2() { - return channel2_->media_receive_channel(); - } - // Accessors that return the FakeMediaChannel object. // Note that these depend on getting the object back that was // passed to the channel constructor. - // T::MediaChannel is either FakeVoiceMediaChannel or FakeVideoMediaChannel. - typename T::MediaChannel* media_send_channel1_impl() { + typename T::MediaChannel* media_channel1() { + RTC_DCHECK(channel1_); + RTC_DCHECK(channel1_->media_channel()); + return static_cast(channel1_->media_channel()); + } + + typename T::MediaChannel* media_channel2() { + RTC_DCHECK(channel2_); + RTC_DCHECK(channel2_->media_channel()); + return static_cast(channel2_->media_channel()); + } + + typename T::MediaSendChannel* media_send_channel1() { RTC_DCHECK(channel1_); RTC_DCHECK(channel1_->media_send_channel()); - return static_cast( - channel1_->media_send_channel()->ImplForTesting()); + return channel1_->media_send_channel(); } - typename T::MediaChannel* media_send_channel2_impl() { + typename T::MediaSendChannel* media_send_channel2() { RTC_DCHECK(channel2_); RTC_DCHECK(channel2_->media_send_channel()); - return static_cast( - channel2_->media_send_channel()->ImplForTesting()); - } - typename T::MediaChannel* media_receive_channel1_impl() { - RTC_DCHECK(channel1_); - RTC_DCHECK(channel1_->media_receive_channel()); - return static_cast( - channel1_->media_receive_channel()->ImplForTesting()); - } - - typename T::MediaChannel* media_receive_channel2_impl() { - RTC_DCHECK(channel2_); - RTC_DCHECK(channel2_->media_receive_channel()); - return static_cast( - channel2_->media_receive_channel()->ImplForTesting()); + return channel2_->media_send_channel(); } rtc::AutoThread main_thread_; @@ -1511,15 +1476,14 @@ template <> std::unique_ptr ChannelTest::CreateChannel( rtc::Thread* worker_thread, rtc::Thread* network_thread, - std::unique_ptr send_ch, - std::unique_ptr receive_ch, + std::unique_ptr ch, webrtc::RtpTransportInternal* rtp_transport, int flags) { rtc::Thread* signaling_thread = rtc::Thread::Current(); auto channel = std::make_unique( - worker_thread, network_thread, signaling_thread, std::move(send_ch), - std::move(receive_ch), cricket::CN_AUDIO, (flags & DTLS) != 0, - webrtc::CryptoOptions(), &ssrc_generator_); + worker_thread, network_thread, signaling_thread, std::move(ch), + cricket::CN_AUDIO, (flags & DTLS) != 0, webrtc::CryptoOptions(), + &ssrc_generator_); SendTask(network_thread, [&]() { RTC_DCHECK_RUN_ON(channel->network_thread()); channel->SetRtpTransport(rtp_transport); @@ -1598,15 +1562,14 @@ template <> std::unique_ptr ChannelTest::CreateChannel( rtc::Thread* worker_thread, rtc::Thread* network_thread, - std::unique_ptr send_ch, - std::unique_ptr receive_ch, + std::unique_ptr ch, webrtc::RtpTransportInternal* rtp_transport, int flags) { rtc::Thread* signaling_thread = rtc::Thread::Current(); auto channel = std::make_unique( - worker_thread, network_thread, signaling_thread, std::move(send_ch), - std::move(receive_ch), cricket::CN_VIDEO, (flags & DTLS) != 0, - webrtc::CryptoOptions(), &ssrc_generator_); + worker_thread, network_thread, signaling_thread, std::move(ch), + cricket::CN_VIDEO, (flags & DTLS) != 0, webrtc::CryptoOptions(), + &ssrc_generator_); SendTask(network_thread, [&]() { RTC_DCHECK_RUN_ON(channel->network_thread()); channel->SetRtpTransport(rtp_transport); @@ -1661,8 +1624,8 @@ class VideoChannelDoubleThreadTest : public ChannelTest { TEST_F(VoiceChannelSingleThreadTest, TestInit) { Base::TestInit(); - EXPECT_FALSE(media_send_channel1_impl()->IsStreamMuted(0)); - EXPECT_TRUE(media_send_channel1_impl()->dtmf_info_queue().empty()); + EXPECT_FALSE(media_channel1()->IsStreamMuted(0)); + EXPECT_TRUE(media_channel1()->dtmf_info_queue().empty()); } TEST_F(VoiceChannelSingleThreadTest, TestDeinit) { @@ -1798,8 +1761,8 @@ TEST_F(VoiceChannelSingleThreadTest, SocketOptionsMergedOnSetTransport) { // VoiceChannelDoubleThreadTest TEST_F(VoiceChannelDoubleThreadTest, TestInit) { Base::TestInit(); - EXPECT_FALSE(media_send_channel1_impl()->IsStreamMuted(0)); - EXPECT_TRUE(media_send_channel1_impl()->dtmf_info_queue().empty()); + EXPECT_FALSE(media_channel1()->IsStreamMuted(0)); + EXPECT_TRUE(media_channel1()->dtmf_info_queue().empty()); } TEST_F(VoiceChannelDoubleThreadTest, TestDeinit) { @@ -2082,15 +2045,14 @@ TEST_F(VideoChannelSingleThreadTest, TestSetLocalOfferWithPacketization) { std::string err; EXPECT_TRUE(channel1_->SetLocalContent(&video, SdpType::kOffer, err)); - EXPECT_THAT(media_send_channel1_impl()->send_codecs(), testing::IsEmpty()); - ASSERT_THAT(media_receive_channel1_impl()->recv_codecs(), testing::SizeIs(2)); - EXPECT_TRUE(media_receive_channel1_impl()->recv_codecs()[0].Matches( - kVp8Codec, &field_trials_)); - EXPECT_EQ(media_receive_channel1_impl()->recv_codecs()[0].packetization, - absl::nullopt); - EXPECT_TRUE(media_receive_channel1_impl()->recv_codecs()[1].Matches( - vp9_codec, &field_trials_)); - EXPECT_EQ(media_receive_channel1_impl()->recv_codecs()[1].packetization, + EXPECT_THAT(media_channel1()->send_codecs(), testing::IsEmpty()); + ASSERT_THAT(media_channel1()->recv_codecs(), testing::SizeIs(2)); + EXPECT_TRUE( + media_channel1()->recv_codecs()[0].Matches(kVp8Codec, &field_trials_)); + EXPECT_EQ(media_channel1()->recv_codecs()[0].packetization, absl::nullopt); + EXPECT_TRUE( + media_channel1()->recv_codecs()[1].Matches(vp9_codec, &field_trials_)); + EXPECT_EQ(media_channel1()->recv_codecs()[1].packetization, cricket::kPacketizationParamRaw); } @@ -2106,15 +2068,14 @@ TEST_F(VideoChannelSingleThreadTest, TestSetRemoteOfferWithPacketization) { std::string err; EXPECT_TRUE(channel1_->SetRemoteContent(&video, SdpType::kOffer, err)); EXPECT_TRUE(err.empty()); - EXPECT_THAT(media_receive_channel1_impl()->recv_codecs(), testing::IsEmpty()); - ASSERT_THAT(media_send_channel1_impl()->send_codecs(), testing::SizeIs(2)); - EXPECT_TRUE(media_send_channel1_impl()->send_codecs()[0].Matches( - kVp8Codec, &field_trials_)); - EXPECT_EQ(media_send_channel1_impl()->send_codecs()[0].packetization, - absl::nullopt); - EXPECT_TRUE(media_send_channel1_impl()->send_codecs()[1].Matches( - vp9_codec, &field_trials_)); - EXPECT_EQ(media_send_channel1_impl()->send_codecs()[1].packetization, + EXPECT_THAT(media_channel1()->recv_codecs(), testing::IsEmpty()); + ASSERT_THAT(media_channel1()->send_codecs(), testing::SizeIs(2)); + EXPECT_TRUE( + media_channel1()->send_codecs()[0].Matches(kVp8Codec, &field_trials_)); + EXPECT_EQ(media_channel1()->send_codecs()[0].packetization, absl::nullopt); + EXPECT_TRUE( + media_channel1()->send_codecs()[1].Matches(vp9_codec, &field_trials_)); + EXPECT_EQ(media_channel1()->send_codecs()[1].packetization, cricket::kPacketizationParamRaw); } @@ -2132,23 +2093,21 @@ TEST_F(VideoChannelSingleThreadTest, TestSetAnswerWithPacketization) { EXPECT_TRUE(err.empty()); EXPECT_TRUE(channel1_->SetRemoteContent(&video, SdpType::kAnswer, err)); EXPECT_TRUE(err.empty()); - ASSERT_THAT(media_receive_channel1_impl()->recv_codecs(), testing::SizeIs(2)); - EXPECT_TRUE(media_receive_channel1_impl()->recv_codecs()[0].Matches( - kVp8Codec, &field_trials_)); - EXPECT_EQ(media_receive_channel1_impl()->recv_codecs()[0].packetization, - absl::nullopt); - EXPECT_TRUE(media_receive_channel1_impl()->recv_codecs()[1].Matches( - vp9_codec, &field_trials_)); - EXPECT_EQ(media_receive_channel1_impl()->recv_codecs()[1].packetization, + ASSERT_THAT(media_channel1()->recv_codecs(), testing::SizeIs(2)); + EXPECT_TRUE( + media_channel1()->recv_codecs()[0].Matches(kVp8Codec, &field_trials_)); + EXPECT_EQ(media_channel1()->recv_codecs()[0].packetization, absl::nullopt); + EXPECT_TRUE( + media_channel1()->recv_codecs()[1].Matches(vp9_codec, &field_trials_)); + EXPECT_EQ(media_channel1()->recv_codecs()[1].packetization, cricket::kPacketizationParamRaw); - EXPECT_THAT(media_send_channel1_impl()->send_codecs(), testing::SizeIs(2)); - EXPECT_TRUE(media_send_channel1_impl()->send_codecs()[0].Matches( - kVp8Codec, &field_trials_)); - EXPECT_EQ(media_send_channel1_impl()->send_codecs()[0].packetization, - absl::nullopt); - EXPECT_TRUE(media_send_channel1_impl()->send_codecs()[1].Matches( - vp9_codec, &field_trials_)); - EXPECT_EQ(media_send_channel1_impl()->send_codecs()[1].packetization, + EXPECT_THAT(media_channel1()->send_codecs(), testing::SizeIs(2)); + EXPECT_TRUE( + media_channel1()->send_codecs()[0].Matches(kVp8Codec, &field_trials_)); + EXPECT_EQ(media_channel1()->send_codecs()[0].packetization, absl::nullopt); + EXPECT_TRUE( + media_channel1()->send_codecs()[1].Matches(vp9_codec, &field_trials_)); + EXPECT_EQ(media_channel1()->send_codecs()[1].packetization, cricket::kPacketizationParamRaw); } @@ -2166,12 +2125,10 @@ TEST_F(VideoChannelSingleThreadTest, TestSetLocalAnswerWithoutPacketization) { std::string err; EXPECT_TRUE(channel1_->SetRemoteContent(&remote_video, SdpType::kOffer, err)); EXPECT_TRUE(channel1_->SetLocalContent(&local_video, SdpType::kAnswer, err)); - ASSERT_THAT(media_receive_channel1_impl()->recv_codecs(), testing::SizeIs(1)); - EXPECT_EQ(media_receive_channel1_impl()->recv_codecs()[0].packetization, - absl::nullopt); - ASSERT_THAT(media_send_channel1_impl()->send_codecs(), testing::SizeIs(1)); - EXPECT_EQ(media_send_channel1_impl()->send_codecs()[0].packetization, - absl::nullopt); + ASSERT_THAT(media_channel1()->recv_codecs(), testing::SizeIs(1)); + EXPECT_EQ(media_channel1()->recv_codecs()[0].packetization, absl::nullopt); + ASSERT_THAT(media_channel1()->send_codecs(), testing::SizeIs(1)); + EXPECT_EQ(media_channel1()->send_codecs()[0].packetization, absl::nullopt); } TEST_F(VideoChannelSingleThreadTest, TestSetRemoteAnswerWithoutPacketization) { @@ -2189,12 +2146,10 @@ TEST_F(VideoChannelSingleThreadTest, TestSetRemoteAnswerWithoutPacketization) { EXPECT_TRUE(channel1_->SetLocalContent(&local_video, SdpType::kOffer, err)); EXPECT_TRUE( channel1_->SetRemoteContent(&remote_video, SdpType::kAnswer, err)); - ASSERT_THAT(media_receive_channel1_impl()->recv_codecs(), testing::SizeIs(1)); - EXPECT_EQ(media_receive_channel1_impl()->recv_codecs()[0].packetization, - absl::nullopt); - ASSERT_THAT(media_send_channel1_impl()->send_codecs(), testing::SizeIs(1)); - EXPECT_EQ(media_send_channel1_impl()->send_codecs()[0].packetization, - absl::nullopt); + ASSERT_THAT(media_channel1()->recv_codecs(), testing::SizeIs(1)); + EXPECT_EQ(media_channel1()->recv_codecs()[0].packetization, absl::nullopt); + ASSERT_THAT(media_channel1()->send_codecs(), testing::SizeIs(1)); + EXPECT_EQ(media_channel1()->send_codecs()[0].packetization, absl::nullopt); } TEST_F(VideoChannelSingleThreadTest, @@ -2216,10 +2171,10 @@ TEST_F(VideoChannelSingleThreadTest, EXPECT_FALSE( channel1_->SetRemoteContent(&remote_video, SdpType::kAnswer, err)); EXPECT_FALSE(err.empty()); - ASSERT_THAT(media_receive_channel1_impl()->recv_codecs(), testing::SizeIs(1)); - EXPECT_EQ(media_receive_channel1_impl()->recv_codecs()[0].packetization, + ASSERT_THAT(media_channel1()->recv_codecs(), testing::SizeIs(1)); + EXPECT_EQ(media_channel1()->recv_codecs()[0].packetization, cricket::kPacketizationParamRaw); - EXPECT_THAT(media_send_channel1_impl()->send_codecs(), testing::IsEmpty()); + EXPECT_THAT(media_channel1()->send_codecs(), testing::IsEmpty()); } TEST_F(VideoChannelSingleThreadTest, @@ -2239,10 +2194,9 @@ TEST_F(VideoChannelSingleThreadTest, EXPECT_TRUE(err.empty()); EXPECT_FALSE(channel1_->SetLocalContent(&local_video, SdpType::kAnswer, err)); EXPECT_FALSE(err.empty()); - EXPECT_THAT(media_receive_channel1_impl()->recv_codecs(), testing::IsEmpty()); - ASSERT_THAT(media_send_channel1_impl()->send_codecs(), testing::SizeIs(1)); - EXPECT_EQ(media_send_channel1_impl()->send_codecs()[0].packetization, - absl::nullopt); + EXPECT_THAT(media_channel1()->recv_codecs(), testing::IsEmpty()); + ASSERT_THAT(media_channel1()->send_codecs(), testing::SizeIs(1)); + EXPECT_EQ(media_channel1()->send_codecs()[0].packetization, absl::nullopt); } // VideoChannelDoubleThreadTest diff --git a/pc/legacy_stats_collector_unittest.cc b/pc/legacy_stats_collector_unittest.cc index 6d3fd9f462..3033172e95 100644 --- a/pc/legacy_stats_collector_unittest.cc +++ b/pc/legacy_stats_collector_unittest.cc @@ -952,9 +952,8 @@ TEST_P(StatsCollectorTrackTest, AudioBandwidthEstimationInfoIsReported) { VoiceMediaInfo voice_info; voice_info.senders.push_back(voice_sender_info); - auto voice_media_channels = pc->AddVoiceChannel("audio", "transport"); - voice_media_channels.first->SetStats(voice_info); - voice_media_channels.second->SetStats(voice_info); + auto* voice_media_channel = pc->AddVoiceChannel("audio", "transport"); + voice_media_channel->SetStats(voice_info); AddOutgoingAudioTrack(pc.get(), stats.get()); @@ -1527,9 +1526,8 @@ TEST_P(StatsCollectorTrackTest, FilterOutNegativeInitialValues) { voice_info.senders.push_back(voice_sender_info); voice_info.receivers.push_back(voice_receiver_info); - auto voice_media_channels = pc->AddVoiceChannel("voice", "transport"); - voice_media_channels.first->SetStats(voice_info); - voice_media_channels.second->SetStats(voice_info); + auto* voice_media_channel = pc->AddVoiceChannel("voice", "transport"); + voice_media_channel->SetStats(voice_info); stats->UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard); @@ -1580,9 +1578,8 @@ TEST_P(StatsCollectorTrackTest, GetStatsFromLocalAudioTrack) { VoiceMediaInfo voice_info; voice_info.senders.push_back(voice_sender_info); - auto voice_media_channels = pc->AddVoiceChannel("audio", "transport"); - voice_media_channels.first->SetStats(voice_info); - voice_media_channels.second->SetStats(voice_info); + auto* voice_media_channel = pc->AddVoiceChannel("audio", "transport"); + voice_media_channel->SetStats(voice_info); StatsReports reports; // returned values. VerifyAudioTrackStats(audio_track_.get(), stats.get(), voice_info, &reports); @@ -1608,9 +1605,8 @@ TEST_P(StatsCollectorTrackTest, GetStatsFromRemoteStream) { VoiceMediaInfo voice_info; voice_info.receivers.push_back(voice_receiver_info); - auto voice_media_channels = pc->AddVoiceChannel("audio", "transport"); - voice_media_channels.first->SetStats(voice_info); - voice_media_channels.second->SetStats(voice_info); + auto* voice_media_channel = pc->AddVoiceChannel("audio", "transport"); + voice_media_channel->SetStats(voice_info); StatsReports reports; // returned values. VerifyAudioTrackStats(audio_track_.get(), stats.get(), voice_info, &reports); @@ -1630,9 +1626,8 @@ TEST_P(StatsCollectorTrackTest, GetStatsAfterRemoveAudioStream) { VoiceMediaInfo voice_info; voice_info.senders.push_back(voice_sender_info); - auto voice_media_channels = pc->AddVoiceChannel("audio", "transport"); - voice_media_channels.first->SetStats(voice_info); - voice_media_channels.second->SetStats(voice_info); + auto* voice_media_channel = pc->AddVoiceChannel("audio", "transport"); + voice_media_channel->SetStats(voice_info); stats->RemoveLocalAudioTrack(audio_track_.get(), kSsrcOfTrack); @@ -1693,9 +1688,8 @@ TEST_P(StatsCollectorTrackTest, LocalAndRemoteTracksWithSameSsrc) { voice_info.receivers.push_back(voice_receiver_info); // Instruct the session to return stats containing the transport channel. - auto voice_media_channels = pc->AddVoiceChannel("audio", "transport"); - voice_media_channels.first->SetStats(voice_info); - voice_media_channels.second->SetStats(voice_info); + auto* voice_media_channel = pc->AddVoiceChannel("audio", "transport"); + voice_media_channel->SetStats(voice_info); stats->UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard); @@ -1750,9 +1744,8 @@ TEST_P(StatsCollectorTrackTest, TwoLocalTracksWithSameSsrc) { VoiceMediaInfo voice_info; voice_info.senders.push_back(voice_sender_info); - auto voice_media_channels = pc->AddVoiceChannel("voice", "transport"); - voice_media_channels.first->SetStats(voice_info); - voice_media_channels.second->SetStats(voice_info); + auto* voice_media_channel = pc->AddVoiceChannel("voice", "transport"); + voice_media_channel->SetStats(voice_info); StatsReports reports; // returned values. VerifyAudioTrackStats(audio_track_.get(), stats.get(), voice_info, &reports); @@ -1778,8 +1771,7 @@ TEST_P(StatsCollectorTrackTest, TwoLocalTracksWithSameSsrc) { &new_voice_sender_info, false); VoiceMediaInfo new_voice_info; new_voice_info.senders.push_back(new_voice_sender_info); - voice_media_channels.first->SetStats(new_voice_info); - voice_media_channels.second->SetStats(new_voice_info); + voice_media_channel->SetStats(new_voice_info); reports.clear(); VerifyAudioTrackStats(new_audio_track.get(), stats.get(), new_voice_info, @@ -1817,9 +1809,8 @@ TEST_P(StatsCollectorTrackTest, TwoLocalSendersWithSameTrack) { voice_info.senders.push_back(first_sender_info); voice_info.senders.push_back(second_sender_info); - auto voice_media_channels = pc->AddVoiceChannel("voice", "transport"); - voice_media_channels.first->SetStats(voice_info); - voice_media_channels.second->SetStats(voice_info); + auto* voice_media_channel = pc->AddVoiceChannel("voice", "transport"); + voice_media_channel->SetStats(voice_info); stats->UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard); diff --git a/pc/peer_connection_media_unittest.cc b/pc/peer_connection_media_unittest.cc index d1a10a4922..322540fa23 100644 --- a/pc/peer_connection_media_unittest.cc +++ b/pc/peer_connection_media_unittest.cc @@ -75,7 +75,6 @@ using RTCOfferAnswerOptions = PeerConnectionInterface::RTCOfferAnswerOptions; using ::testing::Bool; using ::testing::Combine; using ::testing::ElementsAre; -using ::testing::NotNull; using ::testing::Values; class PeerConnectionWrapperForMediaTest : public PeerConnectionWrapper { @@ -290,32 +289,28 @@ TEST_P(PeerConnectionMediaTest, AudioVideoOfferAnswerCreateSendRecvStreams) { ASSERT_TRUE( caller->SetRemoteDescription(callee->CreateAnswerAndSetAsLocal())); - auto* caller_voice_send = caller->media_engine()->GetVoiceChannel(0); - auto* caller_voice_receive = caller->media_engine()->GetVoiceChannel(1); - EXPECT_THAT(GetIds(caller_voice_receive->recv_streams()), + auto* caller_voice = caller->media_engine()->GetVoiceChannel(0); + EXPECT_THAT(GetIds(caller_voice->recv_streams()), ElementsAre(kCalleeAudioId)); - EXPECT_THAT(GetIds(caller_voice_send->send_streams()), + EXPECT_THAT(GetIds(caller_voice->send_streams()), ElementsAre(kCallerAudioId)); - auto* caller_video_send = caller->media_engine()->GetVideoChannel(0); - auto* caller_video_receive = caller->media_engine()->GetVideoChannel(1); - EXPECT_THAT(GetIds(caller_video_receive->recv_streams()), + auto* caller_video = caller->media_engine()->GetVideoChannel(0); + EXPECT_THAT(GetIds(caller_video->recv_streams()), ElementsAre(kCalleeVideoId)); - EXPECT_THAT(GetIds(caller_video_send->send_streams()), + EXPECT_THAT(GetIds(caller_video->send_streams()), ElementsAre(kCallerVideoId)); - auto* callee_voice_send = callee->media_engine()->GetVoiceChannel(0); - auto* callee_voice_receive = callee->media_engine()->GetVoiceChannel(1); - EXPECT_THAT(GetIds(callee_voice_receive->recv_streams()), + auto* callee_voice = callee->media_engine()->GetVoiceChannel(0); + EXPECT_THAT(GetIds(callee_voice->recv_streams()), ElementsAre(kCallerAudioId)); - EXPECT_THAT(GetIds(callee_voice_send->send_streams()), + EXPECT_THAT(GetIds(callee_voice->send_streams()), ElementsAre(kCalleeAudioId)); - auto* callee_video_send = callee->media_engine()->GetVideoChannel(0); - auto* callee_video_receive = callee->media_engine()->GetVideoChannel(1); - EXPECT_THAT(GetIds(callee_video_receive->recv_streams()), + auto* callee_video = callee->media_engine()->GetVideoChannel(0); + EXPECT_THAT(GetIds(callee_video->recv_streams()), ElementsAre(kCallerVideoId)); - EXPECT_THAT(GetIds(callee_video_send->send_streams()), + EXPECT_THAT(GetIds(callee_video->send_streams()), ElementsAre(kCalleeVideoId)); } @@ -457,14 +452,12 @@ TEST_F(PeerConnectionMediaTestPlanB, EmptyLocalAnswerRemovesSendStreams) { ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); - auto callee_voice_send = callee->media_engine()->GetVoiceChannel(0); - auto callee_voice_receive = callee->media_engine()->GetVoiceChannel(1); - auto callee_video_send = callee->media_engine()->GetVideoChannel(0); - auto callee_video_receive = callee->media_engine()->GetVideoChannel(1); - EXPECT_EQ(0u, callee_voice_send->send_streams().size()); - EXPECT_EQ(1u, callee_voice_receive->recv_streams().size()); - EXPECT_EQ(0u, callee_video_send->send_streams().size()); - EXPECT_EQ(1u, callee_video_receive->recv_streams().size()); + auto callee_voice = callee->media_engine()->GetVoiceChannel(0); + auto callee_video = callee->media_engine()->GetVideoChannel(0); + EXPECT_EQ(0u, callee_voice->send_streams().size()); + EXPECT_EQ(1u, callee_voice->recv_streams().size()); + EXPECT_EQ(0u, callee_video->send_streams().size()); + EXPECT_EQ(1u, callee_video->recv_streams().size()); } // Test that a new stream in a subsequent offer causes a new receive stream to @@ -481,10 +474,10 @@ TEST_P(PeerConnectionMediaTest, NewStreamInRemoteOfferAddsRecvStreams) { ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); - auto a1 = callee->media_engine()->GetVoiceChannel(1); - auto a2 = callee->media_engine()->GetVoiceChannel(3); - auto v1 = callee->media_engine()->GetVideoChannel(1); - auto v2 = callee->media_engine()->GetVideoChannel(3); + auto a1 = callee->media_engine()->GetVoiceChannel(0); + auto a2 = callee->media_engine()->GetVoiceChannel(1); + auto v1 = callee->media_engine()->GetVideoChannel(0); + auto v2 = callee->media_engine()->GetVideoChannel(1); if (IsUnifiedPlan()) { ASSERT_TRUE(a1); EXPECT_EQ(1u, a1->recv_streams().size()); @@ -1102,11 +1095,10 @@ TEST_P(PeerConnectionMediaTest, TestAVOfferWithAudioOnlyAnswer) { ASSERT_TRUE(caller->SetRemoteDescription( callee->CreateAnswerAndSetAsLocal(options_reject_video))); - auto caller_voice_send = caller->media_engine()->GetVoiceChannel(0); - auto caller_voice_receive = caller->media_engine()->GetVoiceChannel(1); - ASSERT_TRUE(caller_voice_send && caller_voice_receive); - EXPECT_EQ(0u, caller_voice_receive->recv_streams().size()); - EXPECT_EQ(1u, caller_voice_send->send_streams().size()); + auto caller_voice = caller->media_engine()->GetVoiceChannel(0); + ASSERT_TRUE(caller_voice); + EXPECT_EQ(0u, caller_voice->recv_streams().size()); + EXPECT_EQ(1u, caller_voice->send_streams().size()); auto caller_video = caller->media_engine()->GetVideoChannel(0); EXPECT_FALSE(caller_video); @@ -1118,16 +1110,14 @@ TEST_P(PeerConnectionMediaTest, TestAVOfferWithAudioOnlyAnswer) { ASSERT_TRUE( caller->SetRemoteDescription(callee->CreateAnswerAndSetAsLocal())); - auto callee_voice_send = callee->media_engine()->GetVoiceChannel(0); - auto callee_voice_receive = callee->media_engine()->GetVoiceChannel(1); - ASSERT_TRUE(callee_voice_send && callee_voice_receive); - EXPECT_EQ(1u, callee_voice_receive->recv_streams().size()); - EXPECT_EQ(1u, callee_voice_send->send_streams().size()); - auto callee_video_send = callee->media_engine()->GetVideoChannel(0); - auto callee_video_receive = callee->media_engine()->GetVideoChannel(1); - ASSERT_TRUE(callee_video_send && callee_video_receive); - EXPECT_EQ(1u, callee_video_receive->recv_streams().size()); - EXPECT_EQ(1u, callee_video_send->send_streams().size()); + auto callee_voice = callee->media_engine()->GetVoiceChannel(0); + ASSERT_TRUE(callee_voice); + EXPECT_EQ(1u, callee_voice->recv_streams().size()); + EXPECT_EQ(1u, callee_voice->send_streams().size()); + auto callee_video = callee->media_engine()->GetVideoChannel(0); + ASSERT_TRUE(callee_video); + EXPECT_EQ(1u, callee_video->recv_streams().size()); + EXPECT_EQ(1u, callee_video->send_streams().size()); // Callee removes video but keeps audio and rejects the video once again. callee->pc()->RemoveTrackOrError(callee_video_track); @@ -1135,12 +1125,11 @@ TEST_P(PeerConnectionMediaTest, TestAVOfferWithAudioOnlyAnswer) { ASSERT_TRUE( callee->SetLocalDescription(callee->CreateAnswer(options_reject_video))); - callee_voice_send = callee->media_engine()->GetVoiceChannel(0); - callee_voice_receive = callee->media_engine()->GetVoiceChannel(1); - ASSERT_TRUE(callee_voice_send && callee_voice_receive); - EXPECT_EQ(1u, callee_voice_receive->recv_streams().size()); - EXPECT_EQ(1u, callee_voice_send->send_streams().size()); - auto callee_video = callee->media_engine()->GetVideoChannel(0); + callee_voice = callee->media_engine()->GetVoiceChannel(0); + ASSERT_TRUE(callee_voice); + EXPECT_EQ(1u, callee_voice->recv_streams().size()); + EXPECT_EQ(1u, callee_voice->send_streams().size()); + callee_video = callee->media_engine()->GetVideoChannel(0); EXPECT_FALSE(callee_video); } @@ -1191,16 +1180,14 @@ TEST_P(PeerConnectionMediaTest, TestAVOfferWithVideoOnlyAnswer) { ASSERT_TRUE(caller->SetRemoteDescription( callee->CreateAnswerAndSetAsLocal(options_no_bundle))); - auto callee_voice_send = callee->media_engine()->GetVoiceChannel(0); - auto callee_voice_receive = callee->media_engine()->GetVoiceChannel(1); - ASSERT_TRUE(callee_voice_send && callee_voice_receive); - EXPECT_EQ(1u, callee_voice_receive->recv_streams().size()); - EXPECT_EQ(1u, callee_voice_send->send_streams().size()); - auto callee_video_send = callee->media_engine()->GetVideoChannel(0); - auto callee_video_receive = callee->media_engine()->GetVideoChannel(1); - ASSERT_TRUE(callee_video_send && callee_video_receive); - EXPECT_EQ(1u, callee_video_receive->recv_streams().size()); - EXPECT_EQ(1u, callee_video_send->send_streams().size()); + auto callee_voice = callee->media_engine()->GetVoiceChannel(0); + ASSERT_TRUE(callee_voice); + EXPECT_EQ(1u, callee_voice->recv_streams().size()); + EXPECT_EQ(1u, callee_voice->send_streams().size()); + auto callee_video = callee->media_engine()->GetVideoChannel(0); + ASSERT_TRUE(callee_video); + EXPECT_EQ(1u, callee_video->recv_streams().size()); + EXPECT_EQ(1u, callee_video->send_streams().size()); // Callee removes audio but keeps video and rejects the audio once again. callee->pc()->RemoveTrackOrError(callee_audio_track); @@ -1208,13 +1195,12 @@ TEST_P(PeerConnectionMediaTest, TestAVOfferWithVideoOnlyAnswer) { ASSERT_TRUE( callee->SetLocalDescription(callee->CreateAnswer(options_reject_audio))); - auto callee_voice = callee->media_engine()->GetVoiceChannel(0); + callee_voice = callee->media_engine()->GetVoiceChannel(0); EXPECT_FALSE(callee_voice); - callee_video_send = callee->media_engine()->GetVideoChannel(0); - callee_video_receive = callee->media_engine()->GetVideoChannel(1); - ASSERT_TRUE(callee_video_send && callee_video_receive); - EXPECT_EQ(1u, callee_video_receive->recv_streams().size()); - EXPECT_EQ(1u, callee_video_send->send_streams().size()); + callee_video = callee->media_engine()->GetVideoChannel(0); + ASSERT_TRUE(callee_video); + EXPECT_EQ(1u, callee_video->recv_streams().size()); + EXPECT_EQ(1u, callee_video->send_streams().size()); } // Tests that if the underlying video encoder fails to be initialized (signaled @@ -2250,7 +2236,6 @@ TEST_F(PeerConnectionMediaTestUnifiedPlan, EXPECT_TRUE(video_transceiver->SetCodecPreferences(capabilities.codecs).ok()); auto reoffer = caller->CreateOffer(options); - ASSERT_THAT(reoffer, NotNull()); EXPECT_FALSE(HasPayloadTypeConflict(reoffer->description())); // Sanity check that we got the primary codec and RTX. diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc index 614a147674..2a03c3b78b 100644 --- a/pc/rtc_stats_collector_unittest.cc +++ b/pc/rtc_stats_collector_unittest.cc @@ -814,11 +814,6 @@ class RTCStatsCollectorTest : public ::testing::Test { EXPECT_EQ(*outbound_rtp.codec_id, graph.send_codec_id); EXPECT_EQ(*outbound_rtp.track_id, graph.sender_track_id); EXPECT_EQ(*outbound_rtp.transport_id, graph.transport_id); - EXPECT_TRUE(graph.full_report->Get(graph.inbound_rtp_id)); - // We can't use an ASSERT in a function returning non-void, so just return. - if (!graph.full_report->Get(graph.inbound_rtp_id)) { - return graph; - } const auto& inbound_rtp = graph.full_report->Get(graph.inbound_rtp_id) ->cast_to(); EXPECT_EQ(*inbound_rtp.codec_id, graph.recv_codec_id); @@ -936,11 +931,6 @@ class RTCStatsCollectorTest : public ::testing::Test { EXPECT_EQ(*outbound_rtp.codec_id, graph.send_codec_id); EXPECT_EQ(*outbound_rtp.track_id, graph.sender_track_id); EXPECT_EQ(*outbound_rtp.transport_id, graph.transport_id); - EXPECT_TRUE(graph.full_report->Get(graph.inbound_rtp_id)); - // We can't use ASSERT in a function with a return value. - if (!graph.full_report->Get(graph.inbound_rtp_id)) { - return graph; - } const auto& inbound_rtp = graph.full_report->Get(graph.inbound_rtp_id) ->cast_to(); EXPECT_EQ(*inbound_rtp.codec_id, graph.recv_codec_id); @@ -1187,9 +1177,9 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCodecStatsOnlyIfReferenced) { outbound_video_info.codec_payload_type = 4; video_media_info.senders.push_back(outbound_video_info); - auto audio_channels = + FakeVoiceMediaChannelForStats* audio_channel = pc_->AddVoiceChannel("AudioMid", "TransportName", voice_media_info); - auto video_channels = + FakeVideoMediaChannelForStats* video_channel = pc_->AddVideoChannel("VideoMid", "TransportName", video_media_info); rtc::scoped_refptr report = stats_->GetStatsReport(); @@ -1254,12 +1244,10 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCodecStatsOnlyIfReferenced) { // being exposed, despite `send_codecs` and `receive_codecs` still being set. voice_media_info.senders.clear(); voice_media_info.receivers.clear(); - audio_channels.first->SetStats(voice_media_info); - audio_channels.second->SetStats(voice_media_info); + audio_channel->SetStats(voice_media_info); video_media_info.senders.clear(); video_media_info.receivers.clear(); - video_channels.first->SetStats(video_media_info); - video_channels.second->SetStats(video_media_info); + video_channel->SetStats(video_media_info); stats_->stats_collector()->ClearCachedStatsReport(); report = stats_->GetStatsReport(); EXPECT_FALSE(report->Get(expected_inbound_audio_codec.id())); @@ -2502,7 +2490,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCInboundRTPStreamStats_Audio) { voice_media_info.receive_codecs.insert( std::make_pair(codec_parameters.payload_type, codec_parameters)); - auto voice_media_channels = + auto* voice_media_channel = pc_->AddVoiceChannel("AudioMid", "TransportName", voice_media_info); stats_->SetupRemoteTrackAndReceiver( cricket::MEDIA_TYPE_AUDIO, "RemoteAudioTrackID", "RemoteStreamId", 1); @@ -2568,8 +2556,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCInboundRTPStreamStats_Audio) { expected_audio.last_packet_received_timestamp = 3000.0; voice_media_info.receivers[0].estimated_playout_ntp_timestamp_ms = 4567; expected_audio.estimated_playout_timestamp = 4567; - voice_media_channels.first->SetStats(voice_media_info); - voice_media_channels.second->SetStats(voice_media_info); + voice_media_channel->SetStats(voice_media_info); report = stats_->GetFreshStatsReport(); @@ -2681,7 +2668,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCInboundRTPStreamStats_Video) { video_media_info.receive_codecs.insert( std::make_pair(codec_parameters.payload_type, codec_parameters)); - auto video_media_channels = + auto* video_media_channel = pc_->AddVideoChannel("VideoMid", "TransportName", video_media_info); stats_->SetupRemoteTrackAndReceiver( cricket::MEDIA_TYPE_VIDEO, "RemoteVideoTrackID", "RemoteStreamId", 1); @@ -2751,8 +2738,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCInboundRTPStreamStats_Video) { expected_video.decoder_implementation = "libfoodecoder"; video_media_info.receivers[0].power_efficient_decoder = true; expected_video.power_efficient_decoder = true; - video_media_channels.first->SetStats(video_media_info); - video_media_channels.second->SetStats(video_media_info); + video_media_channel->SetStats(video_media_info); report = stats_->GetFreshStatsReport(); @@ -2941,7 +2927,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Video) { video_media_info.send_codecs.insert( std::make_pair(codec_parameters.payload_type, codec_parameters)); - auto video_media_channels = + auto* video_media_channel = pc_->AddVideoChannel("VideoMid", "TransportName", video_media_info); stats_->SetupLocalTrackAndSender(cricket::MEDIA_TYPE_VIDEO, "LocalVideoTrackID", 1, true, @@ -3011,8 +2997,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Video) { expected_video.encoder_implementation = "libfooencoder"; video_media_info.senders[0].power_efficient_encoder = true; expected_video.power_efficient_encoder = true; - video_media_channels.first->SetStats(video_media_info); - video_media_channels.second->SetStats(video_media_info); + video_media_channel->SetStats(video_media_info); report = stats_->GetFreshStatsReport(); diff --git a/pc/rtp_transceiver.cc b/pc/rtp_transceiver.cc index 7bcaae11d2..c1619c7349 100644 --- a/pc/rtp_transceiver.cc +++ b/pc/rtp_transceiver.cc @@ -209,28 +209,18 @@ RTCError RtpTransceiver::CreateChannel( context()->worker_thread()->BlockingCall([&] { RTC_DCHECK_RUN_ON(context()->worker_thread()); - AudioCodecPairId codec_pair_id = AudioCodecPairId::Create(); - std::unique_ptr media_send_channel = - absl::WrapUnique(media_engine()->voice().CreateMediaChannel( - cricket::MediaChannel::Role::kSend, call_ptr, media_config, - audio_options, crypto_options, codec_pair_id)); - if (!media_send_channel) { - // TODO(bugs.webrtc.org/14912): Consider CHECK or reporting failure - return; - } - std::unique_ptr media_receive_channel = - absl::WrapUnique(media_engine()->voice().CreateMediaChannel( - cricket::MediaChannel::Role::kReceive, call_ptr, media_config, - audio_options, crypto_options, codec_pair_id)); - if (!media_receive_channel) { + cricket::VoiceMediaChannel* media_channel = + media_engine()->voice().CreateMediaChannel( + cricket::MediaChannel::Role::kBoth, call_ptr, media_config, + audio_options, crypto_options, AudioCodecPairId::Create()); + if (!media_channel) { return; } new_channel = std::make_unique( context()->worker_thread(), context()->network_thread(), - context()->signaling_thread(), std::move(media_send_channel), - std::move(media_receive_channel), mid, srtp_required, crypto_options, - context()->ssrc_generator()); + context()->signaling_thread(), absl::WrapUnique(media_channel), mid, + srtp_required, crypto_options, context()->ssrc_generator()); }); } else { RTC_DCHECK_EQ(cricket::MEDIA_TYPE_VIDEO, media_type()); @@ -240,26 +230,18 @@ RTCError RtpTransceiver::CreateChannel( // simply be on the worker thread and use `call_` (update upstream code). context()->worker_thread()->BlockingCall([&] { RTC_DCHECK_RUN_ON(context()->worker_thread()); - std::unique_ptr media_send_channel = - absl::WrapUnique(media_engine()->video().CreateMediaChannel( - cricket::MediaChannel::Role::kSend, call_ptr, media_config, - video_options, crypto_options, video_bitrate_allocator_factory)); - if (!media_send_channel) { - return; - } - std::unique_ptr media_receive_channel = - absl::WrapUnique(media_engine()->video().CreateMediaChannel( - cricket::MediaChannel::Role::kReceive, call_ptr, media_config, - video_options, crypto_options, video_bitrate_allocator_factory)); - if (!media_receive_channel) { + cricket::VideoMediaChannel* media_channel = + media_engine()->video().CreateMediaChannel( + cricket::MediaChannel::Role::kBoth, call_ptr, media_config, + video_options, crypto_options, video_bitrate_allocator_factory); + if (!media_channel) { return; } new_channel = std::make_unique( context()->worker_thread(), context()->network_thread(), - context()->signaling_thread(), std::move(media_send_channel), - std::move(media_receive_channel), mid, srtp_required, crypto_options, - context()->ssrc_generator()); + context()->signaling_thread(), absl::WrapUnique(media_channel), mid, + srtp_required, crypto_options, context()->ssrc_generator()); }); } if (!new_channel) { diff --git a/pc/test/fake_peer_connection_for_stats.h b/pc/test/fake_peer_connection_for_stats.h index 78e3f43f04..d1914fd708 100644 --- a/pc/test/fake_peer_connection_for_stats.h +++ b/pc/test/fake_peer_connection_for_stats.h @@ -29,9 +29,8 @@ namespace webrtc { // Fake VoiceMediaChannel where the result of GetStats can be configured. class FakeVoiceMediaChannelForStats : public cricket::FakeVoiceMediaChannel { public: - explicit FakeVoiceMediaChannelForStats(MediaChannel::Role role, - TaskQueueBase* network_thread) - : cricket::FakeVoiceMediaChannel(role, + explicit FakeVoiceMediaChannelForStats(TaskQueueBase* network_thread) + : cricket::FakeVoiceMediaChannel(MediaChannel::Role::kBoth, nullptr, cricket::AudioOptions(), network_thread) {} @@ -48,7 +47,6 @@ class FakeVoiceMediaChannelForStats : public cricket::FakeVoiceMediaChannel { // VoiceMediaChannel overrides. bool GetSendStats(cricket::VoiceMediaSendInfo* info) override { - RTC_DCHECK(role() == MediaChannel::Role::kSend); if (send_stats_) { *info = *send_stats_; return true; @@ -57,7 +55,6 @@ class FakeVoiceMediaChannelForStats : public cricket::FakeVoiceMediaChannel { } bool GetReceiveStats(cricket::VoiceMediaReceiveInfo* info, bool get_and_clear_legacy_stats) override { - RTC_DCHECK(role() == MediaChannel::Role::kReceive); if (receive_stats_) { *info = *receive_stats_; return true; @@ -73,35 +70,24 @@ class FakeVoiceMediaChannelForStats : public cricket::FakeVoiceMediaChannel { // Fake VideoMediaChannel where the result of GetStats can be configured. class FakeVideoMediaChannelForStats : public cricket::FakeVideoMediaChannel { public: - explicit FakeVideoMediaChannelForStats(cricket::MediaChannel::Role role, - TaskQueueBase* network_thread) - : cricket::FakeVideoMediaChannel(role, + explicit FakeVideoMediaChannelForStats(TaskQueueBase* network_thread) + : cricket::FakeVideoMediaChannel(MediaChannel::Role::kBoth, nullptr, cricket::VideoOptions(), network_thread) {} void SetStats(const cricket::VideoMediaInfo& video_info) { - switch (role()) { - case MediaChannel::Role::kSend: - send_stats_ = cricket::VideoMediaSendInfo(); - send_stats_->senders = video_info.senders; - send_stats_->aggregated_senders = video_info.aggregated_senders; - send_stats_->send_codecs = video_info.send_codecs; - break; - case MediaChannel::Role::kReceive: - receive_stats_ = cricket::VideoMediaReceiveInfo(); - receive_stats_->receivers = video_info.receivers; - receive_stats_->receive_codecs = video_info.receive_codecs; - break; - default: - RTC_CHECK_NOTREACHED(); - } + send_stats_ = cricket::VideoMediaSendInfo(); + send_stats_->senders = video_info.senders; + send_stats_->aggregated_senders = video_info.aggregated_senders; + send_stats_->send_codecs = video_info.send_codecs; + receive_stats_ = cricket::VideoMediaReceiveInfo(); + receive_stats_->receivers = video_info.receivers; + receive_stats_->receive_codecs = video_info.receive_codecs; } // VideoMediaChannel overrides. bool GetSendStats(cricket::VideoMediaSendInfo* info) override { - RTC_DCHECK(role() == MediaChannel::Role::kSend); - if (send_stats_) { *info = *send_stats_; return true; @@ -109,7 +95,6 @@ class FakeVideoMediaChannelForStats : public cricket::FakeVideoMediaChannel { return false; } bool GetReceiveStats(cricket::VideoMediaReceiveInfo* info) override { - RTC_DCHECK(role() == MediaChannel::Role::kReceive); if (receive_stats_) { *info = *receive_stats_; return true; @@ -127,22 +112,19 @@ constexpr bool kDefaultSrtpRequired = true; class VoiceChannelForTesting : public cricket::VoiceChannel { public: - VoiceChannelForTesting( - rtc::Thread* worker_thread, - rtc::Thread* network_thread, - rtc::Thread* signaling_thread, - std::unique_ptr send_channel, - std::unique_ptr receive_channel, - const std::string& content_name, - bool srtp_required, - webrtc::CryptoOptions crypto_options, - rtc::UniqueRandomIdGenerator* ssrc_generator, - std::string transport_name) + VoiceChannelForTesting(rtc::Thread* worker_thread, + rtc::Thread* network_thread, + rtc::Thread* signaling_thread, + std::unique_ptr channel, + const std::string& content_name, + bool srtp_required, + webrtc::CryptoOptions crypto_options, + rtc::UniqueRandomIdGenerator* ssrc_generator, + std::string transport_name) : VoiceChannel(worker_thread, network_thread, signaling_thread, - std::move(send_channel), - std::move(receive_channel), + std::move(channel), content_name, srtp_required, std::move(crypto_options), @@ -159,22 +141,19 @@ class VoiceChannelForTesting : public cricket::VoiceChannel { class VideoChannelForTesting : public cricket::VideoChannel { public: - VideoChannelForTesting( - rtc::Thread* worker_thread, - rtc::Thread* network_thread, - rtc::Thread* signaling_thread, - std::unique_ptr send_channel, - std::unique_ptr receive_channel, - const std::string& content_name, - bool srtp_required, - webrtc::CryptoOptions crypto_options, - rtc::UniqueRandomIdGenerator* ssrc_generator, - std::string transport_name) + VideoChannelForTesting(rtc::Thread* worker_thread, + rtc::Thread* network_thread, + rtc::Thread* signaling_thread, + std::unique_ptr channel, + const std::string& content_name, + bool srtp_required, + webrtc::CryptoOptions crypto_options, + rtc::UniqueRandomIdGenerator* ssrc_generator, + std::string transport_name) : VideoChannel(worker_thread, network_thread, signaling_thread, - std::move(send_channel), - std::move(receive_channel), + std::move(channel), content_name, srtp_required, std::move(crypto_options), @@ -265,23 +244,16 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { ->RemoveReceiver(receiver.get()); } - std::pair - AddVoiceChannel( + FakeVoiceMediaChannelForStats* AddVoiceChannel( const std::string& mid, const std::string& transport_name, cricket::VoiceMediaInfo initial_stats = cricket::VoiceMediaInfo()) { - auto voice_media_send_channel = - std::make_unique( - cricket::MediaChannel::Role::kSend, network_thread_); - auto voice_media_receive_channel = - std::make_unique( - cricket::MediaChannel::Role::kReceive, network_thread_); - auto* voice_media_send_channel_ptr = voice_media_send_channel.get(); - auto* voice_media_receive_channel_ptr = voice_media_receive_channel.get(); + auto voice_media_channel = + std::make_unique(network_thread_); + auto* voice_media_channel_ptr = voice_media_channel.get(); auto voice_channel = std::make_unique( worker_thread_, network_thread_, signaling_thread_, - std::move(voice_media_send_channel), - std::move(voice_media_receive_channel), mid, kDefaultSrtpRequired, + std::move(voice_media_channel), mid, kDefaultSrtpRequired, webrtc::CryptoOptions(), context_->ssrc_generator(), transport_name); auto transceiver = GetOrCreateFirstTransceiverOfType(cricket::MEDIA_TYPE_AUDIO) @@ -294,29 +266,20 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { RTC_DCHECK(!transceiver->channel()); transceiver->SetChannel(std::move(voice_channel), [](const std::string&) { return nullptr; }); - voice_media_send_channel_ptr->SetStats(initial_stats); - voice_media_receive_channel_ptr->SetStats(initial_stats); - return std::make_pair(voice_media_send_channel_ptr, - voice_media_receive_channel_ptr); + voice_media_channel_ptr->SetStats(initial_stats); + return voice_media_channel_ptr; } - std::pair - AddVideoChannel( + FakeVideoMediaChannelForStats* AddVideoChannel( const std::string& mid, const std::string& transport_name, cricket::VideoMediaInfo initial_stats = cricket::VideoMediaInfo()) { - auto video_media_send_channel = - std::make_unique( - cricket::MediaChannel::Role::kSend, network_thread_); - auto video_media_receive_channel = - std::make_unique( - cricket::MediaChannel::Role::kReceive, network_thread_); - auto video_media_send_channel_ptr = video_media_send_channel.get(); - auto video_media_receive_channel_ptr = video_media_receive_channel.get(); + auto video_media_channel = + std::make_unique(network_thread_); + auto video_media_channel_ptr = video_media_channel.get(); auto video_channel = std::make_unique( worker_thread_, network_thread_, signaling_thread_, - std::move(video_media_send_channel), - std::move(video_media_receive_channel), mid, kDefaultSrtpRequired, + std::move(video_media_channel), mid, kDefaultSrtpRequired, webrtc::CryptoOptions(), context_->ssrc_generator(), transport_name); auto transceiver = GetOrCreateFirstTransceiverOfType(cricket::MEDIA_TYPE_VIDEO) @@ -329,10 +292,8 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { RTC_DCHECK(!transceiver->channel()); transceiver->SetChannel(std::move(video_channel), [](const std::string&) { return nullptr; }); - video_media_send_channel_ptr->SetStats(initial_stats); - video_media_receive_channel_ptr->SetStats(initial_stats); - return std::make_pair(video_media_send_channel_ptr, - video_media_receive_channel_ptr); + video_media_channel_ptr->SetStats(initial_stats); + return video_media_channel_ptr; } void AddSctpDataChannel(const std::string& label) { diff --git a/pc/test/mock_channel_interface.h b/pc/test/mock_channel_interface.h index 3d82beb287..41c142d5ba 100644 --- a/pc/test/mock_channel_interface.h +++ b/pc/test/mock_channel_interface.h @@ -27,6 +27,7 @@ class MockChannelInterface : public cricket::ChannelInterface { MOCK_METHOD(cricket::MediaType, media_type, (), (const, override)); MOCK_METHOD(VideoChannel*, AsVideoChannel, (), (override)); MOCK_METHOD(VoiceChannel*, AsVoiceChannel, (), (override)); + MOCK_METHOD(MediaChannel*, media_channel, (), (override)); MOCK_METHOD(MediaChannel*, media_send_channel, (), (override)); MOCK_METHOD(VoiceMediaChannel*, voice_media_send_channel, (), (override)); MOCK_METHOD(VideoMediaChannel*, video_media_send_channel, (), (override)); diff --git a/pc/test/mock_voice_media_channel.h b/pc/test/mock_voice_media_channel.h index 1523c6fbd8..71f7a18860 100644 --- a/pc/test/mock_voice_media_channel.h +++ b/pc/test/mock_voice_media_channel.h @@ -161,10 +161,6 @@ class MockVoiceMediaChannel : public VoiceMediaChannel { GetBaseMinimumPlayoutDelayMs, (uint32_t ssrc), (const, override)); - MOCK_METHOD(bool, SenderNackEnabled, (), (const, override)); - MOCK_METHOD(bool, SenderNonSenderRttEnabled, (), (const, override)); - MOCK_METHOD(void, SetReceiveNackEnabled, (bool enabled), (override)); - MOCK_METHOD(void, SetReceiveNonSenderRttEnabled, (bool enabled), (override)); }; } // namespace cricket