From a104ceb0ceec0f95e199e6d6704f41ec88a51fc5 Mon Sep 17 00:00:00 2001 From: Johannes Kron Date: Fri, 24 Jan 2020 16:04:04 +0000 Subject: [PATCH] Revert "Reland "Reland "Distinguish between send and receive codecs""" This reverts commit 9bac68c0cc4444b852416396f0e0f31ea66a9cfe. Reason for revert: Breaks perf test on iOS. Original change's description: > Reland "Reland "Distinguish between send and receive codecs"" > > This reverts commit 00a30873c415d717af8dcdf21c2df7fd4b6d1ed2. > > Reason for revert: Flaky test in Chromium fixed. > > Original change's description: > > Revert "Reland "Distinguish between send and receive codecs"" > > > > This reverts commit 133bf2bd28596aab5c7684e0ea3da99b1fece77f. > > > > Reason for revert: Breaks Chromium import due to flaky test in Chromium. > > > > Original change's description: > > > Reland "Distinguish between send and receive codecs" > > > > > > This reverts commit e57b266a20334e47f105a0bd777190ec8c6562e8. > > > > > > Reason for revert: Fixed negotiation of send-only clients. > > > > > > Original change's description: > > > > Revert "Distinguish between send and receive codecs" > > > > > > > > This reverts commit c0f25cf762a6946666c812f7a3df3f0a7f98b38d. > > > > > > > > Reason for revert: breaks negotiation with send-only clients > > > > > > > > (webrtc_video_engine.cc:985): SetRecvParameters called with unsupported video codec: VideoCodec[96:H264] > > > > (peer_connection.cc:6043): Failed to set local video description recv parameters. (INVALID_PARAMETER) > > > > (peer_connection.cc:2591): Failed to set local offer sdp: Failed to set local video description recv parameters. > > > > > > > > Original change's description: > > > > > Distinguish between send and receive codecs > > > > > > > > > > Even though send and receive codecs may be the same, they might have > > > > > different support in HW. Distinguish between send and receive codecs > > > > > to be able to keep track of which codecs have HW support. > > > > > > > > > > Bug: chromium:1029737 > > > > > Change-Id: Id119560becadfe0aaf861c892a6485f1c2eb378d > > > > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/165763 > > > > > Commit-Queue: Johannes Kron > > > > > Reviewed-by: Steve Anton > > > > > Cr-Commit-Position: refs/heads/master@{#30284} > > > > > > > > TBR=steveanton@webrtc.org,kron@webrtc.org > > > > > > > > Change-Id: Iacb7059436b2313b52577b65f164ee363c4816aa > > > > No-Presubmit: true > > > > No-Tree-Checks: true > > > > No-Try: true > > > > Bug: chromium:1029737 > > > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/166420 > > > > Reviewed-by: Steve Anton > > > > Commit-Queue: Steve Anton > > > > Cr-Commit-Position: refs/heads/master@{#30292} > > > > > > TBR=steveanton@webrtc.org,kron@webrtc.org > > > > > > > > > Bug: chromium:1029737 > > > Change-Id: I287efcfdcd1c9a3f2c410aeec8fe26a84204d1fd > > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/166604 > > > Reviewed-by: Johannes Kron > > > Reviewed-by: Steve Anton > > > Commit-Queue: Johannes Kron > > > Cr-Commit-Position: refs/heads/master@{#30348} > > > > TBR=steveanton@webrtc.org,kron@webrtc.org > > > > Change-Id: I9f8731309749e07ce7e651e1550ecfabddb1735f > > No-Presubmit: true > > No-Tree-Checks: true > > No-Try: true > > Bug: chromium:1029737 > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/167205 > > Reviewed-by: Johannes Kron > > Commit-Queue: Johannes Kron > > Cr-Commit-Position: refs/heads/master@{#30360} > > TBR=steveanton@webrtc.org,kron@webrtc.org > > Change-Id: I1cc2d83bd884f10685503a9c31288f96c935d6a3 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: chromium:1029737 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/167206 > Reviewed-by: Johannes Kron > Reviewed-by: Steve Anton > Commit-Queue: Johannes Kron > Cr-Commit-Position: refs/heads/master@{#30367} TBR=steveanton@webrtc.org,kron@webrtc.org Change-Id: I0a9b0b58922ce7c558b3d31b64cc12086b2a6a55 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:1029737 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/167364 Commit-Queue: Johannes Kron Reviewed-by: Johannes Kron Cr-Commit-Position: refs/heads/master@{#30373} --- .../video/function_video_decoder_factory.h | 10 +- media/base/fake_media_engine.cc | 24 +-- media/base/fake_media_engine.h | 9 +- media/base/media_engine.h | 4 +- media/engine/fake_webrtc_video_engine.cc | 7 +- media/engine/fake_webrtc_video_engine.h | 2 +- media/engine/null_webrtc_video_engine.h | 6 +- media/engine/webrtc_video_engine.cc | 29 ++- media/engine/webrtc_video_engine.h | 3 +- media/engine/webrtc_video_engine_unittest.cc | 164 +++++++--------- pc/channel.cc | 39 ++-- pc/channel_manager.cc | 21 +- pc/channel_manager.h | 3 +- pc/channel_manager_unittest.cc | 27 +-- pc/media_session.cc | 168 ++++------------ pc/media_session.h | 21 +- pc/media_session_unittest.cc | 72 ++++--- pc/peer_connection_factory.cc | 4 +- pc/peer_connection_integrationtest.cc | 166 ++++------------ pc/peer_connection_media_unittest.cc | 10 +- pc/rtp_transceiver.cc | 180 +++++++++--------- 21 files changed, 336 insertions(+), 633 deletions(-) diff --git a/api/test/video/function_video_decoder_factory.h b/api/test/video/function_video_decoder_factory.h index 23214ccf40..03a4323997 100644 --- a/api/test/video/function_video_decoder_factory.h +++ b/api/test/video/function_video_decoder_factory.h @@ -33,14 +33,11 @@ class FunctionVideoDecoderFactory final : public VideoDecoderFactory { std::function(const SdpVideoFormat&)> create) : create_(std::move(create)) {} - FunctionVideoDecoderFactory( - std::function()> create, - std::vector sdp_video_formats) - : create_([create](const SdpVideoFormat&) { return create(); }), - sdp_video_formats_(sdp_video_formats) {} + // Unused by tests. std::vector GetSupportedFormats() const override { - return sdp_video_formats_; + RTC_NOTREACHED(); + return {}; } std::unique_ptr CreateVideoDecoder( @@ -51,7 +48,6 @@ class FunctionVideoDecoderFactory final : public VideoDecoderFactory { private: const std::function(const SdpVideoFormat&)> create_; - const std::vector sdp_video_formats_; }; } // namespace test diff --git a/media/base/fake_media_engine.cc b/media/base/fake_media_engine.cc index 4a6971adf1..c31ef97786 100644 --- a/media/base/fake_media_engine.cc +++ b/media/base/fake_media_engine.cc @@ -567,8 +567,7 @@ FakeVideoEngine::FakeVideoEngine() : capture_(false), fail_create_channel_(false) { // Add a fake video codec. Note that the name must not be "" as there are // sanity checks against that. - send_codecs_.push_back(VideoCodec(0, "fake_video_codec")); - recv_codecs_.push_back(VideoCodec(0, "fake_video_codec")); + codecs_.push_back(VideoCodec(0, "fake_video_codec")); } RtpCapabilities FakeVideoEngine::GetCapabilities() const { return RtpCapabilities(); @@ -599,22 +598,12 @@ void FakeVideoEngine::UnregisterChannel(VideoMediaChannel* channel) { RTC_DCHECK(it != channels_.end()); channels_.erase(it); } -std::vector FakeVideoEngine::send_codecs() const { - return send_codecs_; +std::vector FakeVideoEngine::codecs() const { + return codecs_; } - -std::vector FakeVideoEngine::recv_codecs() const { - return recv_codecs_; +void FakeVideoEngine::SetCodecs(const std::vector codecs) { + codecs_ = codecs; } - -void FakeVideoEngine::SetSendCodecs(const std::vector& codecs) { - send_codecs_ = codecs; -} - -void FakeVideoEngine::SetRecvCodecs(const std::vector& codecs) { - recv_codecs_ = codecs; -} - bool FakeVideoEngine::SetCapture(bool capture) { capture_ = capture; return true; @@ -638,8 +627,7 @@ void FakeMediaEngine::SetAudioSendCodecs( voice_->SetSendCodecs(codecs); } void FakeMediaEngine::SetVideoCodecs(const std::vector& codecs) { - video_->SetSendCodecs(codecs); - video_->SetRecvCodecs(codecs); + video_->SetCodecs(codecs); } FakeVoiceMediaChannel* FakeMediaEngine::GetVoiceChannel(size_t index) { diff --git a/media/base/fake_media_engine.h b/media/base/fake_media_engine.h index f072dc4550..3df8f85965 100644 --- a/media/base/fake_media_engine.h +++ b/media/base/fake_media_engine.h @@ -559,16 +559,13 @@ class FakeVideoEngine : public VideoEngineInterface { override; FakeVideoMediaChannel* GetChannel(size_t index); void UnregisterChannel(VideoMediaChannel* channel); - std::vector send_codecs() const override; - std::vector recv_codecs() const override; - void SetSendCodecs(const std::vector& codecs); - void SetRecvCodecs(const std::vector& codecs); + std::vector codecs() const override; + void SetCodecs(const std::vector codecs); bool SetCapture(bool capture); private: std::vector channels_; - std::vector send_codecs_; - std::vector recv_codecs_; + std::vector codecs_; bool capture_; VideoOptions options_; bool fail_create_channel_; diff --git a/media/base/media_engine.h b/media/base/media_engine.h index 841b2b6b0c..173df50e34 100644 --- a/media/base/media_engine.h +++ b/media/base/media_engine.h @@ -99,9 +99,7 @@ class VideoEngineInterface { webrtc::VideoBitrateAllocatorFactory* video_bitrate_allocator_factory) = 0; - virtual std::vector send_codecs() const = 0; - virtual std::vector recv_codecs() const = 0; - + virtual std::vector codecs() const = 0; virtual RtpCapabilities GetCapabilities() const = 0; }; diff --git a/media/engine/fake_webrtc_video_engine.cc b/media/engine/fake_webrtc_video_engine.cc index 91f7e53956..0ee2bcc54f 100644 --- a/media/engine/fake_webrtc_video_engine.cc +++ b/media/engine/fake_webrtc_video_engine.cc @@ -113,11 +113,8 @@ void FakeWebRtcVideoDecoderFactory::DecoderDestroyed( } void FakeWebRtcVideoDecoderFactory::AddSupportedVideoCodecType( - const std::string& name) { - // This is to match the default H264 params of cricket::VideoCodec. - cricket::VideoCodec video_codec(name); - supported_codec_formats_.push_back( - webrtc::SdpVideoFormat(video_codec.name, video_codec.params)); + const webrtc::SdpVideoFormat& format) { + supported_codec_formats_.push_back(format); } int FakeWebRtcVideoDecoderFactory::GetNumCreatedDecoders() { diff --git a/media/engine/fake_webrtc_video_engine.h b/media/engine/fake_webrtc_video_engine.h index 28dc4fe99b..7b32ac86cf 100644 --- a/media/engine/fake_webrtc_video_engine.h +++ b/media/engine/fake_webrtc_video_engine.h @@ -67,7 +67,7 @@ class FakeWebRtcVideoDecoderFactory : public webrtc::VideoDecoderFactory { const webrtc::SdpVideoFormat& format) override; void DecoderDestroyed(FakeWebRtcVideoDecoder* decoder); - void AddSupportedVideoCodecType(const std::string& name); + void AddSupportedVideoCodecType(const webrtc::SdpVideoFormat& format); int GetNumCreatedDecoders(); const std::vector& decoders(); diff --git a/media/engine/null_webrtc_video_engine.h b/media/engine/null_webrtc_video_engine.h index 5c31e21ef1..590f0b0be7 100644 --- a/media/engine/null_webrtc_video_engine.h +++ b/media/engine/null_webrtc_video_engine.h @@ -30,11 +30,7 @@ class VideoMediaChannel; // CompositeMediaEngine. class NullWebRtcVideoEngine : public VideoEngineInterface { public: - std::vector send_codecs() const override { - return std::vector(); - } - - std::vector recv_codecs() const override { + std::vector codecs() const override { return std::vector(); } diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index c8c0ae5451..b17938b014 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -139,11 +139,11 @@ std::vector AssignPayloadTypesAndDefaultCodecs( return output_codecs; } -template -std::vector GetPayloadTypesAndDefaultCodecs(const T factory) { - return factory ? AssignPayloadTypesAndDefaultCodecs( - factory->GetSupportedFormats()) - : std::vector(); +std::vector AssignPayloadTypesAndDefaultCodecs( + const webrtc::VideoEncoderFactory* encoder_factory) { + return encoder_factory ? AssignPayloadTypesAndDefaultCodecs( + encoder_factory->GetSupportedFormats()) + : std::vector(); } bool IsTemporalLayersSupported(const std::string& codec_name) { @@ -476,12 +476,8 @@ VideoMediaChannel* WebRtcVideoEngine::CreateMediaChannel( encoder_factory_.get(), decoder_factory_.get(), video_bitrate_allocator_factory); } -std::vector WebRtcVideoEngine::send_codecs() const { - return GetPayloadTypesAndDefaultCodecs(encoder_factory_.get()); -} - -std::vector WebRtcVideoEngine::recv_codecs() const { - return GetPayloadTypesAndDefaultCodecs(decoder_factory_.get()); +std::vector WebRtcVideoEngine::codecs() const { + return AssignPayloadTypesAndDefaultCodecs(encoder_factory_.get()); } RtpCapabilities WebRtcVideoEngine::GetCapabilities() const { @@ -551,9 +547,9 @@ WebRtcVideoChannel::WebRtcVideoChannel( rtcp_receiver_report_ssrc_ = kDefaultRtcpReceiverReportSsrc; sending_ = false; - recv_codecs_ = MapCodecs(GetPayloadTypesAndDefaultCodecs(decoder_factory_)); - recv_flexfec_payload_type_ = - recv_codecs_.empty() ? 0 : recv_codecs_.front().flexfec_payload_type; + recv_codecs_ = + MapCodecs(AssignPayloadTypesAndDefaultCodecs(encoder_factory_)); + recv_flexfec_payload_type_ = recv_codecs_.front().flexfec_payload_type; } WebRtcVideoChannel::~WebRtcVideoChannel() { @@ -980,7 +976,7 @@ bool WebRtcVideoChannel::GetChangedRecvParameters( // Verify that every mapped codec is supported locally. const std::vector local_supported_codecs = - GetPayloadTypesAndDefaultCodecs(decoder_factory_); + AssignPayloadTypesAndDefaultCodecs(encoder_factory_); for (const VideoCodecSettings& mapped_codec : mapped_codecs) { if (!FindMatchingCodec(local_supported_codecs, mapped_codec.codec)) { RTC_LOG(LS_ERROR) @@ -2913,9 +2909,6 @@ bool WebRtcVideoChannel::VideoCodecSettings::operator!=( std::vector WebRtcVideoChannel::MapCodecs(const std::vector& codecs) { - if (codecs.empty()) { - return {}; - } RTC_DCHECK(!codecs.empty()); std::vector video_codecs; diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index b453d869b5..d5ed95b7f0 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -97,8 +97,7 @@ class WebRtcVideoEngine : public VideoEngineInterface { webrtc::VideoBitrateAllocatorFactory* video_bitrate_allocator_factory) override; - std::vector send_codecs() const override; - std::vector recv_codecs() const override; + std::vector codecs() const override; RtpCapabilities GetCapabilities() const override; private: diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index a233a30f8b..975761123a 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -265,7 +265,7 @@ class WebRtcVideoEngineTest : public ::testing::Test { // Find the codec in the engine with the given name. The codec must be // present. cricket::VideoCodec GetEngineCodec(const std::string& name) const; - void AddSupportedVideoCodecType(const std::string& name); + VideoMediaChannel* SetSendParamsWithAllSupportedCodecs(); VideoMediaChannel* SetRecvParamsWithSupportedCodecs( @@ -296,7 +296,7 @@ TEST_F(WebRtcVideoEngineTest, DefaultRtxCodecHasAssociatedPayloadTypeSet) { encoder_factory_->AddSupportedVideoCodecType("VP8"); AssignDefaultCodec(); - std::vector engine_codecs = engine_.send_codecs(); + std::vector engine_codecs = engine_.codecs(); for (size_t i = 0; i < engine_codecs.size(); ++i) { if (engine_codecs[i].name != kRtxCodecName) continue; @@ -375,7 +375,7 @@ TEST_F(WebRtcVideoEngineTest, CVOSetHeaderExtensionBeforeCapturer) { // dtor is called. ::testing::NiceMock video_source; - AddSupportedVideoCodecType("VP8"); + encoder_factory_->AddSupportedVideoCodecType("VP8"); std::unique_ptr channel( SetSendParamsWithAllSupportedCodecs()); @@ -414,7 +414,7 @@ TEST_F(WebRtcVideoEngineTest, CVOSetHeaderExtensionBeforeAddSendStream) { // dtor is called. ::testing::NiceMock video_source; - AddSupportedVideoCodecType("VP8"); + encoder_factory_->AddSupportedVideoCodecType("VP8"); std::unique_ptr channel( SetSendParamsWithAllSupportedCodecs()); @@ -438,8 +438,8 @@ TEST_F(WebRtcVideoEngineTest, CVOSetHeaderExtensionBeforeAddSendStream) { TEST_F(WebRtcVideoEngineTest, CVOSetHeaderExtensionAfterCapturer) { ::testing::NiceMock video_source; - AddSupportedVideoCodecType("VP8"); - AddSupportedVideoCodecType("VP9"); + encoder_factory_->AddSupportedVideoCodecType("VP8"); + encoder_factory_->AddSupportedVideoCodecType("VP9"); std::unique_ptr channel( SetSendParamsWithAllSupportedCodecs()); @@ -483,7 +483,7 @@ TEST_F(WebRtcVideoEngineTest, CVOSetHeaderExtensionAfterCapturer) { } TEST_F(WebRtcVideoEngineTest, SetSendFailsBeforeSettingCodecs) { - AddSupportedVideoCodecType("VP8"); + encoder_factory_->AddSupportedVideoCodecType("VP8"); std::unique_ptr channel(engine_.CreateMediaChannel( call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), @@ -498,7 +498,7 @@ TEST_F(WebRtcVideoEngineTest, SetSendFailsBeforeSettingCodecs) { } TEST_F(WebRtcVideoEngineTest, GetStatsWithoutSendCodecsSetDoesNotCrash) { - AddSupportedVideoCodecType("VP8"); + encoder_factory_->AddSupportedVideoCodecType("VP8"); std::unique_ptr channel(engine_.CreateMediaChannel( call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), @@ -509,7 +509,7 @@ TEST_F(WebRtcVideoEngineTest, GetStatsWithoutSendCodecsSetDoesNotCrash) { } TEST_F(WebRtcVideoEngineTest, UseFactoryForVp8WhenSupported) { - AddSupportedVideoCodecType("VP8"); + encoder_factory_->AddSupportedVideoCodecType("VP8"); std::unique_ptr channel( SetSendParamsWithAllSupportedCodecs()); @@ -569,7 +569,7 @@ TEST_F(WebRtcVideoEngineTest, RtxCodecAddedForH264Codec) { encoder_factory_->AddSupportedVideoCodec(h264_high); // First figure out what payload types the test codecs got assigned. - const std::vector codecs = engine_.send_codecs(); + const std::vector codecs = engine_.codecs(); // Now search for RTX codecs for them. Expect that they all have associated // RTX codecs. EXPECT_TRUE(HasRtxCodec( @@ -586,7 +586,7 @@ TEST_F(WebRtcVideoEngineTest, RtxCodecAddedForH264Codec) { #if defined(RTC_ENABLE_VP9) TEST_F(WebRtcVideoEngineTest, CanConstructDecoderForVp9EncoderFactory) { - AddSupportedVideoCodecType("VP9"); + encoder_factory_->AddSupportedVideoCodecType("VP9"); std::unique_ptr channel( SetSendParamsWithAllSupportedCodecs()); @@ -597,7 +597,7 @@ TEST_F(WebRtcVideoEngineTest, CanConstructDecoderForVp9EncoderFactory) { #endif // defined(RTC_ENABLE_VP9) TEST_F(WebRtcVideoEngineTest, PropagatesInputFrameTimestamp) { - AddSupportedVideoCodecType("VP8"); + encoder_factory_->AddSupportedVideoCodecType("VP8"); FakeCall* fake_call = new FakeCall(); call_.reset(fake_call); std::unique_ptr channel( @@ -651,7 +651,7 @@ TEST_F(WebRtcVideoEngineTest, PropagatesInputFrameTimestamp) { } void WebRtcVideoEngineTest::AssignDefaultAptRtxTypes() { - std::vector engine_codecs = engine_.send_codecs(); + std::vector engine_codecs = engine_.codecs(); RTC_DCHECK(!engine_codecs.empty()); for (const cricket::VideoCodec& codec : engine_codecs) { if (codec.name == "rtx") { @@ -665,7 +665,7 @@ void WebRtcVideoEngineTest::AssignDefaultAptRtxTypes() { } void WebRtcVideoEngineTest::AssignDefaultCodec() { - std::vector engine_codecs = engine_.send_codecs(); + std::vector engine_codecs = engine_.codecs(); RTC_DCHECK(!engine_codecs.empty()); bool codec_set = false; for (const cricket::VideoCodec& codec : engine_codecs) { @@ -681,7 +681,7 @@ void WebRtcVideoEngineTest::AssignDefaultCodec() { size_t WebRtcVideoEngineTest::GetEngineCodecIndex( const std::string& name) const { - const std::vector codecs = engine_.send_codecs(); + const std::vector codecs = engine_.codecs(); for (size_t i = 0; i < codecs.size(); ++i) { const cricket::VideoCodec engine_codec = codecs[i]; if (!absl::EqualsIgnoreCase(name, engine_codec.name)) @@ -705,13 +705,7 @@ size_t WebRtcVideoEngineTest::GetEngineCodecIndex( cricket::VideoCodec WebRtcVideoEngineTest::GetEngineCodec( const std::string& name) const { - return engine_.send_codecs()[GetEngineCodecIndex(name)]; -} - -void WebRtcVideoEngineTest::AddSupportedVideoCodecType( - const std::string& name) { - encoder_factory_->AddSupportedVideoCodecType(name); - decoder_factory_->AddSupportedVideoCodecType(name); + return engine_.codecs()[GetEngineCodecIndex(name)]; } VideoMediaChannel* @@ -760,7 +754,7 @@ void WebRtcVideoEngineTest::ExpectRtpCapabilitySupport(const char* uri, } TEST_F(WebRtcVideoEngineTest, UsesSimulcastAdapterForVp8Factories) { - AddSupportedVideoCodecType("VP8"); + encoder_factory_->AddSupportedVideoCodecType("VP8"); std::unique_ptr channel( SetSendParamsWithAllSupportedCodecs()); @@ -797,8 +791,8 @@ TEST_F(WebRtcVideoEngineTest, UsesSimulcastAdapterForVp8Factories) { } TEST_F(WebRtcVideoEngineTest, ChannelWithH264CanChangeToVp8) { - AddSupportedVideoCodecType("VP8"); - AddSupportedVideoCodecType("H264"); + encoder_factory_->AddSupportedVideoCodecType("VP8"); + encoder_factory_->AddSupportedVideoCodecType("H264"); // Frame source. webrtc::test::FrameForwarder frame_forwarder; @@ -832,8 +826,8 @@ TEST_F(WebRtcVideoEngineTest, ChannelWithH264CanChangeToVp8) { TEST_F(WebRtcVideoEngineTest, UsesSimulcastAdapterForVp8WithCombinedVP8AndH264Factory) { - AddSupportedVideoCodecType("VP8"); - AddSupportedVideoCodecType("H264"); + encoder_factory_->AddSupportedVideoCodecType("VP8"); + encoder_factory_->AddSupportedVideoCodecType("H264"); std::unique_ptr channel(engine_.CreateMediaChannel( call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), @@ -868,8 +862,8 @@ TEST_F(WebRtcVideoEngineTest, TEST_F(WebRtcVideoEngineTest, DestroysNonSimulcastEncoderFromCombinedVP8AndH264Factory) { - AddSupportedVideoCodecType("VP8"); - AddSupportedVideoCodecType("H264"); + encoder_factory_->AddSupportedVideoCodecType("VP8"); + encoder_factory_->AddSupportedVideoCodecType("H264"); std::unique_ptr channel(engine_.CreateMediaChannel( call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), @@ -902,7 +896,7 @@ TEST_F(WebRtcVideoEngineTest, SimulcastEnabledForH264BehindFieldTrial) { RTC_DCHECK(!override_field_trials_); override_field_trials_ = std::make_unique( "WebRTC-H264Simulcast/Enabled/"); - AddSupportedVideoCodecType("H264"); + encoder_factory_->AddSupportedVideoCodecType("H264"); std::unique_ptr channel(engine_.CreateMediaChannel( call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), @@ -942,13 +936,13 @@ TEST_F(WebRtcVideoEngineTest, auto flexfec = Field("name", &VideoCodec::name, "flexfec-03"); // FlexFEC is not active without field trial. - EXPECT_THAT(engine_.send_codecs(), Not(Contains(flexfec))); + EXPECT_THAT(engine_.codecs(), Not(Contains(flexfec))); // FlexFEC is active with field trial. RTC_DCHECK(!override_field_trials_); override_field_trials_ = std::make_unique( "WebRTC-FlexFEC-03-Advertised/Enabled/"); - EXPECT_THAT(engine_.send_codecs(), Contains(flexfec)); + EXPECT_THAT(engine_.codecs(), Contains(flexfec)); } // Test that codecs are added in the order they are reported from the factory. @@ -972,11 +966,11 @@ TEST_F(WebRtcVideoEngineTest, ReportSupportedAddedCodec) { // Set up external encoder factory with first codec, and initialize engine. encoder_factory_->AddSupportedVideoCodecType(kFakeExternalCodecName1); - std::vector codecs_before(engine_.send_codecs()); + std::vector codecs_before(engine_.codecs()); // Add second codec. encoder_factory_->AddSupportedVideoCodecType(kFakeExternalCodecName2); - std::vector codecs_after(engine_.send_codecs()); + std::vector codecs_after(engine_.codecs()); // The codec itself and RTX should have been added. EXPECT_EQ(codecs_before.size() + 2, codecs_after.size()); @@ -992,11 +986,12 @@ TEST_F(WebRtcVideoEngineTest, ReportRtxForExternalCodec) { encoder_factory_->AddSupportedVideoCodecType(kFakeCodecName); const size_t fake_codec_index = GetEngineCodecIndex(kFakeCodecName); - EXPECT_EQ("rtx", engine_.send_codecs().at(fake_codec_index + 1).name); + EXPECT_EQ("rtx", engine_.codecs().at(fake_codec_index + 1).name); } TEST_F(WebRtcVideoEngineTest, RegisterDecodersIfSupported) { - AddSupportedVideoCodecType("VP8"); + encoder_factory_->AddSupportedVideoCodecType("VP8"); + decoder_factory_->AddSupportedVideoCodecType(webrtc::SdpVideoFormat("VP8")); cricket::VideoRecvParameters parameters; parameters.codecs.push_back(GetEngineCodec("VP8")); @@ -1022,7 +1017,10 @@ TEST_F(WebRtcVideoEngineTest, RegisterH264DecoderIfSupported) { // can't even query the WebRtcVideoDecoderFactory for supported codecs. // For now we add a FakeWebRtcVideoEncoderFactory to add H264 to supported // codecs. - AddSupportedVideoCodecType("H264"); + encoder_factory_->AddSupportedVideoCodecType("H264"); + webrtc::SdpVideoFormat supported_h264("H264"); + supported_h264.parameters[kH264FmtpPacketizationMode] = "1"; + decoder_factory_->AddSupportedVideoCodecType(supported_h264); std::vector codecs; codecs.push_back(GetEngineCodec("H264")); @@ -1038,7 +1036,8 @@ TEST_F(WebRtcVideoEngineTest, RegisterH264DecoderIfSupported) { // empty list of RtpSource without crashing. TEST_F(WebRtcVideoEngineTest, GetSourcesWithNonExistingSsrc) { // Setup an recv stream with |kSsrc|. - AddSupportedVideoCodecType("VP8"); + encoder_factory_->AddSupportedVideoCodecType("VP8"); + decoder_factory_->AddSupportedVideoCodecType(webrtc::SdpVideoFormat("VP8")); cricket::VideoRecvParameters parameters; parameters.codecs.push_back(GetEngineCodec("VP8")); std::unique_ptr channel( @@ -1057,8 +1056,7 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, NullFactories) { std::unique_ptr decoder_factory; WebRtcVideoEngine engine(std::move(encoder_factory), std::move(decoder_factory)); - EXPECT_EQ(0u, engine.send_codecs().size()); - EXPECT_EQ(0u, engine.recv_codecs().size()); + EXPECT_EQ(0u, engine.codecs().size()); } TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, EmptyFactories) { @@ -1070,11 +1068,8 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, EmptyFactories) { WebRtcVideoEngine engine( (std::unique_ptr(encoder_factory)), (std::unique_ptr(decoder_factory))); - // TODO(kron): Change to Times(1) once send and receive codecs are changed - // to be treated independently. - EXPECT_CALL(*encoder_factory, GetSupportedFormats()).Times(1); - EXPECT_EQ(0u, engine.send_codecs().size()); - EXPECT_EQ(0u, engine.recv_codecs().size()); + EXPECT_CALL(*encoder_factory, GetSupportedFormats()); + EXPECT_EQ(0u, engine.codecs().size()); EXPECT_CALL(*encoder_factory, Die()); EXPECT_CALL(*decoder_factory, Die()); } @@ -1103,11 +1098,9 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, Vp8) { const std::vector supported_formats = {vp8_format}; EXPECT_CALL(*encoder_factory, GetSupportedFormats()) .WillRepeatedly(::testing::Return(supported_formats)); - EXPECT_CALL(*decoder_factory, GetSupportedFormats()) - .WillRepeatedly(::testing::Return(supported_formats)); // Verify the codecs from the engine. - const std::vector engine_codecs = engine.send_codecs(); + const std::vector engine_codecs = engine.codecs(); // Verify default codecs has been added correctly. EXPECT_EQ(5u, engine_codecs.size()); EXPECT_EQ("VP8", engine_codecs.at(0).name); @@ -1240,14 +1233,12 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, NullDecoder) { const auto call = absl::WrapUnique(webrtc::Call::Create(call_config)); // Create recv channel. - EXPECT_CALL(*decoder_factory, GetSupportedFormats()) - .WillRepeatedly(::testing::Return(supported_formats)); const int recv_ssrc = 321; std::unique_ptr recv_channel(engine.CreateMediaChannel( call.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), rate_allocator_factory.get())); cricket::VideoRecvParameters recv_parameters; - recv_parameters.codecs.push_back(engine.recv_codecs().front()); + recv_parameters.codecs.push_back(engine.codecs().front()); EXPECT_TRUE(recv_channel->SetRecvParameters(recv_parameters)); EXPECT_TRUE(recv_channel->AddRecvStream( cricket::StreamParams::CreateLegacy(recv_ssrc))); @@ -1335,9 +1326,9 @@ class WebRtcVideoChannelEncodedFrameCallbackTest : public ::testing::Test { webrtc::CreateBuiltinVideoBitrateAllocatorFactory()), engine_( webrtc::CreateBuiltinVideoEncoderFactory(), - std::make_unique( - []() { return std::make_unique(); }, - kSdpVideoFormats)), + std::make_unique([]() { + return std::make_unique(); + })), channel_(absl::WrapUnique(static_cast( engine_.CreateMediaChannel( call_.get(), @@ -1348,7 +1339,7 @@ class WebRtcVideoChannelEncodedFrameCallbackTest : public ::testing::Test { network_interface_.SetDestination(channel_.get()); channel_->SetInterface(&network_interface_, webrtc::MediaTransportConfig()); cricket::VideoRecvParameters parameters; - parameters.codecs = engine_.recv_codecs(); + parameters.codecs = engine_.codecs(); channel_->SetRecvParameters(parameters); } @@ -1372,7 +1363,6 @@ class WebRtcVideoChannelEncodedFrameCallbackTest : public ::testing::Test { EXPECT_EQ(0, renderer_.errors()); } - static const std::vector kSdpVideoFormats; webrtc::FieldTrialBasedConfig field_trials_; webrtc::RtcEventLogNull event_log_; std::unique_ptr task_queue_factory_; @@ -1385,10 +1375,6 @@ class WebRtcVideoChannelEncodedFrameCallbackTest : public ::testing::Test { cricket::FakeVideoRenderer renderer_; }; -const std::vector - WebRtcVideoChannelEncodedFrameCallbackTest::kSdpVideoFormats = { - webrtc::SdpVideoFormat("VP8")}; - TEST_F(WebRtcVideoChannelEncodedFrameCallbackTest, SetEncodedFrameBufferFunction_DefaultStream) { testing::MockFunction callback; @@ -1494,7 +1480,7 @@ class WebRtcVideoChannelBaseTest : public ::testing::Test { network_interface_.SetDestination(channel_.get()); channel_->SetInterface(&network_interface_, webrtc::MediaTransportConfig()); cricket::VideoRecvParameters parameters; - parameters.codecs = engine_.send_codecs(); + parameters.codecs = engine_.codecs(); channel_->SetRecvParameters(parameters); EXPECT_TRUE(channel_->AddSendStream(DefaultSendStreamParams())); frame_forwarder_ = std::make_unique(); @@ -1642,7 +1628,7 @@ class WebRtcVideoChannelBaseTest : public ::testing::Test { } cricket::VideoCodec GetEngineCodec(const std::string& name) { - for (const cricket::VideoCodec& engine_codec : engine_.send_codecs()) { + for (const cricket::VideoCodec& engine_codec : engine_.codecs()) { if (absl::EqualsIgnoreCase(name, engine_codec.name)) return engine_codec; } @@ -2419,10 +2405,10 @@ class WebRtcVideoChannelTest : public WebRtcVideoEngineTest { frame_source_(1280, 720, rtc::kNumMicrosecsPerSec / 30), last_ssrc_(0) {} void SetUp() override { - AddSupportedVideoCodecType("VP8"); - AddSupportedVideoCodecType("VP9"); + encoder_factory_->AddSupportedVideoCodecType("VP8"); + encoder_factory_->AddSupportedVideoCodecType("VP9"); #if defined(WEBRTC_USE_H264) - AddSupportedVideoCodecType("H264"); + encoder_factory_->AddSupportedVideoCodecType("H264"); #endif fake_call_.reset(new FakeCall()); @@ -2431,8 +2417,8 @@ class WebRtcVideoChannelTest : public WebRtcVideoEngineTest { webrtc::CryptoOptions(), video_bitrate_allocator_factory_.get())); channel_->OnReadyToSend(true); last_ssrc_ = 123; - send_parameters_.codecs = engine_.send_codecs(); - recv_parameters_.codecs = engine_.recv_codecs(); + send_parameters_.codecs = engine_.codecs(); + recv_parameters_.codecs = engine_.codecs(); ASSERT_TRUE(channel_->SetSendParameters(send_parameters_)); } @@ -2604,7 +2590,7 @@ class WebRtcVideoChannelTest : public WebRtcVideoEngineTest { VerifyCodecHasDefaultFeedbackParams(default_codec_, expect_lntf_enabled); cricket::VideoSendParameters parameters; - parameters.codecs = engine_.send_codecs(); + parameters.codecs = engine_.codecs(); EXPECT_TRUE(channel_->SetSendParameters(parameters)); EXPECT_TRUE(channel_->SetSend(true)); @@ -2749,7 +2735,7 @@ TEST_F(WebRtcVideoChannelTest, SetsSyncGroupFromSyncLabel) { TEST_F(WebRtcVideoChannelTest, RecvStreamWithSimAndRtx) { cricket::VideoSendParameters parameters; - parameters.codecs = engine_.send_codecs(); + parameters.codecs = engine_.codecs(); EXPECT_TRUE(channel_->SetSendParameters(parameters)); EXPECT_TRUE(channel_->SetSend(true)); parameters.conference_mode = true; @@ -3062,7 +3048,7 @@ TEST_F(WebRtcVideoChannelTest, TransportCcCanBeEnabledAndDisabled) { // Verify that transport cc feedback is turned on when setting default codecs // since the default codecs have transport cc feedback enabled. - parameters.codecs = engine_.send_codecs(); + parameters.codecs = engine_.codecs(); EXPECT_TRUE(channel_->SetSendParameters(parameters)); stream = fake_call_->GetVideoReceiveStreams()[0]; EXPECT_TRUE(stream->GetConfig().rtp.transport_cc); @@ -3091,7 +3077,7 @@ TEST_F(WebRtcVideoChannelTest, LossNotificationCanBeEnabledAndDisabled) { { cricket::VideoSendParameters parameters; - parameters.codecs = engine_.send_codecs(); + parameters.codecs = engine_.codecs(); EXPECT_TRUE(channel_->SetSendParameters(parameters)); EXPECT_TRUE(channel_->SetSend(true)); } @@ -3115,7 +3101,7 @@ TEST_F(WebRtcVideoChannelTest, LossNotificationCanBeEnabledAndDisabled) { EXPECT_FALSE(send_stream->GetConfig().rtp.lntf.enabled); // Setting the default codecs again, including VP8, turns LNTF back on. - parameters.codecs = engine_.send_codecs(); + parameters.codecs = engine_.codecs(); EXPECT_TRUE(channel_->SetSendParameters(parameters)); recv_stream = fake_call_->GetVideoReceiveStreams()[0]; EXPECT_TRUE(recv_stream->GetConfig().rtp.lntf.enabled); @@ -3128,7 +3114,7 @@ TEST_F(WebRtcVideoChannelTest, NackIsEnabledByDefault) { VerifyCodecHasDefaultFeedbackParams(default_codec_, false); cricket::VideoSendParameters parameters; - parameters.codecs = engine_.send_codecs(); + parameters.codecs = engine_.codecs(); EXPECT_TRUE(channel_->SetSendParameters(parameters)); EXPECT_TRUE(channel_->SetSend(true)); @@ -3166,7 +3152,7 @@ TEST_F(WebRtcVideoChannelTest, NackCanBeEnabledAndDisabled) { // Verify that NACK is turned on when setting default codecs since the // default codecs have NACK enabled. - parameters.codecs = engine_.send_codecs(); + parameters.codecs = engine_.codecs(); EXPECT_TRUE(channel_->SetSendParameters(parameters)); recv_stream = fake_call_->GetVideoReceiveStreams()[0]; EXPECT_GT(recv_stream->GetConfig().rtp.nack.rtp_history_ms, 0); @@ -3904,7 +3890,7 @@ TEST_F(WebRtcVideoChannelTest, SetDefaultSendCodecs) { VideoCodec codec; EXPECT_TRUE(channel_->GetSendCodec(&codec)); - EXPECT_TRUE(codec.Matches(engine_.send_codecs()[0])); + EXPECT_TRUE(codec.Matches(engine_.codecs()[0])); // Using a RTX setup to verify that the default RTX payload type is good. const std::vector ssrcs = MAKE_VECTOR(kSsrcs1); @@ -4252,7 +4238,7 @@ TEST_F(WebRtcVideoChannelFlexfecRecvTest, TEST_F(WebRtcVideoChannelTest, SetSendCodecRejectsRtxWithoutAssociatedPayloadType) { const int kUnusedPayloadType = 127; - EXPECT_FALSE(FindCodecById(engine_.send_codecs(), kUnusedPayloadType)); + EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType)); cricket::VideoSendParameters parameters; cricket::VideoCodec rtx_codec(kUnusedPayloadType, "rtx"); @@ -4265,8 +4251,8 @@ TEST_F(WebRtcVideoChannelTest, SetSendCodecRejectsRtxWithoutMatchingVideoCodec) { const int kUnusedPayloadType1 = 126; const int kUnusedPayloadType2 = 127; - EXPECT_FALSE(FindCodecById(engine_.send_codecs(), kUnusedPayloadType1)); - EXPECT_FALSE(FindCodecById(engine_.send_codecs(), kUnusedPayloadType2)); + EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType1)); + EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType2)); { cricket::VideoCodec rtx_codec = cricket::VideoCodec::CreateRtxCodec( kUnusedPayloadType1, GetEngineCodec("VP8").id); @@ -4289,8 +4275,8 @@ TEST_F(WebRtcVideoChannelTest, TEST_F(WebRtcVideoChannelTest, SetSendCodecsWithChangedRtxPayloadType) { const int kUnusedPayloadType1 = 126; const int kUnusedPayloadType2 = 127; - EXPECT_FALSE(FindCodecById(engine_.send_codecs(), kUnusedPayloadType1)); - EXPECT_FALSE(FindCodecById(engine_.send_codecs(), kUnusedPayloadType2)); + EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType1)); + EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType2)); // SSRCs for RTX. cricket::StreamParams params = @@ -4691,8 +4677,8 @@ TEST_F(WebRtcVideoChannelTest, SetRecvCodecsWithOnlyVp8) { TEST_F(WebRtcVideoChannelTest, SetRecvCodecsWithRtx) { const int kUnusedPayloadType1 = 126; const int kUnusedPayloadType2 = 127; - EXPECT_FALSE(FindCodecById(engine_.recv_codecs(), kUnusedPayloadType1)); - EXPECT_FALSE(FindCodecById(engine_.recv_codecs(), kUnusedPayloadType2)); + EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType1)); + EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType2)); cricket::VideoRecvParameters parameters; parameters.codecs.push_back(GetEngineCodec("VP8")); @@ -4790,8 +4776,8 @@ TEST_F(WebRtcVideoChannelTest, DuplicateRedCodecIsDropped) { TEST_F(WebRtcVideoChannelTest, SetRecvCodecsWithChangedRtxPayloadType) { const int kUnusedPayloadType1 = 126; const int kUnusedPayloadType2 = 127; - EXPECT_FALSE(FindCodecById(engine_.recv_codecs(), kUnusedPayloadType1)); - EXPECT_FALSE(FindCodecById(engine_.recv_codecs(), kUnusedPayloadType2)); + EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType1)); + EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType2)); // SSRCs for RTX. cricket::StreamParams params = @@ -4839,14 +4825,13 @@ TEST_F(WebRtcVideoChannelTest, SetRecvCodecsDifferentPayloadType) { TEST_F(WebRtcVideoChannelTest, SetRecvCodecsAcceptDefaultCodecs) { cricket::VideoRecvParameters parameters; - parameters.codecs = engine_.recv_codecs(); + parameters.codecs = engine_.codecs(); EXPECT_TRUE(channel_->SetRecvParameters(parameters)); FakeVideoReceiveStream* stream = AddRecvStream(); const webrtc::VideoReceiveStream::Config& config = stream->GetConfig(); - EXPECT_EQ(engine_.recv_codecs()[0].name, - config.decoders[0].video_format.name); - EXPECT_EQ(engine_.recv_codecs()[0].id, config.decoders[0].payload_type); + EXPECT_EQ(engine_.codecs()[0].name, config.decoders[0].video_format.name); + EXPECT_EQ(engine_.codecs()[0].id, config.decoders[0].payload_type); } TEST_F(WebRtcVideoChannelTest, SetRecvCodecsRejectUnsupportedCodec) { @@ -5736,7 +5721,7 @@ void WebRtcVideoChannelTest::TestReceiveUnsignaledSsrcPacket( uint8_t payload_type, bool expect_created_receive_stream) { // kRedRtxPayloadType must currently be unused. - EXPECT_FALSE(FindCodecById(engine_.recv_codecs(), kRedRtxPayloadType)); + EXPECT_FALSE(FindCodecById(engine_.codecs(), kRedRtxPayloadType)); // Add a RED RTX codec. VideoCodec red_rtx_codec = @@ -7593,7 +7578,6 @@ class WebRtcVideoChannelSimulcastTest : public ::testing::Test { void SetUp() override { encoder_factory_->AddSupportedVideoCodecType("VP8"); - decoder_factory_->AddSupportedVideoCodecType("VP8"); channel_.reset(engine_.CreateMediaChannel( &fake_call_, GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), mock_rate_allocator_factory_.get())); diff --git a/pc/channel.cc b/pc/channel.cc index e3f13e27b1..d6f884ce5e 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -993,8 +993,7 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, VideoSendParameters send_params = last_send_params_; bool needs_send_params_update = false; - if ((type == SdpType::kAnswer || type == SdpType::kPrAnswer) && - webrtc::RtpTransceiverDirectionHasSend(video->direction())) { + if (type == SdpType::kAnswer || type == SdpType::kPrAnswer) { for (auto& send_codec : send_params.codecs) { auto* recv_codec = FindMatchingCodec(recv_params.codecs, send_codec); if (recv_codec) { @@ -1011,13 +1010,13 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, } } - if (webrtc::RtpTransceiverDirectionHasRecv(video->direction())) { - if (!media_channel()->SetRecvParameters(recv_params)) { - SafeSetError("Failed to set local video description recv parameters.", - error_desc); - return false; - } + if (!media_channel()->SetRecvParameters(recv_params)) { + SafeSetError("Failed to set local video description recv parameters.", + error_desc); + return false; + } + if (webrtc::RtpTransceiverDirectionHasRecv(video->direction())) { for (const VideoCodec& codec : video->codecs()) { AddHandledPayloadType(codec.id); } @@ -1026,11 +1025,11 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, RTC_LOG(LS_ERROR) << "Failed to set up video demuxing."; return false; } - last_recv_params_ = recv_params; } + last_recv_params_ = recv_params; + if (needs_send_params_update) { - RTC_DCHECK(webrtc::RtpTransceiverDirectionHasSend(video->direction())); if (!media_channel()->SetSendParameters(send_params)) { SafeSetError("Failed to set send parameters.", error_desc); return false; @@ -1080,10 +1079,7 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content, VideoRecvParameters recv_params = last_recv_params_; bool needs_recv_params_update = false; - // Require SEND direction for receive parameters since we're in - // SetRemoteContent_w. - if ((type == SdpType::kAnswer || type == SdpType::kPrAnswer) && - webrtc::RtpTransceiverDirectionHasSend(video->direction())) { + if (type == SdpType::kAnswer || type == SdpType::kPrAnswer) { for (auto& recv_codec : recv_params.codecs) { auto* send_codec = FindMatchingCodec(send_params.codecs, recv_codec); if (send_codec) { @@ -1100,19 +1096,14 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content, } } - // Require RECV direction for send parameters since we're in - // SetRemoteContent_w. - if (webrtc::RtpTransceiverDirectionHasRecv(video->direction())) { - if (!media_channel()->SetSendParameters(send_params)) { - SafeSetError("Failed to set remote video description send parameters.", - error_desc); - return false; - } - last_send_params_ = send_params; + if (!media_channel()->SetSendParameters(send_params)) { + SafeSetError("Failed to set remote video description send parameters.", + error_desc); + return false; } + last_send_params_ = send_params; if (needs_recv_params_update) { - RTC_DCHECK(webrtc::RtpTransceiverDirectionHasSend(video->direction())); if (!media_channel()->SetRecvParameters(recv_params)) { SafeSetError("Failed to set recv parameters.", error_desc); return false; diff --git a/pc/channel_manager.cc b/pc/channel_manager.cc index 16814bd493..ce8f473600 100644 --- a/pc/channel_manager.cc +++ b/pc/channel_manager.cc @@ -87,31 +87,14 @@ void ChannelManager::GetSupportedAudioRtpHeaderExtensions( *ext = media_engine_->voice().GetCapabilities().header_extensions; } -void ChannelManager::GetSupportedVideoSendCodecs( +void ChannelManager::GetSupportedVideoCodecs( std::vector* codecs) const { if (!media_engine_) { return; } codecs->clear(); - std::vector video_codecs = media_engine_->video().send_codecs(); - for (const auto& video_codec : video_codecs) { - if (!enable_rtx_ && - absl::EqualsIgnoreCase(kRtxCodecName, video_codec.name)) { - continue; - } - codecs->push_back(video_codec); - } -} - -void ChannelManager::GetSupportedVideoReceiveCodecs( - std::vector* codecs) const { - if (!media_engine_) { - return; - } - codecs->clear(); - - std::vector video_codecs = media_engine_->video().recv_codecs(); + std::vector video_codecs = media_engine_->video().codecs(); for (const auto& video_codec : video_codecs) { if (!enable_rtx_ && absl::EqualsIgnoreCase(kRtxCodecName, video_codec.name)) { diff --git a/pc/channel_manager.h b/pc/channel_manager.h index f66ad4bfc1..661ab4bbde 100644 --- a/pc/channel_manager.h +++ b/pc/channel_manager.h @@ -76,8 +76,7 @@ class ChannelManager final { void GetSupportedAudioSendCodecs(std::vector* codecs) const; void GetSupportedAudioReceiveCodecs(std::vector* codecs) const; void GetSupportedAudioRtpHeaderExtensions(RtpHeaderExtensions* ext) const; - void GetSupportedVideoSendCodecs(std::vector* codecs) const; - void GetSupportedVideoReceiveCodecs(std::vector* codecs) const; + void GetSupportedVideoCodecs(std::vector* codecs) const; void GetSupportedVideoRtpHeaderExtensions(RtpHeaderExtensions* ext) const; void GetSupportedDataCodecs(std::vector* codecs) const; diff --git a/pc/channel_manager_unittest.cc b/pc/channel_manager_unittest.cc index 6f3128ebde..90785131f9 100644 --- a/pc/channel_manager_unittest.cc +++ b/pc/channel_manager_unittest.cc @@ -142,29 +142,22 @@ TEST_F(ChannelManagerTest, StartupShutdownOnThread) { } TEST_F(ChannelManagerTest, SetVideoRtxEnabled) { - std::vector send_codecs; - std::vector recv_codecs; + std::vector codecs; const VideoCodec rtx_codec(96, "rtx"); // By default RTX is disabled. - cm_->GetSupportedVideoSendCodecs(&send_codecs); - EXPECT_FALSE(ContainsMatchingCodec(send_codecs, rtx_codec)); - cm_->GetSupportedVideoSendCodecs(&recv_codecs); - EXPECT_FALSE(ContainsMatchingCodec(recv_codecs, rtx_codec)); + cm_->GetSupportedVideoCodecs(&codecs); + EXPECT_FALSE(ContainsMatchingCodec(codecs, rtx_codec)); // Enable and check. EXPECT_TRUE(cm_->SetVideoRtxEnabled(true)); - cm_->GetSupportedVideoSendCodecs(&send_codecs); - EXPECT_TRUE(ContainsMatchingCodec(send_codecs, rtx_codec)); - cm_->GetSupportedVideoSendCodecs(&recv_codecs); - EXPECT_TRUE(ContainsMatchingCodec(recv_codecs, rtx_codec)); + cm_->GetSupportedVideoCodecs(&codecs); + EXPECT_TRUE(ContainsMatchingCodec(codecs, rtx_codec)); // Disable and check. EXPECT_TRUE(cm_->SetVideoRtxEnabled(false)); - cm_->GetSupportedVideoSendCodecs(&send_codecs); - EXPECT_FALSE(ContainsMatchingCodec(send_codecs, rtx_codec)); - cm_->GetSupportedVideoSendCodecs(&recv_codecs); - EXPECT_FALSE(ContainsMatchingCodec(recv_codecs, rtx_codec)); + cm_->GetSupportedVideoCodecs(&codecs); + EXPECT_FALSE(ContainsMatchingCodec(codecs, rtx_codec)); // Cannot toggle rtx after initialization. EXPECT_TRUE(cm_->Init()); @@ -174,10 +167,8 @@ TEST_F(ChannelManagerTest, SetVideoRtxEnabled) { // Can set again after terminate. cm_->Terminate(); EXPECT_TRUE(cm_->SetVideoRtxEnabled(true)); - cm_->GetSupportedVideoSendCodecs(&send_codecs); - EXPECT_TRUE(ContainsMatchingCodec(send_codecs, rtx_codec)); - cm_->GetSupportedVideoSendCodecs(&recv_codecs); - EXPECT_TRUE(ContainsMatchingCodec(recv_codecs, rtx_codec)); + cm_->GetSupportedVideoCodecs(&codecs); + EXPECT_TRUE(ContainsMatchingCodec(codecs, rtx_codec)); } TEST_F(ChannelManagerTest, CreateDestroyChannels) { diff --git a/pc/media_session.cc b/pc/media_session.cc index e764101eef..59f140f951 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -1330,12 +1330,10 @@ MediaSessionDescriptionFactory::MediaSessionDescriptionFactory( channel_manager->GetSupportedAudioSendCodecs(&audio_send_codecs_); channel_manager->GetSupportedAudioReceiveCodecs(&audio_recv_codecs_); channel_manager->GetSupportedAudioRtpHeaderExtensions(&audio_rtp_extensions_); - channel_manager->GetSupportedVideoSendCodecs(&video_send_codecs_); - channel_manager->GetSupportedVideoReceiveCodecs(&video_recv_codecs_); + channel_manager->GetSupportedVideoCodecs(&video_codecs_); channel_manager->GetSupportedVideoRtpHeaderExtensions(&video_rtp_extensions_); channel_manager->GetSupportedDataCodecs(&rtp_data_codecs_); ComputeAudioCodecsIntersectionAndUnion(); - ComputeVideoCodecsIntersectionAndUnion(); } const AudioCodecs& MediaSessionDescriptionFactory::audio_sendrecv_codecs() @@ -1359,27 +1357,6 @@ void MediaSessionDescriptionFactory::set_audio_codecs( ComputeAudioCodecsIntersectionAndUnion(); } -const VideoCodecs& MediaSessionDescriptionFactory::video_sendrecv_codecs() - const { - return video_sendrecv_codecs_; -} - -const VideoCodecs& MediaSessionDescriptionFactory::video_send_codecs() const { - return video_send_codecs_; -} - -const VideoCodecs& MediaSessionDescriptionFactory::video_recv_codecs() const { - return video_recv_codecs_; -} - -void MediaSessionDescriptionFactory::set_video_codecs( - const VideoCodecs& send_codecs, - const VideoCodecs& recv_codecs) { - video_send_codecs_ = send_codecs; - video_recv_codecs_ = recv_codecs; - ComputeVideoCodecsIntersectionAndUnion(); -} - static void RemoveUnifiedPlanExtensions(RtpHeaderExtensions* extensions) { RTC_DCHECK(extensions); @@ -1760,41 +1737,6 @@ const AudioCodecs& MediaSessionDescriptionFactory::GetAudioCodecsForAnswer( return audio_sendrecv_codecs_; } -const VideoCodecs& MediaSessionDescriptionFactory::GetVideoCodecsForOffer( - const RtpTransceiverDirection& direction) const { - switch (direction) { - // If stream is inactive - generate list as if sendrecv. - case RtpTransceiverDirection::kSendRecv: - case RtpTransceiverDirection::kInactive: - return video_sendrecv_codecs_; - case RtpTransceiverDirection::kSendOnly: - return video_send_codecs_; - case RtpTransceiverDirection::kRecvOnly: - return video_recv_codecs_; - } - RTC_NOTREACHED(); - return video_sendrecv_codecs_; -} - -const VideoCodecs& MediaSessionDescriptionFactory::GetVideoCodecsForAnswer( - const RtpTransceiverDirection& offer, - const RtpTransceiverDirection& answer) const { - switch (answer) { - // For inactive and sendrecv answers, generate lists as if we were to accept - // the offer's direction. See RFC 3264 Section 6.1. - case RtpTransceiverDirection::kSendRecv: - case RtpTransceiverDirection::kInactive: - return GetVideoCodecsForOffer( - webrtc::RtpTransceiverDirectionReversed(offer)); - case RtpTransceiverDirection::kSendOnly: - return video_send_codecs_; - case RtpTransceiverDirection::kRecvOnly: - return video_recv_codecs_; - } - RTC_NOTREACHED(); - return video_sendrecv_codecs_; -} - void MergeCodecsFromDescription( const std::vector& current_active_contents, AudioCodecs* audio_codecs, @@ -1842,7 +1784,7 @@ void MediaSessionDescriptionFactory::GetCodecsForOffer( // Add our codecs that are not in the current description. MergeCodecs(all_audio_codecs_, audio_codecs, &used_pltypes); - MergeCodecs(all_video_codecs_, video_codecs, &used_pltypes); + MergeCodecs(video_codecs_, video_codecs, &used_pltypes); MergeCodecs(rtp_data_codecs_, rtp_data_codecs, &used_pltypes); } @@ -1890,7 +1832,7 @@ void MediaSessionDescriptionFactory::GetCodecsForAnswer( if (!FindMatchingCodec(video->codecs(), filtered_offered_video_codecs, offered_video_codec, nullptr) && - FindMatchingCodec(video->codecs(), all_video_codecs_, + FindMatchingCodec(video->codecs(), video_codecs_, offered_video_codec, nullptr)) { filtered_offered_video_codecs.push_back(offered_video_codec); } @@ -2097,7 +2039,7 @@ bool MediaSessionDescriptionFactory::AddAudioContentForOffer( IsDtlsActive(current_content, current_description) ? cricket::SEC_DISABLED : secure(); - auto audio = std::make_unique(); + std::unique_ptr audio(new AudioContentDescription()); std::vector crypto_suites; GetSupportedAudioSdesCryptoSuiteNames(session_options.crypto_options, &crypto_suites); @@ -2125,8 +2067,6 @@ bool MediaSessionDescriptionFactory::AddAudioContentForOffer( return true; } -// TODO(kron): This function is very similar to AddAudioContentForOffer. -// Refactor to reuse shared code. bool MediaSessionDescriptionFactory::AddVideoContentForOffer( const MediaDescriptionOptions& media_description_options, const MediaSessionOptions& session_options, @@ -2137,10 +2077,14 @@ bool MediaSessionDescriptionFactory::AddVideoContentForOffer( StreamParamsVec* current_streams, SessionDescription* desc, IceCredentialsIterator* ice_credentials) const { - // Filter video_codecs (which includes all codecs, with correctly remapped - // payload types) based on transceiver direction. - const VideoCodecs& supported_video_codecs = - GetVideoCodecsForOffer(media_description_options.direction); + cricket::SecurePolicy sdes_policy = + IsDtlsActive(current_content, current_description) ? cricket::SEC_DISABLED + : secure(); + + std::unique_ptr video(new VideoContentDescription()); + std::vector crypto_suites; + GetSupportedVideoSdesCryptoSuiteNames(session_options.crypto_options, + &crypto_suites); VideoCodecs filtered_codecs; @@ -2148,7 +2092,7 @@ bool MediaSessionDescriptionFactory::AddVideoContentForOffer( // Add the codecs from the current transceiver's codec preferences. // They override any existing codecs from previous negotiations. filtered_codecs = MatchCodecPreference( - media_description_options.codec_preferences, supported_video_codecs); + media_description_options.codec_preferences, video_codecs_); } else { // Add the codecs from current content if it exists and is not rejected nor // recycled. @@ -2166,11 +2110,11 @@ bool MediaSessionDescriptionFactory::AddVideoContentForOffer( } // Add other supported video codecs. VideoCodec found_codec; - for (const VideoCodec& codec : supported_video_codecs) { - if (FindMatchingCodec(supported_video_codecs, video_codecs, - codec, &found_codec) && - !FindMatchingCodec(supported_video_codecs, - filtered_codecs, codec, nullptr)) { + for (const VideoCodec& codec : video_codecs_) { + if (FindMatchingCodec(video_codecs_, video_codecs, codec, + &found_codec) && + !FindMatchingCodec(video_codecs_, filtered_codecs, codec, + nullptr)) { // Use the |found_codec| from |video_codecs| because it has the // correctly mapped payload type. filtered_codecs.push_back(found_codec); @@ -2186,13 +2130,6 @@ bool MediaSessionDescriptionFactory::AddVideoContentForOffer( } } - cricket::SecurePolicy sdes_policy = - IsDtlsActive(current_content, current_description) ? cricket::SEC_DISABLED - : secure(); - auto video = std::make_unique(); - std::vector crypto_suites; - GetSupportedVideoSdesCryptoSuiteNames(session_options.crypto_options, - &crypto_suites); if (!CreateMediaContentOffer(media_description_options, session_options, filtered_codecs, sdes_policy, GetCryptos(current_content), crypto_suites, @@ -2215,7 +2152,6 @@ bool MediaSessionDescriptionFactory::AddVideoContentForOffer( current_description, desc, ice_credentials)) { return false; } - return true; } @@ -2227,7 +2163,8 @@ bool MediaSessionDescriptionFactory::AddSctpDataContentForOffer( StreamParamsVec* current_streams, SessionDescription* desc, IceCredentialsIterator* ice_credentials) const { - auto data = std::make_unique(); + std::unique_ptr data( + new SctpDataContentDescription()); bool secure_transport = (transport_desc_factory_->secure() != SEC_DISABLED); @@ -2273,7 +2210,8 @@ bool MediaSessionDescriptionFactory::AddRtpDataContentForOffer( StreamParamsVec* current_streams, SessionDescription* desc, IceCredentialsIterator* ice_credentials) const { - auto data = std::make_unique(); + std::unique_ptr data( + new RtpDataContentDescription()); bool secure_transport = (transport_desc_factory_->secure() != SEC_DISABLED); cricket::SecurePolicy sdes_policy = @@ -2413,7 +2351,8 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer( bool bundle_enabled = offer_description->HasGroup(GROUP_TYPE_BUNDLE) && session_options.bundle_enabled; - auto audio_answer = std::make_unique(); + std::unique_ptr audio_answer( + new AudioContentDescription()); // Do not require or create SDES cryptos if DTLS is used. cricket::SecurePolicy sdes_policy = audio_transport->secure() ? cricket::SEC_DISABLED : secure(); @@ -2453,8 +2392,6 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer( return true; } -// TODO(kron): This function is very similar to AddAudioContentForAnswer. -// Refactor to reuse shared code. bool MediaSessionDescriptionFactory::AddVideoContentForAnswer( const MediaDescriptionOptions& media_description_options, const MediaSessionOptions& session_options, @@ -2479,20 +2416,11 @@ bool MediaSessionDescriptionFactory::AddVideoContentForAnswer( return false; } - // Pick codecs based on the requested communications direction in the offer - // and the selected direction in the answer. - // Note these will be filtered one final time in CreateMediaContentAnswer. - auto wants_rtd = media_description_options.direction; - auto offer_rtd = offer_video_description->direction(); - auto answer_rtd = NegotiateRtpTransceiverDirection(offer_rtd, wants_rtd); - VideoCodecs supported_video_codecs = - GetVideoCodecsForAnswer(offer_rtd, answer_rtd); - VideoCodecs filtered_codecs; if (!media_description_options.codec_preferences.empty()) { filtered_codecs = MatchCodecPreference( - media_description_options.codec_preferences, supported_video_codecs); + media_description_options.codec_preferences, video_codecs_); } else { // Add the codecs from current content if it exists and is not rejected nor // recycled. @@ -2509,11 +2437,11 @@ bool MediaSessionDescriptionFactory::AddVideoContentForAnswer( } } // Add other supported video codecs. - for (const VideoCodec& codec : supported_video_codecs) { - if (FindMatchingCodec(supported_video_codecs, video_codecs, - codec, nullptr) && - !FindMatchingCodec(supported_video_codecs, - filtered_codecs, codec, nullptr)) { + for (const VideoCodec& codec : video_codecs_) { + if (FindMatchingCodec(video_codecs_, video_codecs, codec, + nullptr) && + !FindMatchingCodec(video_codecs_, filtered_codecs, codec, + nullptr)) { // We should use the local codec with local parameters and the codec id // would be correctly mapped in |NegotiateCodecs|. filtered_codecs.push_back(codec); @@ -2531,7 +2459,9 @@ bool MediaSessionDescriptionFactory::AddVideoContentForAnswer( bool bundle_enabled = offer_description->HasGroup(GROUP_TYPE_BUNDLE) && session_options.bundle_enabled; - auto video_answer = std::make_unique(); + + std::unique_ptr video_answer( + new VideoContentDescription()); // Do not require or create SDES cryptos if DTLS is used. cricket::SecurePolicy sdes_policy = video_transport->secure() ? cricket::SEC_DISABLED : secure(); @@ -2701,38 +2631,6 @@ void MediaSessionDescriptionFactory::ComputeAudioCodecsIntersectionAndUnion() { &audio_sendrecv_codecs_, true); } -void MediaSessionDescriptionFactory::ComputeVideoCodecsIntersectionAndUnion() { - video_sendrecv_codecs_.clear(); - all_video_codecs_.clear(); - // Compute the video codecs union. - for (const VideoCodec& send : video_send_codecs_) { - all_video_codecs_.push_back(send); - if (!FindMatchingCodec(video_send_codecs_, video_recv_codecs_, - send, nullptr)) { - // TODO(kron): This check is violated by the unit test: - // MediaSessionDescriptionFactoryTest.RtxWithoutApt - // Remove either the test or the check. - - // It doesn't make sense to have an RTX codec we support sending but not - // receiving. - // RTC_DCHECK(!IsRtxCodec(send)); - } - } - for (const VideoCodec& recv : video_recv_codecs_) { - if (!FindMatchingCodec(video_recv_codecs_, video_send_codecs_, - recv, nullptr)) { - all_video_codecs_.push_back(recv); - } - } - // Use NegotiateCodecs to merge our codec lists, since the operation is - // essentially the same. Put send_codecs as the offered_codecs, which is the - // order we'd like to follow. The reasoning is that encoding is usually more - // expensive than decoding, and prioritizing a codec in the send list probably - // means it's a codec we can handle efficiently. - NegotiateCodecs(video_recv_codecs_, video_send_codecs_, - &video_sendrecv_codecs_, true); -} - bool IsMediaContent(const ContentInfo* content) { return (content && (content->type == MediaProtocolType::kRtp || content->type == MediaProtocolType::kSctp)); diff --git a/pc/media_session.h b/pc/media_session.h index ef83834318..235945c4f9 100644 --- a/pc/media_session.h +++ b/pc/media_session.h @@ -151,11 +151,8 @@ class MediaSessionDescriptionFactory { audio_rtp_extensions_ = extensions; } RtpHeaderExtensions audio_rtp_header_extensions() const; - const VideoCodecs& video_sendrecv_codecs() const; - const VideoCodecs& video_send_codecs() const; - const VideoCodecs& video_recv_codecs() const; - void set_video_codecs(const VideoCodecs& send_codecs, - const VideoCodecs& recv_codecs); + const VideoCodecs& video_codecs() const { return video_codecs_; } + void set_video_codecs(const VideoCodecs& codecs) { video_codecs_ = codecs; } void set_video_rtp_header_extensions(const RtpHeaderExtensions& extensions) { video_rtp_extensions_ = extensions; } @@ -189,11 +186,6 @@ class MediaSessionDescriptionFactory { const AudioCodecs& GetAudioCodecsForAnswer( const webrtc::RtpTransceiverDirection& offer, const webrtc::RtpTransceiverDirection& answer) const; - const VideoCodecs& GetVideoCodecsForOffer( - const webrtc::RtpTransceiverDirection& direction) const; - const VideoCodecs& GetVideoCodecsForAnswer( - const webrtc::RtpTransceiverDirection& offer, - const webrtc::RtpTransceiverDirection& answer) const; void GetCodecsForOffer( const std::vector& current_active_contents, AudioCodecs* audio_codecs, @@ -325,8 +317,6 @@ class MediaSessionDescriptionFactory { void ComputeAudioCodecsIntersectionAndUnion(); - void ComputeVideoCodecsIntersectionAndUnion(); - bool is_unified_plan_ = false; AudioCodecs audio_send_codecs_; AudioCodecs audio_recv_codecs_; @@ -335,12 +325,7 @@ class MediaSessionDescriptionFactory { // Union of send and recv. AudioCodecs all_audio_codecs_; RtpHeaderExtensions audio_rtp_extensions_; - VideoCodecs video_send_codecs_; - VideoCodecs video_recv_codecs_; - // Intersection of send and recv. - VideoCodecs video_sendrecv_codecs_; - // Union of send and recv. - VideoCodecs all_video_codecs_; + VideoCodecs video_codecs_; RtpHeaderExtensions video_rtp_extensions_; RtpDataCodecs rtp_data_codecs_; // This object is not owned by the channel so it must outlive it. diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc index a901dedb70..a2416c4dcc 100644 --- a/pc/media_session_unittest.cc +++ b/pc/media_session_unittest.cc @@ -415,13 +415,11 @@ class MediaSessionDescriptionFactoryTest : public ::testing::Test { : f1_(&tdf1_, &ssrc_generator1), f2_(&tdf2_, &ssrc_generator2) { f1_.set_audio_codecs(MAKE_VECTOR(kAudioCodecs1), MAKE_VECTOR(kAudioCodecs1)); - f1_.set_video_codecs(MAKE_VECTOR(kVideoCodecs1), - MAKE_VECTOR(kVideoCodecs1)); + f1_.set_video_codecs(MAKE_VECTOR(kVideoCodecs1)); f1_.set_rtp_data_codecs(MAKE_VECTOR(kDataCodecs1)); f2_.set_audio_codecs(MAKE_VECTOR(kAudioCodecs2), MAKE_VECTOR(kAudioCodecs2)); - f2_.set_video_codecs(MAKE_VECTOR(kVideoCodecs2), - MAKE_VECTOR(kVideoCodecs2)); + f2_.set_video_codecs(MAKE_VECTOR(kVideoCodecs2)); f2_.set_rtp_data_codecs(MAKE_VECTOR(kDataCodecs2)); tdf1_.set_certificate(rtc::RTCCertificate::Create( std::unique_ptr(new rtc::FakeSSLIdentity("id1")))); @@ -799,7 +797,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateVideoOffer) { ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuite); EXPECT_EQ(cricket::kMediaProtocolSavpf, acd->protocol()); EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type()); - EXPECT_EQ(f1_.video_sendrecv_codecs(), vcd->codecs()); + EXPECT_EQ(f1_.video_codecs(), vcd->codecs()); EXPECT_EQ(0U, vcd->first_ssrc()); // no sender is attached EXPECT_EQ(kAutoBandwidth, vcd->bandwidth()); // default bandwidth (auto) EXPECT_TRUE(vcd->rtcp_mux()); // rtcp-mux defaults on @@ -811,7 +809,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateVideoOffer) { // RTP playlod type. The test verifies that the offer don't contain the // duplicate RTP payload types. TEST_F(MediaSessionDescriptionFactoryTest, TestBundleOfferWithSameCodecPlType) { - const VideoCodec& offered_video_codec = f2_.video_sendrecv_codecs()[0]; + const VideoCodec& offered_video_codec = f2_.video_codecs()[0]; const AudioCodec& offered_audio_codec = f2_.audio_sendrecv_codecs()[0]; const RtpDataCodec& offered_data_codec = f2_.rtp_data_codecs()[0]; ASSERT_EQ(offered_video_codec.id, offered_audio_codec.id); @@ -2063,7 +2061,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateMultiStreamVideoOffer) { ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuite); EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type()); - EXPECT_EQ(f1_.video_sendrecv_codecs(), vcd->codecs()); + EXPECT_EQ(f1_.video_codecs(), vcd->codecs()); ASSERT_CRYPTO(vcd, 1U, kDefaultSrtpCryptoSuite); const StreamParamsVec& video_streams = vcd->streams(); @@ -2559,8 +2557,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, // that is being recycled. TEST_F(MediaSessionDescriptionFactoryTest, ReOfferDoesNotReUseRecycledAudioCodecs) { - f1_.set_video_codecs({}, {}); - f2_.set_video_codecs({}, {}); + f1_.set_video_codecs({}); + f2_.set_video_codecs({}); MediaSessionOptions opts; AddMediaDescriptionOptions(MEDIA_TYPE_AUDIO, "a0", @@ -2612,8 +2610,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, // section that is being recycled. TEST_F(MediaSessionDescriptionFactoryTest, ReAnswerDoesNotReUseRecycledAudioCodecs) { - f1_.set_video_codecs({}, {}); - f2_.set_video_codecs({}, {}); + f1_.set_video_codecs({}); + f2_.set_video_codecs({}); // Perform initial offer/answer in reverse (|f2_| as offerer) so that the // second offer/answer is forward (|f1_| as offerer). @@ -2682,12 +2680,12 @@ TEST_F(MediaSessionDescriptionFactoryTest, std::vector f1_codecs = MAKE_VECTOR(kVideoCodecs1); // This creates rtx for H264 with the payload type |f1_| uses. AddRtxCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id), &f1_codecs); - f1_.set_video_codecs(f1_codecs, f1_codecs); + f1_.set_video_codecs(f1_codecs); std::vector f2_codecs = MAKE_VECTOR(kVideoCodecs2); // This creates rtx for H264 with the payload type |f2_| uses. AddRtxCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs2[0].id), &f2_codecs); - f2_.set_video_codecs(f2_codecs, f2_codecs); + f2_.set_video_codecs(f2_codecs); std::unique_ptr offer = f1_.CreateOffer(opts, NULL); ASSERT_TRUE(offer.get() != NULL); @@ -2746,8 +2744,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, std::vector f2_codecs = {vp9, vp9_rtx, vp8_answerer, vp8_answerer_rtx}; - f1_.set_video_codecs(f1_codecs, f1_codecs); - f2_.set_video_codecs(f2_codecs, f2_codecs); + f1_.set_video_codecs(f1_codecs); + f2_.set_video_codecs(f2_codecs); std::vector audio_codecs; f1_.set_audio_codecs(audio_codecs, audio_codecs); f2_.set_audio_codecs(audio_codecs, audio_codecs); @@ -2782,7 +2780,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, std::vector f1_codecs = MAKE_VECTOR(kVideoCodecs1); // This creates rtx for H264 with the payload type |f1_| uses. AddRtxCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id), &f1_codecs); - f1_.set_video_codecs(f1_codecs, f1_codecs); + f1_.set_video_codecs(f1_codecs); MediaSessionOptions opts; AddMediaDescriptionOptions(MEDIA_TYPE_AUDIO, "audio", @@ -2807,7 +2805,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, int used_pl_type = acd->codecs()[0].id; f2_codecs[0].id = used_pl_type; // Set the payload type for H264. AddRtxCodec(VideoCodec::CreateRtxCodec(125, used_pl_type), &f2_codecs); - f2_.set_video_codecs(f2_codecs, f2_codecs); + f2_.set_video_codecs(f2_codecs); std::unique_ptr updated_offer( f2_.CreateOffer(opts, answer.get())); @@ -2843,7 +2841,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, std::vector f2_codecs = MAKE_VECTOR(kVideoCodecs2); // This creates rtx for H264 with the payload type |f2_| uses. AddRtxCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs2[0].id), &f2_codecs); - f2_.set_video_codecs(f2_codecs, f2_codecs); + f2_.set_video_codecs(f2_codecs); std::unique_ptr offer = f1_.CreateOffer(opts, nullptr); ASSERT_TRUE(offer.get() != nullptr); @@ -2882,12 +2880,12 @@ TEST_F(MediaSessionDescriptionFactoryTest, RtxWithoutApt) { std::vector f1_codecs = MAKE_VECTOR(kVideoCodecs1); // This creates RTX without associated payload type parameter. AddRtxCodec(VideoCodec(126, cricket::kRtxCodecName), &f1_codecs); - f1_.set_video_codecs(f1_codecs, f1_codecs); + f1_.set_video_codecs(f1_codecs); std::vector f2_codecs = MAKE_VECTOR(kVideoCodecs2); // This creates RTX for H264 with the payload type |f2_| uses. AddRtxCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs2[0].id), &f2_codecs); - f2_.set_video_codecs(f2_codecs, f2_codecs); + f2_.set_video_codecs(f2_codecs); std::unique_ptr offer = f1_.CreateOffer(opts, NULL); ASSERT_TRUE(offer.get() != NULL); @@ -2925,12 +2923,12 @@ TEST_F(MediaSessionDescriptionFactoryTest, FilterOutRtxIfAptDoesntMatch) { std::vector f1_codecs = MAKE_VECTOR(kVideoCodecs1); // This creates RTX for H264 in sender. AddRtxCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id), &f1_codecs); - f1_.set_video_codecs(f1_codecs, f1_codecs); + f1_.set_video_codecs(f1_codecs); std::vector f2_codecs = MAKE_VECTOR(kVideoCodecs2); // This creates RTX for H263 in receiver. AddRtxCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs2[1].id), &f2_codecs); - f2_.set_video_codecs(f2_codecs, f2_codecs); + f2_.set_video_codecs(f2_codecs); std::unique_ptr offer = f1_.CreateOffer(opts, NULL); ASSERT_TRUE(offer.get() != NULL); @@ -2955,16 +2953,16 @@ TEST_F(MediaSessionDescriptionFactoryTest, std::vector f1_codecs = MAKE_VECTOR(kVideoCodecs1); // This creates RTX for H264-SVC in sender. AddRtxCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs1[0].id), &f1_codecs); - f1_.set_video_codecs(f1_codecs, f1_codecs); + f1_.set_video_codecs(f1_codecs); // This creates RTX for H264 in sender. AddRtxCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id), &f1_codecs); - f1_.set_video_codecs(f1_codecs, f1_codecs); + f1_.set_video_codecs(f1_codecs); std::vector f2_codecs = MAKE_VECTOR(kVideoCodecs2); // This creates RTX for H264 in receiver. AddRtxCodec(VideoCodec::CreateRtxCodec(124, kVideoCodecs2[0].id), &f2_codecs); - f2_.set_video_codecs(f2_codecs, f1_codecs); + f2_.set_video_codecs(f2_codecs); // H264-SVC codec is removed in the answer, therefore, associated RTX codec // for H264-SVC should also be removed. @@ -2991,7 +2989,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, AddSecondRtxInNewOffer) { std::vector f1_codecs = MAKE_VECTOR(kVideoCodecs1); // This creates RTX for H264 for the offerer. AddRtxCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id), &f1_codecs); - f1_.set_video_codecs(f1_codecs, f1_codecs); + f1_.set_video_codecs(f1_codecs); std::unique_ptr offer = f1_.CreateOffer(opts, nullptr); ASSERT_TRUE(offer); @@ -3005,7 +3003,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, AddSecondRtxInNewOffer) { // Now, attempt to add RTX for H264-SVC. AddRtxCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs1[0].id), &f1_codecs); - f1_.set_video_codecs(f1_codecs, f1_codecs); + f1_.set_video_codecs(f1_codecs); std::unique_ptr updated_offer( f1_.CreateOffer(opts, offer.get())); @@ -3032,7 +3030,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, SimSsrcsGenerateMultipleRtxSsrcs) { std::vector f1_codecs; f1_codecs.push_back(VideoCodec(97, "H264")); AddRtxCodec(VideoCodec::CreateRtxCodec(125, 97), &f1_codecs); - f1_.set_video_codecs(f1_codecs, f1_codecs); + f1_.set_video_codecs(f1_codecs); // Ensure that the offer has an RTX ssrc for each regular ssrc, and that there // is a FID ssrc + grouping for each. @@ -3074,7 +3072,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, GenerateFlexfecSsrc) { std::vector f1_codecs; f1_codecs.push_back(VideoCodec(97, "H264")); f1_codecs.push_back(VideoCodec(118, "flexfec-03")); - f1_.set_video_codecs(f1_codecs, f1_codecs); + f1_.set_video_codecs(f1_codecs); // Ensure that the offer has a single FlexFEC ssrc and that // there is no FEC-FR ssrc + grouping for each. @@ -3115,7 +3113,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, SimSsrcsGenerateNoFlexfecSsrcs) { std::vector f1_codecs; f1_codecs.push_back(VideoCodec(97, "H264")); f1_codecs.push_back(VideoCodec(118, "flexfec-03")); - f1_.set_video_codecs(f1_codecs, f1_codecs); + f1_.set_video_codecs(f1_codecs); // Ensure that the offer has no FlexFEC ssrcs for each regular ssrc, and that // there is no FEC-FR ssrc + grouping for each. @@ -4253,9 +4251,9 @@ TEST_F(MediaSessionDescriptionFactoryTest, CreateAnswerWithLocalCodecParams) { video_codecs2[0].SetParam(video_param_name, video_value2); f1_.set_audio_codecs(audio_codecs1, audio_codecs1); - f1_.set_video_codecs(video_codecs1, video_codecs1); + f1_.set_video_codecs(video_codecs1); f2_.set_audio_codecs(audio_codecs2, audio_codecs2); - f2_.set_video_codecs(video_codecs2, video_codecs2); + f2_.set_video_codecs(video_codecs2); MediaSessionOptions opts; AddMediaDescriptionOptions(MEDIA_TYPE_AUDIO, "audio", @@ -4305,8 +4303,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, // Offerer will send both codecs, answerer should choose the one with matching // packetization mode (and not the first one it sees). - f1_.set_video_codecs({h264_pm0, h264_pm1}, {h264_pm0, h264_pm1}); - f2_.set_video_codecs({h264_pm1}, {h264_pm1}); + f1_.set_video_codecs({h264_pm0, h264_pm1}); + f2_.set_video_codecs({h264_pm1}); MediaSessionOptions opts; AddMediaDescriptionOptions(MEDIA_TYPE_VIDEO, "video", @@ -4335,13 +4333,11 @@ class MediaProtocolTest : public ::testing::TestWithParam { : f1_(&tdf1_, &ssrc_generator1), f2_(&tdf2_, &ssrc_generator2) { f1_.set_audio_codecs(MAKE_VECTOR(kAudioCodecs1), MAKE_VECTOR(kAudioCodecs1)); - f1_.set_video_codecs(MAKE_VECTOR(kVideoCodecs1), - MAKE_VECTOR(kVideoCodecs1)); + f1_.set_video_codecs(MAKE_VECTOR(kVideoCodecs1)); f1_.set_rtp_data_codecs(MAKE_VECTOR(kDataCodecs1)); f2_.set_audio_codecs(MAKE_VECTOR(kAudioCodecs2), MAKE_VECTOR(kAudioCodecs2)); - f2_.set_video_codecs(MAKE_VECTOR(kVideoCodecs2), - MAKE_VECTOR(kVideoCodecs2)); + f2_.set_video_codecs(MAKE_VECTOR(kVideoCodecs2)); f2_.set_rtp_data_codecs(MAKE_VECTOR(kDataCodecs2)); f1_.set_secure(SEC_ENABLED); f2_.set_secure(SEC_ENABLED); diff --git a/pc/peer_connection_factory.cc b/pc/peer_connection_factory.cc index c8bb22e43e..4523121b58 100644 --- a/pc/peer_connection_factory.cc +++ b/pc/peer_connection_factory.cc @@ -169,7 +169,7 @@ RtpCapabilities PeerConnectionFactory::GetRtpSenderCapabilities( case cricket::MEDIA_TYPE_VIDEO: { cricket::VideoCodecs cricket_codecs; cricket::RtpHeaderExtensions cricket_extensions; - channel_manager_->GetSupportedVideoSendCodecs(&cricket_codecs); + channel_manager_->GetSupportedVideoCodecs(&cricket_codecs); channel_manager_->GetSupportedVideoRtpHeaderExtensions( &cricket_extensions); return ToRtpCapabilities(cricket_codecs, cricket_extensions); @@ -196,7 +196,7 @@ RtpCapabilities PeerConnectionFactory::GetRtpReceiverCapabilities( case cricket::MEDIA_TYPE_VIDEO: { cricket::VideoCodecs cricket_codecs; cricket::RtpHeaderExtensions cricket_extensions; - channel_manager_->GetSupportedVideoReceiveCodecs(&cricket_codecs); + channel_manager_->GetSupportedVideoCodecs(&cricket_codecs); channel_manager_->GetSupportedVideoRtpHeaderExtensions( &cricket_extensions); return ToRtpCapabilities(cricket_codecs, cricket_extensions); diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc index df231f572c..399001f9f3 100644 --- a/pc/peer_connection_integrationtest.cc +++ b/pc/peer_connection_integrationtest.cc @@ -214,9 +214,7 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, dependencies.cert_generator = std::move(cert_generator); if (!client->Init(nullptr, nullptr, std::move(dependencies), network_thread, worker_thread, nullptr, - /*media_transport_factory=*/nullptr, - /*reset_encoder_factory=*/false, - /*reset_decoder_factory=*/false)) { + /*media_transport_factory=*/nullptr)) { delete client; return nullptr; } @@ -606,9 +604,7 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, rtc::Thread* network_thread, rtc::Thread* worker_thread, std::unique_ptr event_log_factory, - std::unique_ptr media_transport_factory, - bool reset_encoder_factory, - bool reset_decoder_factory) { + std::unique_ptr media_transport_factory) { // There's an error in this test code if Init ends up being called twice. RTC_DCHECK(!peer_connection_); RTC_DCHECK(!peer_connection_factory_); @@ -636,14 +632,6 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, pc_factory_dependencies.task_queue_factory.get(); media_deps.adm = fake_audio_capture_module_; webrtc::SetMediaEngineDefaults(&media_deps); - - if (reset_encoder_factory) { - media_deps.video_encoder_factory.reset(); - } - if (reset_decoder_factory) { - media_deps.video_decoder_factory.reset(); - } - pc_factory_dependencies.media_engine = cricket::CreateMediaEngine(std::move(media_deps)); pc_factory_dependencies.call_factory = webrtc::CreateCallFactory(); @@ -1277,9 +1265,7 @@ class PeerConnectionIntegrationBaseTest : public ::testing::Test { const RTCConfiguration* config, webrtc::PeerConnectionDependencies dependencies, std::unique_ptr event_log_factory, - std::unique_ptr media_transport_factory, - bool reset_encoder_factory, - bool reset_decoder_factory) { + std::unique_ptr media_transport_factory) { RTCConfiguration modified_config; if (config) { modified_config = *config; @@ -1295,8 +1281,7 @@ class PeerConnectionIntegrationBaseTest : public ::testing::Test { if (!client->Init(options, &modified_config, std::move(dependencies), network_thread_.get(), worker_thread_.get(), std::move(event_log_factory), - std::move(media_transport_factory), reset_encoder_factory, - reset_decoder_factory)) { + std::move(media_transport_factory))) { return nullptr; } return client; @@ -1310,11 +1295,10 @@ class PeerConnectionIntegrationBaseTest : public ::testing::Test { webrtc::PeerConnectionDependencies dependencies) { std::unique_ptr event_log_factory( new webrtc::FakeRtcEventLogFactory(rtc::Thread::Current())); - return CreatePeerConnectionWrapper( - debug_name, options, config, std::move(dependencies), - std::move(event_log_factory), - /*media_transport_factory=*/nullptr, /*reset_encoder_factory=*/false, - /*reset_decoder_factory=*/false); + return CreatePeerConnectionWrapper(debug_name, options, config, + std::move(dependencies), + std::move(event_log_factory), + /*media_transport_factory=*/nullptr); } bool CreatePeerConnectionWrappers() { @@ -1335,15 +1319,11 @@ class PeerConnectionIntegrationBaseTest : public ::testing::Test { sdp_semantics_ = caller_semantics; caller_ = CreatePeerConnectionWrapper( "Caller", nullptr, nullptr, webrtc::PeerConnectionDependencies(nullptr), - nullptr, /*media_transport_factory=*/nullptr, - /*reset_encoder_factory=*/false, - /*reset_decoder_factory=*/false); + nullptr, /*media_transport_factory=*/nullptr); sdp_semantics_ = callee_semantics; callee_ = CreatePeerConnectionWrapper( "Callee", nullptr, nullptr, webrtc::PeerConnectionDependencies(nullptr), - nullptr, /*media_transport_factory=*/nullptr, - /*reset_encoder_factory=*/false, - /*reset_decoder_factory=*/false); + nullptr, /*media_transport_factory=*/nullptr); sdp_semantics_ = original_semantics; return caller_ && callee_; } @@ -1354,13 +1334,11 @@ class PeerConnectionIntegrationBaseTest : public ::testing::Test { caller_ = CreatePeerConnectionWrapper( "Caller", nullptr, &caller_config, webrtc::PeerConnectionDependencies(nullptr), nullptr, - /*media_transport_factory=*/nullptr, /*reset_encoder_factory=*/false, - /*reset_decoder_factory=*/false); + /*media_transport_factory=*/nullptr); callee_ = CreatePeerConnectionWrapper( "Callee", nullptr, &callee_config, webrtc::PeerConnectionDependencies(nullptr), nullptr, - /*media_transport_factory=*/nullptr, /*reset_encoder_factory=*/false, - /*reset_decoder_factory=*/false); + /*media_transport_factory=*/nullptr); return caller_ && callee_; } @@ -1369,16 +1347,14 @@ class PeerConnectionIntegrationBaseTest : public ::testing::Test { const PeerConnectionInterface::RTCConfiguration& callee_config, std::unique_ptr caller_factory, std::unique_ptr callee_factory) { - caller_ = CreatePeerConnectionWrapper( - "Caller", nullptr, &caller_config, - webrtc::PeerConnectionDependencies(nullptr), nullptr, - std::move(caller_factory), /*reset_encoder_factory=*/false, - /*reset_decoder_factory=*/false); - callee_ = CreatePeerConnectionWrapper( - "Callee", nullptr, &callee_config, - webrtc::PeerConnectionDependencies(nullptr), nullptr, - std::move(callee_factory), /*reset_encoder_factory=*/false, - /*reset_decoder_factory=*/false); + caller_ = + CreatePeerConnectionWrapper("Caller", nullptr, &caller_config, + webrtc::PeerConnectionDependencies(nullptr), + nullptr, std::move(caller_factory)); + callee_ = + CreatePeerConnectionWrapper("Callee", nullptr, &callee_config, + webrtc::PeerConnectionDependencies(nullptr), + nullptr, std::move(callee_factory)); return caller_ && callee_; } @@ -1387,16 +1363,14 @@ class PeerConnectionIntegrationBaseTest : public ::testing::Test { webrtc::PeerConnectionDependencies caller_dependencies, const PeerConnectionInterface::RTCConfiguration& callee_config, webrtc::PeerConnectionDependencies callee_dependencies) { - caller_ = CreatePeerConnectionWrapper( - "Caller", nullptr, &caller_config, std::move(caller_dependencies), - nullptr, - /*media_transport_factory=*/nullptr, /*reset_encoder_factory=*/false, - /*reset_decoder_factory=*/false); - callee_ = CreatePeerConnectionWrapper( - "Callee", nullptr, &callee_config, std::move(callee_dependencies), - nullptr, - /*media_transport_factory=*/nullptr, /*reset_encoder_factory=*/false, - /*reset_decoder_factory=*/false); + caller_ = + CreatePeerConnectionWrapper("Caller", nullptr, &caller_config, + std::move(caller_dependencies), nullptr, + /*media_transport_factory=*/nullptr); + callee_ = + CreatePeerConnectionWrapper("Callee", nullptr, &callee_config, + std::move(callee_dependencies), nullptr, + /*media_transport_factory=*/nullptr); return caller_ && callee_; } @@ -1406,13 +1380,11 @@ class PeerConnectionIntegrationBaseTest : public ::testing::Test { caller_ = CreatePeerConnectionWrapper( "Caller", &caller_options, nullptr, webrtc::PeerConnectionDependencies(nullptr), nullptr, - /*media_transport_factory=*/nullptr, /*reset_encoder_factory=*/false, - /*reset_decoder_factory=*/false); + /*media_transport_factory=*/nullptr); callee_ = CreatePeerConnectionWrapper( "Callee", &callee_options, nullptr, webrtc::PeerConnectionDependencies(nullptr), nullptr, - /*media_transport_factory=*/nullptr, /*reset_encoder_factory=*/false, - /*reset_decoder_factory=*/false); + /*media_transport_factory=*/nullptr); return caller_ && callee_; } @@ -1435,24 +1407,9 @@ class PeerConnectionIntegrationBaseTest : public ::testing::Test { webrtc::PeerConnectionDependencies dependencies(nullptr); dependencies.cert_generator = std::move(cert_generator); - return CreatePeerConnectionWrapper( - "New Peer", nullptr, nullptr, std::move(dependencies), nullptr, - /*media_transport_factory=*/nullptr, /*reset_encoder_factory=*/false, - /*reset_decoder_factory=*/false); - } - - bool CreateOneDirectionalPeerConnectionWrappers(bool caller_to_callee) { - caller_ = CreatePeerConnectionWrapper( - "Caller", nullptr, nullptr, webrtc::PeerConnectionDependencies(nullptr), - nullptr, /*media_transport_factory=*/nullptr, - /*reset_encoder_factory=*/!caller_to_callee, - /*reset_decoder_factory=*/caller_to_callee); - callee_ = CreatePeerConnectionWrapper( - "Callee", nullptr, nullptr, webrtc::PeerConnectionDependencies(nullptr), - nullptr, /*media_transport_factory=*/nullptr, - /*reset_encoder_factory=*/caller_to_callee, - /*reset_decoder_factory=*/!caller_to_callee); - return caller_ && callee_; + return CreatePeerConnectionWrapper("New Peer", nullptr, nullptr, + std::move(dependencies), nullptr, + /*media_transport_factory=*/nullptr); } cricket::TestTurnServer* CreateTurnServer( @@ -2081,56 +2038,6 @@ TEST_P(PeerConnectionIntegrationTest, OneWayMediaCall) { ASSERT_TRUE(ExpectNewFrames(media_expectations)); } -// Tests that send only works without the caller having a decoder factory and -// the callee having an encoder factory. -TEST_P(PeerConnectionIntegrationTest, EndToEndCallWithSendOnlyVideo) { - ASSERT_TRUE( - CreateOneDirectionalPeerConnectionWrappers(/*caller_to_callee=*/true)); - ConnectFakeSignaling(); - // Add one-directional video, from caller to callee. - rtc::scoped_refptr track = - caller()->CreateLocalVideoTrack(); - caller()->AddTrack(track); - PeerConnectionInterface::RTCOfferAnswerOptions options; - options.offer_to_receive_video = 0; - caller()->SetOfferAnswerOptions(options); - caller()->CreateAndSetAndSignalOffer(); - ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); - ASSERT_EQ(callee()->pc()->GetReceivers().size(), 1u); - - // Expect video to be received in one direction. - MediaExpectations media_expectations; - media_expectations.CallerExpectsNoVideo(); - media_expectations.CalleeExpectsSomeVideo(); - - EXPECT_TRUE(ExpectNewFrames(media_expectations)); -} - -// Tests that receive only works without the caller having an encoder factory -// and the callee having a dncoder factory. -TEST_P(PeerConnectionIntegrationTest, EndToEndCallWithReceiveOnlyVideo) { - ASSERT_TRUE( - CreateOneDirectionalPeerConnectionWrappers(/*caller_to_callee=*/false)); - ConnectFakeSignaling(); - // Add one-directional video, from caller to callee. - rtc::scoped_refptr track = - callee()->CreateLocalVideoTrack(); - callee()->AddTrack(track); - PeerConnectionInterface::RTCOfferAnswerOptions options; - options.offer_to_receive_video = 1; - caller()->SetOfferAnswerOptions(options); - caller()->CreateAndSetAndSignalOffer(); - ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); - ASSERT_EQ(caller()->pc()->GetReceivers().size(), 1u); - - // Expect video to be received in one direction. - MediaExpectations media_expectations; - media_expectations.CallerExpectsSomeVideo(); - media_expectations.CalleeExpectsNoVideo(); - - EXPECT_TRUE(ExpectNewFrames(media_expectations)); -} - // This test sets up a audio call initially, with the callee rejecting video // initially. Then later the callee decides to upgrade to audio/video, and // initiates a new offer/answer exchange. @@ -5349,10 +5256,9 @@ TEST_P(PeerConnectionIntegrationTest, IceTransportFactoryUsedForConnections) { auto ice_transport_factory = std::make_unique(); EXPECT_CALL(*ice_transport_factory, RecordIceTransportCreated()).Times(1); dependencies.ice_transport_factory = std::move(ice_transport_factory); - auto wrapper = CreatePeerConnectionWrapper( - "Caller", nullptr, &default_config, std::move(dependencies), nullptr, - nullptr, /*reset_encoder_factory=*/false, - /*reset_decoder_factory=*/false); + auto wrapper = + CreatePeerConnectionWrapper("Caller", nullptr, &default_config, + std::move(dependencies), nullptr, nullptr); ASSERT_TRUE(wrapper); wrapper->CreateDataChannel(); rtc::scoped_refptr observer( diff --git a/pc/peer_connection_media_unittest.cc b/pc/peer_connection_media_unittest.cc index c9ffd776d9..077c4a3e43 100644 --- a/pc/peer_connection_media_unittest.cc +++ b/pc/peer_connection_media_unittest.cc @@ -1434,11 +1434,9 @@ TEST_F(PeerConnectionMediaTestUnifiedPlan, TEST_F(PeerConnectionMediaTestUnifiedPlan, SetCodecPreferencesVideoRejectsOnlyRtxRedFec) { auto fake_engine = std::make_unique(); - auto video_codecs = fake_engine->video().send_codecs(); + auto video_codecs = fake_engine->video().codecs(); video_codecs.push_back( cricket::VideoCodec(video_codecs.back().id + 1, cricket::kRtxCodecName)); - video_codecs.back().params[cricket::kCodecParamAssociatedPayloadType] = - std::to_string(video_codecs.back().id - 1); video_codecs.push_back( cricket::VideoCodec(video_codecs.back().id + 1, cricket::kRedCodecName)); video_codecs.push_back(cricket::VideoCodec(video_codecs.back().id + 1, @@ -1542,7 +1540,7 @@ TEST_F(PeerConnectionMediaTestUnifiedPlan, TEST_F(PeerConnectionMediaTestUnifiedPlan, SetCodecPreferencesVideoWithRtx) { auto caller_fake_engine = std::make_unique(); - auto caller_video_codecs = caller_fake_engine->video().send_codecs(); + auto caller_video_codecs = caller_fake_engine->video().codecs(); caller_video_codecs.push_back(cricket::VideoCodec( caller_video_codecs.back().id + 1, cricket::kVp8CodecName)); caller_video_codecs.push_back(cricket::VideoCodec( @@ -1594,7 +1592,7 @@ TEST_F(PeerConnectionMediaTestUnifiedPlan, SetCodecPreferencesVideoWithRtx) { TEST_F(PeerConnectionMediaTestUnifiedPlan, SetCodecPreferencesVideoCodecsNegotiation) { auto caller_fake_engine = std::make_unique(); - auto caller_video_codecs = caller_fake_engine->video().send_codecs(); + auto caller_video_codecs = caller_fake_engine->video().codecs(); caller_video_codecs.push_back(cricket::VideoCodec( caller_video_codecs.back().id + 1, cricket::kVp8CodecName)); caller_video_codecs.push_back(cricket::VideoCodec( @@ -1668,7 +1666,7 @@ TEST_F(PeerConnectionMediaTestUnifiedPlan, TEST_F(PeerConnectionMediaTestUnifiedPlan, SetCodecPreferencesVideoCodecsNegotiationReverseOrder) { auto caller_fake_engine = std::make_unique(); - auto caller_video_codecs = caller_fake_engine->video().send_codecs(); + auto caller_video_codecs = caller_fake_engine->video().codecs(); caller_video_codecs.push_back(cricket::VideoCodec( caller_video_codecs.back().id + 1, cricket::kVp8CodecName)); caller_video_codecs.push_back(cricket::VideoCodec( diff --git a/pc/rtp_transceiver.cc b/pc/rtp_transceiver.cc index fcb54b54c2..d3281d5e6e 100644 --- a/pc/rtp_transceiver.cc +++ b/pc/rtp_transceiver.cc @@ -20,83 +20,6 @@ #include "rtc_base/logging.h" namespace webrtc { -namespace { -template -RTCError VerifyCodecPreferences(const std::vector& codecs, - const std::vector& send_codecs, - const std::vector& recv_codecs) { - // 6. If the intersection between codecs and - // RTCRtpSender.getCapabilities(kind).codecs or the intersection between - // codecs and RTCRtpReceiver.getCapabilities(kind).codecs only contains RTX, - // RED or FEC codecs or is an empty set, throw InvalidModificationError. - // This ensures that we always have something to offer, regardless of - // transceiver.direction. - - if (!absl::c_any_of(codecs, [&recv_codecs](const RtpCodecCapability& codec) { - return codec.name != cricket::kRtxCodecName && - codec.name != cricket::kRedCodecName && - codec.name != cricket::kFlexfecCodecName && - absl::c_any_of(recv_codecs, [&codec](const T& recv_codec) { - return recv_codec.MatchesCapability(codec); - }); - })) { - return RTCError(RTCErrorType::INVALID_MODIFICATION, - "Invalid codec preferences: Missing codec from recv " - "codec capabilities."); - } - - if (!absl::c_any_of(codecs, [&send_codecs](const RtpCodecCapability& codec) { - return codec.name != cricket::kRtxCodecName && - codec.name != cricket::kRedCodecName && - codec.name != cricket::kFlexfecCodecName && - absl::c_any_of(send_codecs, [&codec](const T& send_codec) { - return send_codec.MatchesCapability(codec); - }); - })) { - return RTCError(RTCErrorType::INVALID_MODIFICATION, - "Invalid codec preferences: Missing codec from send " - "codec capabilities."); - } - - // 7. Let codecCapabilities be the union of - // RTCRtpSender.getCapabilities(kind).codecs and - // RTCRtpReceiver.getCapabilities(kind).codecs. 8.1 For each codec in - // codecs, If codec is not in codecCapabilities, throw - // InvalidModificationError. - for (const auto& codec_preference : codecs) { - bool is_recv_codec = - absl::c_any_of(recv_codecs, [&codec_preference](const T& codec) { - return codec.MatchesCapability(codec_preference); - }); - - bool is_send_codec = - absl::c_any_of(send_codecs, [&codec_preference](const T& codec) { - return codec.MatchesCapability(codec_preference); - }); - - if (!is_recv_codec && !is_send_codec) { - return RTCError( - RTCErrorType::INVALID_MODIFICATION, - std::string("Invalid codec preferences: invalid codec with name \"") + - codec_preference.name + "\"."); - } - } - - // Check we have a real codec (not just rtx, red or fec) - if (absl::c_all_of(codecs, [](const RtpCodecCapability& codec) { - return codec.name == cricket::kRtxCodecName || - codec.name == cricket::kRedCodecName || - codec.name == cricket::kUlpfecCodecName; - })) { - return RTCError(RTCErrorType::INVALID_MODIFICATION, - "Invalid codec preferences: codec list must have a non " - "RTX, RED or FEC entry."); - } - - return RTCError::OK(); -} - -} // namespace RtpTransceiver::RtpTransceiver(cricket::MediaType media_type) : unified_plan_(false), media_type_(media_type) { @@ -328,26 +251,111 @@ RTCError RtpTransceiver::SetCodecPreferences( return absl::c_linear_search(codecs, codec); }); - RTCError result; if (media_type_ == cricket::MEDIA_TYPE_AUDIO) { + std::vector audio_codecs; + std::vector recv_codecs, send_codecs; channel_manager_->GetSupportedAudioReceiveCodecs(&recv_codecs); channel_manager_->GetSupportedAudioSendCodecs(&send_codecs); - result = VerifyCodecPreferences(codecs, send_codecs, recv_codecs); + // 6. If the intersection between codecs and + // RTCRtpSender.getCapabilities(kind).codecs or the intersection between + // codecs and RTCRtpReceiver.getCapabilities(kind).codecs only contains RTX, + // RED or FEC codecs or is an empty set, throw InvalidModificationError. + // This ensures that we always have something to offer, regardless of + // transceiver.direction. + + if (!absl::c_any_of( + codecs, [&recv_codecs](const RtpCodecCapability& codec) { + return codec.name != cricket::kRtxCodecName && + codec.name != cricket::kRedCodecName && + codec.name != cricket::kFlexfecCodecName && + absl::c_any_of( + recv_codecs, + [&codec](const cricket::AudioCodec& recv_codec) { + return recv_codec.MatchesCapability(codec); + }); + })) { + return RTCError(RTCErrorType::INVALID_MODIFICATION, + "Invalid codec preferences: Missing codec from recv " + "codec capabilities."); + } + + if (!absl::c_any_of( + codecs, [&send_codecs](const RtpCodecCapability& codec) { + return codec.name != cricket::kRtxCodecName && + codec.name != cricket::kRedCodecName && + codec.name != cricket::kFlexfecCodecName && + absl::c_any_of( + send_codecs, + [&codec](const cricket::AudioCodec& send_codec) { + return send_codec.MatchesCapability(codec); + }); + })) { + return RTCError(RTCErrorType::INVALID_MODIFICATION, + "Invalid codec preferences: Missing codec from send " + "codec capabilities."); + } + + // 7. Let codecCapabilities be the union of + // RTCRtpSender.getCapabilities(kind).codecs and + // RTCRtpReceiver.getCapabilities(kind).codecs. 8.1 For each codec in + // codecs, If codec is not in codecCapabilities, throw + // InvalidModificationError. + for (const auto& codec_preference : codecs) { + bool is_recv_codec = absl::c_any_of( + recv_codecs, [&codec_preference](const cricket::AudioCodec& codec) { + return codec.MatchesCapability(codec_preference); + }); + + bool is_send_codec = absl::c_any_of( + send_codecs, [&codec_preference](const cricket::AudioCodec& codec) { + return codec.MatchesCapability(codec_preference); + }); + + if (!is_recv_codec && !is_send_codec) { + return RTCError( + RTCErrorType::INVALID_MODIFICATION, + std::string( + "Invalid codec preferences: invalid codec with name \"") + + codec_preference.name + "\"."); + } + } } else if (media_type_ == cricket::MEDIA_TYPE_VIDEO) { - std::vector recv_codecs, send_codecs; - channel_manager_->GetSupportedVideoReceiveCodecs(&recv_codecs); - channel_manager_->GetSupportedVideoSendCodecs(&send_codecs); + std::vector video_codecs; + // Video codecs are both for the receive and send side, so the checks are + // simpler than the audio ones. + channel_manager_->GetSupportedVideoCodecs(&video_codecs); - result = VerifyCodecPreferences(codecs, send_codecs, recv_codecs); + // Validate codecs + for (const auto& codec_preference : codecs) { + if (!absl::c_any_of(video_codecs, [&codec_preference]( + const cricket::VideoCodec& codec) { + return codec.MatchesCapability(codec_preference); + })) { + return RTCError( + RTCErrorType::INVALID_MODIFICATION, + std::string( + "Invalid codec preferences: invalid codec with name \"") + + codec_preference.name + "\"."); + } + } } - if (result.ok()) { - codec_preferences_ = codecs; + // Check we have a real codec (not just rtx, red or fec) + if (absl::c_all_of(codecs, [](const RtpCodecCapability& codec) { + return codec.name == cricket::kRtxCodecName || + codec.name == cricket::kRedCodecName || + codec.name == cricket::kUlpfecCodecName; + })) { + return RTCError(RTCErrorType::INVALID_MODIFICATION, + "Invalid codec preferences: codec list must have a non " + "RTX, RED or FEC entry."); } - return result; + codec_preferences_ = codecs; + + return RTCError::OK(); } } // namespace webrtc