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}
diff --git a/pc/BUILD.gn b/pc/BUILD.gn
index 9a7a783..bbd339a 100644
--- a/pc/BUILD.gn
+++ b/pc/BUILD.gn
@@ -372,6 +372,7 @@
"../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 @@
"../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 @@
":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 @@
":rtp_transport_internal",
":sctp_transport",
":session_description",
+ ":simulcast_description",
":srtp_session",
":srtp_transport",
":used_ids",
@@ -2008,8 +2013,10 @@
"../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 @@
"../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 @@
"../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",
diff --git a/pc/media_session.cc b/pc/media_session.cc
index 9287155..50c4260 100644
--- a/pc/media_session.cc
+++ b/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 @@
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 @@
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 @@
// 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 @@
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,
diff --git a/pc/media_session.h b/pc/media_session.h
index 33c44de..3228b4f 100644
--- a/pc/media_session.h
+++ b/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 @@
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
- // owned by MediaSessionDescriptionFactory, so they must be kept alive by the
- // user of this class.
+ // The TransportDescriptionFactory, the UniqueRandomIdGenerator, and the
+ // PayloadTypeSuggester are not owned by MediaSessionDescriptionFactory, so
+ // 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 @@
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.
diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc
index 4ac9a5e..ce81d6c 100644
--- a/pc/media_session_unittest.cc
+++ b/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 @@
MediaSessionDescriptionFactoryTest()
: tdf1_(field_trials),
tdf2_(field_trials),
- f1_(nullptr, false, &ssrc_generator1, &tdf1_),
- f2_(nullptr, false, &ssrc_generator2, &tdf2_) {
+ f1_(nullptr, false, &ssrc_generator1, &tdf1_, nullptr),
+ 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 @@
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 @@
// 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 @@
// 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 @@
MediaProtocolTest()
: tdf1_(field_trials_),
tdf2_(field_trials_),
- f1_(nullptr, false, &ssrc_generator1, &tdf1_),
- f2_(nullptr, false, &ssrc_generator2, &tdf2_) {
+ f1_(nullptr, false, &ssrc_generator1, &tdf1_, nullptr),
+ 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 @@
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 @@
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 @@
new rtc::FakeSSLIdentity("answer_id"))));
UniqueRandomIdGenerator ssrc_generator1, ssrc_generator2;
MediaSessionDescriptionFactory offer_factory(nullptr, false, &ssrc_generator1,
- &offer_tdf);
- MediaSessionDescriptionFactory answer_factory(nullptr, false,
- &ssrc_generator2, &answer_tdf);
+ &offer_tdf, nullptr);
+ MediaSessionDescriptionFactory answer_factory(
+ nullptr, false, &ssrc_generator2, &answer_tdf, nullptr);
offer_factory.set_audio_codecs(
VectorFromIndices(kOfferAnswerCodecs, kOfferSendCodecs),
diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index cbf5a68..c4546a9 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -764,8 +764,9 @@
legacy_stats_ = std::make_unique<LegacyStatsCollector>(this);
stats_collector_ = RTCStatsCollector::Create(this);
- sdp_handler_ = SdpOfferAnswerHandler::Create(this, configuration,
- dependencies, context_.get());
+ sdp_handler_ =
+ SdpOfferAnswerHandler::Create(this, configuration, dependencies,
+ context_.get(), transport_controller_copy_);
rtp_manager_ = std::make_unique<RtpTransmissionManager>(
env_, IsUnifiedPlan(), context_.get(), &usage_pattern_, observer_,
diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index 5a50867..9d49296 100644
--- a/pc/sdp_offer_answer.cc
+++ b/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 @@
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 @@
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)
diff --git a/pc/sdp_offer_answer.h b/pc/sdp_offer_answer.h
index 13492b6..793f2c9 100644
--- a/pc/sdp_offer_answer.h
+++ b/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 @@
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 @@
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;
diff --git a/pc/webrtc_session_description_factory.cc b/pc/webrtc_session_description_factory.cc
index 236ec74..8b32191 100644
--- a/pc/webrtc_session_description_factory.cc
+++ b/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 @@
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
diff --git a/pc/webrtc_session_description_factory.h b/pc/webrtc_session_description_factory.h
index eed922e..6c468ac 100644
--- a/pc/webrtc_session_description_factory.h
+++ b/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 @@
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();
