From bdee46d275a3c2fba40f530841eff41ced05626e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85sa=20Persson?= Date: Mon, 25 Jun 2018 11:28:06 +0200 Subject: [PATCH] Add functionality to set min bitrate for single stream through RtpEncodingParameters. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:9341 Change-Id: Ia1e819bd61dbef9c93d0ba84907faeeebf925db2 Reviewed-on: https://webrtc-review.googlesource.com/80261 Commit-Queue: Åsa Persson Reviewed-by: Taylor Brandstetter Cr-Commit-Position: refs/heads/master@{#23722} --- api/rtpparameters.h | 1 - api/video_codecs/video_encoder_config.h | 2 + media/engine/webrtcvideoengine.cc | 21 ++- media/engine/webrtcvideoengine_unittest.cc | 141 ++++++++++++++++++--- 4 files changed, 140 insertions(+), 25 deletions(-) diff --git a/api/rtpparameters.h b/api/rtpparameters.h index 84da811224..ba318ca040 100644 --- a/api/rtpparameters.h +++ b/api/rtpparameters.h @@ -411,7 +411,6 @@ struct RtpEncodingParameters { // Specifies the minimum bitrate in bps for video. // TODO(asapersson): Not implemented for ORTC API. - // TODO(asapersson): Not implemented for single layer. absl::optional min_bitrate_bps; // TODO(deadbeef): Not implemented. diff --git a/api/video_codecs/video_encoder_config.h b/api/video_codecs/video_encoder_config.h index 47ff9254a1..e10f08190c 100644 --- a/api/video_codecs/video_encoder_config.h +++ b/api/video_codecs/video_encoder_config.h @@ -146,6 +146,8 @@ class VideoEncoderConfig { // The simulcast layer's configurations set by the application for this video // sender. These are modified by the video_stream_factory before being passed // down to lower layers for the video encoding. + // |simulcast_layers| is also used for configuring non-simulcast (when there + // is a single VideoStream). std::vector simulcast_layers; // Max number of encoded VideoStreams to produce. diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc index 08de675eda..9466718961 100644 --- a/media/engine/webrtcvideoengine.cc +++ b/media/engine/webrtcvideoengine.cc @@ -1969,6 +1969,8 @@ WebRtcVideoChannel::WebRtcVideoSendStream::CreateVideoEncoderConfig( // Application-controlled state is held in the encoder_config's // simulcast_layers. Currently this is used to control which simulcast layers // are active and for configuring the min/max bitrate. + // The encoder_config's simulcast_layers is also used for non-simulcast (when + // there is a single layer). RTC_DCHECK_GE(rtp_parameters_.encodings.size(), encoder_config.number_of_streams); RTC_DCHECK_GT(encoder_config.number_of_streams, 0); @@ -2710,6 +2712,7 @@ std::vector EncoderStreamFactory::CreateEncoderStreams( if (is_screenshare_ && !screenshare_simulcast_enabled) { RTC_DCHECK_EQ(1, encoder_config.number_of_streams); } + RTC_DCHECK_GT(encoder_config.number_of_streams, 0); RTC_DCHECK_EQ(encoder_config.simulcast_layers.size(), encoder_config.number_of_streams); std::vector layers; @@ -2775,15 +2778,23 @@ std::vector EncoderStreamFactory::CreateEncoderStreams( ? encoder_config.max_bitrate_bps : GetMaxDefaultVideoBitrateKbps(width, height) * 1000; + int min_bitrate_bps = GetMinVideoBitrateBps(); + if (encoder_config.simulcast_layers[0].min_bitrate_bps > 0) { + // Use set min bitrate. + min_bitrate_bps = encoder_config.simulcast_layers[0].min_bitrate_bps; + // If only min bitrate is configured, make sure max is above min. + if (encoder_config.max_bitrate_bps <= 0) + max_bitrate_bps = std::max(min_bitrate_bps, max_bitrate_bps); + } + webrtc::VideoStream layer; layer.width = width; layer.height = height; layer.max_framerate = max_framerate_; - // 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); + + // 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(min_bitrate_bps, 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 039de76c65..5dbd329ee9 100644 --- a/media/engine/webrtcvideoengine_unittest.cc +++ b/media/engine/webrtcvideoengine_unittest.cc @@ -161,6 +161,19 @@ cricket::MediaConfig GetMediaConfig() { return media_config; } +// Values from GetMaxDefaultVideoBitrateKbps in webrtcvideoengine.cc. +int GetMaxDefaultBitrateBps(size_t width, size_t height) { + if (width * height <= 320 * 240) { + return 600000; + } else if (width * height <= 640 * 480) { + return 1700000; + } else if (width * height <= 960 * 540) { + return 2000000; + } else { + return 2500000; + } +} + } // namespace #define EXPECT_FRAME_WAIT(c, w, h, t) \ @@ -5231,7 +5244,8 @@ TEST_F(WebRtcVideoChannelTest, CannotSetMaxBitrateForNonexistentStream) { TEST_F(WebRtcVideoChannelTest, SetLowMaxBitrateOverwritesVideoStreamMinBitrate) { - AddSendStream(); + FakeVideoSendStream* stream = AddSendStream(); + webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(last_ssrc_); EXPECT_EQ(1UL, parameters.encodings.size()); EXPECT_FALSE(parameters.encodings[0].max_bitrate_bps.has_value()); @@ -5241,10 +5255,8 @@ TEST_F(WebRtcVideoChannelTest, // 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); + ASSERT_EQ(1UL, stream->GetVideoStreams().size()); + EXPECT_EQ(kMinVideoBitrateBps, stream->GetVideoStreams()[0].min_bitrate_bps); // Set a low max bitrate & check that VideoStream.min_bitrate_bps is limited // by this amount. @@ -5253,9 +5265,55 @@ TEST_F(WebRtcVideoChannelTest, 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); + ASSERT_EQ(1UL, stream->GetVideoStreams().size()); + EXPECT_EQ(low_max_bitrate_bps, stream->GetVideoStreams()[0].min_bitrate_bps); + EXPECT_EQ(low_max_bitrate_bps, stream->GetVideoStreams()[0].max_bitrate_bps); +} + +TEST_F(WebRtcVideoChannelTest, + SetHighMinBitrateOverwritesVideoStreamMaxBitrate) { + FakeVideoSendStream* stream = AddSendStream(); + + // 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. + ASSERT_EQ(1UL, stream->GetVideoStreams().size()); + int high_min_bitrate_bps = stream->GetVideoStreams()[0].max_bitrate_bps + 1; + + // Set a high min bitrate and check that max_bitrate_bps is adjusted up. + webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(last_ssrc_); + EXPECT_EQ(1UL, parameters.encodings.size()); + parameters.encodings[0].min_bitrate_bps = high_min_bitrate_bps; + EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); + + ASSERT_EQ(1UL, stream->GetVideoStreams().size()); + EXPECT_EQ(high_min_bitrate_bps, stream->GetVideoStreams()[0].min_bitrate_bps); + EXPECT_EQ(high_min_bitrate_bps, stream->GetVideoStreams()[0].max_bitrate_bps); +} + +TEST_F(WebRtcVideoChannelTest, + SetMinBitrateAboveMaxBitrateLimitAdjustsMinBitrateDown) { + send_parameters_.max_bandwidth_bps = 99999; + FakeVideoSendStream* stream = AddSendStream(); + ExpectSetMaxBitrate(send_parameters_.max_bandwidth_bps); + ASSERT_TRUE(channel_->SetSendParameters(send_parameters_)); + ASSERT_EQ(1UL, stream->GetVideoStreams().size()); + EXPECT_EQ(kMinVideoBitrateBps, stream->GetVideoStreams()[0].min_bitrate_bps); + EXPECT_EQ(send_parameters_.max_bandwidth_bps, + stream->GetVideoStreams()[0].max_bitrate_bps); + + // Set min bitrate above global max bitrate and check that min_bitrate_bps is + // adjusted down. + webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(last_ssrc_); + EXPECT_EQ(1UL, parameters.encodings.size()); + parameters.encodings[0].min_bitrate_bps = 99999 + 1; + EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); + ASSERT_EQ(1UL, stream->GetVideoStreams().size()); + EXPECT_EQ(send_parameters_.max_bandwidth_bps, + stream->GetVideoStreams()[0].min_bitrate_bps); + EXPECT_EQ(send_parameters_.max_bandwidth_bps, + stream->GetVideoStreams()[0].max_bitrate_bps); } TEST_F(WebRtcVideoChannelTest, @@ -5732,6 +5790,61 @@ TEST_F(WebRtcVideoChannelTest, EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, nullptr, nullptr)); } +// Test that min and max bitrate values set via RtpParameters are correctly +// propagated to the underlying encoder for a single stream. +TEST_F(WebRtcVideoChannelTest, MinAndMaxBitratePropagatedToEncoder) { + FakeVideoSendStream* stream = AddSendStream(); + EXPECT_TRUE(channel_->SetSend(true)); + EXPECT_TRUE(stream->IsSending()); + + // Set min and max bitrate. + webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(last_ssrc_); + EXPECT_EQ(1u, parameters.encodings.size()); + parameters.encodings[0].min_bitrate_bps = 80000; + parameters.encodings[0].max_bitrate_bps = 150000; + EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); + + // Check that WebRtcVideoSendStream updates VideoEncoderConfig correctly. + webrtc::VideoEncoderConfig encoder_config = stream->GetEncoderConfig().Copy(); + EXPECT_EQ(1u, encoder_config.number_of_streams); + EXPECT_EQ(1u, encoder_config.simulcast_layers.size()); + EXPECT_EQ(80000, encoder_config.simulcast_layers[0].min_bitrate_bps); + EXPECT_EQ(150000, encoder_config.simulcast_layers[0].max_bitrate_bps); + + // FakeVideoSendStream calls CreateEncoderStreams, test that the vector of + // VideoStreams are created appropriately. + EXPECT_EQ(1u, stream->GetVideoStreams().size()); + EXPECT_EQ(80000, stream->GetVideoStreams()[0].min_bitrate_bps); + EXPECT_EQ(150000, stream->GetVideoStreams()[0].target_bitrate_bps); + EXPECT_EQ(150000, stream->GetVideoStreams()[0].max_bitrate_bps); +} + +// Test the default min and max bitrate value are correctly propagated to the +// underlying encoder for a single stream (when the values are not set via +// RtpParameters). +TEST_F(WebRtcVideoChannelTest, DefaultMinAndMaxBitratePropagatedToEncoder) { + FakeVideoSendStream* stream = AddSendStream(); + EXPECT_TRUE(channel_->SetSend(true)); + EXPECT_TRUE(stream->IsSending()); + + // Check that WebRtcVideoSendStream updates VideoEncoderConfig correctly. + webrtc::VideoEncoderConfig encoder_config = stream->GetEncoderConfig().Copy(); + EXPECT_EQ(1u, encoder_config.number_of_streams); + EXPECT_EQ(1u, encoder_config.simulcast_layers.size()); + EXPECT_EQ(-1, encoder_config.simulcast_layers[0].min_bitrate_bps); + EXPECT_EQ(-1, encoder_config.simulcast_layers[0].max_bitrate_bps); + + // FakeVideoSendStream calls CreateEncoderStreams, test that the vector of + // VideoStreams are created appropriately. + EXPECT_EQ(1u, stream->GetVideoStreams().size()); + EXPECT_EQ(cricket::kMinVideoBitrateBps, + stream->GetVideoStreams()[0].min_bitrate_bps); + EXPECT_GT(stream->GetVideoStreams()[0].max_bitrate_bps, + stream->GetVideoStreams()[0].min_bitrate_bps); + EXPECT_EQ(stream->GetVideoStreams()[0].max_bitrate_bps, + stream->GetVideoStreams()[0].target_bitrate_bps); +} + // Test that a stream will not be sending if its encoding is made inactive // through SetRtpSendParameters. TEST_F(WebRtcVideoChannelTest, SetRtpSendParametersOneEncodingActive) { @@ -6200,18 +6313,8 @@ class WebRtcVideoChannelSimulcastTest : public testing::Test { stream.height = capture_height; stream.max_framerate = kDefaultVideoMaxFramerate; stream.min_bitrate_bps = cricket::kMinVideoBitrateBps; - int max_bitrate_kbps; - if (capture_width * capture_height <= 320 * 240) { - max_bitrate_kbps = 600; - } else if (capture_width * capture_height <= 640 * 480) { - max_bitrate_kbps = 1700; - } else if (capture_width * capture_height <= 960 * 540) { - max_bitrate_kbps = 2000; - } else { - max_bitrate_kbps = 2500; - } stream.target_bitrate_bps = stream.max_bitrate_bps = - max_bitrate_kbps * 1000; + GetMaxDefaultBitrateBps(capture_width, capture_height); stream.max_qp = kDefaultQpMax; expected_streams.push_back(stream); }