Add payload type assignment to offer/answer generation.

This adds payload types to the codecs at the time when offer
is being generated, if they are unassigned at that point.

Bug: webrtc:360058654
Change-Id: I231ed057ebaf7fb0fffaf6ff5d600b064ba21f5b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/362282
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43033}

pc/BUILD.gn

@@ -372,6 +372,7 @@ rtc_source_set("media_session") {
     "../api:rtp_parameters",
     "../api:rtp_transceiver_direction",
     "../api/crypto:options",
+    "../call:payload_type",
     "../media:codec",
     "../media:media_constants",
     "../media:media_engine",
@@ -1013,6 +1014,7 @@ rtc_source_set("sdp_offer_answer") {
     "../api/video:builtin_video_bitrate_allocator_factory",
     "../api/video:video_bitrate_allocator_factory",
     "../api/video:video_codec_constants",
+    "../call:payload_type",
     "../media:codec",
     "../media:media_channel",
     "../media:media_engine",
@@ -1346,11 +1348,13 @@ rtc_source_set("webrtc_session_description_factory") {
     ":media_session",
     ":sdp_state_provider",
     ":session_description",
+    "../api:field_trials_view",
     "../api:libjingle_peerconnection_api",
     "../api:rtc_error",
     "../api:scoped_refptr",
     "../api:sequence_checker",
     "../api/task_queue",
+    "../call:payload_type",
     "../p2p:rtc_p2p",
     "../p2p:transport_description",
     "../p2p:transport_description_factory",
@@ -1992,6 +1996,7 @@ if (rtc_include_tests && !build_with_chromium) {
       ":rtp_transport_internal",
       ":sctp_transport",
       ":session_description",
+      ":simulcast_description",
       ":srtp_session",
       ":srtp_transport",
       ":used_ids",
@@ -2008,8 +2013,10 @@ if (rtc_include_tests && !build_with_chromium) {
       "../api:rtc_error",
       "../api:rtp_headers",
       "../api:rtp_parameters",
+      "../api:rtp_transceiver_direction",
       "../api:scoped_refptr",
       "../api:sequence_checker",
+      "../api/audio_codecs:audio_codecs_api",
       "../api/environment:environment_factory",
       "../api/task_queue:pending_task_safety_flag",
       "../api/task_queue:task_queue",
@@ -2027,6 +2034,7 @@ if (rtc_include_tests && !build_with_chromium) {
       "../media:rid_description",
       "../media:rtc_data_sctp_transport_internal",
       "../media:rtc_media_tests_utils",
+      "../media:stream_params",
       "../modules/rtp_rtcp:rtp_rtcp_format",
       "../p2p:candidate_pair_interface",
       "../p2p:dtls_transport_factory",
@@ -2039,6 +2047,7 @@ if (rtc_include_tests && !build_with_chromium) {
       "../p2p:packet_transport_internal",
       "../p2p:rtc_p2p",
       "../p2p:transport_description",
+      "../p2p:transport_description_factory",
       "../p2p:transport_info",
       "../rtc_base:async_packet_socket",
       "../rtc_base:buffer",

pc/media_session.cc

@@ -29,6 +29,7 @@
 #include "api/rtc_error.h"
 #include "api/rtp_parameters.h"
 #include "api/rtp_transceiver_direction.h"
+#include "call/payload_type.h"
 #include "media/base/codec.h"
 #include "media/base/media_constants.h"
 #include "media/base/media_engine.h"
@@ -1353,9 +1354,11 @@ MediaSessionDescriptionFactory::MediaSessionDescriptionFactory(
     cricket::MediaEngineInterface* media_engine,
     bool rtx_enabled,
     rtc::UniqueRandomIdGenerator* ssrc_generator,
-    const TransportDescriptionFactory* transport_desc_factory)
+    const TransportDescriptionFactory* transport_desc_factory,
+    webrtc::PayloadTypeSuggester* pt_suggester)
     : ssrc_generator_(ssrc_generator),
-      transport_desc_factory_(transport_desc_factory) {
+      transport_desc_factory_(transport_desc_factory),
+      pt_suggester_(pt_suggester) {
   RTC_CHECK(transport_desc_factory_);
   if (media_engine) {
     audio_send_codecs_ = media_engine->voice().send_codecs();
@@ -2049,7 +2052,7 @@ RTCError MediaSessionDescriptionFactory::AddRtpContentForOffer(
 
   std::vector<Codec> codecs_to_include;
   if (media_description_options.codecs_to_include.empty()) {
-    const std::vector<Codec>& supported_codecs =
+    std::vector<Codec> supported_codecs =
         media_description_options.type == MEDIA_TYPE_AUDIO
             ? GetAudioCodecsForOffer(media_description_options.direction)
             : GetVideoCodecsForOffer(media_description_options.direction);
@@ -2064,6 +2067,22 @@ RTCError MediaSessionDescriptionFactory::AddRtpContentForOffer(
     // Ignore both the codecs argument and the Get*CodecsForOffer results.
     codecs_to_include = media_description_options.codecs_to_include;
   }
+  for (cricket::Codec& codec : codecs_to_include) {
+    if (codec.id == Codec::kIdNotSet) {
+      // Add payload types to codecs, if needed
+      // This should only happen if WebRTC-PayloadTypesInTransport field trial
+      // is enabled.
+      RTC_CHECK(pt_suggester_);
+      RTC_CHECK(transport_desc_factory_->trials().IsEnabled(
+          "WebRTC-PayloadTypesInTransport"));
+      auto result = pt_suggester_->SuggestPayloadType(
+          media_description_options.mid, codec);
+      if (!result.ok()) {
+        return result.error();
+      }
+      codec.id = result.value();
+    }
+  }
   std::unique_ptr<MediaContentDescription> content_description;
   if (media_description_options.type == MEDIA_TYPE_AUDIO) {
     content_description = std::make_unique<AudioContentDescription>();
@@ -2251,6 +2270,20 @@ RTCError MediaSessionDescriptionFactory::AddRtpContentForAnswer(
                     media_description_options.codec_preferences.empty());
     codecs_to_include = negotiated_codecs;
   }
+  for (cricket::Codec& codec : codecs_to_include) {
+    if (codec.id == Codec::kIdNotSet) {
+      // Add payload types to codecs, if needed
+      RTC_CHECK(pt_suggester_);
+      RTC_CHECK(transport_desc_factory_->trials().IsEnabled(
+          "WebRTC-PayloadTypesInTransport"));
+      auto result = pt_suggester_->SuggestPayloadType(
+          media_description_options.mid, codec);
+      if (!result.ok()) {
+        return result.error();
+      }
+      codec.id = result.value();
+    }
+  }
 
   if (!SetCodecsInAnswer(offer_content_description, codecs_to_include,
                          media_description_options, session_options,

pc/media_session.h

@@ -22,6 +22,7 @@
 #include "api/rtc_error.h"
 #include "api/rtp_parameters.h"
 #include "api/rtp_transceiver_direction.h"
+#include "call/payload_type.h"
 #include "media/base/codec.h"
 #include "media/base/rid_description.h"
 #include "media/base/stream_params.h"
@@ -141,13 +142,14 @@ class MediaSessionDescriptionFactory {
  public:
   // This constructor automatically sets up the factory to get its configuration
   // from the specified MediaEngine (when provided).
-  // The TransportDescriptionFactory and the UniqueRandomIdGenerator are not
+  // The TransportDescriptionFactory, the UniqueRandomIdGenerator, and the
-  // owned by MediaSessionDescriptionFactory, so they must be kept alive by the
+  // PayloadTypeSuggester are not owned by MediaSessionDescriptionFactory, so
-  // user of this class.
+  // they must be kept alive by the user of this class.
   MediaSessionDescriptionFactory(cricket::MediaEngineInterface* media_engine,
                                  bool rtx_enabled,
                                  rtc::UniqueRandomIdGenerator* ssrc_generator,
-                                 const TransportDescriptionFactory* factory);
+                                 const TransportDescriptionFactory* factory,
+                                 webrtc::PayloadTypeSuggester* pt_suggester);
 
   const Codecs& audio_sendrecv_codecs() const;
   const Codecs& audio_send_codecs() const;
@@ -318,6 +320,8 @@ class MediaSessionDescriptionFactory {
       ssrc_generator_;
   bool enable_encrypted_rtp_header_extensions_ = false;
   const TransportDescriptionFactory* transport_desc_factory_;
+  // Payoad type tracker interface. Must live longer than this object.
+  webrtc::PayloadTypeSuggester* pt_suggester_;
 };
 
 // Convenience functions.

pc/media_session_unittest.cc

@@ -12,7 +12,6 @@
 
 #include <stddef.h>
 
-#include <algorithm>
 #include <cstdint>
 #include <map>
 #include <memory>
@@ -24,24 +23,31 @@
 #include "absl/algorithm/container.h"
 #include "absl/strings/match.h"
 #include "absl/strings/string_view.h"
+#include "api/audio_codecs/audio_format.h"
 #include "api/candidate.h"
+#include "api/media_types.h"
 #include "api/rtp_parameters.h"
+#include "api/rtp_transceiver_direction.h"
 #include "media/base/codec.h"
 #include "media/base/media_constants.h"
+#include "media/base/rid_description.h"
+#include "media/base/stream_params.h"
 #include "media/base/test_utils.h"
 #include "media/sctp/sctp_transport_internal.h"
 #include "p2p/base/p2p_constants.h"
 #include "p2p/base/transport_description.h"
+#include "p2p/base/transport_description_factory.h"
 #include "p2p/base/transport_info.h"
 #include "pc/media_protocol_names.h"
 #include "pc/rtp_media_utils.h"
 #include "pc/rtp_parameters_conversion.h"
-#include "rtc_base/arraysize.h"
+#include "pc/session_description.h"
+#include "pc/simulcast_description.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/fake_ssl_identity.h"
+#include "rtc_base/logging.h"
 #include "rtc_base/rtc_certificate.h"
 #include "rtc_base/ssl_identity.h"
-#include "rtc_base/ssl_stream_adapter.h"
 #include "rtc_base/string_encode.h"
 #include "rtc_base/strings/string_builder.h"
 #include "rtc_base/unique_id_generator.h"
@@ -382,8 +388,8 @@ class MediaSessionDescriptionFactoryTest : public testing::Test {
   MediaSessionDescriptionFactoryTest()
       : tdf1_(field_trials),
         tdf2_(field_trials),
-        f1_(nullptr, false, &ssrc_generator1, &tdf1_),
+        f1_(nullptr, false, &ssrc_generator1, &tdf1_, nullptr),
-        f2_(nullptr, false, &ssrc_generator2, &tdf2_) {
+        f2_(nullptr, false, &ssrc_generator2, &tdf2_, nullptr) {
     f1_.set_audio_codecs(MAKE_VECTOR(kAudioCodecs1),
                          MAKE_VECTOR(kAudioCodecs1));
     f1_.set_video_codecs(MAKE_VECTOR(kVideoCodecs1),
@@ -742,12 +748,14 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateOfferWithCustomCodecs) {
 
   webrtc::SdpAudioFormat audio_format("custom-audio", 8000, 2);
   Codec custom_audio_codec = CreateAudioCodec(audio_format);
+  custom_audio_codec.id = 123;  // picked at random, but valid
   auto audio_options = MediaDescriptionOptions(
       MEDIA_TYPE_AUDIO, "0", RtpTransceiverDirection::kSendRecv, kActive);
   audio_options.codecs_to_include.push_back(custom_audio_codec);
   opts.media_description_options.push_back(audio_options);
 
   Codec custom_video_codec = CreateVideoCodec("custom-video");
+  custom_video_codec.id = 124;  // picked at random, but valid
   auto video_options = MediaDescriptionOptions(
       MEDIA_TYPE_VIDEO, "1", RtpTransceiverDirection::kSendRecv, kActive);
   video_options.codecs_to_include.push_back(custom_video_codec);
@@ -769,6 +777,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateOfferWithCustomCodecs) {
   // Fields in codec are set during the gen process, so simple compare
   // does not work.
   EXPECT_EQ(acd->codecs()[0].name, custom_audio_codec.name);
+  RTC_LOG(LS_ERROR) << "DEBUG: audio PT assigned is " << acd->codecs()[0].id;
 
   EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type());
   ASSERT_EQ(vcd->codecs().size(), 1U);
@@ -786,12 +795,14 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAnswerWithCustomCodecs) {
   // on the caller, not on this function.
   webrtc::SdpAudioFormat audio_format("custom-audio", 8000, 2);
   Codec custom_audio_codec = CreateAudioCodec(audio_format);
+  custom_audio_codec.id = 123;  // picked at random, but valid
   auto audio_options = MediaDescriptionOptions(
       MEDIA_TYPE_AUDIO, "audio", RtpTransceiverDirection::kSendRecv, kActive);
   audio_options.codecs_to_include.push_back(custom_audio_codec);
   answer_opts.media_description_options.push_back(audio_options);
 
   Codec custom_video_codec = CreateVideoCodec("custom-video");
+  custom_video_codec.id = 124;
   auto video_options = MediaDescriptionOptions(
       MEDIA_TYPE_VIDEO, "video", RtpTransceiverDirection::kSendRecv, kActive);
   video_options.codecs_to_include.push_back(custom_video_codec);
@@ -4395,8 +4406,8 @@ class MediaProtocolTest : public testing::TestWithParam<const char*> {
   MediaProtocolTest()
       : tdf1_(field_trials_),
         tdf2_(field_trials_),
-        f1_(nullptr, false, &ssrc_generator1, &tdf1_),
+        f1_(nullptr, false, &ssrc_generator1, &tdf1_, nullptr),
-        f2_(nullptr, false, &ssrc_generator2, &tdf2_) {
+        f2_(nullptr, false, &ssrc_generator2, &tdf2_, nullptr) {
     f1_.set_audio_codecs(MAKE_VECTOR(kAudioCodecs1),
                          MAKE_VECTOR(kAudioCodecs1));
     f1_.set_video_codecs(MAKE_VECTOR(kVideoCodecs1),
@@ -4459,7 +4470,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestSetAudioCodecs) {
       std::unique_ptr<rtc::SSLIdentity>(new rtc::FakeSSLIdentity("id"))));
 
   UniqueRandomIdGenerator ssrc_generator;
-  MediaSessionDescriptionFactory sf(nullptr, false, &ssrc_generator, &tdf);
+  MediaSessionDescriptionFactory sf(nullptr, false, &ssrc_generator, &tdf,
+                                    nullptr);
   std::vector<Codec> send_codecs = MAKE_VECTOR(kAudioCodecs1);
   std::vector<Codec> recv_codecs = MAKE_VECTOR(kAudioCodecs2);
 
@@ -4530,7 +4542,8 @@ void TestAudioCodecsOffer(RtpTransceiverDirection direction) {
       std::unique_ptr<rtc::SSLIdentity>(new rtc::FakeSSLIdentity("id"))));
 
   UniqueRandomIdGenerator ssrc_generator;
-  MediaSessionDescriptionFactory sf(nullptr, false, &ssrc_generator, &tdf);
+  MediaSessionDescriptionFactory sf(nullptr, false, &ssrc_generator, &tdf,
+                                    nullptr);
   const std::vector<Codec> send_codecs = MAKE_VECTOR(kAudioCodecs1);
   const std::vector<Codec> recv_codecs = MAKE_VECTOR(kAudioCodecs2);
   const std::vector<Codec> sendrecv_codecs = MAKE_VECTOR(kAudioCodecsAnswer);
@@ -4634,9 +4647,9 @@ void TestAudioCodecsAnswer(RtpTransceiverDirection offer_direction,
           new rtc::FakeSSLIdentity("answer_id"))));
   UniqueRandomIdGenerator ssrc_generator1, ssrc_generator2;
   MediaSessionDescriptionFactory offer_factory(nullptr, false, &ssrc_generator1,
-                                               &offer_tdf);
+                                               &offer_tdf, nullptr);
-  MediaSessionDescriptionFactory answer_factory(nullptr, false,
+  MediaSessionDescriptionFactory answer_factory(
-                                                &ssrc_generator2, &answer_tdf);
+      nullptr, false, &ssrc_generator2, &answer_tdf, nullptr);
 
   offer_factory.set_audio_codecs(
       VectorFromIndices(kOfferAnswerCodecs, kOfferSendCodecs),

pc/peer_connection.cc

@@ -764,8 +764,9 @@ RTCError PeerConnection::Initialize(
   legacy_stats_ = std::make_unique<LegacyStatsCollector>(this);
   stats_collector_ = RTCStatsCollector::Create(this);
 
-  sdp_handler_ = SdpOfferAnswerHandler::Create(this, configuration,
+  sdp_handler_ =
-                                               dependencies, context_.get());
+      SdpOfferAnswerHandler::Create(this, configuration, dependencies,
+                                    context_.get(), transport_controller_copy_);
 
   rtp_manager_ = std::make_unique<RtpTransmissionManager>(
       env_, IsUnifiedPlan(), context_.get(), &usage_pattern_, observer_,

pc/sdp_offer_answer.cc

@@ -50,6 +50,7 @@
 #include "api/uma_metrics.h"
 #include "api/video/builtin_video_bitrate_allocator_factory.h"
 #include "api/video/video_codec_constants.h"
+#include "call/payload_type.h"
 #include "media/base/codec.h"
 #include "media/base/media_engine.h"
 #include "media/base/rid_description.h"
@@ -1379,16 +1380,18 @@ std::unique_ptr<SdpOfferAnswerHandler> SdpOfferAnswerHandler::Create(
     PeerConnectionSdpMethods* pc,
     const PeerConnectionInterface::RTCConfiguration& configuration,
     PeerConnectionDependencies& dependencies,
-    ConnectionContext* context) {
+    ConnectionContext* context,
+    PayloadTypeSuggester* pt_suggester) {
   auto handler = absl::WrapUnique(new SdpOfferAnswerHandler(pc, context));
-  handler->Initialize(configuration, dependencies, context);
+  handler->Initialize(configuration, dependencies, context, pt_suggester);
   return handler;
 }
 
 void SdpOfferAnswerHandler::Initialize(
     const PeerConnectionInterface::RTCConfiguration& configuration,
     PeerConnectionDependencies& dependencies,
-    ConnectionContext* context) {
+    ConnectionContext* context,
+    PayloadTypeSuggester* pt_suggester) {
   RTC_DCHECK_RUN_ON(signaling_thread());
   // 100 kbps is used by default, but can be overriden by a non-standard
   // RTCConfiguration value (not available on Web).
@@ -1421,7 +1424,7 @@ void SdpOfferAnswerHandler::Initialize(
             RTC_DCHECK_RUN_ON(signaling_thread());
             transport_controller_s()->SetLocalCertificate(certificate);
           },
-          pc_->trials());
+          pt_suggester, pc_->trials());
 
   if (pc_->options()->disable_encryption) {
     RTC_LOG(LS_INFO)

pc/sdp_offer_answer.h

@@ -38,6 +38,7 @@
 #include "api/set_remote_description_observer_interface.h"
 #include "api/uma_metrics.h"
 #include "api/video/video_bitrate_allocator_factory.h"
+#include "call/payload_type.h"
 #include "media/base/media_channel.h"
 #include "media/base/stream_params.h"
 #include "p2p/base/port_allocator.h"
@@ -80,7 +81,8 @@ class SdpOfferAnswerHandler : public SdpStateProvider {
       PeerConnectionSdpMethods* pc,
       const PeerConnectionInterface::RTCConfiguration& configuration,
       PeerConnectionDependencies& dependencies,
-      ConnectionContext* context);
+      ConnectionContext* context,
+      PayloadTypeSuggester* pt_suggester);
 
   void ResetSessionDescFactory() {
     RTC_DCHECK_RUN_ON(signaling_thread());
@@ -210,7 +212,8 @@ class SdpOfferAnswerHandler : public SdpStateProvider {
   void Initialize(
       const PeerConnectionInterface::RTCConfiguration& configuration,
       PeerConnectionDependencies& dependencies,
-      ConnectionContext* context);
+      ConnectionContext* context,
+      PayloadTypeSuggester* pt_suggester);
 
   rtc::Thread* signaling_thread() const;
   rtc::Thread* network_thread() const;

pc/webrtc_session_description_factory.cc

@@ -12,23 +12,33 @@
 
 #include <stddef.h>
 
+#include <cstdint>
+#include <functional>
+#include <memory>
 #include <optional>
 #include <queue>
 #include <string>
-#include <type_traits>
 #include <utility>
 #include <vector>
 
 #include "absl/algorithm/container.h"
+#include "absl/functional/any_invocable.h"
+#include "api/field_trials_view.h"
 #include "api/jsep.h"
 #include "api/jsep_session_description.h"
+#include "api/peer_connection_interface.h"
 #include "api/rtc_error.h"
+#include "api/scoped_refptr.h"
 #include "api/sequence_checker.h"
+#include "call/payload_type.h"
 #include "pc/connection_context.h"
+#include "pc/media_session.h"
 #include "pc/sdp_state_provider.h"
 #include "pc/session_description.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/logging.h"
+#include "rtc_base/rtc_certificate.h"
+#include "rtc_base/rtc_certificate_generator.h"
 #include "rtc_base/ssl_identity.h"
 #include "rtc_base/ssl_stream_adapter.h"
 #include "rtc_base/string_encode.h"
@@ -108,13 +118,15 @@ WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory(
     rtc::scoped_refptr<rtc::RTCCertificate> certificate,
     std::function<void(const rtc::scoped_refptr<rtc::RTCCertificate>&)>
         on_certificate_ready,
+    PayloadTypeSuggester* pt_suggester,
     const FieldTrialsView& field_trials)
     : signaling_thread_(context->signaling_thread()),
       transport_desc_factory_(field_trials),
       session_desc_factory_(context->media_engine(),
                             context->use_rtx(),
                             context->ssrc_generator(),
-                            &transport_desc_factory_),
+                            &transport_desc_factory_,
+                            pt_suggester),
       // RFC 4566 suggested a Network Time Protocol (NTP) format timestamp
       // as the session id and session version. To simplify, it should be fine
       // to just use a random number as session id and start version from

pc/webrtc_session_description_factory.h

@@ -19,17 +19,18 @@
 #include <string>
 
 #include "absl/functional/any_invocable.h"
+#include "api/field_trials_view.h"
 #include "api/jsep.h"
 #include "api/peer_connection_interface.h"
+#include "api/rtc_error.h"
 #include "api/scoped_refptr.h"
 #include "api/task_queue/task_queue_base.h"
-#include "p2p/base/transport_description.h"
+#include "call/payload_type.h"
 #include "p2p/base/transport_description_factory.h"
 #include "pc/media_session.h"
 #include "pc/sdp_state_provider.h"
 #include "rtc_base/rtc_certificate.h"
 #include "rtc_base/rtc_certificate_generator.h"
-#include "rtc_base/unique_id_generator.h"
 #include "rtc_base/weak_ptr.h"
 
 namespace webrtc {
@@ -53,6 +54,7 @@ class WebRtcSessionDescriptionFactory {
       rtc::scoped_refptr<rtc::RTCCertificate> certificate,
       std::function<void(const rtc::scoped_refptr<rtc::RTCCertificate>&)>
           on_certificate_ready,
+      PayloadTypeSuggester* pt_suggester,
       const FieldTrialsView& field_trials);
   ~WebRtcSessionDescriptionFactory();