Implement read-only codecPayloadType in RtpParameters
Bug: webrtc:7580
Change-Id: I6d901afa97262b6c6d9fe6c7366df465ec77bfb3
Reviewed-on: https://webrtc-review.googlesource.com/c/113944
Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
Reviewed-by: Seth Hampson <shampson@webrtc.org>
Reviewed-by: Anders Carlsson <andersc@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25993}diff --git a/api/rtpparameters.h b/api/rtpparameters.h
index 47df22e..f4b5198 100644
--- a/api/rtpparameters.h
+++ b/api/rtpparameters.h
@@ -377,12 +377,7 @@
// unset SSRC acts as a "wildcard" SSRC.
absl::optional<uint32_t> 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<int> codec_payload_type;
// Specifies the FEC mechanism, if set.
diff --git a/media/base/mediaengine.cc b/media/base/mediaengine.cc
index bcdd6b6..7d9143b 100644
--- a/media/base/mediaengine.cc
+++ b/media/base/mediaengine.cc
@@ -73,6 +73,12 @@
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 e429231..1bac48a 100644
--- a/media/engine/webrtcvideoengine.cc
+++ b/media/engine/webrtcvideoengine.cc
@@ -1765,6 +1765,10 @@
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 1660bd8..4505f04 100644
--- a/media/engine/webrtcvoiceengine.cc
+++ b/media/engine/webrtcvoiceengine.cc
@@ -1050,6 +1050,9 @@
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 4bdaead..45e8c71 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -1443,6 +1443,12 @@
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 8ce09d2..8f4ac85 100644
--- a/pc/peerconnection_integrationtest.cc
+++ b/pc/peerconnection_integrationtest.cc
@@ -4640,6 +4640,34 @@
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 c1f7656..b39b0c2 100644
--- a/pc/peerconnection_rtp_unittest.cc
+++ b/pc/peerconnection_rtp_unittest.cc
@@ -1424,7 +1424,7 @@
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 @@
.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 ec99a4b..df14772 100644
--- a/pc/rtpsender.cc
+++ b/pc/rtpsender.cc
@@ -38,8 +38,7 @@
// 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 037446f..97e144d 100644
--- a/pc/rtpsenderreceiver_unittest.cc
+++ b/pc/rtpsenderreceiver_unittest.cc
@@ -798,19 +798,33 @@
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 @@
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 @@
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 @@
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 5158c13..7b523b7 100644
--- a/sdk/android/api/org/webrtc/RtpParameters.java
+++ b/sdk/android/api/org/webrtc/RtpParameters.java
@@ -30,6 +30,9 @@
// 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 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 @@
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 3c7a9a9..a05942c 100644
--- a/sdk/android/src/jni/pc/rtpparameters.cc
+++ b/sdk/android/src/jni/pc/rtpparameters.cc
@@ -24,7 +24,9 @@
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 @@
encoding.active = Java_Encoding_getActive(jni, j_encoding_parameters);
ScopedJavaLocalRef<jobject> j_max_bitrate =
Java_Encoding_getMaxBitrateBps(jni, j_encoding_parameters);
+ ScopedJavaLocalRef<jobject> 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<jobject> 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 ba50bde..1406213 100644
--- a/sdk/objc/api/peerconnection/RTCRtpEncodingParameters.h
+++ b/sdk/objc/api/peerconnection/RTCRtpEncodingParameters.h
@@ -17,6 +17,11 @@
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 270f1b2..0351939 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<int>(_codecPayloadType.intValue);
+ }
parameters.active = _isActive;
if (_maxBitrateBps != nil) {
parameters.max_bitrate_bps = absl::optional<int>(_maxBitrateBps.intValue);
