diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index b97e829f3e..fbf9226c4e 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -29,6 +29,7 @@ #include "api/media_stream_interface.h" #include "api/media_types.h" #include "api/priority.h" +#include "api/rtc_error.h" #include "api/rtp_transceiver_direction.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" @@ -47,6 +48,7 @@ #include "common_video/frame_counts.h" #include "common_video/include/quality_limitation_reason.h" #include "media/base/codec.h" +#include "media/base/media_channel.h" #include "media/base/media_constants.h" #include "media/base/rid_description.h" #include "media/base/rtp_utils.h" @@ -1336,17 +1338,24 @@ webrtc::RTCError WebRtcVideoSendChannel::SetRtpSendParameters( break; } + // Since we validate that all layers have the same value, we can just check + // the first layer. // TODO(orphis): Support mixed-codec simulcast if (parameters.encodings[0].codec && send_codec_ && !send_codec_->codec.MatchesRtpCodec(*parameters.encodings[0].codec)) { - RTC_LOG(LS_ERROR) << "Trying to change codec to " - << parameters.encodings[0].codec->name; + RTC_LOG(LS_VERBOSE) << "Trying to change codec to " + << parameters.encodings[0].codec->name; auto matched_codec = absl::c_find_if(negotiated_codecs_, [&](auto negotiated_codec) { return negotiated_codec.codec.MatchesRtpCodec( *parameters.encodings[0].codec); }); - RTC_CHECK(matched_codec != negotiated_codecs_.end()); + if (matched_codec == negotiated_codecs_.end()) { + return webrtc::InvokeSetParametersCallback( + callback, webrtc::RTCError( + webrtc::RTCErrorType::INVALID_MODIFICATION, + "Attempted to use an unsupported codec for layer 0")); + } ChangedSenderParameters params; params.send_codec = *matched_codec; diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index e8d4435611..adf8b5c51d 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -1871,6 +1871,8 @@ webrtc::RTCError WebRtcVoiceSendChannel::SetRtpSendParameters( SetPreferredDscp(new_dscp); absl::optional send_codec = GetSendCodec(); + // Since we validate that all layers have the same value, we can just check + // the first layer. // TODO(orphis): Support mixed-codec simulcast if (parameters.encodings[0].codec && send_codec && !send_codec->MatchesRtpCodec(*parameters.encodings[0].codec)) { @@ -1881,7 +1883,13 @@ webrtc::RTCError WebRtcVoiceSendChannel::SetRtpSendParameters( return negotiated_codec.MatchesRtpCodec( *parameters.encodings[0].codec); }); - RTC_DCHECK(matched_codec != send_codecs_.end()); + + if (matched_codec == send_codecs_.end()) { + return webrtc::InvokeSetParametersCallback( + callback, webrtc::RTCError( + webrtc::RTCErrorType::INVALID_MODIFICATION, + "Attempted to use an unsupported codec for layer 0")); + } SetSendCodecs(send_codecs_, *matched_codec); } diff --git a/pc/peer_connection_encodings_integrationtest.cc b/pc/peer_connection_encodings_integrationtest.cc index 4e502f73d7..4a786e0733 100644 --- a/pc/peer_connection_encodings_integrationtest.cc +++ b/pc/peer_connection_encodings_integrationtest.cc @@ -1463,6 +1463,69 @@ TEST_F(PeerConnectionEncodingsIntegrationTest, EXPECT_EQ(error.type(), RTCErrorType::INVALID_MODIFICATION); } +TEST_F(PeerConnectionEncodingsIntegrationTest, + SetParametersRejectsNonRemotelyNegotiatedCodecParameterAudio) { + rtc::scoped_refptr local_pc_wrapper = CreatePc(); + rtc::scoped_refptr remote_pc_wrapper = CreatePc(); + ExchangeIceCandidates(local_pc_wrapper, remote_pc_wrapper); + + absl::optional opus = + local_pc_wrapper->FindFirstSendCodecWithName(cricket::MEDIA_TYPE_AUDIO, + "opus"); + ASSERT_TRUE(opus); + + std::vector not_opus_codecs = + local_pc_wrapper->pc_factory() + ->GetRtpSenderCapabilities(cricket::MEDIA_TYPE_AUDIO) + .codecs; + not_opus_codecs.erase( + std::remove_if(not_opus_codecs.begin(), not_opus_codecs.end(), + [&](const auto& codec) { + return absl::EqualsIgnoreCase(codec.name, opus->name); + }), + not_opus_codecs.end()); + + auto transceiver_or_error = + local_pc_wrapper->pc()->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); + ASSERT_TRUE(transceiver_or_error.ok()); + rtc::scoped_refptr audio_transceiver = + transceiver_or_error.MoveValue(); + + // Negotiation, create offer and apply it + std::unique_ptr offer = + CreateOffer(local_pc_wrapper); + rtc::scoped_refptr p1 = + SetLocalDescription(local_pc_wrapper, offer.get()); + rtc::scoped_refptr p2 = + SetRemoteDescription(remote_pc_wrapper, offer.get()); + EXPECT_TRUE(Await({p1, p2})); + + // Update the remote transceiver to reject Opus + std::vector> remote_transceivers = + remote_pc_wrapper->pc()->GetTransceivers(); + ASSERT_TRUE(!remote_transceivers.empty()); + rtc::scoped_refptr remote_audio_transceiver = + remote_transceivers[0]; + ASSERT_TRUE( + remote_audio_transceiver->SetCodecPreferences(not_opus_codecs).ok()); + + // Create answer and apply it + std::unique_ptr answer = + CreateAnswer(remote_pc_wrapper); + p1 = SetLocalDescription(remote_pc_wrapper, answer.get()); + p2 = SetRemoteDescription(local_pc_wrapper, answer.get()); + EXPECT_TRUE(Await({p1, p2})); + + local_pc_wrapper->WaitForConnection(); + remote_pc_wrapper->WaitForConnection(); + + webrtc::RtpParameters parameters = + audio_transceiver->sender()->GetParameters(); + parameters.encodings[0].codec = opus; + RTCError error = audio_transceiver->sender()->SetParameters(parameters); + EXPECT_EQ(error.type(), RTCErrorType::INVALID_MODIFICATION); +} + TEST_F(PeerConnectionEncodingsIntegrationTest, SetParametersRejectsNonNegotiatedCodecParameterVideo) { rtc::scoped_refptr local_pc_wrapper = CreatePc(); @@ -1503,6 +1566,69 @@ TEST_F(PeerConnectionEncodingsIntegrationTest, EXPECT_EQ(error.type(), RTCErrorType::INVALID_MODIFICATION); } +TEST_F(PeerConnectionEncodingsIntegrationTest, + SetParametersRejectsNonRemotelyNegotiatedCodecParameterVideo) { + rtc::scoped_refptr local_pc_wrapper = CreatePc(); + rtc::scoped_refptr remote_pc_wrapper = CreatePc(); + ExchangeIceCandidates(local_pc_wrapper, remote_pc_wrapper); + + absl::optional vp8 = + local_pc_wrapper->FindFirstSendCodecWithName(cricket::MEDIA_TYPE_VIDEO, + "vp8"); + ASSERT_TRUE(vp8); + + std::vector not_vp8_codecs = + local_pc_wrapper->pc_factory() + ->GetRtpSenderCapabilities(cricket::MEDIA_TYPE_VIDEO) + .codecs; + not_vp8_codecs.erase( + std::remove_if(not_vp8_codecs.begin(), not_vp8_codecs.end(), + [&](const auto& codec) { + return absl::EqualsIgnoreCase(codec.name, vp8->name); + }), + not_vp8_codecs.end()); + + auto transceiver_or_error = + local_pc_wrapper->pc()->AddTransceiver(cricket::MEDIA_TYPE_VIDEO); + ASSERT_TRUE(transceiver_or_error.ok()); + rtc::scoped_refptr video_transceiver = + transceiver_or_error.MoveValue(); + + // Negotiation, create offer and apply it + std::unique_ptr offer = + CreateOffer(local_pc_wrapper); + rtc::scoped_refptr p1 = + SetLocalDescription(local_pc_wrapper, offer.get()); + rtc::scoped_refptr p2 = + SetRemoteDescription(remote_pc_wrapper, offer.get()); + EXPECT_TRUE(Await({p1, p2})); + + // Update the remote transceiver to reject VP8 + std::vector> remote_transceivers = + remote_pc_wrapper->pc()->GetTransceivers(); + ASSERT_TRUE(!remote_transceivers.empty()); + rtc::scoped_refptr remote_video_transceiver = + remote_transceivers[0]; + ASSERT_TRUE( + remote_video_transceiver->SetCodecPreferences(not_vp8_codecs).ok()); + + // Create answer and apply it + std::unique_ptr answer = + CreateAnswer(remote_pc_wrapper); + p1 = SetLocalDescription(remote_pc_wrapper, answer.get()); + p2 = SetRemoteDescription(local_pc_wrapper, answer.get()); + EXPECT_TRUE(Await({p1, p2})); + + local_pc_wrapper->WaitForConnection(); + remote_pc_wrapper->WaitForConnection(); + + webrtc::RtpParameters parameters = + video_transceiver->sender()->GetParameters(); + parameters.encodings[0].codec = vp8; + RTCError error = video_transceiver->sender()->SetParameters(parameters); + EXPECT_EQ(error.type(), RTCErrorType::INVALID_MODIFICATION); +} + TEST_F(PeerConnectionEncodingsIntegrationTest, EncodingParametersCodecRemovedAfterNegotiationAudio) { rtc::scoped_refptr local_pc_wrapper = CreatePc();