From 51542be8ce35503c3cec4755ded415ecb1da2d37 Mon Sep 17 00:00:00 2001 From: nisse Date: Fri, 12 Feb 2016 02:27:06 -0800 Subject: [PATCH] Introduce struct MediaConfig, with construction-time settings. Pass it to MediaController constructor and down to WebRtcVideoEngine2 and WebRtcVoiceEngine. Follows discussion on https://codereview.webrtc.org/1646253004/ TBR=pthatcher@webrtc.org BUG=webrtc:5438 Review URL: https://codereview.webrtc.org/1670153003 Cr-Commit-Position: refs/heads/master@{#11595} --- webrtc/api/fakemediacontroller.h | 3 + webrtc/api/mediacontroller.cc | 13 ++- webrtc/api/mediacontroller.h | 3 + webrtc/api/peerconnection.cc | 15 ++- webrtc/api/peerconnectionfactory.cc | 6 +- webrtc/api/peerconnectionfactory.h | 3 +- .../api/peerconnectioninterface_unittest.cc | 99 +++++++++++++++++++ webrtc/api/statscollector_unittest.cc | 3 +- webrtc/api/webrtcsession.cc | 16 --- webrtc/api/webrtcsession_unittest.cc | 25 +---- webrtc/media/base/fakemediaengine.h | 2 + webrtc/media/base/mediachannel.h | 95 +++++++++--------- webrtc/media/base/mediaengine.h | 13 ++- webrtc/media/base/videoengine_unittest.h | 4 +- webrtc/media/engine/nullwebrtcvideoengine.h | 3 +- webrtc/media/engine/webrtcvideoengine2.cc | 46 ++++----- webrtc/media/engine/webrtcvideoengine2.h | 9 +- .../engine/webrtcvideoengine2_unittest.cc | 59 ++++++----- webrtc/media/engine/webrtcvoiceengine.cc | 27 ++--- webrtc/media/engine/webrtcvoiceengine.h | 4 + .../engine/webrtcvoiceengine_unittest.cc | 76 ++++++++------ webrtc/pc/channelmanager.cc | 8 +- 22 files changed, 322 insertions(+), 210 deletions(-) diff --git a/webrtc/api/fakemediacontroller.h b/webrtc/api/fakemediacontroller.h index 56af5cfe38..2e539e813a 100644 --- a/webrtc/api/fakemediacontroller.h +++ b/webrtc/api/fakemediacontroller.h @@ -13,6 +13,7 @@ #include "webrtc/api/mediacontroller.h" #include "webrtc/base/checks.h" +#include "webrtc/media/base/mediachannel.h" namespace cricket { @@ -29,8 +30,10 @@ class FakeMediaController : public webrtc::MediaControllerInterface { cricket::ChannelManager* channel_manager() const override { return channel_manager_; } + const MediaConfig& config() const override { return media_config_; } private: + const MediaConfig media_config_ = MediaConfig(); cricket::ChannelManager* channel_manager_; webrtc::Call* call_; }; diff --git a/webrtc/api/mediacontroller.cc b/webrtc/api/mediacontroller.cc index 0c7d73389e..341c013ed1 100644 --- a/webrtc/api/mediacontroller.cc +++ b/webrtc/api/mediacontroller.cc @@ -14,6 +14,7 @@ #include "webrtc/base/checks.h" #include "webrtc/call.h" #include "webrtc/pc/channelmanager.h" +#include "webrtc/media/base/mediachannel.h" namespace { @@ -24,9 +25,12 @@ const int kMaxBandwidthBps = 2000000; class MediaController : public webrtc::MediaControllerInterface, public sigslot::has_slots<> { public: - MediaController(rtc::Thread* worker_thread, + MediaController(const cricket::MediaConfig& config, + rtc::Thread* worker_thread, cricket::ChannelManager* channel_manager) - : worker_thread_(worker_thread), channel_manager_(channel_manager) { + : worker_thread_(worker_thread), + config_(config), + channel_manager_(channel_manager) { RTC_DCHECK(nullptr != worker_thread); worker_thread_->Invoke( rtc::Bind(&MediaController::Construct_w, this, @@ -44,6 +48,7 @@ class MediaController : public webrtc::MediaControllerInterface, cricket::ChannelManager* channel_manager() const override { return channel_manager_; } + const cricket::MediaConfig& config() const override { return config_; } private: void Construct_w(cricket::MediaEngineInterface* media_engine) { @@ -62,6 +67,7 @@ class MediaController : public webrtc::MediaControllerInterface, } rtc::Thread* const worker_thread_; + const cricket::MediaConfig config_; cricket::ChannelManager* const channel_manager_; rtc::scoped_ptr call_; @@ -72,8 +78,9 @@ class MediaController : public webrtc::MediaControllerInterface, namespace webrtc { MediaControllerInterface* MediaControllerInterface::Create( + const cricket::MediaConfig& config, rtc::Thread* worker_thread, cricket::ChannelManager* channel_manager) { - return new MediaController(worker_thread, channel_manager); + return new MediaController(config, worker_thread, channel_manager); } } // namespace webrtc diff --git a/webrtc/api/mediacontroller.h b/webrtc/api/mediacontroller.h index 5721ffe80e..7b6a2a3e3e 100644 --- a/webrtc/api/mediacontroller.h +++ b/webrtc/api/mediacontroller.h @@ -15,6 +15,7 @@ namespace cricket { class ChannelManager; +struct MediaConfig; } // namespace cricket namespace webrtc { @@ -26,12 +27,14 @@ class VoiceEngine; class MediaControllerInterface { public: static MediaControllerInterface* Create( + const cricket::MediaConfig& config, rtc::Thread* worker_thread, cricket::ChannelManager* channel_manager); virtual ~MediaControllerInterface() {} virtual webrtc::Call* call_w() = 0; virtual cricket::ChannelManager* channel_manager() const = 0; + virtual const cricket::MediaConfig& config() const = 0; }; } // namespace webrtc diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc index 70800880fb..eacbeaedc3 100644 --- a/webrtc/api/peerconnection.cc +++ b/webrtc/api/peerconnection.cc @@ -616,7 +616,20 @@ bool PeerConnection::Initialize( // No step delay is used while allocating ports. port_allocator_->set_step_delay(cricket::kMinimumStepDelay); - media_controller_.reset(factory_->CreateMediaController()); + // We rely on default values when constraints aren't found. + cricket::MediaConfig media_config; + + media_config.disable_prerenderer_smoothing = + configuration.disable_prerenderer_smoothing; + + // Find DSCP constraint. + FindConstraint(constraints, MediaConstraintsInterface::kEnableDscp, + &media_config.enable_dscp, NULL); + // Find constraints for cpu overuse detection. + FindConstraint(constraints, MediaConstraintsInterface::kCpuOveruseDetection, + &media_config.enable_cpu_overuse_detection, NULL); + + media_controller_.reset(factory_->CreateMediaController(media_config)); remote_stream_factory_.reset(new RemoteMediaStreamFactory( factory_->signaling_thread(), media_controller_->channel_manager())); diff --git a/webrtc/api/peerconnectionfactory.cc b/webrtc/api/peerconnectionfactory.cc index c3dec666f5..c05cd852f6 100644 --- a/webrtc/api/peerconnectionfactory.cc +++ b/webrtc/api/peerconnectionfactory.cc @@ -286,10 +286,10 @@ PeerConnectionFactory::CreateAudioTrack(const std::string& id, return AudioTrackProxy::Create(signaling_thread_, track); } -webrtc::MediaControllerInterface* PeerConnectionFactory::CreateMediaController() - const { +webrtc::MediaControllerInterface* PeerConnectionFactory::CreateMediaController( + const cricket::MediaConfig& config) const { RTC_DCHECK(signaling_thread_->IsCurrent()); - return MediaControllerInterface::Create(worker_thread_, + return MediaControllerInterface::Create(config, worker_thread_, channel_manager_.get()); } diff --git a/webrtc/api/peerconnectionfactory.h b/webrtc/api/peerconnectionfactory.h index 548986cb10..3ba4124096 100644 --- a/webrtc/api/peerconnectionfactory.h +++ b/webrtc/api/peerconnectionfactory.h @@ -70,7 +70,8 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface { bool StartRtcEventLog(rtc::PlatformFile file) override; void StopRtcEventLog() override; - virtual webrtc::MediaControllerInterface* CreateMediaController() const; + virtual webrtc::MediaControllerInterface* CreateMediaController( + const cricket::MediaConfig& config) const; virtual rtc::Thread* signaling_thread(); virtual rtc::Thread* worker_thread(); const Options& options() const { return options_; } diff --git a/webrtc/api/peerconnectioninterface_unittest.cc b/webrtc/api/peerconnectioninterface_unittest.cc index b9629f5043..2c7cc128ad 100644 --- a/webrtc/api/peerconnectioninterface_unittest.cc +++ b/webrtc/api/peerconnectioninterface_unittest.cc @@ -2328,6 +2328,105 @@ TEST_F(PeerConnectionInterfaceTest, SignalSameTracksInSeparateMediaStream) { EXPECT_TRUE(ContainsSender(senders, kVideoTracks[0])); } +// The PeerConnectionMediaConfig tests below verify that configuration +// and constraints are propagated into the MediaConfig passed to +// CreateMediaController. These settings are intended for MediaChannel +// constructors, but that is not exercised by these unittest. +class PeerConnectionFactoryForTest : public webrtc::PeerConnectionFactory { + public: + webrtc::MediaControllerInterface* CreateMediaController( + const cricket::MediaConfig& config) const override { + create_media_controller_called_ = true; + create_media_controller_config_ = config; + + webrtc::MediaControllerInterface* mc = + PeerConnectionFactory::CreateMediaController(config); + EXPECT_TRUE(mc != nullptr); + return mc; + } + + // Mutable, so they can be modified in the above const-declared method. + mutable bool create_media_controller_called_ = false; + mutable cricket::MediaConfig create_media_controller_config_; +}; + +class PeerConnectionMediaConfigTest : public testing::Test { + protected: + void SetUp() override { + pcf_= new rtc::RefCountedObject(); + pcf_->Initialize(); + } + const cricket::MediaConfig& TestCreatePeerConnection( + const PeerConnectionInterface::RTCConfiguration& config, + const MediaConstraintsInterface *constraints) { + pcf_->create_media_controller_called_ = false; + + scoped_refptr pc( + pcf_->CreatePeerConnection(config, constraints, nullptr, nullptr, + &observer_)); + EXPECT_TRUE(pc.get()); + EXPECT_TRUE(pcf_->create_media_controller_called_); + return pcf_->create_media_controller_config_; + } + + scoped_refptr pcf_; + MockPeerConnectionObserver observer_; +}; + +// This test verifies the default behaviour with no constraints and a +// default RTCConfiguration. +TEST_F(PeerConnectionMediaConfigTest, TestDefaults) { + PeerConnectionInterface::RTCConfiguration config; + FakeConstraints constraints; + + const cricket::MediaConfig& media_config = + TestCreatePeerConnection(config, &constraints); + + EXPECT_FALSE(media_config.enable_dscp); + EXPECT_TRUE(media_config.enable_cpu_overuse_detection); + EXPECT_FALSE(media_config.disable_prerenderer_smoothing); +} + +// This test verifies the DSCP constraint is recognized and passed to +// the CreateMediaController call. +TEST_F(PeerConnectionMediaConfigTest, TestDscpConstraintTrue) { + PeerConnectionInterface::RTCConfiguration config; + FakeConstraints constraints; + + constraints.AddOptional(webrtc::MediaConstraintsInterface::kEnableDscp, true); + const cricket::MediaConfig& media_config = + TestCreatePeerConnection(config, &constraints); + + EXPECT_TRUE(media_config.enable_dscp); +} + +// This test verifies the cpu overuse detection constraint is +// recognized and passed to the CreateMediaController call. +TEST_F(PeerConnectionMediaConfigTest, TestCpuOveruseConstraintFalse) { + PeerConnectionInterface::RTCConfiguration config; + FakeConstraints constraints; + + constraints.AddOptional( + webrtc::MediaConstraintsInterface::kCpuOveruseDetection, false); + const cricket::MediaConfig media_config = + TestCreatePeerConnection(config, &constraints); + + EXPECT_FALSE(media_config.enable_cpu_overuse_detection); +} + +// This test verifies that the disable_prerenderer_smoothing flag is +// propagated from RTCConfiguration to the CreateMediaController call. +TEST_F(PeerConnectionMediaConfigTest, TestDisablePrerendererSmoothingTrue) { + PeerConnectionInterface::RTCConfiguration config; + FakeConstraints constraints; + + config.disable_prerenderer_smoothing = true; + const cricket::MediaConfig& media_config = + TestCreatePeerConnection(config, &constraints); + + EXPECT_TRUE(media_config.disable_prerenderer_smoothing); +} + // The following tests verify that session options are created correctly. // TODO(deadbeef): Convert these tests to be more end-to-end. Instead of // "verify options are converted correctly", should be "pass options into diff --git a/webrtc/api/statscollector_unittest.cc b/webrtc/api/statscollector_unittest.cc index cdb4bd0d75..d4a59a1a6d 100644 --- a/webrtc/api/statscollector_unittest.cc +++ b/webrtc/api/statscollector_unittest.cc @@ -489,7 +489,8 @@ class StatsCollectorTest : public testing::Test { channel_manager_( new cricket::ChannelManager(media_engine_, rtc::Thread::Current())), media_controller_( - webrtc::MediaControllerInterface::Create(rtc::Thread::Current(), + webrtc::MediaControllerInterface::Create(cricket::MediaConfig(), + rtc::Thread::Current(), channel_manager_.get())), session_(media_controller_.get()) { // By default, we ignore session GetStats calls. diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc index 56bea6403d..c693cb1077 100644 --- a/webrtc/api/webrtcsession.cc +++ b/webrtc/api/webrtcsession.cc @@ -584,8 +584,6 @@ bool WebRtcSession::Initialize( const PeerConnectionInterface::RTCConfiguration& rtc_configuration) { bundle_policy_ = rtc_configuration.bundle_policy; rtcp_mux_policy_ = rtc_configuration.rtcp_mux_policy; - video_options_.disable_prerenderer_smoothing = - rtc::Optional(rtc_configuration.disable_prerenderer_smoothing); transport_controller_->SetSslMaxProtocolVersion(options.ssl_max_version); // Obtain a certificate from RTCConfiguration if any were provided (optional). @@ -632,15 +630,6 @@ bool WebRtcSession::Initialize( } } - // Find DSCP constraint. - if (FindConstraint( - constraints, - MediaConstraintsInterface::kEnableDscp, - &value, NULL)) { - audio_options_.dscp = rtc::Optional(value); - video_options_.dscp = rtc::Optional(value); - } - // Find Suspend Below Min Bitrate constraint. if (FindConstraint( constraints, @@ -654,11 +643,6 @@ bool WebRtcSession::Initialize( MediaConstraintsInterface::kScreencastMinBitrate, &video_options_.screencast_min_bitrate_kbps); - // Find constraints for cpu overuse detection. - SetOptionFromOptionalConstraint(constraints, - MediaConstraintsInterface::kCpuOveruseDetection, - &video_options_.cpu_overuse_detection); - SetOptionFromOptionalConstraint(constraints, MediaConstraintsInterface::kCombinedAudioVideoBwe, &audio_options_.combined_audio_video_bwe); diff --git a/webrtc/api/webrtcsession_unittest.cc b/webrtc/api/webrtcsession_unittest.cc index 1fe3a81592..16ce621eb8 100644 --- a/webrtc/api/webrtcsession_unittest.cc +++ b/webrtc/api/webrtcsession_unittest.cc @@ -329,7 +329,8 @@ class WebRtcSessionTest rtc::Thread::Current())), fake_call_(webrtc::Call::Config()), media_controller_( - webrtc::MediaControllerInterface::Create(rtc::Thread::Current(), + webrtc::MediaControllerInterface::Create(cricket::MediaConfig(), + rtc::Thread::Current(), channel_manager_.get())), tdesc_factory_(new cricket::TransportDescriptionFactory()), desc_factory_( @@ -4039,28 +4040,6 @@ TEST_F(WebRtcSessionTest, TestSetRemoteOfferFailIfDtlsDisabledAndNoCrypto) { offer); } -// This test verifies DSCP is properly applied on the media channels. -TEST_F(WebRtcSessionTest, TestDscpConstraint) { - constraints_.reset(new FakeConstraints()); - constraints_->AddOptional( - webrtc::MediaConstraintsInterface::kEnableDscp, true); - Init(); - SendAudioVideoStream1(); - SessionDescriptionInterface* offer = CreateOffer(); - - SetLocalDescriptionWithoutError(offer); - - video_channel_ = media_engine_->GetVideoChannel(0); - voice_channel_ = media_engine_->GetVoiceChannel(0); - - ASSERT_TRUE(video_channel_ != NULL); - ASSERT_TRUE(voice_channel_ != NULL); - const cricket::AudioOptions& audio_options = voice_channel_->options(); - const cricket::VideoOptions& video_options = video_channel_->options(); - EXPECT_EQ(rtc::Optional(true), audio_options.dscp); - EXPECT_EQ(rtc::Optional(true), video_options.dscp); -} - TEST_F(WebRtcSessionTest, TestSuspendBelowMinBitrateConstraint) { constraints_.reset(new FakeConstraints()); constraints_->AddOptional( diff --git a/webrtc/media/base/fakemediaengine.h b/webrtc/media/base/fakemediaengine.h index 49790cb053..f2de5ac653 100644 --- a/webrtc/media/base/fakemediaengine.h +++ b/webrtc/media/base/fakemediaengine.h @@ -665,6 +665,7 @@ class FakeVoiceEngine : public FakeBaseEngine { } VoiceMediaChannel* CreateChannel(webrtc::Call* call, + const MediaConfig& config, const AudioOptions& options) { if (fail_create_channel_) { return nullptr; @@ -728,6 +729,7 @@ class FakeVideoEngine : public FakeBaseEngine { } VideoMediaChannel* CreateChannel(webrtc::Call* call, + const MediaConfig& config, const VideoOptions& options) { if (fail_create_channel_) { return NULL; diff --git a/webrtc/media/base/mediachannel.h b/webrtc/media/base/mediachannel.h index e32eb6078e..3f6c8dda63 100644 --- a/webrtc/media/base/mediachannel.h +++ b/webrtc/media/base/mediachannel.h @@ -78,6 +78,40 @@ static std::string VectorToString(const std::vector& vals) { return ost.str(); } +// Construction-time settings, passed to +// MediaControllerInterface::Create, and passed on when creating +// MediaChannels. +struct MediaConfig { + // Set DSCP value on packets. This flag comes from the + // PeerConnection constraint 'googDscp'. + bool enable_dscp = false; + + // Video-specific config + + // Enable WebRTC CPU Overuse Detection. This flag comes from the + // PeerConnection constraint 'googCpuOveruseDetection' and is + // checked in WebRtcVideoChannel2::OnLoadUpdate, where it's passed + // to VideoCapturer::video_adapter()->OnCpuResolutionRequest. + bool enable_cpu_overuse_detection = true; + + // Set to true if the renderer has an algorithm of frame selection. + // If the value is true, then WebRTC will hand over a frame as soon as + // possible without delay, and rendering smoothness is completely the duty + // of the renderer; + // If the value is false, then WebRTC is responsible to delay frame release + // in order to increase rendering smoothness. + // + // This flag comes from PeerConnection's RtcConfiguration, but is + // currently only set by the command line flag + // 'disable-rtc-smoothness-algorithm'. + // WebRtcVideoChannel2::AddRecvStream copies it to the created + // WebRtcVideoReceiveStream, where it is returned by the + // SmoothsRenderedFrames method. This method is used by the + // VideoReceiveStream, where the value is passed on to the + // IncomingVideoStream constructor. + bool disable_prerenderer_smoothing = false; +}; + // Options that can be applied to a VoiceMediaChannel or a VoiceMediaEngine. // Used to be flags, but that makes it hard to selectively apply options. // We are moving all of the setting of options to structs like this, @@ -108,7 +142,6 @@ struct AudioOptions { SetFrom(&tx_agc_limiter, change.tx_agc_limiter); SetFrom(&recording_sample_rate, change.recording_sample_rate); SetFrom(&playout_sample_rate, change.playout_sample_rate); - SetFrom(&dscp, change.dscp); SetFrom(&combined_audio_video_bwe, change.combined_audio_video_bwe); } @@ -135,7 +168,6 @@ struct AudioOptions { tx_agc_limiter == o.tx_agc_limiter && recording_sample_rate == o.recording_sample_rate && playout_sample_rate == o.playout_sample_rate && - dscp == o.dscp && combined_audio_video_bwe == o.combined_audio_video_bwe; } @@ -166,7 +198,6 @@ struct AudioOptions { ost << ToStringIfSet("tx_agc_limiter", tx_agc_limiter); ost << ToStringIfSet("recording_sample_rate", recording_sample_rate); ost << ToStringIfSet("playout_sample_rate", playout_sample_rate); - ost << ToStringIfSet("dscp", dscp); ost << ToStringIfSet("combined_audio_video_bwe", combined_audio_video_bwe); ost << "}"; return ost.str(); @@ -203,9 +234,10 @@ struct AudioOptions { rtc::Optional tx_agc_limiter; rtc::Optional recording_sample_rate; rtc::Optional playout_sample_rate; - // Set DSCP value for packet sent from audio channel. - rtc::Optional dscp; // Enable combined audio+bandwidth BWE. + // TODO(pthatcher): This flag is set from the + // "googCombinedAudioVideoBwe", but not used anywhere. So delete it, + // and check if any other AudioOptions members are unused. rtc::Optional combined_audio_video_bwe; private: @@ -224,32 +256,23 @@ struct AudioOptions { struct VideoOptions { void SetAll(const VideoOptions& change) { SetFrom(&video_noise_reduction, change.video_noise_reduction); - SetFrom(&cpu_overuse_detection, change.cpu_overuse_detection); SetFrom(&conference_mode, change.conference_mode); - SetFrom(&dscp, change.dscp); SetFrom(&suspend_below_min_bitrate, change.suspend_below_min_bitrate); SetFrom(&screencast_min_bitrate_kbps, change.screencast_min_bitrate_kbps); - SetFrom(&disable_prerenderer_smoothing, - change.disable_prerenderer_smoothing); } bool operator==(const VideoOptions& o) const { return video_noise_reduction == o.video_noise_reduction && - cpu_overuse_detection == o.cpu_overuse_detection && conference_mode == o.conference_mode && - dscp == o.dscp && suspend_below_min_bitrate == o.suspend_below_min_bitrate && - screencast_min_bitrate_kbps == o.screencast_min_bitrate_kbps && - disable_prerenderer_smoothing == o.disable_prerenderer_smoothing; + screencast_min_bitrate_kbps == o.screencast_min_bitrate_kbps; } std::string ToString() const { std::ostringstream ost; ost << "VideoOptions {"; ost << ToStringIfSet("noise reduction", video_noise_reduction); - ost << ToStringIfSet("cpu overuse detection", cpu_overuse_detection); ost << ToStringIfSet("conference mode", conference_mode); - ost << ToStringIfSet("dscp", dscp); ost << ToStringIfSet("suspend below min bitrate", suspend_below_min_bitrate); ost << ToStringIfSet("screencast min bitrate kbps", @@ -262,11 +285,6 @@ struct VideoOptions { // constraint 'googNoiseReduction', and WebRtcVideoEngine2 passes it // on to the codec options. Disabled by default. rtc::Optional video_noise_reduction; - // Enable WebRTC Cpu Overuse Detection. This flag comes from the - // PeerConnection constraint 'googCpuOveruseDetection' and is - // checked in WebRtcVideoChannel2::OnLoadUpdate, where it's passed - // to VideoCapturer::video_adapter()->OnCpuResolutionRequest. - rtc::Optional cpu_overuse_detection; // Use conference mode? This flag comes from the remote // description's SDP line 'a=x-google-flag:conference', copied over // by VideoChannel::SetRemoteContent_w, and ultimately used by @@ -274,12 +292,6 @@ struct VideoOptions { // WebRtcVideoChannel2::WebRtcVideoSendStream::CreateVideoEncoderConfig. // The special screencast behaviour is disabled by default. rtc::Optional conference_mode; - // Set DSCP value for packet sent from video channel. This flag - // comes from the PeerConnection constraint 'googDscp' and, - // WebRtcVideoChannel2::SetOptions checks it before calling - // MediaChannel::SetDscp. If enabled, rtc::DSCP_AF41 is used. If - // disabled, which is the default, rtc::DSCP_DEFAULT is used. - rtc::Optional dscp; // Enable WebRTC suspension of video. No video frames will be sent // when the bitrate is below the configured minimum bitrate. This // flag comes from the PeerConnection constraint @@ -290,22 +302,6 @@ struct VideoOptions { // the PeerConnection constraint 'googScreencastMinBitrate'. It is // copied to the encoder config by WebRtcVideoChannel2. rtc::Optional screencast_min_bitrate_kbps; - // Set to true if the renderer has an algorithm of frame selection. - // If the value is true, then WebRTC will hand over a frame as soon as - // possible without delay, and rendering smoothness is completely the duty - // of the renderer; - // If the value is false, then WebRTC is responsible to delay frame release - // in order to increase rendering smoothness. - // - // This flag comes from PeerConnection's RtcConfiguration, but is - // currently only set by the command line flag - // 'disable-rtc-smoothness-algorithm'. - // WebRtcVideoChannel2::AddRecvStream copies it to the created - // WebRtcVideoReceiveStream, where it is returned by the - // SmoothsRenderedFrames method. This method is used by the - // VideoReceiveStream, where the value is passed on to the - // IncomingVideoStream constructor. - rtc::Optional disable_prerenderer_smoothing; private: template @@ -368,15 +364,20 @@ class MediaChannel : public sigslot::has_slots<> { virtual ~NetworkInterface() {} }; - MediaChannel() : network_interface_(NULL) {} + MediaChannel(const MediaConfig& config) + : enable_dscp_(config.enable_dscp), network_interface_(NULL) {} + MediaChannel() : enable_dscp_(false), network_interface_(NULL) {} virtual ~MediaChannel() {} // Sets the abstract interface class for sending RTP/RTCP data. virtual void SetInterface(NetworkInterface *iface) { rtc::CritScope cs(&network_interface_crit_); network_interface_ = iface; + SetDscp(enable_dscp_ ? PreferredDscp() : rtc::DSCP_DEFAULT); + } + virtual rtc::DiffServCodePoint PreferredDscp() const { + return rtc::DSCP_DEFAULT; } - // Called when a RTP packet is received. virtual void OnPacketReceived(rtc::Buffer* packet, const rtc::PacketTime& packet_time) = 0; @@ -424,7 +425,7 @@ class MediaChannel : public sigslot::has_slots<> { return network_interface_->SetOption(type, opt, option); } - protected: + private: // This method sets DSCP |value| on both RTP and RTCP channels. int SetDscp(rtc::DiffServCodePoint value) { int ret; @@ -439,7 +440,6 @@ class MediaChannel : public sigslot::has_slots<> { return ret; } - private: bool DoSendPacket(rtc::Buffer* packet, bool rtcp, const rtc::PacketOptions& options) { @@ -451,6 +451,7 @@ class MediaChannel : public sigslot::has_slots<> { : network_interface_->SendRtcp(packet, options); } + const bool enable_dscp_; // |network_interface_| can be accessed from the worker_thread and // from any MediaEngine threads. This critical section is to protect accessing // of network_interface_ object. @@ -904,6 +905,7 @@ class VoiceMediaChannel : public MediaChannel { }; VoiceMediaChannel() {} + VoiceMediaChannel(const MediaConfig& config) : MediaChannel(config) {} virtual ~VoiceMediaChannel() {} virtual bool SetSendParameters(const AudioSendParameters& params) = 0; virtual bool SetRecvParameters(const AudioRecvParameters& params) = 0; @@ -967,6 +969,7 @@ class VideoMediaChannel : public MediaChannel { }; VideoMediaChannel() {} + VideoMediaChannel(const MediaConfig& config) : MediaChannel(config) {} virtual ~VideoMediaChannel() {} virtual bool SetSendParameters(const VideoSendParameters& params) = 0; diff --git a/webrtc/media/base/mediaengine.h b/webrtc/media/base/mediaengine.h index 43e1538a00..479d1acf4e 100644 --- a/webrtc/media/base/mediaengine.h +++ b/webrtc/media/base/mediaengine.h @@ -62,13 +62,14 @@ class MediaEngineInterface { // MediaChannel creation // Creates a voice media channel. Returns NULL on failure. - virtual VoiceMediaChannel* CreateChannel( - webrtc::Call* call, - const AudioOptions& options) = 0; + virtual VoiceMediaChannel* CreateChannel(webrtc::Call* call, + const MediaConfig& config, + const AudioOptions& options) = 0; // Creates a video media channel, paired with the specified voice channel. // Returns NULL on failure. virtual VideoMediaChannel* CreateVideoChannel( webrtc::Call* call, + const MediaConfig& config, const VideoOptions& options) = 0; // Device configuration @@ -139,12 +140,14 @@ class CompositeMediaEngine : public MediaEngineInterface { return voice_.GetAudioState(); } virtual VoiceMediaChannel* CreateChannel(webrtc::Call* call, + const MediaConfig& config, const AudioOptions& options) { - return voice_.CreateChannel(call, options); + return voice_.CreateChannel(call, config, options); } virtual VideoMediaChannel* CreateVideoChannel(webrtc::Call* call, + const MediaConfig& config, const VideoOptions& options) { - return video_.CreateChannel(call, options); + return video_.CreateChannel(call, config, options); } virtual bool GetOutputVolume(int* level) { diff --git a/webrtc/media/base/videoengine_unittest.h b/webrtc/media/base/videoengine_unittest.h index 2541c4acd3..b7fbd4bd55 100644 --- a/webrtc/media/base/videoengine_unittest.h +++ b/webrtc/media/base/videoengine_unittest.h @@ -125,8 +125,8 @@ class VideoMediaChannelTest : public testing::Test, virtual void SetUp() { cricket::Device device("test", "device"); engine_.Init(); - channel_.reset( - engine_.CreateChannel(call_.get(), cricket::VideoOptions())); + channel_.reset(engine_.CreateChannel(call_.get(), cricket::MediaConfig(), + cricket::VideoOptions())); EXPECT_TRUE(channel_.get() != NULL); network_interface_.SetDestination(channel_.get()); channel_->SetInterface(&network_interface_); diff --git a/webrtc/media/engine/nullwebrtcvideoengine.h b/webrtc/media/engine/nullwebrtcvideoengine.h index 488f02159d..2fe87f4bad 100644 --- a/webrtc/media/engine/nullwebrtcvideoengine.h +++ b/webrtc/media/engine/nullwebrtcvideoengine.h @@ -51,7 +51,8 @@ class NullWebRtcVideoEngine { } VideoMediaChannel* CreateChannel(webrtc::Call* call, - const VideoOptions& options) { + const MediaConfig& config, + const VideoOptions& options) { return nullptr; } diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index f747da997b..509c8a69ab 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -509,11 +509,13 @@ void WebRtcVideoEngine2::Init() { WebRtcVideoChannel2* WebRtcVideoEngine2::CreateChannel( webrtc::Call* call, + const MediaConfig& config, const VideoOptions& options) { RTC_DCHECK(initialized_); LOG(LS_INFO) << "CreateChannel. Options: " << options.ToString(); - return new WebRtcVideoChannel2(call, options, video_codecs_, - external_encoder_factory_, external_decoder_factory_); + return new WebRtcVideoChannel2(call, config, options, video_codecs_, + external_encoder_factory_, + external_decoder_factory_); } const std::vector& WebRtcVideoEngine2::codecs() const { @@ -602,19 +604,20 @@ std::vector WebRtcVideoEngine2::GetSupportedCodecs() const { WebRtcVideoChannel2::WebRtcVideoChannel2( webrtc::Call* call, + const MediaConfig& config, const VideoOptions& options, const std::vector& recv_codecs, WebRtcVideoEncoderFactory* external_encoder_factory, WebRtcVideoDecoderFactory* external_decoder_factory) - : call_(call), + : VideoMediaChannel(config), + call_(call), unsignalled_ssrc_handler_(&default_unsignalled_ssrc_handler_), + signal_cpu_adaptation_(config.enable_cpu_overuse_detection), + disable_prerenderer_smoothing_(config.disable_prerenderer_smoothing), external_encoder_factory_(external_encoder_factory), external_decoder_factory_(external_decoder_factory) { RTC_DCHECK(thread_checker_.CalledOnValidThread()); - SetDefaultOptions(); options_.SetAll(options); - if (options_.cpu_overuse_detection) - signal_cpu_adaptation_ = *options_.cpu_overuse_detection; rtcp_receiver_report_ssrc_ = kDefaultRtcpReceiverReportSsrc; sending_ = false; default_send_ssrc_ = 0; @@ -622,13 +625,6 @@ WebRtcVideoChannel2::WebRtcVideoChannel2( recv_codecs_ = FilterSupportedCodecs(MapCodecs(recv_codecs)); } -void WebRtcVideoChannel2::SetDefaultOptions() { - options_.cpu_overuse_detection = rtc::Optional(true); - options_.dscp = rtc::Optional(false); - options_.suspend_below_min_bitrate = rtc::Optional(false); - options_.screencast_min_bitrate_kbps = rtc::Optional(0); -} - WebRtcVideoChannel2::~WebRtcVideoChannel2() { for (auto& kv : send_streams_) delete kv.second; @@ -757,6 +753,10 @@ bool WebRtcVideoChannel2::GetChangedSendParameters( return true; } +rtc::DiffServCodePoint WebRtcVideoChannel2::PreferredDscp() const { + return rtc::DSCP_AF41; +} + bool WebRtcVideoChannel2::SetSendParameters(const VideoSendParameters& params) { TRACE_EVENT0("webrtc", "WebRtcVideoChannel2::SetSendParameters"); LOG(LS_INFO) << "SetSendParameters: " << params.ToString(); @@ -801,18 +801,8 @@ bool WebRtcVideoChannel2::SetSendParameters(const VideoSendParameters& params) { call_->SetBitrateConfig(bitrate_config_); } - if (changed_params.options) { + if (changed_params.options) options_.SetAll(*changed_params.options); - { - rtc::CritScope lock(&capturer_crit_); - if (options_.cpu_overuse_detection) { - signal_cpu_adaptation_ = *options_.cpu_overuse_detection; - } - } - rtc::DiffServCodePoint dscp = - options_.dscp.value_or(false) ? rtc::DSCP_AF41 : rtc::DSCP_DEFAULT; - MediaChannel::SetDscp(dscp); - } { rtc::CritScope stream_lock(&stream_crit_); @@ -1138,7 +1128,7 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp, receive_streams_[ssrc] = new WebRtcVideoReceiveStream( call_, sp, config, external_decoder_factory_, default_stream, - recv_codecs_, options_.disable_prerenderer_smoothing.value_or(false)); + recv_codecs_, disable_prerenderer_smoothing_); return true; } @@ -1767,9 +1757,8 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetCodecAndOptions( parameters_.config.rtp.nack.rtp_history_ms = HasNack(codec_settings.codec) ? kNackHistoryMs : 0; - RTC_CHECK(options.suspend_below_min_bitrate); parameters_.config.suspend_below_min_bitrate = - *options.suspend_below_min_bitrate; + options.suspend_below_min_bitrate.value_or(false); parameters_.codec_settings = rtc::Optional(codec_settings); @@ -1835,9 +1824,8 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::CreateVideoEncoderConfig( const VideoCodec& codec) const { webrtc::VideoEncoderConfig encoder_config; if (dimensions.is_screencast) { - RTC_CHECK(parameters_.options.screencast_min_bitrate_kbps); encoder_config.min_transmit_bitrate_bps = - *parameters_.options.screencast_min_bitrate_kbps * 1000; + 1000 * parameters_.options.screencast_min_bitrate_kbps.value_or(0); encoder_config.content_type = webrtc::VideoEncoderConfig::ContentType::kScreen; } else { diff --git a/webrtc/media/engine/webrtcvideoengine2.h b/webrtc/media/engine/webrtcvideoengine2.h index 40485eeb17..a8218324f8 100644 --- a/webrtc/media/engine/webrtcvideoengine2.h +++ b/webrtc/media/engine/webrtcvideoengine2.h @@ -34,6 +34,7 @@ namespace webrtc { class VideoDecoder; class VideoEncoder; +struct MediaConfig; } namespace rtc { @@ -98,6 +99,7 @@ class WebRtcVideoEngine2 { void Init(); WebRtcVideoChannel2* CreateChannel(webrtc::Call* call, + const MediaConfig& config, const VideoOptions& options); const std::vector& codecs() const; @@ -130,6 +132,7 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::LoadObserver { public: WebRtcVideoChannel2(webrtc::Call* call, + const MediaConfig& config, const VideoOptions& options, const std::vector& recv_codecs, WebRtcVideoEncoderFactory* external_encoder_factory, @@ -137,6 +140,8 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, ~WebRtcVideoChannel2() override; // VideoMediaChannel implementation + rtc::DiffServCodePoint PreferredDscp() const override; + bool SetSendParameters(const VideoSendParameters& params) override; bool SetRecvParameters(const VideoRecvParameters& params) override; bool GetSendCodec(VideoCodec* send_codec) override; @@ -479,11 +484,13 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, DefaultUnsignalledSsrcHandler default_unsignalled_ssrc_handler_; UnsignalledSsrcHandler* const unsignalled_ssrc_handler_; + const bool signal_cpu_adaptation_; + const bool disable_prerenderer_smoothing_; + // Separate list of set capturers used to signal CPU adaptation. These should // not be locked while calling methods that take other locks to prevent // lock-order inversions. rtc::CriticalSection capturer_crit_; - bool signal_cpu_adaptation_ GUARDED_BY(capturer_crit_); std::map capturers_ GUARDED_BY(capturer_crit_); rtc::CriticalSection stream_crit_; diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index b089d86ffa..65b5c6b1e1 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -285,7 +285,7 @@ TEST_F(WebRtcVideoEngine2Test, CVOSetHeaderExtensionAfterCapturer) { TEST_F(WebRtcVideoEngine2Test, SetSendFailsBeforeSettingCodecs) { engine_.Init(); rtc::scoped_ptr channel( - engine_.CreateChannel(call_.get(), cricket::VideoOptions())); + engine_.CreateChannel(call_.get(), MediaConfig(), VideoOptions())); EXPECT_TRUE(channel->AddSendStream(StreamParams::CreateLegacy(123))); @@ -298,7 +298,7 @@ TEST_F(WebRtcVideoEngine2Test, SetSendFailsBeforeSettingCodecs) { TEST_F(WebRtcVideoEngine2Test, GetStatsWithoutSendCodecsSetDoesNotCrash) { engine_.Init(); rtc::scoped_ptr channel( - engine_.CreateChannel(call_.get(), cricket::VideoOptions())); + engine_.CreateChannel(call_.get(), MediaConfig(), VideoOptions())); EXPECT_TRUE(channel->AddSendStream(StreamParams::CreateLegacy(123))); VideoMediaInfo info; channel->GetStats(&info); @@ -355,7 +355,8 @@ void WebRtcVideoEngine2Test::TestExtendedEncoderOveruse( SetUpForExternalEncoderFactory(&encoder_factory, parameters.codecs)); } else { engine_.Init(); - channel.reset(engine_.CreateChannel(call_.get(), cricket::VideoOptions())); + channel.reset( + engine_.CreateChannel(call_.get(), MediaConfig(), VideoOptions())); } ASSERT_TRUE( channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrc))); @@ -513,7 +514,7 @@ VideoMediaChannel* WebRtcVideoEngine2Test::SetUpForExternalEncoderFactory( engine_.Init(); VideoMediaChannel* channel = - engine_.CreateChannel(call_.get(), cricket::VideoOptions()); + engine_.CreateChannel(call_.get(), MediaConfig(), VideoOptions()); cricket::VideoSendParameters parameters; parameters.codecs = codecs; EXPECT_TRUE(channel->SetSendParameters(parameters)); @@ -528,7 +529,7 @@ VideoMediaChannel* WebRtcVideoEngine2Test::SetUpForExternalDecoderFactory( engine_.Init(); VideoMediaChannel* channel = - engine_.CreateChannel(call_.get(), cricket::VideoOptions()); + engine_.CreateChannel(call_.get(), MediaConfig(), VideoOptions()); cricket::VideoRecvParameters parameters; parameters.codecs = codecs; EXPECT_TRUE(channel->SetRecvParameters(parameters)); @@ -848,7 +849,7 @@ class WebRtcVideoChannel2Test : public WebRtcVideoEngine2Test { fake_call_.reset(new FakeCall(webrtc::Call::Config())); engine_.Init(); channel_.reset( - engine_.CreateChannel(fake_call_.get(), cricket::VideoOptions())); + engine_.CreateChannel(fake_call_.get(), MediaConfig(), VideoOptions())); last_ssrc_ = 123; send_parameters_.codecs = engine_.codecs(); recv_parameters_.codecs = engine_.codecs(); @@ -1737,9 +1738,14 @@ void WebRtcVideoChannel2Test::TestCpuAdaptation(bool enable_overuse, cricket::VideoCodec codec = kVp8Codec720p; cricket::VideoSendParameters parameters; parameters.codecs.push_back(codec); + + MediaConfig media_config = MediaConfig(); if (!enable_overuse) { - parameters.options.cpu_overuse_detection = rtc::Optional(false); + media_config.enable_cpu_overuse_detection = false; } + channel_.reset( + engine_.CreateChannel(fake_call_.get(), media_config, VideoOptions())); + EXPECT_TRUE(channel_->SetSendParameters(parameters)); AddSendStream(); @@ -2266,21 +2272,25 @@ TEST_F(WebRtcVideoChannel2Test, SetSend) { TEST_F(WebRtcVideoChannel2Test, TestSetDscpOptions) { rtc::scoped_ptr network_interface( new cricket::FakeNetworkInterface); - channel_->SetInterface(network_interface.get()); - cricket::VideoSendParameters parameters = send_parameters_; - EXPECT_TRUE(channel_->SetSendParameters(parameters)); - EXPECT_EQ(rtc::DSCP_NO_CHANGE, network_interface->dscp()); - parameters.options.dscp = rtc::Optional(true); - EXPECT_TRUE(channel_->SetSendParameters(parameters)); - EXPECT_EQ(rtc::DSCP_AF41, network_interface->dscp()); - // Verify previous value is not modified if dscp option is not set. - cricket::VideoSendParameters parameters1 = send_parameters_; - EXPECT_TRUE(channel_->SetSendParameters(parameters1)); - EXPECT_EQ(rtc::DSCP_AF41, network_interface->dscp()); - parameters1.options.dscp = rtc::Optional(false); - EXPECT_TRUE(channel_->SetSendParameters(parameters1)); + MediaConfig config; + rtc::scoped_ptr channel; + + channel.reset(engine_.CreateChannel(call_.get(), config, VideoOptions())); + channel->SetInterface(network_interface.get()); + // Default value when DSCP is disabled should be DSCP_DEFAULT. + EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface->dscp()); + + config.enable_dscp = true; + channel.reset(engine_.CreateChannel(call_.get(), config, VideoOptions())); + channel->SetInterface(network_interface.get()); + EXPECT_EQ(rtc::DSCP_AF41, network_interface->dscp()); + + // Verify that setting the option to false resets the + // DiffServCodePoint. + config.enable_dscp = false; + channel.reset(engine_.CreateChannel(call_.get(), config, VideoOptions())); + channel->SetInterface(network_interface.get()); EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface->dscp()); - channel_->SetInterface(NULL); } // This test verifies that the RTCP reduced size mode is properly applied to @@ -2403,8 +2413,6 @@ TEST_F(WebRtcVideoChannel2Test, GetStatsTracksAdaptationStats) { EXPECT_TRUE(channel_->SetSend(true)); // Verify that the CpuOveruseObserver is registered and trigger downgrade. - parameters.options.cpu_overuse_detection = rtc::Optional(true); - EXPECT_TRUE(channel_->SetSendParameters(parameters)); // Trigger overuse. ASSERT_EQ(1u, fake_call_->GetVideoSendStreams().size()); @@ -2480,8 +2488,6 @@ TEST_F(WebRtcVideoChannel2Test, GetStatsTracksAdaptationAndBandwidthStats) { EXPECT_TRUE(channel_->SetSend(true)); // Verify that the CpuOveruseObserver is registered and trigger downgrade. - parameters.options.cpu_overuse_detection = rtc::Optional(true); - EXPECT_TRUE(channel_->SetSendParameters(parameters)); // Trigger overuse -> adapt CPU. ASSERT_EQ(1u, fake_call_->GetVideoSendStreams().size()); @@ -2941,7 +2947,8 @@ class WebRtcVideoChannel2SimulcastTest : public testing::Test { void SetUp() override { engine_.Init(); - channel_.reset(engine_.CreateChannel(&fake_call_, VideoOptions())); + channel_.reset( + engine_.CreateChannel(&fake_call_, MediaConfig(), VideoOptions())); last_ssrc_ = 123; } diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc index c4e92c0024..b6c2deaf53 100644 --- a/webrtc/media/engine/webrtcvoiceengine.cc +++ b/webrtc/media/engine/webrtcvoiceengine.cc @@ -649,10 +649,12 @@ rtc::scoped_refptr return audio_state_; } -VoiceMediaChannel* WebRtcVoiceEngine::CreateChannel(webrtc::Call* call, +VoiceMediaChannel* WebRtcVoiceEngine::CreateChannel( + webrtc::Call* call, + const MediaConfig& config, const AudioOptions& options) { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); - return new WebRtcVoiceMediaChannel(this, options, call); + return new WebRtcVoiceMediaChannel(this, config, options, call); } bool WebRtcVoiceEngine::ApplyOptions(const AudioOptions& options_in) { @@ -1366,9 +1368,10 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream { }; WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel(WebRtcVoiceEngine* engine, + const MediaConfig& config, const AudioOptions& options, webrtc::Call* call) - : engine_(engine), call_(call) { + : VoiceMediaChannel(config), engine_(engine), call_(call) { LOG(LS_VERBOSE) << "WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel"; RTC_DCHECK(call); engine->RegisterChannel(this); @@ -1390,6 +1393,10 @@ WebRtcVoiceMediaChannel::~WebRtcVoiceMediaChannel() { engine()->UnregisterChannel(this); } +rtc::DiffServCodePoint WebRtcVoiceMediaChannel::PreferredDscp() const { + return kAudioDscpValue; +} + bool WebRtcVoiceMediaChannel::SetSendParameters( const AudioSendParameters& params) { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); @@ -1453,9 +1460,6 @@ bool WebRtcVoiceMediaChannel::SetOptions(const AudioOptions& options) { LOG(LS_INFO) << "Setting voice channel options: " << options.ToString(); - // Check if DSCP value is changed from previous. - bool dscp_option_changed = (options_.dscp != options.dscp); - // We retain all of the existing options, and apply the given ones // on top. This means there is no way to "clear" options such that // they go back to the engine default. @@ -1465,17 +1469,6 @@ bool WebRtcVoiceMediaChannel::SetOptions(const AudioOptions& options) { "Failed to apply engine options during channel SetOptions."; return false; } - - if (dscp_option_changed) { - rtc::DiffServCodePoint dscp = rtc::DSCP_DEFAULT; - if (options_.dscp.value_or(false)) { - dscp = kAudioDscpValue; - } - if (MediaChannel::SetDscp(dscp) != 0) { - LOG(LS_WARNING) << "Failed to set DSCP settings for audio channel"; - } - } - LOG(LS_INFO) << "Set voice channel options. Current options: " << options_.ToString(); return true; diff --git a/webrtc/media/engine/webrtcvoiceengine.h b/webrtc/media/engine/webrtcvoiceengine.h index fac7fa6f30..55655e3887 100644 --- a/webrtc/media/engine/webrtcvoiceengine.h +++ b/webrtc/media/engine/webrtcvoiceengine.h @@ -52,6 +52,7 @@ class WebRtcVoiceEngine final : public webrtc::TraceCallback { rtc::scoped_refptr GetAudioState() const; VoiceMediaChannel* CreateChannel(webrtc::Call* call, + const MediaConfig& config, const AudioOptions& options); bool GetOutputVolume(int* level); @@ -140,12 +141,15 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, public webrtc::Transport { public: WebRtcVoiceMediaChannel(WebRtcVoiceEngine* engine, + const MediaConfig& config, const AudioOptions& options, webrtc::Call* call); ~WebRtcVoiceMediaChannel() override; const AudioOptions& options() const { return options_; } + rtc::DiffServCodePoint PreferredDscp() const override; + bool SetSendParameters(const AudioSendParameters& params) override; bool SetRecvParameters(const AudioRecvParameters& params) override; bool SetPlayout(bool playout) override; diff --git a/webrtc/media/engine/webrtcvoiceengine_unittest.cc b/webrtc/media/engine/webrtcvoiceengine_unittest.cc index 2eafb47961..3953be6736 100644 --- a/webrtc/media/engine/webrtcvoiceengine_unittest.cc +++ b/webrtc/media/engine/webrtcvoiceengine_unittest.cc @@ -77,7 +77,8 @@ class WebRtcVoiceEngineTestFake : public testing::Test { if (!engine_.Init(rtc::Thread::Current())) { return false; } - channel_ = engine_.CreateChannel(&call_, cricket::AudioOptions()); + channel_ = engine_.CreateChannel(&call_, cricket::MediaConfig(), + cricket::AudioOptions()); return (channel_ != nullptr); } bool SetupEngineWithRecvStream() { @@ -137,7 +138,8 @@ class WebRtcVoiceEngineTestFake : public testing::Test { void TestInsertDtmf(uint32_t ssrc, bool caller) { EXPECT_TRUE(engine_.Init(rtc::Thread::Current())); - channel_ = engine_.CreateChannel(&call_, cricket::AudioOptions()); + channel_ = engine_.CreateChannel(&call_, cricket::MediaConfig(), + cricket::AudioOptions()); EXPECT_TRUE(channel_ != nullptr); if (caller) { // If this is a caller, local description will be applied and add the @@ -414,7 +416,8 @@ TEST_F(WebRtcVoiceEngineTestFake, StartupShutdown) { // Tests that we can create and destroy a channel. TEST_F(WebRtcVoiceEngineTestFake, CreateChannel) { EXPECT_TRUE(engine_.Init(rtc::Thread::Current())); - channel_ = engine_.CreateChannel(&call_, cricket::AudioOptions()); + channel_ = engine_.CreateChannel(&call_, cricket::MediaConfig(), + cricket::AudioOptions()); EXPECT_TRUE(channel_ != nullptr); } @@ -1701,7 +1704,8 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsCNandDTMFAsCaller) { // Test that we set VAD and DTMF types correctly as callee. TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsCNandDTMFAsCallee) { EXPECT_TRUE(engine_.Init(rtc::Thread::Current())); - channel_ = engine_.CreateChannel(&call_, cricket::AudioOptions()); + channel_ = engine_.CreateChannel(&call_, cricket::MediaConfig(), + cricket::AudioOptions()); EXPECT_TRUE(channel_ != nullptr); cricket::AudioSendParameters parameters; @@ -1818,7 +1822,8 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsREDAsCaller) { // Test that we set up RED correctly as callee. TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsREDAsCallee) { EXPECT_TRUE(engine_.Init(rtc::Thread::Current())); - channel_ = engine_.CreateChannel(&call_, cricket::AudioOptions()); + channel_ = engine_.CreateChannel(&call_, cricket::MediaConfig(), + cricket::AudioOptions()); EXPECT_TRUE(channel_ != nullptr); cricket::AudioSendParameters parameters; @@ -2349,7 +2354,8 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendSsrcWithMultipleStreams) { // receive channel is created before the send channel. TEST_F(WebRtcVoiceEngineTestFake, SetSendSsrcAfterCreatingReceiveChannel) { EXPECT_TRUE(engine_.Init(rtc::Thread::Current())); - channel_ = engine_.CreateChannel(&call_, cricket::AudioOptions()); + channel_ = engine_.CreateChannel(&call_, cricket::MediaConfig(), + cricket::AudioOptions()); EXPECT_TRUE(channel_->AddRecvStream(cricket::StreamParams::CreateLegacy(1))); int receive_channel_num = voe_.GetLastChannel(); @@ -2777,11 +2783,11 @@ TEST_F(WebRtcVoiceEngineTestFake, InitDoesNotOverwriteDefaultAgcConfig) { TEST_F(WebRtcVoiceEngineTestFake, SetOptionOverridesViaChannels) { EXPECT_TRUE(SetupEngineWithSendStream()); rtc::scoped_ptr channel1( - static_cast( - engine_.CreateChannel(&call_, cricket::AudioOptions()))); + static_cast(engine_.CreateChannel( + &call_, cricket::MediaConfig(), cricket::AudioOptions()))); rtc::scoped_ptr channel2( - static_cast( - engine_.CreateChannel(&call_, cricket::AudioOptions()))); + static_cast(engine_.CreateChannel( + &call_, cricket::MediaConfig(), cricket::AudioOptions()))); // Have to add a stream to make SetSend work. cricket::StreamParams stream1; @@ -2877,21 +2883,29 @@ TEST_F(WebRtcVoiceEngineTestFake, SetOptionOverridesViaChannels) { // This test verifies DSCP settings are properly applied on voice media channel. TEST_F(WebRtcVoiceEngineTestFake, TestSetDscpOptions) { EXPECT_TRUE(SetupEngineWithSendStream()); - rtc::scoped_ptr channel( - engine_.CreateChannel(&call_, cricket::AudioOptions())); - rtc::scoped_ptr network_interface( - new cricket::FakeNetworkInterface); - channel->SetInterface(network_interface.get()); - cricket::AudioSendParameters parameters = send_parameters_; - parameters.options.dscp = rtc::Optional(true); - EXPECT_TRUE(channel->SetSendParameters(parameters)); - EXPECT_EQ(rtc::DSCP_EF, network_interface->dscp()); - // Verify previous value is not modified if dscp option is not set. - EXPECT_TRUE(channel->SetSendParameters(send_parameters_)); - EXPECT_EQ(rtc::DSCP_EF, network_interface->dscp()); - parameters.options.dscp = rtc::Optional(false); - EXPECT_TRUE(channel->SetSendParameters(parameters)); - EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface->dscp()); + cricket::FakeNetworkInterface network_interface; + cricket::MediaConfig config; + rtc::scoped_ptr channel; + + channel.reset(engine_.CreateChannel(&call_, config, cricket::AudioOptions())); + channel->SetInterface(&network_interface); + // Default value when DSCP is disabled should be DSCP_DEFAULT. + EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface.dscp()); + + config.enable_dscp = true; + channel.reset(engine_.CreateChannel(&call_, config, cricket::AudioOptions())); + channel->SetInterface(&network_interface); + EXPECT_EQ(rtc::DSCP_EF, network_interface.dscp()); + + // Verify that setting the option to false resets the + // DiffServCodePoint. + config.enable_dscp = false; + channel.reset(engine_.CreateChannel(&call_, config, cricket::AudioOptions())); + channel->SetInterface(&network_interface); + // Default value when DSCP is disabled should be DSCP_DEFAULT. + EXPECT_EQ(rtc::DSCP_DEFAULT, network_interface.dscp()); + + channel->SetInterface(nullptr); } TEST_F(WebRtcVoiceEngineTestFake, TestGetReceiveChannelId) { @@ -3149,8 +3163,8 @@ TEST(WebRtcVoiceEngineTest, StartupShutdown) { EXPECT_TRUE(engine.Init(rtc::Thread::Current())); rtc::scoped_ptr call( webrtc::Call::Create(webrtc::Call::Config())); - cricket::VoiceMediaChannel* channel = - engine.CreateChannel(call.get(), cricket::AudioOptions()); + cricket::VoiceMediaChannel* channel = engine.CreateChannel( + call.get(), cricket::MediaConfig(), cricket::AudioOptions()); EXPECT_TRUE(channel != nullptr); delete channel; engine.Terminate(); @@ -3255,8 +3269,8 @@ TEST(WebRtcVoiceEngineTest, Has32Channels) { cricket::VoiceMediaChannel* channels[32]; int num_channels = 0; while (num_channels < arraysize(channels)) { - cricket::VoiceMediaChannel* channel = - engine.CreateChannel(call.get(), cricket::AudioOptions()); + cricket::VoiceMediaChannel* channel = engine.CreateChannel( + call.get(), cricket::MediaConfig(), cricket::AudioOptions()); if (!channel) break; channels[num_channels++] = channel; @@ -3277,8 +3291,8 @@ TEST(WebRtcVoiceEngineTest, SetRecvCodecs) { EXPECT_TRUE(engine.Init(rtc::Thread::Current())); rtc::scoped_ptr call( webrtc::Call::Create(webrtc::Call::Config())); - cricket::WebRtcVoiceMediaChannel channel(&engine, cricket::AudioOptions(), - call.get()); + cricket::WebRtcVoiceMediaChannel channel(&engine, cricket::MediaConfig(), + cricket::AudioOptions(), call.get()); cricket::AudioRecvParameters parameters; parameters.codecs = engine.codecs(); EXPECT_TRUE(channel.SetRecvParameters(parameters)); diff --git a/webrtc/pc/channelmanager.cc b/webrtc/pc/channelmanager.cc index ca8471d05c..c3e5893446 100644 --- a/webrtc/pc/channelmanager.cc +++ b/webrtc/pc/channelmanager.cc @@ -250,8 +250,8 @@ VoiceChannel* ChannelManager::CreateVoiceChannel_w( ASSERT(initialized_); ASSERT(worker_thread_ == rtc::Thread::Current()); ASSERT(nullptr != media_controller); - VoiceMediaChannel* media_channel = - media_engine_->CreateChannel(media_controller->call_w(), options); + VoiceMediaChannel* media_channel = media_engine_->CreateChannel( + media_controller->call_w(), media_controller->config(), options); if (!media_channel) return nullptr; @@ -308,8 +308,8 @@ VideoChannel* ChannelManager::CreateVideoChannel_w( ASSERT(initialized_); ASSERT(worker_thread_ == rtc::Thread::Current()); ASSERT(nullptr != media_controller); - VideoMediaChannel* media_channel = - media_engine_->CreateVideoChannel(media_controller->call_w(), options); + VideoMediaChannel* media_channel = media_engine_->CreateVideoChannel( + media_controller->call_w(), media_controller->config(), options); if (media_channel == NULL) { return NULL; }