diff --git a/api/rtpparameters.h b/api/rtpparameters.h index 47df22e7ff..f4b5198353 100644 --- a/api/rtpparameters.h +++ b/api/rtpparameters.h @@ -377,12 +377,7 @@ struct RtpEncodingParameters { // unset SSRC acts as a "wildcard" SSRC. absl::optional ssrc; - // Can be used to reference a codec in the |codecs| member of the - // RtpParameters that contains this RtpEncodingParameters. If unset, the - // implementation will choose the first possible codec (if a sender), or - // prepare to receive any codec (for a receiver). - // TODO(deadbeef): Not implemented. Implementation of RtpSender will always - // choose the first codec from the list. + // Read-only parameter indicating the payload type of the codec being used. absl::optional codec_payload_type; // Specifies the FEC mechanism, if set. diff --git a/media/base/mediaengine.cc b/media/base/mediaengine.cc index bcdd6b6b9b..7d9143be4e 100644 --- a/media/base/mediaengine.cc +++ b/media/base/mediaengine.cc @@ -73,6 +73,12 @@ webrtc::RTCError ValidateRtpParameters( LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_MODIFICATION, "Attempted to set RtpParameters with modified SSRC"); } + if (rtp_parameters.encodings[i].codec_payload_type != + old_rtp_parameters.encodings[i].codec_payload_type) { + LOG_AND_RETURN_ERROR( + RTCErrorType::INVALID_MODIFICATION, + "Attempted to set RtpParameters with modified codecPayloadType"); + } if (rtp_parameters.encodings[i].bitrate_priority <= 0) { LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_RANGE, "Attempted to set RtpParameters bitrate_priority to " diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc index e4292314b8..1bac48a1b8 100644 --- a/media/engine/webrtcvideoengine.cc +++ b/media/engine/webrtcvideoengine.cc @@ -1765,6 +1765,10 @@ void WebRtcVideoChannel::WebRtcVideoSendStream::SetCodec( parameters_.codec_settings = codec_settings; + for (auto& encoding : rtp_parameters_.encodings) { + encoding.codec_payload_type = codec_settings.codec.id; + } + // TODO(nisse): Avoid recreation, it should be enough to call // ReconfigureEncoder. RTC_LOG(LS_INFO) << "RecreateWebRtcStream (send) because of SetCodec."; diff --git a/media/engine/webrtcvoiceengine.cc b/media/engine/webrtcvoiceengine.cc index 1660bd86e1..4505f04011 100644 --- a/media/engine/webrtcvoiceengine.cc +++ b/media/engine/webrtcvoiceengine.cc @@ -1050,6 +1050,9 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream max_send_bitrate_bps_, rtp_parameters_.encodings[0].max_bitrate_bps, *audio_codec_spec_); + rtp_parameters_.encodings[0].codec_payload_type = + send_codec_spec.payload_type; + UpdateAllowedBitrateRange(); } diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index 4bdaead317..45e8c71a47 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -1443,6 +1443,12 @@ PeerConnection::AddTransceiver( RTCErrorType::UNSUPPORTED_PARAMETER, "Attempted to set an unimplemented parameter of RtpParameters."); } + + if (encoding.codec_payload_type.has_value()) { + LOG_AND_RETURN_ERROR( + RTCErrorType::INVALID_MODIFICATION, + "Attempted to set a read-only value in RtpParameters."); + } } RtpParameters parameters; diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc index 8ce09d2546..8f4ac8540f 100644 --- a/pc/peerconnection_integrationtest.cc +++ b/pc/peerconnection_integrationtest.cc @@ -4640,6 +4640,34 @@ TEST_P(PeerConnectionIntegrationTest, GetSourcesVideo) { EXPECT_EQ(webrtc::RtpSourceType::SSRC, sources[0].source_type()); } +TEST_P(PeerConnectionIntegrationTest, GetParametersCodecPayloadTypeAudio) { + ASSERT_TRUE(CreatePeerConnectionWrappers()); + ConnectFakeSignaling(); + caller()->AddAudioTrack(); + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + ASSERT_EQ(caller()->pc()->GetSenders().size(), 1u); + auto sender = caller()->pc()->GetSenders()[0]; + ASSERT_EQ(sender->media_type(), cricket::MEDIA_TYPE_AUDIO); + ASSERT_GT(sender->GetParameters().encodings.size(), 0u); + EXPECT_TRUE( + sender->GetParameters().encodings[0].codec_payload_type.has_value()); +} + +TEST_P(PeerConnectionIntegrationTest, GetParametersCodecPayloadTypeVideo) { + ASSERT_TRUE(CreatePeerConnectionWrappers()); + ConnectFakeSignaling(); + caller()->AddVideoTrack(); + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + ASSERT_EQ(caller()->pc()->GetSenders().size(), 1u); + auto sender = caller()->pc()->GetSenders()[0]; + ASSERT_EQ(sender->media_type(), cricket::MEDIA_TYPE_VIDEO); + ASSERT_GT(sender->GetParameters().encodings.size(), 0u); + EXPECT_TRUE( + sender->GetParameters().encodings[0].codec_payload_type.has_value()); +} + // Test that if a track is removed and added again with a different stream ID, // the new stream ID is successfully communicated in SDP and media continues to // flow end-to-end. diff --git a/pc/peerconnection_rtp_unittest.cc b/pc/peerconnection_rtp_unittest.cc index c1f7656178..b39b0c2448 100644 --- a/pc/peerconnection_rtp_unittest.cc +++ b/pc/peerconnection_rtp_unittest.cc @@ -1424,7 +1424,7 @@ TEST_F(PeerConnectionRtpTestUnifiedPlan, auto default_send_encodings = init.send_encodings; - // Unimplemented RtpParameters: ssrc, codec_payload_type, fec, rtx, dtx, + // Unimplemented RtpParameters: ssrc, fec, rtx, dtx, // ptime, scale_resolution_down_by, scale_framerate_down_by, rid, // dependency_rids. init.send_encodings[0].ssrc = 1; @@ -1435,14 +1435,6 @@ TEST_F(PeerConnectionRtpTestUnifiedPlan, .type()); init.send_encodings = default_send_encodings; - init.send_encodings[0].codec_payload_type = 1; - EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, - caller->pc() - ->AddTransceiver(cricket::MEDIA_TYPE_AUDIO, init) - .error() - .type()); - init.send_encodings = default_send_encodings; - init.send_encodings[0].fec = RtpFecParameters(); EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, caller->pc() diff --git a/pc/rtpsender.cc b/pc/rtpsender.cc index ec99a4b40d..df14772368 100644 --- a/pc/rtpsender.cc +++ b/pc/rtpsender.cc @@ -38,8 +38,7 @@ int GenerateUniqueId() { // contains a value. bool UnimplementedRtpEncodingParameterHasValue( const RtpEncodingParameters& encoding_params) { - if (encoding_params.codec_payload_type.has_value() || - encoding_params.fec.has_value() || encoding_params.rtx.has_value() || + if (encoding_params.fec.has_value() || encoding_params.rtx.has_value() || encoding_params.dtx.has_value() || encoding_params.ptime.has_value() || !encoding_params.rid.empty() || encoding_params.scale_resolution_down_by.has_value() || diff --git a/pc/rtpsenderreceiver_unittest.cc b/pc/rtpsenderreceiver_unittest.cc index 037446fc0b..97e144d986 100644 --- a/pc/rtpsenderreceiver_unittest.cc +++ b/pc/rtpsenderreceiver_unittest.cc @@ -798,19 +798,33 @@ TEST_F(RtpSenderReceiverTest, AudioSenderCantSetUnimplementedRtpParameters) { DestroyAudioRtpSender(); } +TEST_F(RtpSenderReceiverTest, AudioSenderCantSetReadOnlyEncodingParameters) { + CreateAudioRtpSender(); + RtpParameters params = audio_rtp_sender_->GetParameters(); + + for (size_t i = 0; i < params.encodings.size(); i++) { + params.encodings[i].ssrc = 1337; + EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION, + audio_rtp_sender_->SetParameters(params).type()); + params = audio_rtp_sender_->GetParameters(); + + params.encodings[i].codec_payload_type = 42; + EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION, + audio_rtp_sender_->SetParameters(params).type()); + params = audio_rtp_sender_->GetParameters(); + } + + DestroyAudioRtpSender(); +} + TEST_F(RtpSenderReceiverTest, AudioSenderCantSetUnimplementedRtpEncodingParameters) { CreateAudioRtpSender(); RtpParameters params = audio_rtp_sender_->GetParameters(); EXPECT_EQ(1u, params.encodings.size()); - // Unimplemented RtpParameters: codec_payload_type, fec, rtx, dtx, ptime, + // Unimplemented RtpParameters: fec, rtx, dtx, ptime, // scale_resolution_down_by, scale_framerate_down_by, rid, dependency_rids. - params.encodings[0].codec_payload_type = 1; - EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, - audio_rtp_sender_->SetParameters(params).type()); - params = audio_rtp_sender_->GetParameters(); - params.encodings[0].fec = RtpFecParameters(); EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, audio_rtp_sender_->SetParameters(params).type()); @@ -1079,13 +1093,8 @@ TEST_F(RtpSenderReceiverTest, RtpParameters params = video_rtp_sender_->GetParameters(); EXPECT_EQ(1u, params.encodings.size()); - // Unimplemented RtpParameters: codec_payload_type, fec, rtx, dtx, ptime, + // Unimplemented RtpParameters: fec, rtx, dtx, ptime, // scale_resolution_down_by, scale_framerate_down_by, rid, dependency_rids. - params.encodings[0].codec_payload_type = 1; - EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, - video_rtp_sender_->SetParameters(params).type()); - params = video_rtp_sender_->GetParameters(); - params.encodings[0].fec = RtpFecParameters(); EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, video_rtp_sender_->SetParameters(params).type()); @@ -1129,14 +1138,9 @@ TEST_F(RtpSenderReceiverTest, RtpParameters params = video_rtp_sender_->GetParameters(); EXPECT_EQ(kVideoSimulcastLayerCount, params.encodings.size()); - // Unimplemented RtpParameters: codec_payload_type, fec, rtx, dtx, ptime, + // Unimplemented RtpParameters: fec, rtx, dtx, ptime, // scale_resolution_down_by, scale_framerate_down_by, rid, dependency_rids. for (size_t i = 0; i < params.encodings.size(); i++) { - params.encodings[i].codec_payload_type = 1; - EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, - video_rtp_sender_->SetParameters(params).type()); - params = video_rtp_sender_->GetParameters(); - params.encodings[i].fec = RtpFecParameters(); EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, video_rtp_sender_->SetParameters(params).type()); @@ -1205,6 +1209,11 @@ TEST_F(RtpSenderReceiverTest, VideoSenderCantSetReadOnlyEncodingParameters) { EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION, video_rtp_sender_->SetParameters(params).type()); params = video_rtp_sender_->GetParameters(); + + params.encodings[i].codec_payload_type = 1337; + EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION, + video_rtp_sender_->SetParameters(params).type()); + params = video_rtp_sender_->GetParameters(); } DestroyVideoRtpSender(); diff --git a/sdk/android/api/org/webrtc/RtpParameters.java b/sdk/android/api/org/webrtc/RtpParameters.java index 5158c13aeb..7b523b7252 100644 --- a/sdk/android/api/org/webrtc/RtpParameters.java +++ b/sdk/android/api/org/webrtc/RtpParameters.java @@ -30,6 +30,9 @@ public class RtpParameters { // Set to true to cause this encoding to be sent, and false for it not to // be sent. public boolean active = true; + // The payloadType of the codec used by the sender. + // Can't be changed between getParameters/setParameters. + @Nullable public Integer codecPayloadType; // If non-null, this represents the Transport Independent Application // Specific maximum bandwidth defined in RFC3890. If null, there is no // maximum bitrate. @@ -45,9 +48,10 @@ public class RtpParameters { public Long ssrc; @CalledByNative("Encoding") - Encoding(boolean active, Integer maxBitrateBps, Integer minBitrateBps, Integer maxFramerate, - Integer numTemporalLayers, Long ssrc) { + Encoding(boolean active, Integer codecPayloadType, Integer maxBitrateBps, Integer minBitrateBps, + Integer maxFramerate, Integer numTemporalLayers, Long ssrc) { this.active = active; + this.codecPayloadType = codecPayloadType; this.maxBitrateBps = maxBitrateBps; this.minBitrateBps = minBitrateBps; this.maxFramerate = maxFramerate; @@ -55,6 +59,12 @@ public class RtpParameters { this.ssrc = ssrc; } + @Nullable + @CalledByNative("Encoding") + Integer getCodecPayloadType() { + return codecPayloadType; + } + @CalledByNative("Encoding") boolean getActive() { return active; diff --git a/sdk/android/src/jni/pc/rtpparameters.cc b/sdk/android/src/jni/pc/rtpparameters.cc index 3c7a9a9103..a05942c718 100644 --- a/sdk/android/src/jni/pc/rtpparameters.cc +++ b/sdk/android/src/jni/pc/rtpparameters.cc @@ -24,7 +24,9 @@ ScopedJavaLocalRef NativeToJavaRtpEncodingParameter( JNIEnv* env, const RtpEncodingParameters& encoding) { return Java_Encoding_Constructor( - env, encoding.active, NativeToJavaInteger(env, encoding.max_bitrate_bps), + env, encoding.active, + NativeToJavaInteger(env, encoding.codec_payload_type), + NativeToJavaInteger(env, encoding.max_bitrate_bps), NativeToJavaInteger(env, encoding.min_bitrate_bps), NativeToJavaInteger(env, encoding.max_framerate), NativeToJavaInteger(env, encoding.num_temporal_layers), @@ -66,6 +68,10 @@ RtpEncodingParameters JavaToNativeRtpEncodingParameters( encoding.active = Java_Encoding_getActive(jni, j_encoding_parameters); ScopedJavaLocalRef j_max_bitrate = Java_Encoding_getMaxBitrateBps(jni, j_encoding_parameters); + ScopedJavaLocalRef j_codec_payload_type = + Java_Encoding_getCodecPayloadType(jni, j_encoding_parameters); + encoding.codec_payload_type = + JavaToNativeOptionalInt(jni, j_codec_payload_type); encoding.max_bitrate_bps = JavaToNativeOptionalInt(jni, j_max_bitrate); ScopedJavaLocalRef j_min_bitrate = Java_Encoding_getMinBitrateBps(jni, j_encoding_parameters); diff --git a/sdk/objc/api/peerconnection/RTCRtpEncodingParameters.h b/sdk/objc/api/peerconnection/RTCRtpEncodingParameters.h index ba50bde649..14062133dd 100644 --- a/sdk/objc/api/peerconnection/RTCRtpEncodingParameters.h +++ b/sdk/objc/api/peerconnection/RTCRtpEncodingParameters.h @@ -17,6 +17,11 @@ NS_ASSUME_NONNULL_BEGIN RTC_OBJC_EXPORT @interface RTCRtpEncodingParameters : NSObject +/** The codec payloadType used by the encoder, or nil if it is not currently + * available. + */ +@property(nonatomic, readonly, nullable) NSNumber *codecPayloadType; + /** Controls whether the encoding is currently transmitted. */ @property(nonatomic, assign) BOOL isActive; diff --git a/sdk/objc/api/peerconnection/RTCRtpEncodingParameters.mm b/sdk/objc/api/peerconnection/RTCRtpEncodingParameters.mm index 270f1b2e3b..0351939a6c 100644 --- a/sdk/objc/api/peerconnection/RTCRtpEncodingParameters.mm +++ b/sdk/objc/api/peerconnection/RTCRtpEncodingParameters.mm @@ -12,6 +12,7 @@ @implementation RTCRtpEncodingParameters +@synthesize codecPayloadType = _codecPayloadType; @synthesize isActive = _isActive; @synthesize maxBitrateBps = _maxBitrateBps; @synthesize minBitrateBps = _minBitrateBps; @@ -26,6 +27,9 @@ - (instancetype)initWithNativeParameters: (const webrtc::RtpEncodingParameters &)nativeParameters { if (self = [self init]) { + if (nativeParameters.codec_payload_type) { + _codecPayloadType = [NSNumber numberWithInt:*nativeParameters.codec_payload_type]; + } _isActive = nativeParameters.active; if (nativeParameters.max_bitrate_bps) { _maxBitrateBps = @@ -50,6 +54,9 @@ - (webrtc::RtpEncodingParameters)nativeParameters { webrtc::RtpEncodingParameters parameters; + if (_codecPayloadType != nil) { + parameters.codec_payload_type = absl::optional(_codecPayloadType.intValue); + } parameters.active = _isActive; if (_maxBitrateBps != nil) { parameters.max_bitrate_bps = absl::optional(_maxBitrateBps.intValue);