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

Perform packetization verification until a match is found

Browse Source 
If there happens to be an asymmetry between local and remote codecs we
shouldn't validate that there's a 1:1 packetization mapping for every
codec. It's sufficient to check that there's at least one matching
packetization per codec.

Bug: webrtc:15473
Change-Id: Ib4fc8fdd54bb4dccf96f0c802746c848e2deed83
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/320440
Commit-Queue: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Auto-Submit: Emil Lundmark <lndmrk@webrtc.org>
Reviewed-by: Sergey Sukhanov <sergeysu@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40760}
This commit is contained in:
Emil Lundmark 2023-09-15 16:33:03 +02:00 committed by WebRTC LUCI CQ
parent f96430e433
commit 17304c3bf8
3 changed files with 144 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
pc/BUILD.gn
View File

@ -105,6 +105,7 @@ rtc_source_set("channel") {
"../rtc_base/third_party/sigslot",
]
absl_deps = [
"//third_party/abseil-cpp/absl/algorithm:container",
"//third_party/abseil-cpp/absl/strings",
"//third_party/abseil-cpp/absl/types:optional",
]

83
pc/channel.cc
View File

@ -16,6 +16,7 @@
#include <type_traits>
#include <utility>
#include "absl/algorithm/container.h"
#include "absl/strings/string_view.h"
#include "api/rtp_parameters.h"
#include "api/sequence_checker.h"
@ -1036,19 +1037,34 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content,
bool needs_send_params_update = false;
if (type == SdpType::kAnswer || type == SdpType::kPrAnswer) {
for (auto& send_codec : send_params.codecs) {
auto* recv_codec = FindMatchingCodec(recv_params.codecs, send_codec);
if (recv_codec) {
if (!recv_codec->packetization && send_codec.packetization) {
send_codec.packetization.reset();
needs_send_params_update = true;
} else if (recv_codec->packetization != send_codec.packetization) {
error_desc = StringFormat(
"Failed to set local answer due to invalid codec packetization "
"specified in m-section with mid='%s'.",
mid().c_str());
return false;
}
webrtc::flat_set<const VideoCodec*> matched_codecs;
for (VideoCodec& send_codec : send_params.codecs) {
if (absl::c_any_of(matched_codecs, [&](const VideoCodec* c) {
return send_codec.Matches(*c);
})) {
continue;
}
const VideoCodec* recv_codec =
FindMatchingCodec(recv_params.codecs, send_codec);
if (recv_codec == nullptr) {
continue;
}
if (!recv_codec->packetization.has_value() &&
send_codec.packetization.has_value()) {
send_codec.packetization = absl::nullopt;
needs_send_params_update = true;
} else if (recv_codec->packetization != send_codec.packetization) {
error_desc = StringFormat(
"Failed to set local answer due to incompatible codec "
"packetization for pt='%d' specified in m-section with mid='%s'.",
send_codec.id, mid().c_str());
return false;
}
if (recv_codec->packetization == send_codec.packetization) {
matched_codecs.insert(&send_codec);
}
}
}
@ -1121,19 +1137,34 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content,
bool needs_recv_params_update = false;
if (type == SdpType::kAnswer || type == SdpType::kPrAnswer) {
for (auto& recv_codec : recv_params.codecs) {
auto* send_codec = FindMatchingCodec(send_params.codecs, recv_codec);
if (send_codec) {
if (!send_codec->packetization && recv_codec.packetization) {
recv_codec.packetization.reset();
needs_recv_params_update = true;
} else if (send_codec->packetization != recv_codec.packetization) {
error_desc = StringFormat(
"Failed to set remote answer due to invalid codec packetization "
"specifid in m-section with mid='%s'.",
mid().c_str());
return false;
}
webrtc::flat_set<const VideoCodec*> matched_codecs;
for (VideoCodec& recv_codec : recv_params.codecs) {
if (absl::c_any_of(matched_codecs, [&](const VideoCodec* c) {
return recv_codec.Matches(*c);
})) {
continue;
}
const VideoCodec* send_codec =
FindMatchingCodec(send_params.codecs, recv_codec);
if (send_codec == nullptr) {
continue;
}
if (!send_codec->packetization.has_value() &&
recv_codec.packetization.has_value()) {
recv_codec.packetization = absl::nullopt;
needs_recv_params_update = true;
} else if (send_codec->packetization != recv_codec.packetization) {
error_desc = StringFormat(
"Failed to set remote answer due to incompatible codec "
"packetization for pt='%d' specified in m-section with mid='%s'.",
recv_codec.id, mid().c_str());
return false;
}
if (send_codec->packetization == recv_codec.packetization) {
matched_codecs.insert(&recv_codec);
}
}
}

97
pc/channel_unittest.cc
View File

@ -48,16 +48,20 @@
#include "test/gtest.h"
#include "test/scoped_key_value_config.h"
using cricket::DtlsTransportInternal;
using cricket::FakeVoiceMediaReceiveChannel;
using cricket::FakeVoiceMediaSendChannel;
using cricket::RidDescription;
using cricket::RidDirection;
using cricket::StreamParams;
using webrtc::RtpTransceiverDirection;
using webrtc::SdpType;
namespace {
using ::cricket::DtlsTransportInternal;
using ::cricket::FakeVoiceMediaReceiveChannel;
using ::cricket::FakeVoiceMediaSendChannel;
using ::cricket::RidDescription;
using ::cricket::RidDirection;
using ::cricket::StreamParams;
using ::testing::AllOf;
using ::testing::ElementsAre;
using ::testing::Field;
using ::webrtc::RtpTransceiverDirection;
using ::webrtc::SdpType;
const cricket::AudioCodec kPcmuCodec =
cricket::CreateAudioCodec(0, "PCMU", 64000, 1);
const cricket::AudioCodec kPcmaCodec =
@ -75,7 +79,6 @@ const int kAudioPts[] = {0, 8};
const int kVideoPts[] = {97, 99};
enum class NetworkIsWorker { Yes, No };
} // namespace
template <class ChannelT,
class MediaSendChannelT,
@ -2274,6 +2277,78 @@ TEST_F(VideoChannelSingleThreadTest,
absl::nullopt);
}
TEST_F(VideoChannelSingleThreadTest,
StopsPacketizationVerificationWhenMatchIsFoundInRemoteAnswer) {
cricket::VideoCodec vp8_foo = cricket::CreateVideoCodec(96, "VP8");
vp8_foo.packetization = "foo";
cricket::VideoCodec vp8_bar = cricket::CreateVideoCodec(97, "VP8");
vp8_bar.packetization = "bar";
cricket::VideoCodec vp9 = cricket::CreateVideoCodec(98, "VP9");
cricket::VideoCodec vp9_foo = cricket::CreateVideoCodec(99, "VP9");
vp9_foo.packetization = "bar";
cricket::VideoContentDescription local;
local.set_codecs({vp8_foo, vp8_bar, vp9_foo});
cricket::VideoContentDescription remote;
remote.set_codecs({vp8_foo, vp9});
CreateChannels(0, 0);
std::string err;
ASSERT_TRUE(channel1_->SetLocalContent(&local, SdpType::kOffer, err)) << err;
ASSERT_TRUE(channel1_->SetRemoteContent(&remote, SdpType::kAnswer, err))
<< err;
EXPECT_THAT(
media_receive_channel1_impl()->recv_codecs(),
ElementsAre(AllOf(Field(&cricket::Codec::id, 96),
Field(&cricket::Codec::packetization, "foo")),
AllOf(Field(&cricket::Codec::id, 97),
Field(&cricket::Codec::packetization, "bar")),
AllOf(Field(&cricket::Codec::id, 99),
Field(&cricket::Codec::packetization, absl::nullopt))));
EXPECT_THAT(
media_send_channel1_impl()->send_codecs(),
ElementsAre(AllOf(Field(&cricket::Codec::id, 96),
Field(&cricket::Codec::packetization, "foo")),
AllOf(Field(&cricket::Codec::id, 98),
Field(&cricket::Codec::packetization, absl::nullopt))));
}
TEST_F(VideoChannelSingleThreadTest,
StopsPacketizationVerificationWhenMatchIsFoundInLocalAnswer) {
cricket::VideoCodec vp8_foo = cricket::CreateVideoCodec(96, "VP8");
vp8_foo.packetization = "foo";
cricket::VideoCodec vp8_bar = cricket::CreateVideoCodec(97, "VP8");
vp8_bar.packetization = "bar";
cricket::VideoCodec vp9 = cricket::CreateVideoCodec(98, "VP9");
cricket::VideoCodec vp9_foo = cricket::CreateVideoCodec(99, "VP9");
vp9_foo.packetization = "bar";
cricket::VideoContentDescription local;
local.set_codecs({vp8_foo, vp9});
cricket::VideoContentDescription remote;
remote.set_codecs({vp8_foo, vp8_bar, vp9_foo});
CreateChannels(0, 0);
std::string err;
ASSERT_TRUE(channel1_->SetRemoteContent(&remote, SdpType::kOffer, err))
<< err;
ASSERT_TRUE(channel1_->SetLocalContent(&local, SdpType::kAnswer, err)) << err;
EXPECT_THAT(
media_receive_channel1_impl()->recv_codecs(),
ElementsAre(AllOf(Field(&cricket::Codec::id, 96),
Field(&cricket::Codec::packetization, "foo")),
AllOf(Field(&cricket::Codec::id, 98),
Field(&cricket::Codec::packetization, absl::nullopt))));
EXPECT_THAT(
media_send_channel1_impl()->send_codecs(),
ElementsAre(AllOf(Field(&cricket::Codec::id, 96),
Field(&cricket::Codec::packetization, "foo")),
AllOf(Field(&cricket::Codec::id, 97),
Field(&cricket::Codec::packetization, "bar")),
AllOf(Field(&cricket::Codec::id, 99),
Field(&cricket::Codec::packetization, absl::nullopt))));
}
// VideoChannelDoubleThreadTest
TEST_F(VideoChannelDoubleThreadTest, TestInit) {
Base::TestInit();
@ -2409,4 +2484,4 @@ TEST_F(VideoChannelDoubleThreadTest, SocketOptionsMergedOnSetTransport) {
Base::SocketOptionsMergedOnSetTransport();
}
// TODO(pthatcher): TestSetReceiver?
} // namespace