Revert "Implement read-only codecPayloadType in RtpParameters"
This reverts commit 806e06d1366b58878ced05cdd8d1d56394982fe6.
Reason for revert: Breaks WebRTC roll to Chromium. https://chromium-review.googlesource.com/c/chromium/src/+/1375538
02:52:35.346 7748 [6936:11248:1213/025234.206:ERROR:mediaengine.cc(80)] Attempted to set RtpParameters with modified codecPayloadType (INVALID_MODIFICATION)
Original change's description:
> 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}
TBR=steveanton@webrtc.org,sakal@webrtc.org,andersc@webrtc.org,shampson@webrtc.org,orphis@webrtc.org
Change-Id: I157f9a79ae7133395431891e15e2c053559d359b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:7580
Reviewed-on: https://webrtc-review.googlesource.com/c/114300
Reviewed-by: Henrik Grunell <henrikg@webrtc.org>
Commit-Queue: Henrik Grunell <henrikg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26000}diff --git a/api/rtpparameters.h b/api/rtpparameters.h
index f4b5198..47df22e 100644
--- a/api/rtpparameters.h
+++ b/api/rtpparameters.h
@@ -377,7 +377,12 @@
// unset SSRC acts as a "wildcard" SSRC.
absl::optional<uint32_t> ssrc;
- // Read-only parameter indicating the payload type of the codec being used.
+ // 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.
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 7d9143b..bcdd6b6 100644
--- a/media/base/mediaengine.cc
+++ b/media/base/mediaengine.cc
@@ -73,12 +73,6 @@
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 1bac48a..e429231 100644
--- a/media/engine/webrtcvideoengine.cc
+++ b/media/engine/webrtcvideoengine.cc
@@ -1765,10 +1765,6 @@
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 4505f04..1660bd8 100644
--- a/media/engine/webrtcvoiceengine.cc
+++ b/media/engine/webrtcvoiceengine.cc
@@ -1050,9 +1050,6 @@
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 f24073d..c9f2302 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -1443,12 +1443,6 @@
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 0b817c7..fa1fb23 100644
--- a/pc/peerconnection_integrationtest.cc
+++ b/pc/peerconnection_integrationtest.cc
@@ -4586,34 +4586,6 @@
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 b39b0c2..c1f7656 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, fec, rtx, dtx,
+ // Unimplemented RtpParameters: ssrc, codec_payload_type, fec, rtx, dtx,
// ptime, scale_resolution_down_by, scale_framerate_down_by, rid,
// dependency_rids.
init.send_encodings[0].ssrc = 1;
@@ -1435,6 +1435,14 @@
.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 df14772..ec99a4b 100644
--- a/pc/rtpsender.cc
+++ b/pc/rtpsender.cc
@@ -38,7 +38,8 @@
// contains a value.
bool UnimplementedRtpEncodingParameterHasValue(
const RtpEncodingParameters& encoding_params) {
- if (encoding_params.fec.has_value() || encoding_params.rtx.has_value() ||
+ if (encoding_params.codec_payload_type.has_value() ||
+ 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 97e144d..037446f 100644
--- a/pc/rtpsenderreceiver_unittest.cc
+++ b/pc/rtpsenderreceiver_unittest.cc
@@ -798,33 +798,19 @@
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: fec, rtx, dtx, ptime,
+ // Unimplemented RtpParameters: codec_payload_type, 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());
@@ -1093,8 +1079,13 @@
RtpParameters params = video_rtp_sender_->GetParameters();
EXPECT_EQ(1u, params.encodings.size());
- // Unimplemented RtpParameters: fec, rtx, dtx, ptime,
+ // Unimplemented RtpParameters: codec_payload_type, 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());
@@ -1138,9 +1129,14 @@
RtpParameters params = video_rtp_sender_->GetParameters();
EXPECT_EQ(kVideoSimulcastLayerCount, params.encodings.size());
- // Unimplemented RtpParameters: fec, rtx, dtx, ptime,
+ // Unimplemented RtpParameters: codec_payload_type, 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());
@@ -1209,11 +1205,6 @@
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 7b523b7..5158c13 100644
--- a/sdk/android/api/org/webrtc/RtpParameters.java
+++ b/sdk/android/api/org/webrtc/RtpParameters.java
@@ -30,9 +30,6 @@
// 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.
@@ -48,10 +45,9 @@
public Long ssrc;
@CalledByNative("Encoding")
- Encoding(boolean active, Integer codecPayloadType, Integer maxBitrateBps, Integer minBitrateBps,
- Integer maxFramerate, Integer numTemporalLayers, Long ssrc) {
+ Encoding(boolean active, 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;
@@ -59,12 +55,6 @@
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 a05942c..3c7a9a9 100644
--- a/sdk/android/src/jni/pc/rtpparameters.cc
+++ b/sdk/android/src/jni/pc/rtpparameters.cc
@@ -24,9 +24,7 @@
JNIEnv* env,
const RtpEncodingParameters& encoding) {
return Java_Encoding_Constructor(
- env, encoding.active,
- NativeToJavaInteger(env, encoding.codec_payload_type),
- NativeToJavaInteger(env, encoding.max_bitrate_bps),
+ env, encoding.active, NativeToJavaInteger(env, encoding.max_bitrate_bps),
NativeToJavaInteger(env, encoding.min_bitrate_bps),
NativeToJavaInteger(env, encoding.max_framerate),
NativeToJavaInteger(env, encoding.num_temporal_layers),
@@ -68,10 +66,6 @@
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 1406213..ba50bde 100644
--- a/sdk/objc/api/peerconnection/RTCRtpEncodingParameters.h
+++ b/sdk/objc/api/peerconnection/RTCRtpEncodingParameters.h
@@ -17,11 +17,6 @@
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 0351939..270f1b2 100644
--- a/sdk/objc/api/peerconnection/RTCRtpEncodingParameters.mm
+++ b/sdk/objc/api/peerconnection/RTCRtpEncodingParameters.mm
@@ -12,7 +12,6 @@
@implementation RTCRtpEncodingParameters
-@synthesize codecPayloadType = _codecPayloadType;
@synthesize isActive = _isActive;
@synthesize maxBitrateBps = _maxBitrateBps;
@synthesize minBitrateBps = _minBitrateBps;
@@ -27,9 +26,6 @@
- (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 =
@@ -54,9 +50,6 @@
- (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);
