zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Filter out duplicate receive codecs in the media engine

Browse Source 
A malformed session description can assign the same codec to
different payload types which would hit a DCHECK in the
WebRtcVideoEngine. This changes the video engine to just ignore
the duplicate payload type instead of failing.

Bug: chromium:987598
Change-Id: I2034dd11d315ef05448630c860c7ca3f69ef700b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/147943
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Amit Hilbuch <amithi@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28796}
This commit is contained in:
Steve Anton
2019-08-07 09:57:59 -07:00
committed by Commit Bot
parent 3cc2f70bc3
commit 2d2bbb16e5
3 changed files with 127 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

110
media/engine/webrtc_video_engine.cc
View File

@ -2887,46 +2887,58 @@ WebRtcVideoChannel::MapCodecs(const std::vector<VideoCodec>& codecs) {
RTC_DCHECK(!codecs.empty());
std::vector<VideoCodecSettings> video_codecs;
std::map<int, bool> payload_used;
std::map<int, VideoCodec::CodecType> payload_codec_type;
// |rtx_mapping| maps video payload type to rtx payload type.
std::map<int, int> rtx_mapping;
webrtc::UlpfecConfig ulpfec_config;
int flexfec_payload_type = -1;
absl::optional<int> flexfec_payload_type;
for (size_t i = 0; i < codecs.size(); ++i) {
const VideoCodec& in_codec = codecs[i];
int payload_type = in_codec.id;
for (const VideoCodec& in_codec : codecs) {
const int payload_type = in_codec.id;
if (payload_used[payload_type]) {
if (payload_codec_type.find(payload_type) != payload_codec_type.end()) {
RTC_LOG(LS_ERROR) << "Payload type already registered: "
<< in_codec.ToString();
return std::vector<VideoCodecSettings>();
return {};
}
payload_used[payload_type] = true;
payload_codec_type[payload_type] = in_codec.GetCodecType();
switch (in_codec.GetCodecType()) {
case VideoCodec::CODEC_RED: {
// RED payload type, should not have duplicates.
RTC_DCHECK_EQ(-1, ulpfec_config.red_payload_type);
ulpfec_config.red_payload_type = in_codec.id;
continue;
if (ulpfec_config.red_payload_type != -1) {
RTC_LOG(LS_ERROR)
<< "Duplicate RED codec: ignoring PT=" << payload_type
<< " in favor of PT=" << ulpfec_config.red_payload_type
<< " which was specified first.";
break;
}
ulpfec_config.red_payload_type = payload_type;
break;
}
case VideoCodec::CODEC_ULPFEC: {
// ULPFEC payload type, should not have duplicates.
RTC_DCHECK_EQ(-1, ulpfec_config.ulpfec_payload_type);
ulpfec_config.ulpfec_payload_type = in_codec.id;
continue;
if (ulpfec_config.ulpfec_payload_type != -1) {
RTC_LOG(LS_ERROR)
<< "Duplicate ULPFEC codec: ignoring PT=" << payload_type
<< " in favor of PT=" << ulpfec_config.ulpfec_payload_type
<< " which was specified first.";
break;
}
ulpfec_config.ulpfec_payload_type = payload_type;
break;
}
case VideoCodec::CODEC_FLEXFEC: {
// FlexFEC payload type, should not have duplicates.
RTC_DCHECK_EQ(-1, flexfec_payload_type);
flexfec_payload_type = in_codec.id;
continue;
if (flexfec_payload_type) {
RTC_LOG(LS_ERROR)
<< "Duplicate FLEXFEC codec: ignoring PT=" << payload_type
<< " in favor of PT=" << *flexfec_payload_type
<< " which was specified first.";
break;
}
flexfec_payload_type = payload_type;
break;
}
case VideoCodec::CODEC_RTX: {
@ -2937,49 +2949,57 @@ WebRtcVideoChannel::MapCodecs(const std::vector<VideoCodec>& codecs) {
RTC_LOG(LS_ERROR)
<< "RTX codec with invalid or no associated payload type: "
<< in_codec.ToString();
return std::vector<VideoCodecSettings>();
return {};
}
rtx_mapping[associated_payload_type] = in_codec.id;
continue;
rtx_mapping[associated_payload_type] = payload_type;
break;
}
case VideoCodec::CODEC_VIDEO:
case VideoCodec::CODEC_VIDEO: {
video_codecs.emplace_back();
video_codecs.back().codec = in_codec;
break;
}
}
video_codecs.push_back(VideoCodecSettings());
video_codecs.back().codec = in_codec;
}
// One of these codecs should have been a video codec. Only having FEC
// parameters into this code is a logic error.
RTC_DCHECK(!video_codecs.empty());
for (std::map<int, int>::const_iterator it = rtx_mapping.begin();
it != rtx_mapping.end(); ++it) {
if (!payload_used[it->first]) {
RTC_LOG(LS_ERROR) << "RTX mapped to payload not in codec list.";
return std::vector<VideoCodecSettings>();
for (const auto& entry : rtx_mapping) {
const int associated_payload_type = entry.first;
const int rtx_payload_type = entry.second;
auto it = payload_codec_type.find(associated_payload_type);
if (it == payload_codec_type.end()) {
RTC_LOG(LS_ERROR) << "RTX codec (PT=" << rtx_payload_type
<< ") mapped to PT=" << associated_payload_type
<< " which is not in the codec list.";
return {};
}
if (payload_codec_type[it->first] != VideoCodec::CODEC_VIDEO &&
payload_codec_type[it->first] != VideoCodec::CODEC_RED) {
const VideoCodec::CodecType associated_codec_type = it->second;
if (associated_codec_type != VideoCodec::CODEC_VIDEO &&
associated_codec_type != VideoCodec::CODEC_RED) {
RTC_LOG(LS_ERROR)
<< "RTX not mapped to regular video codec or RED codec.";
return std::vector<VideoCodecSettings>();
<< "RTX PT=" << rtx_payload_type
<< " not mapped to regular video codec or RED codec (PT="
<< associated_payload_type << ").";
return {};
}
if (it->first == ulpfec_config.red_payload_type) {
ulpfec_config.red_rtx_payload_type = it->second;
if (associated_payload_type == ulpfec_config.red_payload_type) {
ulpfec_config.red_rtx_payload_type = rtx_payload_type;
}
}
for (size_t i = 0; i < video_codecs.size(); ++i) {
video_codecs[i].ulpfec = ulpfec_config;
video_codecs[i].flexfec_payload_type = flexfec_payload_type;
if (rtx_mapping[video_codecs[i].codec.id] != 0 &&
rtx_mapping[video_codecs[i].codec.id] !=
ulpfec_config.red_payload_type) {
video_codecs[i].rtx_payload_type = rtx_mapping[video_codecs[i].codec.id];
for (VideoCodecSettings& codec_settings : video_codecs) {
const int payload_type = codec_settings.codec.id;
codec_settings.ulpfec = ulpfec_config;
codec_settings.flexfec_payload_type = flexfec_payload_type.value_or(-1);
auto it = rtx_mapping.find(payload_type);
if (it != rtx_mapping.end()) {
const int rtx_payload_type = it->second;
codec_settings.rtx_payload_type = rtx_payload_type;
}
}

8
media/engine/webrtc_video_engine.h
View File

@ -227,8 +227,8 @@ class WebRtcVideoChannel : public VideoMediaChannel,
VideoCodec codec;
webrtc::UlpfecConfig ulpfec;
int flexfec_payload_type;
int rtx_payload_type;
int flexfec_payload_type; // -1 if absent.
int rtx_payload_type; // -1 if absent.
};
struct ChangedSendParameters {
@ -481,6 +481,10 @@ class WebRtcVideoChannel : public VideoMediaChannel,
const webrtc::PacketOptions& options) override;
bool SendRtcp(const uint8_t* data, size_t len) override;
// Generate the list of codec parameters to pass down based on the negotiated
// "codecs". Note that VideoCodecSettings correspond to concrete codecs like
// VP8, VP9, H264 while VideoCodecs correspond also to "virtual" codecs like
// RTX, ULPFEC, FLEXFEC.
static std::vector<VideoCodecSettings> MapCodecs(
const std::vector<VideoCodec>& codecs);
// Get all codecs that are compatible with the receiver.

56
media/engine/webrtc_video_engine_unittest.cc
View File

@ -3848,6 +3848,28 @@ TEST_F(WebRtcVideoChannelFlexfecRecvTest,
EXPECT_EQ(1, video_stream.GetNumRemovedSecondarySinks());
}
TEST_F(WebRtcVideoChannelFlexfecRecvTest, DuplicateFlexfecCodecIsDropped) {
constexpr int kUnusedPayloadType1 = 127;
cricket::VideoRecvParameters recv_parameters;
recv_parameters.codecs.push_back(GetEngineCodec("VP8"));
recv_parameters.codecs.push_back(GetEngineCodec("flexfec-03"));
cricket::VideoCodec duplicate = GetEngineCodec("flexfec-03");
duplicate.id = kUnusedPayloadType1;
recv_parameters.codecs.push_back(duplicate);
ASSERT_TRUE(channel_->SetRecvParameters(recv_parameters));
AddRecvStream(
CreatePrimaryWithFecFrStreamParams("cname", kSsrcs1[0], kFlexfecSsrc));
const std::vector<FakeFlexfecReceiveStream*>& streams =
fake_call_->GetFlexfecReceiveStreams();
ASSERT_EQ(1U, streams.size());
const FakeFlexfecReceiveStream* stream = streams.front();
const webrtc::FlexfecReceiveStream::Config& config = stream->GetConfig();
EXPECT_EQ(GetEngineCodec("flexfec-03").id, config.payload_type);
}
// TODO(brandtr): When FlexFEC is no longer behind a field trial, merge all
// tests that use this test fixture into the corresponding "non-field trial"
// tests.
@ -4555,6 +4577,40 @@ TEST_F(WebRtcVideoChannelTest, SetRecvCodecsWithPacketizationRecreatesStream) {
EXPECT_EQ(fake_call_->GetNumCreatedReceiveStreams(), 2);
}
TEST_F(WebRtcVideoChannelTest, DuplicateUlpfecCodecIsDropped) {
constexpr int kFirstUlpfecPayloadType = 126;
constexpr int kSecondUlpfecPayloadType = 127;
cricket::VideoRecvParameters parameters;
parameters.codecs.push_back(GetEngineCodec("VP8"));
parameters.codecs.push_back(
cricket::VideoCodec(kFirstUlpfecPayloadType, cricket::kUlpfecCodecName));
parameters.codecs.push_back(
cricket::VideoCodec(kSecondUlpfecPayloadType, cricket::kUlpfecCodecName));
ASSERT_TRUE(channel_->SetRecvParameters(parameters));
FakeVideoReceiveStream* recv_stream = AddRecvStream();
EXPECT_EQ(kFirstUlpfecPayloadType,
recv_stream->GetConfig().rtp.ulpfec_payload_type);
}
TEST_F(WebRtcVideoChannelTest, DuplicateRedCodecIsDropped) {
constexpr int kFirstRedPayloadType = 126;
constexpr int kSecondRedPayloadType = 127;
cricket::VideoRecvParameters parameters;
parameters.codecs.push_back(GetEngineCodec("VP8"));
parameters.codecs.push_back(
cricket::VideoCodec(kFirstRedPayloadType, cricket::kRedCodecName));
parameters.codecs.push_back(
cricket::VideoCodec(kSecondRedPayloadType, cricket::kRedCodecName));
ASSERT_TRUE(channel_->SetRecvParameters(parameters));
FakeVideoReceiveStream* recv_stream = AddRecvStream();
EXPECT_EQ(kFirstRedPayloadType,
recv_stream->GetConfig().rtp.red_payload_type);
}
TEST_F(WebRtcVideoChannelTest, SetRecvCodecsWithChangedRtxPayloadType) {
const int kUnusedPayloadType1 = 126;
const int kUnusedPayloadType2 = 127;