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}

api/rtpparameters.h

@@ -377,12 +377,7 @@ struct RtpEncodingParameters {
   // 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.

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 "

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.";

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();
   }
 

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;

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.

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()

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() ||

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();

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;

sdk/android/src/jni/pc/rtpparameters.cc

@@ -24,7 +24,9 @@ ScopedJavaLocalRef<jobject> 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<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);

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;
 

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);