From c0f25cf762a6946666c812f7a3df3f0a7f98b38d Mon Sep 17 00:00:00 2001 From: Johannes Kron Date: Thu, 16 Jan 2020 15:00:56 +0100 Subject: [PATCH] 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} --- .../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 | 23 ++- media/engine/webrtc_video_engine.h | 3 +- media/engine/webrtc_video_engine_unittest.cc | 164 +++++++++------- 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_media_unittest.cc | 10 +- pc/rtp_transceiver.cc | 180 +++++++++--------- 19 files changed, 474 insertions(+), 284 deletions(-) diff --git a/api/test/video/function_video_decoder_factory.h b/api/test/video/function_video_decoder_factory.h index 03a4323997..23214ccf40 100644 --- a/api/test/video/function_video_decoder_factory.h +++ b/api/test/video/function_video_decoder_factory.h @@ -33,11 +33,14 @@ 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 { - RTC_NOTREACHED(); - return {}; + return sdp_video_formats_; } std::unique_ptr CreateVideoDecoder( @@ -48,6 +51,7 @@ 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 c31ef97786..4a6971adf1 100644 --- a/media/base/fake_media_engine.cc +++ b/media/base/fake_media_engine.cc @@ -567,7 +567,8 @@ 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. - codecs_.push_back(VideoCodec(0, "fake_video_codec")); + send_codecs_.push_back(VideoCodec(0, "fake_video_codec")); + recv_codecs_.push_back(VideoCodec(0, "fake_video_codec")); } RtpCapabilities FakeVideoEngine::GetCapabilities() const { return RtpCapabilities(); @@ -598,12 +599,22 @@ void FakeVideoEngine::UnregisterChannel(VideoMediaChannel* channel) { RTC_DCHECK(it != channels_.end()); channels_.erase(it); } -std::vector FakeVideoEngine::codecs() const { - return codecs_; +std::vector FakeVideoEngine::send_codecs() const { + return send_codecs_; } -void FakeVideoEngine::SetCodecs(const std::vector codecs) { - codecs_ = codecs; + +std::vector FakeVideoEngine::recv_codecs() const { + return recv_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; @@ -627,7 +638,8 @@ void FakeMediaEngine::SetAudioSendCodecs( voice_->SetSendCodecs(codecs); } void FakeMediaEngine::SetVideoCodecs(const std::vector& codecs) { - video_->SetCodecs(codecs); + video_->SetSendCodecs(codecs); + video_->SetRecvCodecs(codecs); } FakeVoiceMediaChannel* FakeMediaEngine::GetVoiceChannel(size_t index) { diff --git a/media/base/fake_media_engine.h b/media/base/fake_media_engine.h index 3df8f85965..f072dc4550 100644 --- a/media/base/fake_media_engine.h +++ b/media/base/fake_media_engine.h @@ -559,13 +559,16 @@ class FakeVideoEngine : public VideoEngineInterface { override; FakeVideoMediaChannel* GetChannel(size_t index); void UnregisterChannel(VideoMediaChannel* channel); - std::vector codecs() const override; - void SetCodecs(const std::vector codecs); + std::vector send_codecs() const override; + std::vector recv_codecs() const override; + void SetSendCodecs(const std::vector& codecs); + void SetRecvCodecs(const std::vector& codecs); bool SetCapture(bool capture); private: std::vector channels_; - std::vector codecs_; + std::vector send_codecs_; + std::vector recv_codecs_; bool capture_; VideoOptions options_; bool fail_create_channel_; diff --git a/media/base/media_engine.h b/media/base/media_engine.h index 173df50e34..841b2b6b0c 100644 --- a/media/base/media_engine.h +++ b/media/base/media_engine.h @@ -99,7 +99,9 @@ class VideoEngineInterface { webrtc::VideoBitrateAllocatorFactory* video_bitrate_allocator_factory) = 0; - virtual std::vector codecs() const = 0; + virtual std::vector send_codecs() const = 0; + virtual std::vector recv_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 0ee2bcc54f..91f7e53956 100644 --- a/media/engine/fake_webrtc_video_engine.cc +++ b/media/engine/fake_webrtc_video_engine.cc @@ -113,8 +113,11 @@ void FakeWebRtcVideoDecoderFactory::DecoderDestroyed( } void FakeWebRtcVideoDecoderFactory::AddSupportedVideoCodecType( - const webrtc::SdpVideoFormat& format) { - supported_codec_formats_.push_back(format); + 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)); } int FakeWebRtcVideoDecoderFactory::GetNumCreatedDecoders() { diff --git a/media/engine/fake_webrtc_video_engine.h b/media/engine/fake_webrtc_video_engine.h index 7b32ac86cf..28dc4fe99b 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 webrtc::SdpVideoFormat& format); + void AddSupportedVideoCodecType(const std::string& name); 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 590f0b0be7..5c31e21ef1 100644 --- a/media/engine/null_webrtc_video_engine.h +++ b/media/engine/null_webrtc_video_engine.h @@ -30,7 +30,11 @@ class VideoMediaChannel; // CompositeMediaEngine. class NullWebRtcVideoEngine : public VideoEngineInterface { public: - std::vector codecs() const override { + std::vector send_codecs() const override { + return std::vector(); + } + + std::vector recv_codecs() const override { return std::vector(); } diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index a2944d5a9a..75cee32c69 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; } -std::vector AssignPayloadTypesAndDefaultCodecs( - const webrtc::VideoEncoderFactory* encoder_factory) { - return encoder_factory ? AssignPayloadTypesAndDefaultCodecs( - encoder_factory->GetSupportedFormats()) - : std::vector(); +template +std::vector GetPayloadTypesAndDefaultCodecs(const T factory) { + return factory ? AssignPayloadTypesAndDefaultCodecs( + factory->GetSupportedFormats()) + : std::vector(); } bool IsTemporalLayersSupported(const std::string& codec_name) { @@ -476,8 +476,12 @@ VideoMediaChannel* WebRtcVideoEngine::CreateMediaChannel( encoder_factory_.get(), decoder_factory_.get(), video_bitrate_allocator_factory); } -std::vector WebRtcVideoEngine::codecs() const { - return AssignPayloadTypesAndDefaultCodecs(encoder_factory_.get()); +std::vector WebRtcVideoEngine::send_codecs() const { + return GetPayloadTypesAndDefaultCodecs(encoder_factory_.get()); +} + +std::vector WebRtcVideoEngine::recv_codecs() const { + return GetPayloadTypesAndDefaultCodecs(decoder_factory_.get()); } RtpCapabilities WebRtcVideoEngine::GetCapabilities() const { @@ -547,8 +551,7 @@ WebRtcVideoChannel::WebRtcVideoChannel( rtcp_receiver_report_ssrc_ = kDefaultRtcpReceiverReportSsrc; sending_ = false; - recv_codecs_ = - MapCodecs(AssignPayloadTypesAndDefaultCodecs(encoder_factory_)); + recv_codecs_ = MapCodecs(GetPayloadTypesAndDefaultCodecs(decoder_factory_)); recv_flexfec_payload_type_ = recv_codecs_.front().flexfec_payload_type; } @@ -976,7 +979,7 @@ bool WebRtcVideoChannel::GetChangedRecvParameters( // Verify that every mapped codec is supported locally. const std::vector local_supported_codecs = - AssignPayloadTypesAndDefaultCodecs(encoder_factory_); + GetPayloadTypesAndDefaultCodecs(decoder_factory_); for (const VideoCodecSettings& mapped_codec : mapped_codecs) { if (!FindMatchingCodec(local_supported_codecs, mapped_codec.codec)) { RTC_LOG(LS_ERROR) diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index 9e0ede95a7..482006eb8b 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -97,7 +97,8 @@ class WebRtcVideoEngine : public VideoEngineInterface { webrtc::VideoBitrateAllocatorFactory* video_bitrate_allocator_factory) override; - std::vector codecs() const override; + std::vector send_codecs() const override; + std::vector recv_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 975761123a..a233a30f8b 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_.codecs(); + std::vector engine_codecs = engine_.send_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; - encoder_factory_->AddSupportedVideoCodecType("VP8"); + AddSupportedVideoCodecType("VP8"); std::unique_ptr channel( SetSendParamsWithAllSupportedCodecs()); @@ -414,7 +414,7 @@ TEST_F(WebRtcVideoEngineTest, CVOSetHeaderExtensionBeforeAddSendStream) { // dtor is called. ::testing::NiceMock video_source; - encoder_factory_->AddSupportedVideoCodecType("VP8"); + AddSupportedVideoCodecType("VP8"); std::unique_ptr channel( SetSendParamsWithAllSupportedCodecs()); @@ -438,8 +438,8 @@ TEST_F(WebRtcVideoEngineTest, CVOSetHeaderExtensionBeforeAddSendStream) { TEST_F(WebRtcVideoEngineTest, CVOSetHeaderExtensionAfterCapturer) { ::testing::NiceMock video_source; - encoder_factory_->AddSupportedVideoCodecType("VP8"); - encoder_factory_->AddSupportedVideoCodecType("VP9"); + AddSupportedVideoCodecType("VP8"); + AddSupportedVideoCodecType("VP9"); std::unique_ptr channel( SetSendParamsWithAllSupportedCodecs()); @@ -483,7 +483,7 @@ TEST_F(WebRtcVideoEngineTest, CVOSetHeaderExtensionAfterCapturer) { } TEST_F(WebRtcVideoEngineTest, SetSendFailsBeforeSettingCodecs) { - encoder_factory_->AddSupportedVideoCodecType("VP8"); + 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) { - encoder_factory_->AddSupportedVideoCodecType("VP8"); + 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) { - encoder_factory_->AddSupportedVideoCodecType("VP8"); + 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_.codecs(); + const std::vector codecs = engine_.send_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) { - encoder_factory_->AddSupportedVideoCodecType("VP9"); + AddSupportedVideoCodecType("VP9"); std::unique_ptr channel( SetSendParamsWithAllSupportedCodecs()); @@ -597,7 +597,7 @@ TEST_F(WebRtcVideoEngineTest, CanConstructDecoderForVp9EncoderFactory) { #endif // defined(RTC_ENABLE_VP9) TEST_F(WebRtcVideoEngineTest, PropagatesInputFrameTimestamp) { - encoder_factory_->AddSupportedVideoCodecType("VP8"); + 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_.codecs(); + std::vector engine_codecs = engine_.send_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_.codecs(); + std::vector engine_codecs = engine_.send_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_.codecs(); + const std::vector codecs = engine_.send_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,7 +705,13 @@ size_t WebRtcVideoEngineTest::GetEngineCodecIndex( cricket::VideoCodec WebRtcVideoEngineTest::GetEngineCodec( const std::string& name) const { - return engine_.codecs()[GetEngineCodecIndex(name)]; + return engine_.send_codecs()[GetEngineCodecIndex(name)]; +} + +void WebRtcVideoEngineTest::AddSupportedVideoCodecType( + const std::string& name) { + encoder_factory_->AddSupportedVideoCodecType(name); + decoder_factory_->AddSupportedVideoCodecType(name); } VideoMediaChannel* @@ -754,7 +760,7 @@ void WebRtcVideoEngineTest::ExpectRtpCapabilitySupport(const char* uri, } TEST_F(WebRtcVideoEngineTest, UsesSimulcastAdapterForVp8Factories) { - encoder_factory_->AddSupportedVideoCodecType("VP8"); + AddSupportedVideoCodecType("VP8"); std::unique_ptr channel( SetSendParamsWithAllSupportedCodecs()); @@ -791,8 +797,8 @@ TEST_F(WebRtcVideoEngineTest, UsesSimulcastAdapterForVp8Factories) { } TEST_F(WebRtcVideoEngineTest, ChannelWithH264CanChangeToVp8) { - encoder_factory_->AddSupportedVideoCodecType("VP8"); - encoder_factory_->AddSupportedVideoCodecType("H264"); + AddSupportedVideoCodecType("VP8"); + AddSupportedVideoCodecType("H264"); // Frame source. webrtc::test::FrameForwarder frame_forwarder; @@ -826,8 +832,8 @@ TEST_F(WebRtcVideoEngineTest, ChannelWithH264CanChangeToVp8) { TEST_F(WebRtcVideoEngineTest, UsesSimulcastAdapterForVp8WithCombinedVP8AndH264Factory) { - encoder_factory_->AddSupportedVideoCodecType("VP8"); - encoder_factory_->AddSupportedVideoCodecType("H264"); + AddSupportedVideoCodecType("VP8"); + AddSupportedVideoCodecType("H264"); std::unique_ptr channel(engine_.CreateMediaChannel( call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), @@ -862,8 +868,8 @@ TEST_F(WebRtcVideoEngineTest, TEST_F(WebRtcVideoEngineTest, DestroysNonSimulcastEncoderFromCombinedVP8AndH264Factory) { - encoder_factory_->AddSupportedVideoCodecType("VP8"); - encoder_factory_->AddSupportedVideoCodecType("H264"); + AddSupportedVideoCodecType("VP8"); + AddSupportedVideoCodecType("H264"); std::unique_ptr channel(engine_.CreateMediaChannel( call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), @@ -896,7 +902,7 @@ TEST_F(WebRtcVideoEngineTest, SimulcastEnabledForH264BehindFieldTrial) { RTC_DCHECK(!override_field_trials_); override_field_trials_ = std::make_unique( "WebRTC-H264Simulcast/Enabled/"); - encoder_factory_->AddSupportedVideoCodecType("H264"); + AddSupportedVideoCodecType("H264"); std::unique_ptr channel(engine_.CreateMediaChannel( call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(), @@ -936,13 +942,13 @@ TEST_F(WebRtcVideoEngineTest, auto flexfec = Field("name", &VideoCodec::name, "flexfec-03"); // FlexFEC is not active without field trial. - EXPECT_THAT(engine_.codecs(), Not(Contains(flexfec))); + EXPECT_THAT(engine_.send_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_.codecs(), Contains(flexfec)); + EXPECT_THAT(engine_.send_codecs(), Contains(flexfec)); } // Test that codecs are added in the order they are reported from the factory. @@ -966,11 +972,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_.codecs()); + std::vector codecs_before(engine_.send_codecs()); // Add second codec. encoder_factory_->AddSupportedVideoCodecType(kFakeExternalCodecName2); - std::vector codecs_after(engine_.codecs()); + std::vector codecs_after(engine_.send_codecs()); // The codec itself and RTX should have been added. EXPECT_EQ(codecs_before.size() + 2, codecs_after.size()); @@ -986,12 +992,11 @@ TEST_F(WebRtcVideoEngineTest, ReportRtxForExternalCodec) { encoder_factory_->AddSupportedVideoCodecType(kFakeCodecName); const size_t fake_codec_index = GetEngineCodecIndex(kFakeCodecName); - EXPECT_EQ("rtx", engine_.codecs().at(fake_codec_index + 1).name); + EXPECT_EQ("rtx", engine_.send_codecs().at(fake_codec_index + 1).name); } TEST_F(WebRtcVideoEngineTest, RegisterDecodersIfSupported) { - encoder_factory_->AddSupportedVideoCodecType("VP8"); - decoder_factory_->AddSupportedVideoCodecType(webrtc::SdpVideoFormat("VP8")); + AddSupportedVideoCodecType("VP8"); cricket::VideoRecvParameters parameters; parameters.codecs.push_back(GetEngineCodec("VP8")); @@ -1017,10 +1022,7 @@ 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. - encoder_factory_->AddSupportedVideoCodecType("H264"); - webrtc::SdpVideoFormat supported_h264("H264"); - supported_h264.parameters[kH264FmtpPacketizationMode] = "1"; - decoder_factory_->AddSupportedVideoCodecType(supported_h264); + AddSupportedVideoCodecType("H264"); std::vector codecs; codecs.push_back(GetEngineCodec("H264")); @@ -1036,8 +1038,7 @@ TEST_F(WebRtcVideoEngineTest, RegisterH264DecoderIfSupported) { // empty list of RtpSource without crashing. TEST_F(WebRtcVideoEngineTest, GetSourcesWithNonExistingSsrc) { // Setup an recv stream with |kSsrc|. - encoder_factory_->AddSupportedVideoCodecType("VP8"); - decoder_factory_->AddSupportedVideoCodecType(webrtc::SdpVideoFormat("VP8")); + AddSupportedVideoCodecType("VP8"); cricket::VideoRecvParameters parameters; parameters.codecs.push_back(GetEngineCodec("VP8")); std::unique_ptr channel( @@ -1056,7 +1057,8 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, NullFactories) { std::unique_ptr decoder_factory; WebRtcVideoEngine engine(std::move(encoder_factory), std::move(decoder_factory)); - EXPECT_EQ(0u, engine.codecs().size()); + EXPECT_EQ(0u, engine.send_codecs().size()); + EXPECT_EQ(0u, engine.recv_codecs().size()); } TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, EmptyFactories) { @@ -1068,8 +1070,11 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, EmptyFactories) { WebRtcVideoEngine engine( (std::unique_ptr(encoder_factory)), (std::unique_ptr(decoder_factory))); - EXPECT_CALL(*encoder_factory, GetSupportedFormats()); - EXPECT_EQ(0u, engine.codecs().size()); + // 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, Die()); EXPECT_CALL(*decoder_factory, Die()); } @@ -1098,9 +1103,11 @@ 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.codecs(); + const std::vector engine_codecs = engine.send_codecs(); // Verify default codecs has been added correctly. EXPECT_EQ(5u, engine_codecs.size()); EXPECT_EQ("VP8", engine_codecs.at(0).name); @@ -1233,12 +1240,14 @@ 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.codecs().front()); + recv_parameters.codecs.push_back(engine.recv_codecs().front()); EXPECT_TRUE(recv_channel->SetRecvParameters(recv_parameters)); EXPECT_TRUE(recv_channel->AddRecvStream( cricket::StreamParams::CreateLegacy(recv_ssrc))); @@ -1326,9 +1335,9 @@ class WebRtcVideoChannelEncodedFrameCallbackTest : public ::testing::Test { webrtc::CreateBuiltinVideoBitrateAllocatorFactory()), engine_( webrtc::CreateBuiltinVideoEncoderFactory(), - std::make_unique([]() { - return std::make_unique(); - })), + std::make_unique( + []() { return std::make_unique(); }, + kSdpVideoFormats)), channel_(absl::WrapUnique(static_cast( engine_.CreateMediaChannel( call_.get(), @@ -1339,7 +1348,7 @@ class WebRtcVideoChannelEncodedFrameCallbackTest : public ::testing::Test { network_interface_.SetDestination(channel_.get()); channel_->SetInterface(&network_interface_, webrtc::MediaTransportConfig()); cricket::VideoRecvParameters parameters; - parameters.codecs = engine_.codecs(); + parameters.codecs = engine_.recv_codecs(); channel_->SetRecvParameters(parameters); } @@ -1363,6 +1372,7 @@ 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_; @@ -1375,6 +1385,10 @@ class WebRtcVideoChannelEncodedFrameCallbackTest : public ::testing::Test { cricket::FakeVideoRenderer renderer_; }; +const std::vector + WebRtcVideoChannelEncodedFrameCallbackTest::kSdpVideoFormats = { + webrtc::SdpVideoFormat("VP8")}; + TEST_F(WebRtcVideoChannelEncodedFrameCallbackTest, SetEncodedFrameBufferFunction_DefaultStream) { testing::MockFunction callback; @@ -1480,7 +1494,7 @@ class WebRtcVideoChannelBaseTest : public ::testing::Test { network_interface_.SetDestination(channel_.get()); channel_->SetInterface(&network_interface_, webrtc::MediaTransportConfig()); cricket::VideoRecvParameters parameters; - parameters.codecs = engine_.codecs(); + parameters.codecs = engine_.send_codecs(); channel_->SetRecvParameters(parameters); EXPECT_TRUE(channel_->AddSendStream(DefaultSendStreamParams())); frame_forwarder_ = std::make_unique(); @@ -1628,7 +1642,7 @@ class WebRtcVideoChannelBaseTest : public ::testing::Test { } cricket::VideoCodec GetEngineCodec(const std::string& name) { - for (const cricket::VideoCodec& engine_codec : engine_.codecs()) { + for (const cricket::VideoCodec& engine_codec : engine_.send_codecs()) { if (absl::EqualsIgnoreCase(name, engine_codec.name)) return engine_codec; } @@ -2405,10 +2419,10 @@ class WebRtcVideoChannelTest : public WebRtcVideoEngineTest { frame_source_(1280, 720, rtc::kNumMicrosecsPerSec / 30), last_ssrc_(0) {} void SetUp() override { - encoder_factory_->AddSupportedVideoCodecType("VP8"); - encoder_factory_->AddSupportedVideoCodecType("VP9"); + AddSupportedVideoCodecType("VP8"); + AddSupportedVideoCodecType("VP9"); #if defined(WEBRTC_USE_H264) - encoder_factory_->AddSupportedVideoCodecType("H264"); + AddSupportedVideoCodecType("H264"); #endif fake_call_.reset(new FakeCall()); @@ -2417,8 +2431,8 @@ class WebRtcVideoChannelTest : public WebRtcVideoEngineTest { webrtc::CryptoOptions(), video_bitrate_allocator_factory_.get())); channel_->OnReadyToSend(true); last_ssrc_ = 123; - send_parameters_.codecs = engine_.codecs(); - recv_parameters_.codecs = engine_.codecs(); + send_parameters_.codecs = engine_.send_codecs(); + recv_parameters_.codecs = engine_.recv_codecs(); ASSERT_TRUE(channel_->SetSendParameters(send_parameters_)); } @@ -2590,7 +2604,7 @@ class WebRtcVideoChannelTest : public WebRtcVideoEngineTest { VerifyCodecHasDefaultFeedbackParams(default_codec_, expect_lntf_enabled); cricket::VideoSendParameters parameters; - parameters.codecs = engine_.codecs(); + parameters.codecs = engine_.send_codecs(); EXPECT_TRUE(channel_->SetSendParameters(parameters)); EXPECT_TRUE(channel_->SetSend(true)); @@ -2735,7 +2749,7 @@ TEST_F(WebRtcVideoChannelTest, SetsSyncGroupFromSyncLabel) { TEST_F(WebRtcVideoChannelTest, RecvStreamWithSimAndRtx) { cricket::VideoSendParameters parameters; - parameters.codecs = engine_.codecs(); + parameters.codecs = engine_.send_codecs(); EXPECT_TRUE(channel_->SetSendParameters(parameters)); EXPECT_TRUE(channel_->SetSend(true)); parameters.conference_mode = true; @@ -3048,7 +3062,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_.codecs(); + parameters.codecs = engine_.send_codecs(); EXPECT_TRUE(channel_->SetSendParameters(parameters)); stream = fake_call_->GetVideoReceiveStreams()[0]; EXPECT_TRUE(stream->GetConfig().rtp.transport_cc); @@ -3077,7 +3091,7 @@ TEST_F(WebRtcVideoChannelTest, LossNotificationCanBeEnabledAndDisabled) { { cricket::VideoSendParameters parameters; - parameters.codecs = engine_.codecs(); + parameters.codecs = engine_.send_codecs(); EXPECT_TRUE(channel_->SetSendParameters(parameters)); EXPECT_TRUE(channel_->SetSend(true)); } @@ -3101,7 +3115,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_.codecs(); + parameters.codecs = engine_.send_codecs(); EXPECT_TRUE(channel_->SetSendParameters(parameters)); recv_stream = fake_call_->GetVideoReceiveStreams()[0]; EXPECT_TRUE(recv_stream->GetConfig().rtp.lntf.enabled); @@ -3114,7 +3128,7 @@ TEST_F(WebRtcVideoChannelTest, NackIsEnabledByDefault) { VerifyCodecHasDefaultFeedbackParams(default_codec_, false); cricket::VideoSendParameters parameters; - parameters.codecs = engine_.codecs(); + parameters.codecs = engine_.send_codecs(); EXPECT_TRUE(channel_->SetSendParameters(parameters)); EXPECT_TRUE(channel_->SetSend(true)); @@ -3152,7 +3166,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_.codecs(); + parameters.codecs = engine_.send_codecs(); EXPECT_TRUE(channel_->SetSendParameters(parameters)); recv_stream = fake_call_->GetVideoReceiveStreams()[0]; EXPECT_GT(recv_stream->GetConfig().rtp.nack.rtp_history_ms, 0); @@ -3890,7 +3904,7 @@ TEST_F(WebRtcVideoChannelTest, SetDefaultSendCodecs) { VideoCodec codec; EXPECT_TRUE(channel_->GetSendCodec(&codec)); - EXPECT_TRUE(codec.Matches(engine_.codecs()[0])); + EXPECT_TRUE(codec.Matches(engine_.send_codecs()[0])); // Using a RTX setup to verify that the default RTX payload type is good. const std::vector ssrcs = MAKE_VECTOR(kSsrcs1); @@ -4238,7 +4252,7 @@ TEST_F(WebRtcVideoChannelFlexfecRecvTest, TEST_F(WebRtcVideoChannelTest, SetSendCodecRejectsRtxWithoutAssociatedPayloadType) { const int kUnusedPayloadType = 127; - EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType)); + EXPECT_FALSE(FindCodecById(engine_.send_codecs(), kUnusedPayloadType)); cricket::VideoSendParameters parameters; cricket::VideoCodec rtx_codec(kUnusedPayloadType, "rtx"); @@ -4251,8 +4265,8 @@ TEST_F(WebRtcVideoChannelTest, SetSendCodecRejectsRtxWithoutMatchingVideoCodec) { const int kUnusedPayloadType1 = 126; const int kUnusedPayloadType2 = 127; - EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType1)); - EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType2)); + EXPECT_FALSE(FindCodecById(engine_.send_codecs(), kUnusedPayloadType1)); + EXPECT_FALSE(FindCodecById(engine_.send_codecs(), kUnusedPayloadType2)); { cricket::VideoCodec rtx_codec = cricket::VideoCodec::CreateRtxCodec( kUnusedPayloadType1, GetEngineCodec("VP8").id); @@ -4275,8 +4289,8 @@ TEST_F(WebRtcVideoChannelTest, TEST_F(WebRtcVideoChannelTest, SetSendCodecsWithChangedRtxPayloadType) { const int kUnusedPayloadType1 = 126; const int kUnusedPayloadType2 = 127; - EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType1)); - EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType2)); + EXPECT_FALSE(FindCodecById(engine_.send_codecs(), kUnusedPayloadType1)); + EXPECT_FALSE(FindCodecById(engine_.send_codecs(), kUnusedPayloadType2)); // SSRCs for RTX. cricket::StreamParams params = @@ -4677,8 +4691,8 @@ TEST_F(WebRtcVideoChannelTest, SetRecvCodecsWithOnlyVp8) { TEST_F(WebRtcVideoChannelTest, SetRecvCodecsWithRtx) { const int kUnusedPayloadType1 = 126; const int kUnusedPayloadType2 = 127; - EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType1)); - EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType2)); + EXPECT_FALSE(FindCodecById(engine_.recv_codecs(), kUnusedPayloadType1)); + EXPECT_FALSE(FindCodecById(engine_.recv_codecs(), kUnusedPayloadType2)); cricket::VideoRecvParameters parameters; parameters.codecs.push_back(GetEngineCodec("VP8")); @@ -4776,8 +4790,8 @@ TEST_F(WebRtcVideoChannelTest, DuplicateRedCodecIsDropped) { TEST_F(WebRtcVideoChannelTest, SetRecvCodecsWithChangedRtxPayloadType) { const int kUnusedPayloadType1 = 126; const int kUnusedPayloadType2 = 127; - EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType1)); - EXPECT_FALSE(FindCodecById(engine_.codecs(), kUnusedPayloadType2)); + EXPECT_FALSE(FindCodecById(engine_.recv_codecs(), kUnusedPayloadType1)); + EXPECT_FALSE(FindCodecById(engine_.recv_codecs(), kUnusedPayloadType2)); // SSRCs for RTX. cricket::StreamParams params = @@ -4825,13 +4839,14 @@ TEST_F(WebRtcVideoChannelTest, SetRecvCodecsDifferentPayloadType) { TEST_F(WebRtcVideoChannelTest, SetRecvCodecsAcceptDefaultCodecs) { cricket::VideoRecvParameters parameters; - parameters.codecs = engine_.codecs(); + parameters.codecs = engine_.recv_codecs(); EXPECT_TRUE(channel_->SetRecvParameters(parameters)); FakeVideoReceiveStream* stream = AddRecvStream(); const webrtc::VideoReceiveStream::Config& config = stream->GetConfig(); - EXPECT_EQ(engine_.codecs()[0].name, config.decoders[0].video_format.name); - EXPECT_EQ(engine_.codecs()[0].id, config.decoders[0].payload_type); + 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); } TEST_F(WebRtcVideoChannelTest, SetRecvCodecsRejectUnsupportedCodec) { @@ -5721,7 +5736,7 @@ void WebRtcVideoChannelTest::TestReceiveUnsignaledSsrcPacket( uint8_t payload_type, bool expect_created_receive_stream) { // kRedRtxPayloadType must currently be unused. - EXPECT_FALSE(FindCodecById(engine_.codecs(), kRedRtxPayloadType)); + EXPECT_FALSE(FindCodecById(engine_.recv_codecs(), kRedRtxPayloadType)); // Add a RED RTX codec. VideoCodec red_rtx_codec = @@ -7578,6 +7593,7 @@ 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_manager.cc b/pc/channel_manager.cc index ce8f473600..16814bd493 100644 --- a/pc/channel_manager.cc +++ b/pc/channel_manager.cc @@ -87,14 +87,31 @@ void ChannelManager::GetSupportedAudioRtpHeaderExtensions( *ext = media_engine_->voice().GetCapabilities().header_extensions; } -void ChannelManager::GetSupportedVideoCodecs( +void ChannelManager::GetSupportedVideoSendCodecs( std::vector* codecs) const { if (!media_engine_) { return; } codecs->clear(); - std::vector video_codecs = media_engine_->video().codecs(); + 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(); 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 661ab4bbde..f66ad4bfc1 100644 --- a/pc/channel_manager.h +++ b/pc/channel_manager.h @@ -76,7 +76,8 @@ class ChannelManager final { void GetSupportedAudioSendCodecs(std::vector* codecs) const; void GetSupportedAudioReceiveCodecs(std::vector* codecs) const; void GetSupportedAudioRtpHeaderExtensions(RtpHeaderExtensions* ext) const; - void GetSupportedVideoCodecs(std::vector* codecs) const; + void GetSupportedVideoSendCodecs(std::vector* codecs) const; + void GetSupportedVideoReceiveCodecs(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 90785131f9..6f3128ebde 100644 --- a/pc/channel_manager_unittest.cc +++ b/pc/channel_manager_unittest.cc @@ -142,22 +142,29 @@ TEST_F(ChannelManagerTest, StartupShutdownOnThread) { } TEST_F(ChannelManagerTest, SetVideoRtxEnabled) { - std::vector codecs; + std::vector send_codecs; + std::vector recv_codecs; const VideoCodec rtx_codec(96, "rtx"); // By default RTX is disabled. - cm_->GetSupportedVideoCodecs(&codecs); - EXPECT_FALSE(ContainsMatchingCodec(codecs, rtx_codec)); + cm_->GetSupportedVideoSendCodecs(&send_codecs); + EXPECT_FALSE(ContainsMatchingCodec(send_codecs, rtx_codec)); + cm_->GetSupportedVideoSendCodecs(&recv_codecs); + EXPECT_FALSE(ContainsMatchingCodec(recv_codecs, rtx_codec)); // Enable and check. EXPECT_TRUE(cm_->SetVideoRtxEnabled(true)); - cm_->GetSupportedVideoCodecs(&codecs); - EXPECT_TRUE(ContainsMatchingCodec(codecs, rtx_codec)); + cm_->GetSupportedVideoSendCodecs(&send_codecs); + EXPECT_TRUE(ContainsMatchingCodec(send_codecs, rtx_codec)); + cm_->GetSupportedVideoSendCodecs(&recv_codecs); + EXPECT_TRUE(ContainsMatchingCodec(recv_codecs, rtx_codec)); // Disable and check. EXPECT_TRUE(cm_->SetVideoRtxEnabled(false)); - cm_->GetSupportedVideoCodecs(&codecs); - EXPECT_FALSE(ContainsMatchingCodec(codecs, rtx_codec)); + cm_->GetSupportedVideoSendCodecs(&send_codecs); + EXPECT_FALSE(ContainsMatchingCodec(send_codecs, rtx_codec)); + cm_->GetSupportedVideoSendCodecs(&recv_codecs); + EXPECT_FALSE(ContainsMatchingCodec(recv_codecs, rtx_codec)); // Cannot toggle rtx after initialization. EXPECT_TRUE(cm_->Init()); @@ -167,8 +174,10 @@ TEST_F(ChannelManagerTest, SetVideoRtxEnabled) { // Can set again after terminate. cm_->Terminate(); EXPECT_TRUE(cm_->SetVideoRtxEnabled(true)); - cm_->GetSupportedVideoCodecs(&codecs); - EXPECT_TRUE(ContainsMatchingCodec(codecs, rtx_codec)); + cm_->GetSupportedVideoSendCodecs(&send_codecs); + EXPECT_TRUE(ContainsMatchingCodec(send_codecs, rtx_codec)); + cm_->GetSupportedVideoSendCodecs(&recv_codecs); + EXPECT_TRUE(ContainsMatchingCodec(recv_codecs, rtx_codec)); } TEST_F(ChannelManagerTest, CreateDestroyChannels) { diff --git a/pc/media_session.cc b/pc/media_session.cc index 59f140f951..e764101eef 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -1330,10 +1330,12 @@ MediaSessionDescriptionFactory::MediaSessionDescriptionFactory( channel_manager->GetSupportedAudioSendCodecs(&audio_send_codecs_); channel_manager->GetSupportedAudioReceiveCodecs(&audio_recv_codecs_); channel_manager->GetSupportedAudioRtpHeaderExtensions(&audio_rtp_extensions_); - channel_manager->GetSupportedVideoCodecs(&video_codecs_); + channel_manager->GetSupportedVideoSendCodecs(&video_send_codecs_); + channel_manager->GetSupportedVideoReceiveCodecs(&video_recv_codecs_); channel_manager->GetSupportedVideoRtpHeaderExtensions(&video_rtp_extensions_); channel_manager->GetSupportedDataCodecs(&rtp_data_codecs_); ComputeAudioCodecsIntersectionAndUnion(); + ComputeVideoCodecsIntersectionAndUnion(); } const AudioCodecs& MediaSessionDescriptionFactory::audio_sendrecv_codecs() @@ -1357,6 +1359,27 @@ 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); @@ -1737,6 +1760,41 @@ 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, @@ -1784,7 +1842,7 @@ void MediaSessionDescriptionFactory::GetCodecsForOffer( // Add our codecs that are not in the current description. MergeCodecs(all_audio_codecs_, audio_codecs, &used_pltypes); - MergeCodecs(video_codecs_, video_codecs, &used_pltypes); + MergeCodecs(all_video_codecs_, video_codecs, &used_pltypes); MergeCodecs(rtp_data_codecs_, rtp_data_codecs, &used_pltypes); } @@ -1832,7 +1890,7 @@ void MediaSessionDescriptionFactory::GetCodecsForAnswer( if (!FindMatchingCodec(video->codecs(), filtered_offered_video_codecs, offered_video_codec, nullptr) && - FindMatchingCodec(video->codecs(), video_codecs_, + FindMatchingCodec(video->codecs(), all_video_codecs_, offered_video_codec, nullptr)) { filtered_offered_video_codecs.push_back(offered_video_codec); } @@ -2039,7 +2097,7 @@ bool MediaSessionDescriptionFactory::AddAudioContentForOffer( IsDtlsActive(current_content, current_description) ? cricket::SEC_DISABLED : secure(); - std::unique_ptr audio(new AudioContentDescription()); + auto audio = std::make_unique(); std::vector crypto_suites; GetSupportedAudioSdesCryptoSuiteNames(session_options.crypto_options, &crypto_suites); @@ -2067,6 +2125,8 @@ 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, @@ -2077,14 +2137,10 @@ bool MediaSessionDescriptionFactory::AddVideoContentForOffer( StreamParamsVec* current_streams, SessionDescription* desc, IceCredentialsIterator* ice_credentials) const { - 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); + // 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); VideoCodecs filtered_codecs; @@ -2092,7 +2148,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, video_codecs_); + media_description_options.codec_preferences, supported_video_codecs); } else { // Add the codecs from current content if it exists and is not rejected nor // recycled. @@ -2110,11 +2166,11 @@ bool MediaSessionDescriptionFactory::AddVideoContentForOffer( } // Add other supported video codecs. VideoCodec found_codec; - for (const VideoCodec& codec : video_codecs_) { - if (FindMatchingCodec(video_codecs_, video_codecs, codec, - &found_codec) && - !FindMatchingCodec(video_codecs_, filtered_codecs, codec, - nullptr)) { + 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)) { // Use the |found_codec| from |video_codecs| because it has the // correctly mapped payload type. filtered_codecs.push_back(found_codec); @@ -2130,6 +2186,13 @@ 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, @@ -2152,6 +2215,7 @@ bool MediaSessionDescriptionFactory::AddVideoContentForOffer( current_description, desc, ice_credentials)) { return false; } + return true; } @@ -2163,8 +2227,7 @@ bool MediaSessionDescriptionFactory::AddSctpDataContentForOffer( StreamParamsVec* current_streams, SessionDescription* desc, IceCredentialsIterator* ice_credentials) const { - std::unique_ptr data( - new SctpDataContentDescription()); + auto data = std::make_unique(); bool secure_transport = (transport_desc_factory_->secure() != SEC_DISABLED); @@ -2210,8 +2273,7 @@ bool MediaSessionDescriptionFactory::AddRtpDataContentForOffer( StreamParamsVec* current_streams, SessionDescription* desc, IceCredentialsIterator* ice_credentials) const { - std::unique_ptr data( - new RtpDataContentDescription()); + auto data = std::make_unique(); bool secure_transport = (transport_desc_factory_->secure() != SEC_DISABLED); cricket::SecurePolicy sdes_policy = @@ -2351,8 +2413,7 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer( bool bundle_enabled = offer_description->HasGroup(GROUP_TYPE_BUNDLE) && session_options.bundle_enabled; - std::unique_ptr audio_answer( - new AudioContentDescription()); + auto audio_answer = std::make_unique(); // Do not require or create SDES cryptos if DTLS is used. cricket::SecurePolicy sdes_policy = audio_transport->secure() ? cricket::SEC_DISABLED : secure(); @@ -2392,6 +2453,8 @@ 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, @@ -2416,11 +2479,20 @@ 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, video_codecs_); + media_description_options.codec_preferences, supported_video_codecs); } else { // Add the codecs from current content if it exists and is not rejected nor // recycled. @@ -2437,11 +2509,11 @@ bool MediaSessionDescriptionFactory::AddVideoContentForAnswer( } } // Add other supported video codecs. - for (const VideoCodec& codec : video_codecs_) { - if (FindMatchingCodec(video_codecs_, video_codecs, codec, - nullptr) && - !FindMatchingCodec(video_codecs_, filtered_codecs, codec, - nullptr)) { + for (const VideoCodec& codec : supported_video_codecs) { + if (FindMatchingCodec(supported_video_codecs, video_codecs, + codec, nullptr) && + !FindMatchingCodec(supported_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); @@ -2459,9 +2531,7 @@ bool MediaSessionDescriptionFactory::AddVideoContentForAnswer( bool bundle_enabled = offer_description->HasGroup(GROUP_TYPE_BUNDLE) && session_options.bundle_enabled; - - std::unique_ptr video_answer( - new VideoContentDescription()); + auto video_answer = std::make_unique(); // Do not require or create SDES cryptos if DTLS is used. cricket::SecurePolicy sdes_policy = video_transport->secure() ? cricket::SEC_DISABLED : secure(); @@ -2631,6 +2701,38 @@ 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 235945c4f9..ef83834318 100644 --- a/pc/media_session.h +++ b/pc/media_session.h @@ -151,8 +151,11 @@ class MediaSessionDescriptionFactory { audio_rtp_extensions_ = extensions; } RtpHeaderExtensions audio_rtp_header_extensions() const; - const VideoCodecs& video_codecs() const { return video_codecs_; } - void set_video_codecs(const VideoCodecs& codecs) { video_codecs_ = codecs; } + 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); void set_video_rtp_header_extensions(const RtpHeaderExtensions& extensions) { video_rtp_extensions_ = extensions; } @@ -186,6 +189,11 @@ 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, @@ -317,6 +325,8 @@ class MediaSessionDescriptionFactory { void ComputeAudioCodecsIntersectionAndUnion(); + void ComputeVideoCodecsIntersectionAndUnion(); + bool is_unified_plan_ = false; AudioCodecs audio_send_codecs_; AudioCodecs audio_recv_codecs_; @@ -325,7 +335,12 @@ class MediaSessionDescriptionFactory { // Union of send and recv. AudioCodecs all_audio_codecs_; RtpHeaderExtensions audio_rtp_extensions_; - VideoCodecs video_codecs_; + 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_; 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 a2416c4dcc..a901dedb70 100644 --- a/pc/media_session_unittest.cc +++ b/pc/media_session_unittest.cc @@ -415,11 +415,13 @@ 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)); + f1_.set_video_codecs(MAKE_VECTOR(kVideoCodecs1), + 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)); + f2_.set_video_codecs(MAKE_VECTOR(kVideoCodecs2), + MAKE_VECTOR(kVideoCodecs2)); f2_.set_rtp_data_codecs(MAKE_VECTOR(kDataCodecs2)); tdf1_.set_certificate(rtc::RTCCertificate::Create( std::unique_ptr(new rtc::FakeSSLIdentity("id1")))); @@ -797,7 +799,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_codecs(), vcd->codecs()); + EXPECT_EQ(f1_.video_sendrecv_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 @@ -809,7 +811,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_codecs()[0]; + const VideoCodec& offered_video_codec = f2_.video_sendrecv_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); @@ -2061,7 +2063,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateMultiStreamVideoOffer) { ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuite); EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type()); - EXPECT_EQ(f1_.video_codecs(), vcd->codecs()); + EXPECT_EQ(f1_.video_sendrecv_codecs(), vcd->codecs()); ASSERT_CRYPTO(vcd, 1U, kDefaultSrtpCryptoSuite); const StreamParamsVec& video_streams = vcd->streams(); @@ -2557,8 +2559,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", @@ -2610,8 +2612,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). @@ -2680,12 +2682,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_.set_video_codecs(f1_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_.set_video_codecs(f2_codecs, f2_codecs); std::unique_ptr offer = f1_.CreateOffer(opts, NULL); ASSERT_TRUE(offer.get() != NULL); @@ -2744,8 +2746,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, std::vector f2_codecs = {vp9, vp9_rtx, vp8_answerer, vp8_answerer_rtx}; - f1_.set_video_codecs(f1_codecs); - f2_.set_video_codecs(f2_codecs); + f1_.set_video_codecs(f1_codecs, f1_codecs); + f2_.set_video_codecs(f2_codecs, f2_codecs); std::vector audio_codecs; f1_.set_audio_codecs(audio_codecs, audio_codecs); f2_.set_audio_codecs(audio_codecs, audio_codecs); @@ -2780,7 +2782,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_.set_video_codecs(f1_codecs, f1_codecs); MediaSessionOptions opts; AddMediaDescriptionOptions(MEDIA_TYPE_AUDIO, "audio", @@ -2805,7 +2807,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_.set_video_codecs(f2_codecs, f2_codecs); std::unique_ptr updated_offer( f2_.CreateOffer(opts, answer.get())); @@ -2841,7 +2843,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_.set_video_codecs(f2_codecs, f2_codecs); std::unique_ptr offer = f1_.CreateOffer(opts, nullptr); ASSERT_TRUE(offer.get() != nullptr); @@ -2880,12 +2882,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_.set_video_codecs(f1_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_.set_video_codecs(f2_codecs, f2_codecs); std::unique_ptr offer = f1_.CreateOffer(opts, NULL); ASSERT_TRUE(offer.get() != NULL); @@ -2923,12 +2925,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_.set_video_codecs(f1_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_.set_video_codecs(f2_codecs, f2_codecs); std::unique_ptr offer = f1_.CreateOffer(opts, NULL); ASSERT_TRUE(offer.get() != NULL); @@ -2953,16 +2955,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_.set_video_codecs(f1_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_.set_video_codecs(f1_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); + f2_.set_video_codecs(f2_codecs, f1_codecs); // H264-SVC codec is removed in the answer, therefore, associated RTX codec // for H264-SVC should also be removed. @@ -2989,7 +2991,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_.set_video_codecs(f1_codecs, f1_codecs); std::unique_ptr offer = f1_.CreateOffer(opts, nullptr); ASSERT_TRUE(offer); @@ -3003,7 +3005,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_.set_video_codecs(f1_codecs, f1_codecs); std::unique_ptr updated_offer( f1_.CreateOffer(opts, offer.get())); @@ -3030,7 +3032,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_.set_video_codecs(f1_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. @@ -3072,7 +3074,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_.set_video_codecs(f1_codecs, f1_codecs); // Ensure that the offer has a single FlexFEC ssrc and that // there is no FEC-FR ssrc + grouping for each. @@ -3113,7 +3115,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_.set_video_codecs(f1_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. @@ -4251,9 +4253,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); + f1_.set_video_codecs(video_codecs1, video_codecs1); f2_.set_audio_codecs(audio_codecs2, audio_codecs2); - f2_.set_video_codecs(video_codecs2); + f2_.set_video_codecs(video_codecs2, video_codecs2); MediaSessionOptions opts; AddMediaDescriptionOptions(MEDIA_TYPE_AUDIO, "audio", @@ -4303,8 +4305,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}); - f2_.set_video_codecs({h264_pm1}); + f1_.set_video_codecs({h264_pm0, h264_pm1}, {h264_pm0, h264_pm1}); + f2_.set_video_codecs({h264_pm1}, {h264_pm1}); MediaSessionOptions opts; AddMediaDescriptionOptions(MEDIA_TYPE_VIDEO, "video", @@ -4333,11 +4335,13 @@ 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)); + f1_.set_video_codecs(MAKE_VECTOR(kVideoCodecs1), + 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)); + f2_.set_video_codecs(MAKE_VECTOR(kVideoCodecs2), + 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 4523121b58..c8bb22e43e 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_->GetSupportedVideoCodecs(&cricket_codecs); + channel_manager_->GetSupportedVideoSendCodecs(&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_->GetSupportedVideoCodecs(&cricket_codecs); + channel_manager_->GetSupportedVideoReceiveCodecs(&cricket_codecs); channel_manager_->GetSupportedVideoRtpHeaderExtensions( &cricket_extensions); return ToRtpCapabilities(cricket_codecs, cricket_extensions); diff --git a/pc/peer_connection_media_unittest.cc b/pc/peer_connection_media_unittest.cc index 077c4a3e43..c9ffd776d9 100644 --- a/pc/peer_connection_media_unittest.cc +++ b/pc/peer_connection_media_unittest.cc @@ -1434,9 +1434,11 @@ TEST_F(PeerConnectionMediaTestUnifiedPlan, TEST_F(PeerConnectionMediaTestUnifiedPlan, SetCodecPreferencesVideoRejectsOnlyRtxRedFec) { auto fake_engine = std::make_unique(); - auto video_codecs = fake_engine->video().codecs(); + auto video_codecs = fake_engine->video().send_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, @@ -1540,7 +1542,7 @@ TEST_F(PeerConnectionMediaTestUnifiedPlan, TEST_F(PeerConnectionMediaTestUnifiedPlan, SetCodecPreferencesVideoWithRtx) { auto caller_fake_engine = std::make_unique(); - auto caller_video_codecs = caller_fake_engine->video().codecs(); + auto caller_video_codecs = caller_fake_engine->video().send_codecs(); caller_video_codecs.push_back(cricket::VideoCodec( caller_video_codecs.back().id + 1, cricket::kVp8CodecName)); caller_video_codecs.push_back(cricket::VideoCodec( @@ -1592,7 +1594,7 @@ TEST_F(PeerConnectionMediaTestUnifiedPlan, SetCodecPreferencesVideoWithRtx) { TEST_F(PeerConnectionMediaTestUnifiedPlan, SetCodecPreferencesVideoCodecsNegotiation) { auto caller_fake_engine = std::make_unique(); - auto caller_video_codecs = caller_fake_engine->video().codecs(); + auto caller_video_codecs = caller_fake_engine->video().send_codecs(); caller_video_codecs.push_back(cricket::VideoCodec( caller_video_codecs.back().id + 1, cricket::kVp8CodecName)); caller_video_codecs.push_back(cricket::VideoCodec( @@ -1666,7 +1668,7 @@ TEST_F(PeerConnectionMediaTestUnifiedPlan, TEST_F(PeerConnectionMediaTestUnifiedPlan, SetCodecPreferencesVideoCodecsNegotiationReverseOrder) { auto caller_fake_engine = std::make_unique(); - auto caller_video_codecs = caller_fake_engine->video().codecs(); + auto caller_video_codecs = caller_fake_engine->video().send_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 d3281d5e6e..fcb54b54c2 100644 --- a/pc/rtp_transceiver.cc +++ b/pc/rtp_transceiver.cc @@ -20,6 +20,83 @@ #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) { @@ -251,111 +328,26 @@ 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); - // 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 + "\"."); - } - } + result = VerifyCodecPreferences(codecs, send_codecs, recv_codecs); } else if (media_type_ == cricket::MEDIA_TYPE_VIDEO) { - 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); + std::vector recv_codecs, send_codecs; + channel_manager_->GetSupportedVideoReceiveCodecs(&recv_codecs); + channel_manager_->GetSupportedVideoSendCodecs(&send_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 + "\"."); - } - } + result = VerifyCodecPreferences(codecs, send_codecs, recv_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."); + if (result.ok()) { + codec_preferences_ = codecs; } - codec_preferences_ = codecs; - - return RTCError::OK(); + return result; } } // namespace webrtc