Return error when requested codec is preferred but not negotiated
Because of our asymmetrical codec situation, it's possible to have
send only codecs that we cannot negotiate even with ourselves.
This means that we should not have a DCHECK, but just a plain error.
Bug: webrtc:15064
Change-Id: I0c170e5c7f356197bcb04bcecb8259c344423ccb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/323183
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40939}
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index b97e829..fbf9226 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 @@
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 e8d4435..adf8b5c 100644
--- a/media/engine/webrtc_voice_engine.cc
+++ b/media/engine/webrtc_voice_engine.cc
@@ -1871,6 +1871,8 @@
SetPreferredDscp(new_dscp);
absl::optional<cricket::Codec> 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 @@
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 4e502f7..4a786e0 100644
--- a/pc/peer_connection_encodings_integrationtest.cc
+++ b/pc/peer_connection_encodings_integrationtest.cc
@@ -1464,6 +1464,69 @@
}
TEST_F(PeerConnectionEncodingsIntegrationTest,
+ SetParametersRejectsNonRemotelyNegotiatedCodecParameterAudio) {
+ rtc::scoped_refptr<PeerConnectionTestWrapper> local_pc_wrapper = CreatePc();
+ rtc::scoped_refptr<PeerConnectionTestWrapper> remote_pc_wrapper = CreatePc();
+ ExchangeIceCandidates(local_pc_wrapper, remote_pc_wrapper);
+
+ absl::optional<webrtc::RtpCodecCapability> opus =
+ local_pc_wrapper->FindFirstSendCodecWithName(cricket::MEDIA_TYPE_AUDIO,
+ "opus");
+ ASSERT_TRUE(opus);
+
+ std::vector<webrtc::RtpCodecCapability> 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<RtpTransceiverInterface> audio_transceiver =
+ transceiver_or_error.MoveValue();
+
+ // Negotiation, create offer and apply it
+ std::unique_ptr<SessionDescriptionInterface> offer =
+ CreateOffer(local_pc_wrapper);
+ rtc::scoped_refptr<MockSetSessionDescriptionObserver> p1 =
+ SetLocalDescription(local_pc_wrapper, offer.get());
+ rtc::scoped_refptr<MockSetSessionDescriptionObserver> p2 =
+ SetRemoteDescription(remote_pc_wrapper, offer.get());
+ EXPECT_TRUE(Await({p1, p2}));
+
+ // Update the remote transceiver to reject Opus
+ std::vector<rtc::scoped_refptr<RtpTransceiverInterface>> remote_transceivers =
+ remote_pc_wrapper->pc()->GetTransceivers();
+ ASSERT_TRUE(!remote_transceivers.empty());
+ rtc::scoped_refptr<RtpTransceiverInterface> remote_audio_transceiver =
+ remote_transceivers[0];
+ ASSERT_TRUE(
+ remote_audio_transceiver->SetCodecPreferences(not_opus_codecs).ok());
+
+ // Create answer and apply it
+ std::unique_ptr<SessionDescriptionInterface> 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<PeerConnectionTestWrapper> local_pc_wrapper = CreatePc();
rtc::scoped_refptr<PeerConnectionTestWrapper> remote_pc_wrapper = CreatePc();
@@ -1504,6 +1567,69 @@
}
TEST_F(PeerConnectionEncodingsIntegrationTest,
+ SetParametersRejectsNonRemotelyNegotiatedCodecParameterVideo) {
+ rtc::scoped_refptr<PeerConnectionTestWrapper> local_pc_wrapper = CreatePc();
+ rtc::scoped_refptr<PeerConnectionTestWrapper> remote_pc_wrapper = CreatePc();
+ ExchangeIceCandidates(local_pc_wrapper, remote_pc_wrapper);
+
+ absl::optional<webrtc::RtpCodecCapability> vp8 =
+ local_pc_wrapper->FindFirstSendCodecWithName(cricket::MEDIA_TYPE_VIDEO,
+ "vp8");
+ ASSERT_TRUE(vp8);
+
+ std::vector<webrtc::RtpCodecCapability> 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<RtpTransceiverInterface> video_transceiver =
+ transceiver_or_error.MoveValue();
+
+ // Negotiation, create offer and apply it
+ std::unique_ptr<SessionDescriptionInterface> offer =
+ CreateOffer(local_pc_wrapper);
+ rtc::scoped_refptr<MockSetSessionDescriptionObserver> p1 =
+ SetLocalDescription(local_pc_wrapper, offer.get());
+ rtc::scoped_refptr<MockSetSessionDescriptionObserver> p2 =
+ SetRemoteDescription(remote_pc_wrapper, offer.get());
+ EXPECT_TRUE(Await({p1, p2}));
+
+ // Update the remote transceiver to reject VP8
+ std::vector<rtc::scoped_refptr<RtpTransceiverInterface>> remote_transceivers =
+ remote_pc_wrapper->pc()->GetTransceivers();
+ ASSERT_TRUE(!remote_transceivers.empty());
+ rtc::scoped_refptr<RtpTransceiverInterface> remote_video_transceiver =
+ remote_transceivers[0];
+ ASSERT_TRUE(
+ remote_video_transceiver->SetCodecPreferences(not_vp8_codecs).ok());
+
+ // Create answer and apply it
+ std::unique_ptr<SessionDescriptionInterface> 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<PeerConnectionTestWrapper> local_pc_wrapper = CreatePc();
rtc::scoped_refptr<PeerConnectionTestWrapper> remote_pc_wrapper = CreatePc();
