diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index 129eb3288a..335ee3a691 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -1560,6 +1560,11 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::WebRtcVideoSendStream( sp.GetPrimarySsrcs(¶meters_.config.rtp.ssrcs); + // ValidateStreamParams should prevent this from happening. + RTC_CHECK(!parameters_.config.rtp.ssrcs.empty()); + rtp_parameters_.encodings[0].ssrc = + rtc::Optional(parameters_.config.rtp.ssrcs[0]); + // RTX. sp.GetFidSsrcs(parameters_.config.rtp.ssrcs, ¶meters_.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; } diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index b8495b0546..5944883e1c 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -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(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(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 = diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc index a36f328705..9871d3a21f 100644 --- a/webrtc/media/engine/webrtcvoiceengine.cc +++ b/webrtc/media/engine/webrtcvoiceengine.cc @@ -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: " diff --git a/webrtc/media/engine/webrtcvoiceengine_unittest.cc b/webrtc/media/engine/webrtcvoiceengine_unittest.cc index 3ca7ec8b05..0cfbeab42b 100644 --- a/webrtc/media/engine/webrtcvoiceengine_unittest.cc +++ b/webrtc/media/engine/webrtcvoiceengine_unittest.cc @@ -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(0xdeadbeef); + EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrc1, parameters)); } // Test that a stream will not be sending if its encoding is made