use [35,65] rtp payload type range for new codecs
Changes the video payload type allocation to use the
lower dynamic payload type range [35,65] described in
https://www.iana.org/assignments/rtp-parameters/rtp-parameters.xhtml#rtp-parameters-1
for new codecs such as FlexFEC.
BUG=webrtc:12194
Change-Id: I71782199074e0fc94fa6aa8c36afeb68221a2839
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/195822
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32799}
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index 5837899..a7ca967 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -101,18 +101,24 @@
}
}
-// This function will assign dynamic payload types (in the range [96, 127]) to
-// the input codecs, and also add ULPFEC, RED, FlexFEC, and associated RTX
-// codecs for recognized codecs (VP8, VP9, H264, and RED). It will also add
-// default feedback params to the codecs.
+// This function will assign dynamic payload types (in the range [96, 127] and
+// for new additions the [35, 65] range) to the input codecs, and also add
+// ULPFEC, RED, FlexFEC, and associated RTX codecs for recognized codecs (VP8,
+// VP9, H264 and RED). It will also add default feedback params to the codecs.
std::vector<VideoCodec> AssignPayloadTypesAndDefaultCodecs(
std::vector<webrtc::SdpVideoFormat> input_formats,
const webrtc::WebRtcKeyValueConfig& trials) {
if (input_formats.empty())
return std::vector<VideoCodec>();
- static const int kFirstDynamicPayloadType = 96;
- static const int kLastDynamicPayloadType = 127;
- int payload_type = kFirstDynamicPayloadType;
+ // Due to interoperability issues with old Chrome/WebRTC versions only use
+ // the lower range for new codecs.
+ static const int kFirstDynamicPayloadTypeLowerRange = 35;
+ static const int kLastDynamicPayloadTypeLowerRange = 65;
+
+ static const int kFirstDynamicPayloadTypeUpperRange = 96;
+ static const int kLastDynamicPayloadTypeUpperRange = 127;
+ int payload_type_upper = kFirstDynamicPayloadTypeUpperRange;
+ int payload_type_lower = kFirstDynamicPayloadTypeLowerRange;
input_formats.push_back(webrtc::SdpVideoFormat(kRedCodecName));
input_formats.push_back(webrtc::SdpVideoFormat(kUlpfecCodecName));
@@ -130,27 +136,54 @@
std::vector<VideoCodec> output_codecs;
for (const webrtc::SdpVideoFormat& format : input_formats) {
VideoCodec codec(format);
- codec.id = payload_type;
+ bool isCodecValidForLowerRange =
+ absl::EqualsIgnoreCase(codec.name, kFlexfecCodecName);
+ if (!isCodecValidForLowerRange) {
+ codec.id = payload_type_upper++;
+ } else {
+ codec.id = payload_type_lower++;
+ }
AddDefaultFeedbackParams(&codec, trials);
output_codecs.push_back(codec);
- // Increment payload type.
- ++payload_type;
- if (payload_type > kLastDynamicPayloadType) {
- RTC_LOG(LS_ERROR) << "Out of dynamic payload types, skipping the rest.";
+ if (payload_type_upper > kLastDynamicPayloadTypeUpperRange) {
+ RTC_LOG(LS_ERROR)
+ << "Out of dynamic payload types [96,127], skipping the rest.";
+ // TODO(https://bugs.chromium.org/p/webrtc/issues/detail?id=12194):
+ // continue in lower range.
+ break;
+ }
+ if (payload_type_lower > kLastDynamicPayloadTypeLowerRange) {
+ // TODO(https://bugs.chromium.org/p/webrtc/issues/detail?id=12248):
+ // return an error.
+ RTC_LOG(LS_ERROR)
+ << "Out of dynamic payload types [35,65], skipping the rest.";
break;
}
// Add associated RTX codec for non-FEC codecs.
if (!absl::EqualsIgnoreCase(codec.name, kUlpfecCodecName) &&
!absl::EqualsIgnoreCase(codec.name, kFlexfecCodecName)) {
- output_codecs.push_back(
- VideoCodec::CreateRtxCodec(payload_type, codec.id));
+ if (!isCodecValidForLowerRange) {
+ output_codecs.push_back(
+ VideoCodec::CreateRtxCodec(payload_type_upper++, codec.id));
+ } else {
+ output_codecs.push_back(
+ VideoCodec::CreateRtxCodec(payload_type_lower++, codec.id));
+ }
- // Increment payload type.
- ++payload_type;
- if (payload_type > kLastDynamicPayloadType) {
- RTC_LOG(LS_ERROR) << "Out of dynamic payload types, skipping the rest.";
+ if (payload_type_upper > kLastDynamicPayloadTypeUpperRange) {
+ RTC_LOG(LS_ERROR)
+ << "Out of dynamic payload types [96,127], skipping rtx.";
+ // TODO(https://bugs.chromium.org/p/webrtc/issues/detail?id=12194):
+ // continue in lower range.
+ break;
+ }
+ if (payload_type_lower > kLastDynamicPayloadTypeLowerRange) {
+ // TODO(https://bugs.chromium.org/p/webrtc/issues/detail?id=12248):
+ // return an error.
+ RTC_LOG(LS_ERROR)
+ << "Out of dynamic payload types [35,65], skipping rtx.";
break;
}
}
diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc
index d4b2621..63d134d 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -971,6 +971,26 @@
EXPECT_THAT(engine_.send_codecs(), Contains(flexfec));
}
+// Test that the FlexFEC "codec" gets assigned to the lower payload type range
+TEST_F(WebRtcVideoEngineTest, Flexfec03LowerPayloadTypeRange) {
+ encoder_factory_->AddSupportedVideoCodecType("VP8");
+
+ auto flexfec = Field("name", &VideoCodec::name, "flexfec-03");
+
+ // FlexFEC is active with field trial.
+ RTC_DCHECK(!override_field_trials_);
+ override_field_trials_ = std::make_unique<webrtc::test::ScopedFieldTrials>(
+ "WebRTC-FlexFEC-03-Advertised/Enabled/");
+ auto send_codecs = engine_.send_codecs();
+ auto it = std::find_if(send_codecs.begin(), send_codecs.end(),
+ [](const cricket::VideoCodec& codec) {
+ return codec.name == "flexfec-03";
+ });
+ ASSERT_NE(it, send_codecs.end());
+ EXPECT_LE(35, it->id);
+ EXPECT_GE(65, it->id);
+}
+
// Test that codecs are added in the order they are reported from the factory.
TEST_F(WebRtcVideoEngineTest, ReportSupportedCodecs) {
encoder_factory_->AddSupportedVideoCodecType("VP8");
