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

If encoding is inactive, don't start sending when stream is reconfigured.

Browse Source 
RecreateWebRtcStream was checking the "sending_" flag, but wasn't
checking rtp_parameters_.encodings[0].active. As a result, if an
application calls "RtpSender.setParameters(inactive_params)" then later
the stream is recreated due to some change in SDP parameters, the stream
would incorrectly start sending.

R=pthatcher@webrtc.org, skvlad@webrtc.org

Review URL: https://codereview.webrtc.org/2246313002 .

Cr-Commit-Position: refs/heads/master@{#14116}
This commit is contained in:
Taylor Brandstetter
2016-09-07 17:16:54 -07:00
parent 7610f85a2b
commit 14b9d79bd6
2 changed files with 44 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
webrtc/media/engine/webrtcvideoengine2.cc
View File

@ -2246,9 +2246,8 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::RecreateWebRtcStream() {
parameters_.encoder_config.encoder_specific_settings = NULL;
pending_encoder_reconfiguration_ = false;
if (sending_) {
stream_->Start();
}
// Call stream_->Start() if necessary conditions are met.
UpdateSendState();
}
WebRtcVideoChannel2::WebRtcVideoReceiveStream::WebRtcVideoReceiveStream(

44
webrtc/media/engine/webrtcvideoengine2_unittest.cc
View File

@ -3459,8 +3459,8 @@ TEST_F(WebRtcVideoChannel2Test,
EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters));
}
// Test that a stream will not be sending if its encoding is made
// inactive through SetRtpSendParameters.
// Test that a stream will not be sending if its encoding is made inactive
// through SetRtpSendParameters.
// TODO(deadbeef): Update this test when we start supporting setting parameters
// for each encoding individually.
TEST_F(WebRtcVideoChannel2Test, SetRtpSendParametersEncodingsActive) {
@ -3482,6 +3482,46 @@ TEST_F(WebRtcVideoChannel2Test, SetRtpSendParametersEncodingsActive) {
EXPECT_TRUE(stream->IsSending());
}
// Test that if a stream is reconfigured (due to a codec change or other
// change) while its encoding is still inactive, it doesn't start sending.
TEST_F(WebRtcVideoChannel2Test,
InactiveStreamDoesntStartSendingWhenReconfigured) {
// Set an initial codec list, which will be modified later.
cricket::VideoSendParameters parameters1;
parameters1.codecs.push_back(kVp8Codec);
parameters1.codecs.push_back(kVp9Codec);
EXPECT_TRUE(channel_->SetSendParameters(parameters1));
FakeVideoSendStream* stream = AddSendStream();
EXPECT_TRUE(channel_->SetSend(true));
EXPECT_TRUE(stream->IsSending());
// Get current parameters and change "active" to false.
webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(last_ssrc_);
ASSERT_EQ(1u, parameters.encodings.size());
ASSERT_TRUE(parameters.encodings[0].active);
parameters.encodings[0].active = false;
EXPECT_EQ(1u, GetFakeSendStreams().size());
EXPECT_EQ(1, fake_call_->GetNumCreatedSendStreams());
EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters));
EXPECT_FALSE(stream->IsSending());
// Reorder the codec list, causing the stream to be reconfigured.
cricket::VideoSendParameters parameters2;
parameters2.codecs.push_back(kVp9Codec);
parameters2.codecs.push_back(kVp8Codec);
EXPECT_TRUE(channel_->SetSendParameters(parameters2));
auto new_streams = GetFakeSendStreams();
// Assert that a new underlying stream was created due to the codec change.
// Otherwise, this test isn't testing what it set out to test.
EXPECT_EQ(1u, GetFakeSendStreams().size());
EXPECT_EQ(2, fake_call_->GetNumCreatedSendStreams());
// Verify that we still are not sending anything, due to the inactive
// encoding.
EXPECT_FALSE(new_streams[0]->IsSending());
}
// Test that GetRtpSendParameters returns the currently configured codecs.
TEST_F(WebRtcVideoChannel2Test, GetRtpSendParametersCodecs) {
AddSendStream();