Reject StreamParams with RTX SSRCs not in ssrcs.
BUG=1788, chromium:470122 R=stefan@webrtc.org Review URL: https://webrtc-codereview.appspot.com/44859004 Cr-Commit-Position: refs/heads/master@{#8855}
This commit is contained in:
parent
a49f515786
commit
d4362cd336
@ -86,6 +86,40 @@ static bool ValidateCodecFormats(const std::vector<VideoCodec>& codecs) {
|
||||
return true;
|
||||
}
|
||||
|
||||
static bool ValidateStreamParams(const StreamParams& sp) {
|
||||
if (sp.ssrcs.empty()) {
|
||||
LOG(LS_ERROR) << "No SSRCs in stream parameters: " << sp.ToString();
|
||||
return false;
|
||||
}
|
||||
|
||||
std::vector<uint32> primary_ssrcs;
|
||||
sp.GetPrimarySsrcs(&primary_ssrcs);
|
||||
std::vector<uint32> rtx_ssrcs;
|
||||
sp.GetFidSsrcs(primary_ssrcs, &rtx_ssrcs);
|
||||
for (uint32_t rtx_ssrc : rtx_ssrcs) {
|
||||
bool rtx_ssrc_present = false;
|
||||
for (uint32_t sp_ssrc : sp.ssrcs) {
|
||||
if (sp_ssrc == rtx_ssrc) {
|
||||
rtx_ssrc_present = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!rtx_ssrc_present) {
|
||||
LOG(LS_ERROR) << "RTX SSRC '" << rtx_ssrc
|
||||
<< "' missing from StreamParams ssrcs: " << sp.ToString();
|
||||
return false;
|
||||
}
|
||||
}
|
||||
if (!rtx_ssrcs.empty() && primary_ssrcs.size() != rtx_ssrcs.size()) {
|
||||
LOG(LS_ERROR)
|
||||
<< "RTX SSRCs exist, but don't cover all SSRCs (unsupported): "
|
||||
<< sp.ToString();
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
static std::string RtpExtensionsToString(
|
||||
const std::vector<RtpHeaderExtension>& extensions) {
|
||||
std::stringstream out;
|
||||
@ -799,10 +833,8 @@ bool WebRtcVideoChannel2::SetSend(bool send) {
|
||||
|
||||
bool WebRtcVideoChannel2::AddSendStream(const StreamParams& sp) {
|
||||
LOG(LS_INFO) << "AddSendStream: " << sp.ToString();
|
||||
if (sp.ssrcs.empty()) {
|
||||
LOG(LS_ERROR) << "No SSRCs in stream parameters.";
|
||||
if (!ValidateStreamParams(sp))
|
||||
return false;
|
||||
}
|
||||
|
||||
uint32 ssrc = sp.first_ssrc();
|
||||
assert(ssrc != 0);
|
||||
@ -814,17 +846,6 @@ bool WebRtcVideoChannel2::AddSendStream(const StreamParams& sp) {
|
||||
return false;
|
||||
}
|
||||
|
||||
std::vector<uint32> primary_ssrcs;
|
||||
sp.GetPrimarySsrcs(&primary_ssrcs);
|
||||
std::vector<uint32> rtx_ssrcs;
|
||||
sp.GetFidSsrcs(primary_ssrcs, &rtx_ssrcs);
|
||||
if (!rtx_ssrcs.empty() && primary_ssrcs.size() != rtx_ssrcs.size()) {
|
||||
LOG(LS_ERROR)
|
||||
<< "RTX SSRCs exist, but don't cover all SSRCs (unsupported): "
|
||||
<< sp.ToString();
|
||||
return false;
|
||||
}
|
||||
|
||||
WebRtcVideoSendStream* stream =
|
||||
new WebRtcVideoSendStream(call_.get(),
|
||||
external_encoder_factory_,
|
||||
@ -889,8 +910,10 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp) {
|
||||
|
||||
bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp,
|
||||
bool default_stream) {
|
||||
LOG(LS_INFO) << "AddRecvStream: " << sp.ToString();
|
||||
assert(sp.ssrcs.size() > 0);
|
||||
LOG(LS_INFO) << "AddRecvStream" << (default_stream ? " (default stream)" : "")
|
||||
<< ": " << sp.ToString();
|
||||
if (!ValidateStreamParams(sp))
|
||||
return false;
|
||||
|
||||
uint32 ssrc = sp.first_ssrc();
|
||||
assert(ssrc != 0); // TODO(pbos): Is this ever valid?
|
||||
|
@ -2353,37 +2353,26 @@ TEST_F(WebRtcVideoChannel2Test, DefaultReceiveStreamReconfiguresToUseRtx) {
|
||||
recv_stream->GetConfig().rtp.rtx.begin()->second.ssrc);
|
||||
}
|
||||
|
||||
TEST_F(WebRtcVideoChannel2Test, RejectsAddingStreamsWithMissingSsrcsForRtx) {
|
||||
EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs()));
|
||||
|
||||
const std::vector<uint32> ssrcs = MAKE_VECTOR(kSsrcs1);
|
||||
const std::vector<uint32> rtx_ssrcs = MAKE_VECTOR(kRtxSsrcs1);
|
||||
|
||||
StreamParams sp =
|
||||
cricket::CreateSimWithRtxStreamParams("cname", ssrcs, rtx_ssrcs);
|
||||
sp.ssrcs = ssrcs; // Without RTXs, this is the important part.
|
||||
|
||||
EXPECT_FALSE(channel_->AddSendStream(sp));
|
||||
EXPECT_FALSE(channel_->AddRecvStream(sp));
|
||||
}
|
||||
|
||||
class WebRtcVideoEngine2SimulcastTest : public testing::Test {
|
||||
public:
|
||||
WebRtcVideoEngine2SimulcastTest()
|
||||
: engine_(nullptr), engine_codecs_(engine_.codecs()) {
|
||||
assert(!engine_codecs_.empty());
|
||||
|
||||
bool codec_set = false;
|
||||
for (size_t i = 0; i < engine_codecs_.size(); ++i) {
|
||||
if (engine_codecs_[i].name == "red") {
|
||||
default_red_codec_ = engine_codecs_[i];
|
||||
} else if (engine_codecs_[i].name == "ulpfec") {
|
||||
default_ulpfec_codec_ = engine_codecs_[i];
|
||||
} else if (engine_codecs_[i].name == "rtx") {
|
||||
default_rtx_codec_ = engine_codecs_[i];
|
||||
} else if (!codec_set) {
|
||||
default_codec_ = engine_codecs_[i];
|
||||
codec_set = true;
|
||||
}
|
||||
}
|
||||
|
||||
assert(codec_set);
|
||||
}
|
||||
WebRtcVideoEngine2SimulcastTest() : engine_(nullptr) {}
|
||||
|
||||
protected:
|
||||
WebRtcVideoEngine2 engine_;
|
||||
VideoCodec default_codec_;
|
||||
VideoCodec default_red_codec_;
|
||||
VideoCodec default_ulpfec_codec_;
|
||||
VideoCodec default_rtx_codec_;
|
||||
// TODO(pbos): Remove engine_codecs_ unless used a lot.
|
||||
std::vector<VideoCodec> engine_codecs_;
|
||||
};
|
||||
|
||||
class WebRtcVideoChannel2SimulcastTest : public WebRtcVideoEngine2SimulcastTest,
|
||||
|
Loading…
Reference in New Issue
Block a user