Skip to content

Commit

Permalink
Update to allow the application to set a low max bitrate.
Browse files Browse the repository at this point in the history
A bug surfaced when setting a low max bitrate with
30kbps hard-coded min bitrate value then a DCHECK was hit in the
VideoCodecInitializer, expecting the max bitrate to be higher than the
min bitrate. This change allows the application to set a max bitrate
below 30kbps, and adjusts the min bitrate to the value set for the
max bitrate.

RtpSender: :setParameters. If the value set was lower than the
Bug: webrtc:9141
Change-Id: I9b43ee7814b1a2caba00bc9614fc66d4438d66d8
Reviewed-on: https://webrtc-review.googlesource.com/74641
Commit-Queue: Seth Hampson <[email protected]>
Reviewed-by: Taylor Brandstetter <[email protected]>
Cr-Commit-Position: refs/heads/master@{#23179}
  • Loading branch information
Seth Hampson authored and Commit Bot committed May 8, 2018
1 parent 4e268ed commit 7c682e0
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 2 deletions.
6 changes: 5 additions & 1 deletion media/engine/webrtcvideoengine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2688,7 +2688,11 @@ std::vector<webrtc::VideoStream> EncoderStreamFactory::CreateEncoderStreams(
layer.width = width;
layer.height = height;
layer.max_framerate = max_framerate_;
layer.min_bitrate_bps = GetMinVideoBitrateBps();
// The min bitrate is hardcoded, but the max_bitrate_bps is set by the
// application. In the case that the application sets a max bitrate
// that's lower than the min bitrate, we adjust it down (see
// bugs.webrtc.org/9141).
layer.min_bitrate_bps = std::min(GetMinVideoBitrateBps(), max_bitrate_bps);
layer.target_bitrate_bps = layer.max_bitrate_bps = max_bitrate_bps;
layer.max_qp = max_qp_;
layer.bitrate_priority = encoder_config.bitrate_priority;
Expand Down
31 changes: 30 additions & 1 deletion media/engine/webrtcvideoengine_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5157,7 +5157,7 @@ TEST_F(WebRtcVideoChannelTest,
EXPECT_EQ(kSsrcs3[1], recv_stream1->GetConfig().rtp.remote_ssrc);
}

TEST_F(WebRtcVideoChannelTest, CanSentMaxBitrateForExistingStream) {
TEST_F(WebRtcVideoChannelTest, CanSetMaxBitrateForExistingStream) {
AddSendStream();

FakeVideoCapturerWithTaskQueue capturer;
Expand Down Expand Up @@ -5199,6 +5199,35 @@ TEST_F(WebRtcVideoChannelTest, CannotSetMaxBitrateForNonexistentStream) {
channel_->SetRtpSendParameters(last_ssrc_, nonexistent_parameters).ok());
}

TEST_F(WebRtcVideoChannelTest,
SetLowMaxBitrateOverwritesVideoStreamMinBitrate) {
AddSendStream();
webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(last_ssrc_);
EXPECT_EQ(1UL, parameters.encodings.size());
EXPECT_FALSE(parameters.encodings[0].max_bitrate_bps.has_value());
EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok());

// Note that this is testing the behavior of the FakeVideoSendStream, which
// also calls to CreateEncoderStreams to get the VideoStreams, so essentially
// we are just testing the behavior of
// EncoderStreamFactory::CreateEncoderStreams.
std::vector<webrtc::VideoStream> video_streams =
fake_call_->GetVideoSendStreams().front()->GetVideoStreams();
ASSERT_EQ(1UL, video_streams.size());
EXPECT_EQ(kMinVideoBitrateBps, video_streams[0].min_bitrate_bps);

// Set a low max bitrate & check that VideoStream.min_bitrate_bps is limited
// by this amount.
parameters = channel_->GetRtpSendParameters(last_ssrc_);
int low_max_bitrate_bps = kMinVideoBitrateBps - 1000;
parameters.encodings[0].max_bitrate_bps = low_max_bitrate_bps;
EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok());

video_streams = fake_call_->GetVideoSendStreams().front()->GetVideoStreams();
ASSERT_EQ(1UL, video_streams.size());
EXPECT_GE(low_max_bitrate_bps, video_streams[0].min_bitrate_bps);
}

TEST_F(WebRtcVideoChannelTest,
CannotSetRtpSendParametersWithIncorrectNumberOfEncodings) {
AddSendStream();
Expand Down

0 comments on commit 7c682e0

Please sign in to comment.