Add video send SSRC to RtpParameters, and don't allow changing SSRC.

With this, RtpSender and RtpReceiver will always return an SSRC if one
is available. Also, attempts to change the SSRC with SetParameters will
fail, rather than silently doing nothing.

BUG=webrtc:6888

Review-Url: https://codereview.webrtc.org/2567333004
Cr-Commit-Position: refs/heads/master@{#15939}
This commit is contained in:
deadbeef 2017-01-06 23:05:37 -08:00 committed by Commit bot
parent 0aeaa91219
commit fb2aceded6
4 changed files with 62 additions and 30 deletions

View File

@ -1560,6 +1560,11 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::WebRtcVideoSendStream(
sp.GetPrimarySsrcs(&parameters_.config.rtp.ssrcs);
// ValidateStreamParams should prevent this from happening.
RTC_CHECK(!parameters_.config.rtp.ssrcs.empty());
rtp_parameters_.encodings[0].ssrc =
rtc::Optional<uint32_t>(parameters_.config.rtp.ssrcs[0]);
// RTX.
sp.GetFidSsrcs(parameters_.config.rtp.ssrcs,
&parameters_.config.rtp.rtx.ssrcs);
@ -1862,11 +1867,16 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::GetRtpParameters() const {
bool WebRtcVideoChannel2::WebRtcVideoSendStream::ValidateRtpParameters(
const webrtc::RtpParameters& rtp_parameters) {
RTC_DCHECK_RUN_ON(&thread_checker_);
if (rtp_parameters.encodings.size() != 1) {
LOG(LS_ERROR)
<< "Attempted to set RtpParameters without exactly one encoding";
return false;
}
if (rtp_parameters.encodings[0].ssrc != rtp_parameters_.encodings[0].ssrc) {
LOG(LS_ERROR) << "Attempted to set RtpParameters with modified SSRC";
return false;
}
return true;
}

View File

@ -3661,16 +3661,21 @@ TEST_F(WebRtcVideoChannel2Test,
// for each encoding individually.
AddSendStream();
// Setting RtpParameters with no encoding is expected to fail.
webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(last_ssrc_);
parameters.encodings.clear();
EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters));
// Setting RtpParameters with exactly one encoding should succeed.
parameters.encodings.push_back(webrtc::RtpEncodingParameters());
EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters));
// Two or more encodings should result in failure.
parameters.encodings.push_back(webrtc::RtpEncodingParameters());
EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters));
// Zero encodings should also fail.
parameters.encodings.clear();
EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters));
}
// Changing the SSRC through RtpParameters is not allowed.
TEST_F(WebRtcVideoChannel2Test, CannotSetSsrcInRtpSendParameters) {
AddSendStream();
webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(last_ssrc_);
parameters.encodings[0].ssrc = rtc::Optional<uint32_t>(0xdeadbeef);
EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters));
}
// Test that a stream will not be sending if its encoding is made inactive
@ -3753,6 +3758,18 @@ TEST_F(WebRtcVideoChannel2Test, GetRtpSendParametersCodecs) {
rtp_parameters.codecs[1]);
}
// Test that RtpParameters for send stream has one encoding and it has
// the correct SSRC.
TEST_F(WebRtcVideoChannel2Test, GetRtpSendParametersSsrc) {
AddSendStream();
webrtc::RtpParameters rtp_parameters =
channel_->GetRtpSendParameters(last_ssrc_);
ASSERT_EQ(1u, rtp_parameters.encodings.size());
EXPECT_EQ(rtc::Optional<uint32_t>(last_ssrc_),
rtp_parameters.encodings[0].ssrc);
}
// Test that if we set/get parameters multiple times, we get the same results.
TEST_F(WebRtcVideoChannel2Test, SetAndGetRtpSendParameters) {
AddSendStream();
@ -3826,7 +3843,7 @@ TEST_F(WebRtcVideoChannel2Test, DISABLED_GetRtpReceiveFmtpSprop) {
// Test that RtpParameters for receive stream has one encoding and it has
// the correct SSRC.
TEST_F(WebRtcVideoChannel2Test, RtpEncodingParametersSsrcIsSet) {
TEST_F(WebRtcVideoChannel2Test, GetRtpReceiveParametersSsrc) {
AddRecvStream();
webrtc::RtpParameters rtp_parameters =

View File

@ -1384,8 +1384,23 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
return rtp_parameters_;
}
bool ValidateRtpParameters(const webrtc::RtpParameters& rtp_parameters) {
if (rtp_parameters.encodings.size() != 1) {
LOG(LS_ERROR)
<< "Attempted to set RtpParameters without exactly one encoding";
return false;
}
if (rtp_parameters.encodings[0].ssrc != rtp_parameters_.encodings[0].ssrc) {
LOG(LS_ERROR) << "Attempted to set RtpParameters with modified SSRC";
return false;
}
return true;
}
bool SetRtpParameters(const webrtc::RtpParameters& parameters) {
RTC_CHECK_EQ(1UL, parameters.encodings.size());
if (!ValidateRtpParameters(parameters)) {
return false;
}
auto send_rate = ComputeSendBitrate(max_send_bitrate_bps_,
parameters.encodings[0].max_bitrate_bps,
send_codec_spec_.codec_inst);
@ -1704,9 +1719,6 @@ bool WebRtcVoiceMediaChannel::SetRtpSendParameters(
uint32_t ssrc,
const webrtc::RtpParameters& parameters) {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
if (!ValidateRtpParameters(parameters)) {
return false;
}
auto it = send_streams_.find(ssrc);
if (it == send_streams_.end()) {
LOG(LS_WARNING) << "Attempting to set RTP send parameters for stream "
@ -1760,9 +1772,6 @@ bool WebRtcVoiceMediaChannel::SetRtpReceiveParameters(
uint32_t ssrc,
const webrtc::RtpParameters& parameters) {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
if (!ValidateRtpParameters(parameters)) {
return false;
}
auto it = recv_streams_.find(ssrc);
if (it == recv_streams_.end()) {
LOG(LS_WARNING) << "Attempting to set RTP receive parameters for stream "
@ -1779,16 +1788,6 @@ bool WebRtcVoiceMediaChannel::SetRtpReceiveParameters(
return true;
}
bool WebRtcVoiceMediaChannel::ValidateRtpParameters(
const webrtc::RtpParameters& rtp_parameters) {
if (rtp_parameters.encodings.size() != 1) {
LOG(LS_ERROR)
<< "Attempted to set RtpParameters without exactly one encoding";
return false;
}
return true;
}
bool WebRtcVoiceMediaChannel::SetOptions(const AudioOptions& options) {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
LOG(LS_INFO) << "Setting voice channel options: "

View File

@ -1039,15 +1039,21 @@ TEST_F(WebRtcVoiceEngineTestFake,
// for each encoding individually.
EXPECT_TRUE(SetupSendStream());
// Setting RtpParameters with no encoding is expected to fail.
webrtc::RtpParameters parameters;
EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrc1, parameters));
// Setting RtpParameters with exactly one encoding should succeed.
parameters.encodings.push_back(webrtc::RtpEncodingParameters());
EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrc1, parameters));
webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(kSsrc1);
// Two or more encodings should result in failure.
parameters.encodings.push_back(webrtc::RtpEncodingParameters());
EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrc1, parameters));
// Zero encodings should also fail.
parameters.encodings.clear();
EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrc1, parameters));
}
// Changing the SSRC through RtpParameters is not allowed.
TEST_F(WebRtcVoiceEngineTestFake, CannotSetSsrcInRtpSendParameters) {
EXPECT_TRUE(SetupSendStream());
webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(kSsrc1);
parameters.encodings[0].ssrc = rtc::Optional<uint32_t>(0xdeadbeef);
EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrc1, parameters));
}
// Test that a stream will not be sending if its encoding is made