admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Filter fmtp parameters from RED capabilities

Browse Source 
and ensure there is only one, similar to what is done with RTX.
This avoids exposing a payload type there.

See also
  https://github.com/w3c/webrtc-pc/issues/2696

BUG=webrtc:42221750,webrtc:360058654

Change-Id: Id7c2ddeaf47a3169db9be43c9c5b8e59346f1d57
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/376760
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@meta.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43929}
This commit is contained in:
Philipp Hancke 2025-02-14 11:43:11 -08:00 committed by WebRTC LUCI CQ
parent 0ed7980a6e
commit 7233df9751
4 changed files with 32 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
media/base/codec_comparators.cc
View File

@ -19,6 +19,7 @@
#include "absl/functional/any_invocable.h"
#include "absl/strings/match.h"
#include "absl/strings/string_view.h"
#include "api/media_types.h"
#include "api/rtp_parameters.h"
#include "api/video_codecs/av1_profile.h"
#include "api/video_codecs/h264_profile_level_id.h"
@ -407,6 +408,12 @@ bool IsSameRtpCodecIgnoringLevel(const Codec& codec,
return IsSameCodecSpecific(rtp_codec.name, params1, rtp_codec2.name,
params2);
}
// audio/RED should ignore the parameters which specify payload types so
// can not be compared.
if (rtp_codec.kind == cricket::MediaType::MEDIA_TYPE_AUDIO &&
rtp_codec.name == cricket::kRedCodecName) {
return true;
}
return params1 == params2;
}

2
media/engine/webrtc_voice_engine.cc
View File

@ -333,7 +333,7 @@ bool CheckRedParameters(
// Check the FMTP line for the empty parameter which should match
// <primary codec>/<primary codec>[/...]
auto red_parameters = red_codec.params.find("");
auto red_parameters = red_codec.params.find(kCodecParamNotInNameValueFormat);
if (red_parameters == red_codec.params.end()) {
RTC_LOG(LS_WARNING) << "audio/RED missing fmtp parameters.";
return false;

26
pc/rtp_parameters_conversion.cc
View File

@ -10,21 +10,16 @@
#include "pc/rtp_parameters_conversion.h"
#include <cstdint>
#include <set>
#include <optional>
#include <string>
#include <type_traits>
#include <utility>
#include <vector>
#include "api/array_view.h"
#include "api/media_types.h"
#include "api/rtc_error.h"
#include "api/rtp_parameters.h"
#include "media/base/codec.h"
#include "media/base/media_constants.h"
#include "media/base/rtp_utils.h"
#include "rtc_base/checks.h"
#include "pc/session_description.h"
#include "rtc_base/logging.h"
#include "rtc_base/strings/string_builder.h"
namespace webrtc {
@ -118,6 +113,10 @@ RtpCapabilities ToRtpCapabilities(
bool have_rtx = false;
for (const cricket::Codec& cricket_codec : cricket_codecs) {
if (cricket_codec.name == cricket::kRedCodecName) {
if (have_red) {
// There should only be one RED codec entry in caps.
continue;
}
have_red = true;
} else if (cricket_codec.name == cricket::kUlpfecCodecName) {
have_ulpfec = true;
@ -125,14 +124,17 @@ RtpCapabilities ToRtpCapabilities(
have_flexfec = true;
} else if (cricket_codec.name == cricket::kRtxCodecName) {
if (have_rtx) {
// There should only be one RTX codec entry
// There should only be one RTX codec entry in caps.
continue;
}
have_rtx = true;
}
auto codec_capability = ToRtpCodecCapability(cricket_codec);
if (cricket_codec.name == cricket::kRtxCodecName) {
// RTX codec should not have any parameter
if (cricket_codec.name == cricket::kRtxCodecName ||
cricket_codec.name == cricket::kRedCodecName) {
// For RTX this removes the APT which points to a payload type.
// For RED this removes the redundancy spec which points to a payload
// type.
codec_capability.parameters.clear();
}
capabilities.codecs.push_back(codec_capability);

19
pc/rtp_parameters_conversion_unittest.cc
View File

@ -10,12 +10,15 @@
#include "pc/rtp_parameters_conversion.h"
#include <cstdint>
#include <map>
#include <optional>
#include <string>
#include "api/media_types.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"
@ -137,19 +140,15 @@ TEST(RtpParametersConversionTest, ToRtpCodecCapabilityUnknownFeedbackParam) {
// the "fec" list is assembled correctly.
TEST(RtpParametersConversionTest, ToRtpCapabilities) {
cricket::Codec vp8 = cricket::CreateVideoCodec(101, "VP8");
vp8.clockrate = 90000;
cricket::Codec red = cricket::CreateVideoCodec(102, "red");
red.clockrate = 90000;
// Note: fmtp not usually done for video-red but we want it filtered.
red.SetParam(cricket::kCodecParamNotInNameValueFormat, "101/101");
cricket::Codec red2 = cricket::CreateVideoCodec(127, "red");
cricket::Codec ulpfec = cricket::CreateVideoCodec(103, "ulpfec");
ulpfec.clockrate = 90000;
cricket::Codec flexfec = cricket::CreateVideoCodec(102, "flexfec-03");
flexfec.clockrate = 90000;
cricket::Codec rtx = cricket::CreateVideoRtxCodec(014, 101);
cricket::Codec rtx2 = cricket::CreateVideoRtxCodec(105, 109);
RtpCapabilities capabilities =
@ -166,7 +165,7 @@ TEST(RtpParametersConversionTest, ToRtpCapabilities) {
EXPECT_EQ(3, capabilities.header_extensions[1].preferred_id);
EXPECT_EQ(0u, capabilities.fec.size());
capabilities = ToRtpCapabilities({vp8, red, ulpfec, rtx},
capabilities = ToRtpCapabilities({vp8, red, red2, ulpfec, rtx},
cricket::RtpHeaderExtensions());
EXPECT_EQ(4u, capabilities.codecs.size());
EXPECT_THAT(
@ -178,6 +177,8 @@ TEST(RtpParametersConversionTest, ToRtpCapabilities) {
EXPECT_EQ(3u, capabilities.codecs.size());
EXPECT_THAT(capabilities.fec,
UnorderedElementsAre(FecMechanism::RED, FecMechanism::FLEXFEC));
EXPECT_EQ(capabilities.codecs[1].name, "red");
EXPECT_TRUE(capabilities.codecs[1].parameters.empty());
}
} // namespace webrtc