Reland "Make RtpHeaderExtensionId(int) explicit" This reverts commit 031d798a20cc5d9c867598a38632accb20c6effe. Reason for revert: Revised constructor deprecation approach Original change's description: > Revert "Make RtpHeaderExtensionId(int) explicit" > > This reverts commit 9cb2a61b06bacaeb9a7abedc1d00517d7f79548d. > > Reason for revert: Suspected downstream failures. > > Failure Link: https://fusion2.corp.google.com/invocations/abc66f5c-f5bc-4767-8ff8-e67efeaa0f7c/targets/%2F%2Fbuzz%2Ftest%2Fpclf%2Ftests:absolute_capture_time_test_experiments_launched;config=a544fae14671853db631fbeeb923d84e665cd2045db88e266d17af360c6b5925/tests?q=status:failed%20OR%20status:broken%20OR%20status:cancelled%20OR%20status:running%20OR%20status:timed_out%20OR%20status:tool_failure%20label:buildbucket-8678846321400548705 > > Original change's description: > > Make RtpHeaderExtensionId(int) explicit > > > > Make RtpHeaderExtensionId(int) constructor explicit to prevent > > implicit conversions. Added a deprecated template constructor to > > allow implicit conversion downstream to avoid breaking them. Also > > added explicit constructor for uint8_t to avoid matching the template > > in WebRTC. > > > > Bug: webrtc:514817938 > > Change-Id: I5f8a03dc2ffb6d6d76164e734e1627f07727da49 > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/482540 > > Commit-Queue: Harald Alvestrand <hta@webrtc.org> > > Reviewed-by: Danil Chapovalov <danilchap@webrtc.org> > > Cr-Commit-Position: refs/heads/main@{#48017} > > Bug: webrtc:514817938 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Change-Id: I367b804a5f3af448f1ce49b8f178a079edab1612 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/482800 > Auto-Submit: Harald Alvestrand <hta@webrtc.org> > Commit-Queue: Jeremy Leconte <jleconte@google.com> > Reviewed-by: Jeremy Leconte <jleconte@google.com> > Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com> > Reviewed-by: Danil Chapovalov <danilchap@webrtc.org> > Commit-Queue: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com> > Cr-Commit-Position: refs/heads/main@{#48018} Bug: webrtc:514817938 Change-Id: I476b96e2bdf869a27abe5dd46f4f67693d82d5cd Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/482820 Commit-Queue: Harald Alvestrand <hta@webrtc.org> Reviewed-by: Danil Chapovalov <danilchap@webrtc.org> Cr-Commit-Position: refs/heads/main@{#48037}
diff --git a/api/BUILD.gn b/api/BUILD.gn index ba45f58..0b3de97 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn
@@ -1750,6 +1750,7 @@ "peer_connection_interface_unittest.cc", "rtc_error_unittest.cc", "rtc_event_log_output_file_unittest.cc", + "rtp_header_extension_id_unittest.cc", "rtp_packet_info_unittest.cc", "rtp_packet_infos_unittest.cc", "rtp_parameters_unittest.cc",
diff --git a/api/rtp_header_extension_id.h b/api/rtp_header_extension_id.h index 740adb3..731df9a 100644 --- a/api/rtp_header_extension_id.h +++ b/api/rtp_header_extension_id.h
@@ -11,6 +11,9 @@ #ifndef API_RTP_HEADER_EXTENSION_ID_H_ #define API_RTP_HEADER_EXTENSION_ID_H_ +#include <concepts> +#include <type_traits> + #include "absl/base/macros.h" #include "absl/strings/str_format.h" #include "rtc_base/strong_alias.h" @@ -34,21 +37,50 @@ // Factory function for the NotSet value. static constexpr RtpHeaderExtensionId NotSet() { - return RtpHeaderExtensionId(Internal{}, 0); + return RtpHeaderExtensionId(); } // The default constructor makes a NotSet. constexpr RtpHeaderExtensionId() : StrongAlias(0) {} - // Implicit conversion from and to int, required for downstream - // during conversion. - // TODO: bugs.webrtc.org/514817938 - make explicit when downstream fixed. - constexpr RtpHeaderExtensionId(int id) // NOLINT: explicit - : StrongAlias(id) { - // TODO: bugs.webrtc.org/514817938 - enable these checks when tests fixed. - // RTC_DCHECK_GE(id, kMinId.value()); - // RTC_DCHECK_LE(id, kMaxId.value()); - } - // TODO: bugs.webrtc.org/514817938 - RTC_DCHECK(id is valid). + // This constructor is finagled via templates to allow declaring + // both an explicit and an implicit variant, deprecating the + // implicit one. When it is no longer needed, it should just be: + // explicit constexpr RtpHeaderExtensionId(int id) + // : StrongAlias(id) { + // TODO: bugs.webrtc.org/514817938 - enable these checks when tests fixed. + // RTC_DCHECK_GE(id, kMinId.value()); + // RTC_DCHECK_LE(id, kMaxId.value()); + // } + template <typename T> + requires std::is_integral_v<T> && std::convertible_to<T, int> + explicit constexpr RtpHeaderExtensionId(T id) + : StrongAlias(static_cast<int>(id)) {} + + template <typename T> + requires std::is_enum_v<T> && std::convertible_to<T, int> + explicit constexpr RtpHeaderExtensionId(T id) + : StrongAlias(static_cast<int>(id)) {} + + template <typename T> + requires std::is_enum_v<T> && (!std::convertible_to<T, int>) + explicit constexpr RtpHeaderExtensionId(T id) + : StrongAlias(static_cast<int>(id)) {} + + template <typename T> + requires std::convertible_to<T, int> && (!std::is_integral_v<T>) && + (!std::is_enum_v<T>) + explicit constexpr RtpHeaderExtensionId(T id) + : StrongAlias(static_cast<int>(id)) {} + + template <typename T = void> + requires std::convertible_to<T, int> + [[deprecated("Use explicit conversion")]] + constexpr RtpHeaderExtensionId(T id) // NOLINT: explicit + : StrongAlias(static_cast<int>(id)) {} + + // Deprecated operator to allow implicit conversion to int in + // downstream code. + // TODO: bugs.webrtc.org/514817938 - remove when downstream fixed. [[deprecated]] ABSL_REFACTOR_INLINE // constexpr operator int() const& { // NOLINT: explicit @@ -66,20 +98,15 @@ friend void AbslStringify(Sink& sink, RtpHeaderExtensionId id) { absl::Format(&sink, "%d", id.value()); } - - private: - class Internal {}; - explicit constexpr RtpHeaderExtensionId(Internal tag, int id) - : StrongAlias(id) {} }; inline constexpr RtpHeaderExtensionId RtpHeaderExtensionId::kMinId = - RtpHeaderExtensionId(Internal{}, 1); + RtpHeaderExtensionId(1); inline constexpr RtpHeaderExtensionId RtpHeaderExtensionId::kMaxId = - RtpHeaderExtensionId(Internal{}, 255); + RtpHeaderExtensionId(255); inline constexpr RtpHeaderExtensionId RtpHeaderExtensionId::kOneByteHeaderExtensionMaxId = - RtpHeaderExtensionId(Internal{}, 14); + RtpHeaderExtensionId(14); } // namespace webrtc
diff --git a/api/rtp_header_extension_id_unittest.cc b/api/rtp_header_extension_id_unittest.cc new file mode 100644 index 0000000..70edea6 --- /dev/null +++ b/api/rtp_header_extension_id_unittest.cc
@@ -0,0 +1,58 @@ +/* + * Copyright (c) 2026 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "api/rtp_header_extension_id.h" + +#include <cstdint> + +#include "test/gtest.h" + +namespace webrtc { +namespace { + +enum Unscoped { kUnscopedVal = 1 }; +enum class Scoped { kScopedVal = 1 }; + +class ConvertTo { + public: + operator int() const { return 5; } +}; + +TEST(RtpHeaderExtensionId, AppropriateConstructorsChosen) { + // All these should compile without warnings (errors). + RtpHeaderExtensionId value; // default constructor + (void)value; + RtpHeaderExtensionId t1(5); // Explicit int + RtpHeaderExtensionId t2(uint8_t{5}); // Explicit uint8_t + RtpHeaderExtensionId t3(kUnscopedVal); // Explicit unscoped enum + RtpHeaderExtensionId t4(Scoped::kScopedVal); // Explicit scoped enum + ConvertTo c; + RtpHeaderExtensionId t5(c); // Explicit other convertible + + (void)t1; + (void)t2; + (void)t3; + (void)t4; + (void)t5; + + // These should compile when deprecation warnings are ignored. +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" + RtpHeaderExtensionId t6 = 5; // Implicit int + RtpHeaderExtensionId t7 = kUnscopedVal; // Implicit unscoped enum + RtpHeaderExtensionId t8 = c; // Implicit other convertible +#pragma clang diagnostic pop + (void)t6; + (void)t7; + (void)t8; +} + +} // namespace +} // namespace webrtc
diff --git a/call/payload_type_picker.cc b/call/payload_type_picker.cc index 45105a10..daaa921 100644 --- a/call/payload_type_picker.cc +++ b/call/payload_type_picker.cc
@@ -448,7 +448,8 @@ // We prefer to allocate from the top of the range (14 down to 1). for (RtpHeaderExtensionId id = RtpHeaderExtensionId::kOneByteHeaderExtensionMaxId; - id >= RtpHeaderExtensionId::kMinId; id = id.value() - 1) { + id >= RtpHeaderExtensionId::kMinId; + id = RtpHeaderExtensionId(id.value() - 1)) { if (!seen_ids_.contains(id)) { AddMapping(id, uri, encrypt); return id;
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc index 39cd0c2..a93efc0 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc
@@ -87,13 +87,9 @@ constexpr int kHeight = 100; constexpr int kCaptureTimeMsToRtpTimestamp = 90; // 90 kHz clock. constexpr TimeDelta kDefaultReportInterval = TimeDelta::Millis(1000); - -// RTP header extension ids. -enum : int { - kAbsoluteSendTimeExtensionId = 1, - kTransportSequenceNumberExtensionId, - kTransmissionOffsetExtensionId, -}; +constexpr RtpHeaderExtensionId kAbsoluteSendTimeExtensionId(1); +constexpr RtpHeaderExtensionId kTransportSequenceNumberExtensionId(2); +constexpr RtpHeaderExtensionId kTransmissionOffsetExtensionId(3); class RtcpRttStatsTestImpl : public RtcpRttStats { public: