From c262375415c369f3cd6bc92bf9e2689f1064fb55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Mon, 3 Feb 2025 14:18:23 +0100 Subject: [PATCH] Add test for preferring RTX payload to be "primary codec + 1". MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds test coverage for the following mitigations (need both to pass): 1. https://webrtc-review.googlesource.com/c/src/+/375847 2. https://webrtc-review.googlesource.com/c/src/+/376022 Like the comment says, this is neither mandated by the spec or guaranteed, but when the number of codecs is quite small like it is in this test it will be true for all RTX codecs. Bug: webrtc:360058654 Change-Id: Ib73cea59d06a62390dd039eb2dc04677d6178460 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/375865 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Henrik Boström Cr-Commit-Position: refs/heads/main@{#43841} --- media/engine/webrtc_video_engine_unittest.cc | 28 ++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index a76b209dd2..911a34bde0 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -471,6 +471,34 @@ TEST_F(WebRtcVideoEngineTest, DefaultRtxCodecHasAssociatedPayloadTypeSet) { FAIL() << "No RTX codec found among default codecs."; } +// Test that we prefer to assign RTX payload types as "primary codec PT + 1". +// This is purely for backwards compatibility (see https://crbug.com/391132280). +// The spec does NOT mandate we do this and note that this is best-effort, if +// "PT + 1" is already in-use the PT suggester would pick a different PT. +TEST_F(WebRtcVideoEngineTest, + DefaultRtxCodecIsAssignedAssociatedPayloadTypePlusOne) { + AddSupportedVideoCodecType("VP8"); + AddSupportedVideoCodecType("VP9"); + AddSupportedVideoCodecType("AV1"); + AddSupportedVideoCodecType("H264"); + for (const Codec& codec : engine_.send_codecs()) { + if (codec.name != kRtxCodecName) + continue; + int associated_payload_type; + ASSERT_TRUE(codec.GetParam(kCodecParamAssociatedPayloadType, + &associated_payload_type)); + EXPECT_EQ(codec.id, associated_payload_type + 1); + } + for (const Codec& codec : engine_.recv_codecs()) { + if (codec.name != kRtxCodecName) + continue; + int associated_payload_type; + ASSERT_TRUE(codec.GetParam(kCodecParamAssociatedPayloadType, + &associated_payload_type)); + EXPECT_EQ(codec.id, associated_payload_type + 1); + } +} + TEST_F(WebRtcVideoEngineTest, SupportsTimestampOffsetHeaderExtension) { ExpectRtpCapabilitySupport(RtpExtension::kTimestampOffsetUri, true); }