diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc index a1f15e18c5..5932dbf048 100644 --- a/media/engine/webrtcvideoengine.cc +++ b/media/engine/webrtcvideoengine.cc @@ -2688,7 +2688,11 @@ std::vector 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; diff --git a/media/engine/webrtcvideoengine_unittest.cc b/media/engine/webrtcvideoengine_unittest.cc index 54f64707b4..31f56568e7 100644 --- a/media/engine/webrtcvideoengine_unittest.cc +++ b/media/engine/webrtcvideoengine_unittest.cc @@ -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; @@ -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 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();