Use new RtpHeaderExtensionCapability constructors Use constructors that omit preferred_id where it is not needed (tests) or represents 0/unset ID. Update wrappers and PeerConnectionFactory to handle capabilities without preferred IDs. Remove old ToRtpCapabilities overload. Bug: webrtc:514817938 Change-Id: Iced03d2071b99e6bf094f5bf566d8bd2eee72bbc Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/478920 Reviewed-by: Danil Chapovalov <danilchap@webrtc.org> Auto-Submit: Harald Alvestrand <hta@webrtc.org> Commit-Queue: Harald Alvestrand <hta@webrtc.org> Cr-Commit-Position: refs/heads/main@{#47926}
diff --git a/api/rtp_parameters.cc b/api/rtp_parameters.cc index 35f15db..f274342 100644 --- a/api/rtp_parameters.cc +++ b/api/rtp_parameters.cc
@@ -148,20 +148,33 @@ : uri(uri) {} RtpHeaderExtensionCapability::RtpHeaderExtensionCapability( absl::string_view uri, + RtpTransceiverDirection direction) + : uri(uri), direction(direction) {} +RtpHeaderExtensionCapability::RtpHeaderExtensionCapability( + absl::string_view uri, + bool preferred_encrypt, + RtpTransceiverDirection direction) + : uri(uri), preferred_encrypt(preferred_encrypt), direction(direction) {} +RtpHeaderExtensionCapability::RtpHeaderExtensionCapability( + absl::string_view uri, RtpHeaderExtensionId preferred_id) : uri(uri), preferred_id(preferred_id) {} RtpHeaderExtensionCapability::RtpHeaderExtensionCapability( absl::string_view uri, RtpHeaderExtensionId preferred_id, RtpTransceiverDirection direction) - : uri(uri), preferred_id(preferred_id), direction(direction) {} + : uri(uri), + preferred_id(preferred_id.IsSet() ? std::optional(preferred_id) + : std::nullopt), + direction(direction) {} RtpHeaderExtensionCapability::RtpHeaderExtensionCapability( absl::string_view uri, RtpHeaderExtensionId preferred_id, bool preferred_encrypt, RtpTransceiverDirection direction) : uri(uri), - preferred_id(preferred_id), + preferred_id(preferred_id.IsSet() ? std::optional(preferred_id) + : std::nullopt), preferred_encrypt(preferred_encrypt), direction(direction) {}
diff --git a/api/rtp_parameters.h b/api/rtp_parameters.h index 4e04cbe..17bc423 100644 --- a/api/rtp_parameters.h +++ b/api/rtp_parameters.h
@@ -305,6 +305,11 @@ RtpHeaderExtensionCapability(); explicit RtpHeaderExtensionCapability(absl::string_view uri); RtpHeaderExtensionCapability(absl::string_view uri, + RtpTransceiverDirection direction); + RtpHeaderExtensionCapability(absl::string_view uri, + bool preferred_encrypt, + RtpTransceiverDirection direction); + RtpHeaderExtensionCapability(absl::string_view uri, RtpHeaderExtensionId preferred_id); RtpHeaderExtensionCapability(absl::string_view uri, RtpHeaderExtensionId preferred_id, @@ -315,6 +320,9 @@ RtpTransceiverDirection direction); // Backwards compatibility overloads. // TODO: bugs.webrtc.org/514817938 - Remove when downstream is updated. + // Note: the "uri, preferred id(int), direction" cannot be overloaded + // because compilers can't tell the difference between that one + // and "uri, preferred_encrypt(bool), direction". [[deprecated]] ABSL_REFACTOR_INLINE RtpHeaderExtensionCapability( absl::string_view uri, int preferred_id) @@ -322,13 +330,6 @@ [[deprecated]] ABSL_REFACTOR_INLINE RtpHeaderExtensionCapability( absl::string_view uri, int preferred_id, - RtpTransceiverDirection direction) - : RtpHeaderExtensionCapability(uri, - RtpHeaderExtensionId(preferred_id), - direction) {} - [[deprecated]] ABSL_REFACTOR_INLINE RtpHeaderExtensionCapability( - absl::string_view uri, - int preferred_id, bool preferred_encrypt, RtpTransceiverDirection direction) : RtpHeaderExtensionCapability(uri,
diff --git a/media/BUILD.gn b/media/BUILD.gn index 5556284..a5c97aa 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn
@@ -239,6 +239,7 @@ "../api:audio_options_api", "../api:field_trials_view", "../api:rtc_error", + "../api:rtp_header_extension_id", "../api:rtp_headers", "../api:rtp_parameters", "../api:rtp_transceiver_direction",
diff --git a/media/base/media_engine.cc b/media/base/media_engine.cc index 9840b71..5fa4529 100644 --- a/media/base/media_engine.cc +++ b/media/base/media_engine.cc
@@ -22,6 +22,7 @@ #include "absl/algorithm/container.h" #include "api/field_trials_view.h" #include "api/rtc_error.h" +#include "api/rtp_header_extension_id.h" #include "api/rtp_headers.h" #include "api/rtp_parameters.h" #include "api/rtp_transceiver_direction.h" @@ -78,14 +79,28 @@ return parameters; } +std::vector<RtpHeaderExtensionCapability> +GetDefaultEnabledRtpHeaderCapabilities( + const RtpHeaderExtensionQueryInterface& query_interface, + const FieldTrialsView* field_trials) { + std::vector<RtpHeaderExtensionCapability> extensions; + for (const RtpHeaderExtensionCapability& entry : + query_interface.GetRtpHeaderExtensions(field_trials)) { + if (entry.direction != RtpTransceiverDirection::kStopped) { + extensions.push_back(entry); + } + } + return extensions; +} + std::vector<RtpExtension> GetDefaultEnabledRtpHeaderExtensions( const RtpHeaderExtensionQueryInterface& query_interface, const FieldTrialsView* field_trials) { std::vector<RtpExtension> extensions; - for (const auto& entry : - query_interface.GetRtpHeaderExtensions(field_trials)) { - if (entry.direction != RtpTransceiverDirection::kStopped) - extensions.emplace_back(entry.uri, *entry.preferred_id); + for (const RtpHeaderExtensionCapability& entry : + GetDefaultEnabledRtpHeaderCapabilities(query_interface, field_trials)) { + extensions.emplace_back( + entry.uri, entry.preferred_id.value_or(RtpHeaderExtensionId::NotSet())); } return extensions; }
diff --git a/media/base/media_engine.h b/media/base/media_engine.h index 7b23c85..c5abbe3 100644 --- a/media/base/media_engine.h +++ b/media/base/media_engine.h
@@ -313,6 +313,11 @@ const RtpHeaderExtensionQueryInterface& query_interface, const FieldTrialsView* field_trials); +std::vector<RtpHeaderExtensionCapability> +GetDefaultEnabledRtpHeaderCapabilities( + const RtpHeaderExtensionQueryInterface& query_interface, + const FieldTrialsView* field_trials); + } // namespace webrtc
diff --git a/pc/peer_connection_factory.cc b/pc/peer_connection_factory.cc index 94c08d5..5c7c50e 100644 --- a/pc/peer_connection_factory.cc +++ b/pc/peer_connection_factory.cc
@@ -147,15 +147,17 @@ case MediaType::AUDIO: { Codecs cricket_codecs; cricket_codecs = codec_vendor_.audio_send_codecs().codecs(); - auto extensions = GetDefaultEnabledRtpHeaderExtensions( - media_engine()->voice(), /* field_trials= */ nullptr); + std::vector<RtpHeaderExtensionCapability> extensions = + GetDefaultEnabledRtpHeaderCapabilities(media_engine()->voice(), + /* field_trials= */ nullptr); return ToRtpCapabilities(cricket_codecs, extensions); } case MediaType::VIDEO: { Codecs cricket_codecs; cricket_codecs = codec_vendor_.video_send_codecs().codecs(); - auto extensions = GetDefaultEnabledRtpHeaderExtensions( - media_engine()->video(), /* field_trials= */ nullptr); + std::vector<RtpHeaderExtensionCapability> extensions = + GetDefaultEnabledRtpHeaderCapabilities(media_engine()->video(), + /* field_trials= */ nullptr); return ToRtpCapabilities(cricket_codecs, extensions); } default: @@ -172,14 +174,16 @@ case MediaType::AUDIO: { Codecs cricket_codecs; cricket_codecs = codec_vendor_.audio_recv_codecs().codecs(); - auto extensions = GetDefaultEnabledRtpHeaderExtensions( - media_engine()->voice(), /* field_trials= */ nullptr); + std::vector<RtpHeaderExtensionCapability> extensions = + GetDefaultEnabledRtpHeaderCapabilities(media_engine()->voice(), + /* field_trials= */ nullptr); return ToRtpCapabilities(cricket_codecs, extensions); } case MediaType::VIDEO: { Codecs cricket_codecs = codec_vendor_.video_recv_codecs().codecs(); - auto extensions = GetDefaultEnabledRtpHeaderExtensions( - media_engine()->video(), /* field_trials= */ nullptr); + std::vector<RtpHeaderExtensionCapability> extensions = + GetDefaultEnabledRtpHeaderCapabilities(media_engine()->video(), + /* field_trials= */ nullptr); return ToRtpCapabilities(cricket_codecs, extensions); } default:
diff --git a/pc/rtp_parameters_conversion.cc b/pc/rtp_parameters_conversion.cc index ca19b21..3153b46 100644 --- a/pc/rtp_parameters_conversion.cc +++ b/pc/rtp_parameters_conversion.cc
@@ -11,6 +11,7 @@ #include "pc/rtp_parameters_conversion.h" #include <optional> +#include <span> #include <string> #include <vector> @@ -18,7 +19,6 @@ #include "api/rtp_parameters.h" #include "media/base/codec.h" #include "media/base/media_constants.h" -#include "pc/session_description.h" #include "rtc_base/logging.h" namespace webrtc { @@ -102,9 +102,9 @@ return codec; } -RtpCapabilities ToRtpCapabilities( - const std::vector<Codec>& cricket_codecs, - const RtpHeaderExtensions& cricket_extensions) { +namespace { +RtpCapabilities ToRtpCapabilitiesWithoutExtensions( + const std::vector<Codec>& cricket_codecs) { RtpCapabilities capabilities; bool have_red = false; bool have_ulpfec = false; @@ -138,10 +138,6 @@ } capabilities.codecs.push_back(codec_capability); } - for (const RtpExtension& cricket_extension : cricket_extensions) { - capabilities.header_extensions.emplace_back(cricket_extension.uri, - cricket_extension.id); - } if (have_red) { capabilities.fec.push_back(FecMechanism::RED); } @@ -153,5 +149,15 @@ } return capabilities; } +} // namespace + +RtpCapabilities ToRtpCapabilities( + const std::vector<Codec>& cricket_codecs, + std::span<const RtpHeaderExtensionCapability> extensions) { + RtpCapabilities capabilities = + ToRtpCapabilitiesWithoutExtensions(cricket_codecs); + capabilities.header_extensions.assign(extensions.begin(), extensions.end()); + return capabilities; +} } // namespace webrtc
diff --git a/pc/rtp_parameters_conversion.h b/pc/rtp_parameters_conversion.h index 2b83117..d81ec3a 100644 --- a/pc/rtp_parameters_conversion.h +++ b/pc/rtp_parameters_conversion.h
@@ -12,11 +12,11 @@ #define PC_RTP_PARAMETERS_CONVERSION_H_ #include <optional> +#include <span> #include <vector> #include "api/rtp_parameters.h" #include "media/base/codec.h" -#include "pc/session_description.h" namespace webrtc { @@ -39,7 +39,7 @@ RtpCapabilities ToRtpCapabilities( const std::vector<Codec>& cricket_codecs, - const RtpHeaderExtensions& cricket_extensions); + std::span<const RtpHeaderExtensionCapability> extensions); } // namespace webrtc
diff --git a/pc/rtp_parameters_conversion_unittest.cc b/pc/rtp_parameters_conversion_unittest.cc index 84d55b4..297bdd2 100644 --- a/pc/rtp_parameters_conversion_unittest.cc +++ b/pc/rtp_parameters_conversion_unittest.cc
@@ -13,13 +13,13 @@ #include <map> #include <optional> #include <string> +#include <vector> #include "api/media_types.h" #include "api/rtp_header_extension_id.h" #include "api/rtp_parameters.h" #include "media/base/codec.h" #include "media/base/media_constants.h" -#include "pc/session_description.h" #include "test/gmock.h" #include "test/gtest.h" @@ -152,10 +152,12 @@ Codec rtx = CreateVideoRtxCodec(014, 101); Codec rtx2 = CreateVideoRtxCodec(105, 109); + std::vector<RtpHeaderExtensionCapability> header_extension_caps = { + {RtpHeaderExtensionCapability("uri", RtpHeaderExtensionId(1)), + RtpHeaderExtensionCapability("uri2", RtpHeaderExtensionId(3))}}; + RtpCapabilities capabilities = - ToRtpCapabilities({vp8, ulpfec, rtx, rtx2}, - {RtpExtension("uri", RtpHeaderExtensionId(1)), - RtpExtension("uri2", RtpHeaderExtensionId(3))}); + ToRtpCapabilities({vp8, ulpfec, rtx, rtx2}, header_extension_caps); ASSERT_EQ(3u, capabilities.codecs.size()); EXPECT_EQ("VP8", capabilities.codecs[0].name); EXPECT_EQ("ulpfec", capabilities.codecs[1].name); @@ -170,14 +172,13 @@ capabilities.header_extensions[1].preferred_id); EXPECT_EQ(0u, capabilities.fec.size()); - capabilities = - ToRtpCapabilities({vp8, red, red2, ulpfec, rtx}, RtpHeaderExtensions()); + capabilities = ToRtpCapabilities({vp8, red, red2, ulpfec, rtx}, {}); EXPECT_EQ(4u, capabilities.codecs.size()); EXPECT_THAT( capabilities.fec, UnorderedElementsAre(FecMechanism::RED, FecMechanism::RED_AND_ULPFEC)); - capabilities = ToRtpCapabilities({vp8, red, flexfec}, RtpHeaderExtensions()); + capabilities = ToRtpCapabilities({vp8, red, flexfec}, {}); EXPECT_EQ(3u, capabilities.codecs.size()); EXPECT_THAT(capabilities.fec, UnorderedElementsAre(FecMechanism::RED, FecMechanism::FLEXFEC));
diff --git a/pc/rtp_transceiver_unittest.cc b/pc/rtp_transceiver_unittest.cc index 42e6cb7..1968e77 100644 --- a/pc/rtp_transceiver_unittest.cc +++ b/pc/rtp_transceiver_unittest.cc
@@ -732,16 +732,12 @@ RtpTransceiverTestForHeaderExtensions() : extensions_( {RtpHeaderExtensionCapability("uri1", - RtpHeaderExtensionId(1), RtpTransceiverDirection::kSendOnly), RtpHeaderExtensionCapability("uri2", - RtpHeaderExtensionId(2), RtpTransceiverDirection::kRecvOnly), RtpHeaderExtensionCapability(RtpExtension::kMidUri, - RtpHeaderExtensionId(3), RtpTransceiverDirection::kSendRecv), RtpHeaderExtensionCapability(RtpExtension::kVideoRotationUri, - RtpHeaderExtensionId(4), RtpTransceiverDirection::kSendRecv)}), transceiver_(make_ref_counted<RtpTransceiver>( env(), @@ -1042,9 +1038,9 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, SimulcastOrSvcEnablesExtensionsByDefault) { std::vector<RtpHeaderExtensionCapability> extensions = { - {RtpExtension::kDependencyDescriptorUri, RtpHeaderExtensionId(1), + {RtpExtension::kDependencyDescriptorUri, RtpTransceiverDirection::kStopped}, - {RtpExtension::kVideoLayersAllocationUri, RtpHeaderExtensionId(2), + {RtpExtension::kVideoLayersAllocationUri, RtpTransceiverDirection::kStopped}, };
diff --git a/sdk/android/src/jni/pc/rtp_capabilities.cc b/sdk/android/src/jni/pc/rtp_capabilities.cc index dd87102..d2b1246 100644 --- a/sdk/android/src/jni/pc/rtp_capabilities.cc +++ b/sdk/android/src/jni/pc/rtp_capabilities.cc
@@ -56,15 +56,21 @@ Java_RtpCapabilities_getHeaderExtensions(jni, j_capabilities); for (const JavaRef<jobject>& j_header_extension : Iterable(jni, j_header_extensions)) { - RtpHeaderExtensionCapability header_extension; - header_extension.uri = JavaToStdString( + std::string uri = JavaToStdString( jni, Java_HeaderExtensionCapability_getUri(jni, j_header_extension)); - header_extension.preferred_id = RtpHeaderExtensionId( - Java_HeaderExtensionCapability_getPreferredId(jni, j_header_extension)); - header_extension.preferred_encrypt = + int id = + Java_HeaderExtensionCapability_getPreferredId(jni, j_header_extension); + bool preferred_encrypt = Java_HeaderExtensionCapability_getPreferredEncrypted( jni, j_header_extension); - capabilities.header_extensions.push_back(header_extension); + if (id == 0) { + capabilities.header_extensions.emplace_back( + uri, preferred_encrypt, RtpTransceiverDirection::kSendRecv); + } else { + capabilities.header_extensions.emplace_back( + uri, RtpHeaderExtensionId(id), preferred_encrypt, + RtpTransceiverDirection::kSendRecv); + } } // Convert codecs.
diff --git a/sdk/objc/api/peerconnection/RTCRtpHeaderExtensionCapability.mm b/sdk/objc/api/peerconnection/RTCRtpHeaderExtensionCapability.mm index 7ba028a..ef40aca 100644 --- a/sdk/objc/api/peerconnection/RTCRtpHeaderExtensionCapability.mm +++ b/sdk/objc/api/peerconnection/RTCRtpHeaderExtensionCapability.mm
@@ -55,16 +55,18 @@ } - (webrtc::RtpHeaderExtensionCapability)nativeRtpHeaderExtensionCapability { - webrtc::RtpHeaderExtensionCapability rtpHeaderExtensionCapability; - rtpHeaderExtensionCapability.uri = [NSString stdStringForString:_uri]; - if (_preferredId != nil) { - rtpHeaderExtensionCapability.preferred_id = - std::optional<webrtc::RtpHeaderExtensionId>(_preferredId.intValue); - } - rtpHeaderExtensionCapability.preferred_encrypt = _preferredEncrypted; - rtpHeaderExtensionCapability.direction = [RTC_OBJC_TYPE(RTCRtpTransceiver) + webrtc::RtpTransceiverDirection direction = [RTC_OBJC_TYPE(RTCRtpTransceiver) nativeRtpTransceiverDirectionFromDirection:_direction]; - return rtpHeaderExtensionCapability; + if (_preferredId != nil) { + return webrtc::RtpHeaderExtensionCapability( + [NSString stdStringForString:_uri], + webrtc::RtpHeaderExtensionId(_preferredId.intValue), + _preferredEncrypted, + direction); + } else { + return webrtc::RtpHeaderExtensionCapability( + [NSString stdStringForString:_uri], _preferredEncrypted, direction); + } } @end