Revert of Drop frames until specified bitrate is achieved. (patchset #12 id:240001 of https://codereview.webrtc.org/2630333002/ ) Reason for revert: due to failures on perf tests (not on perf stats, but fails running due to dcheck failures), see e.g., https://build.chromium.org/p/client.webrtc.perf/builders/Android32%20Tests%20(K%20Nexus5) Original issue's description: > Drop frames until specified bitrate is achieved. > > This CL fixes a regression introduced with the new quality scaler > where the video would no longer start in a scaled mode. This CL adds > code that compares incoming captured frames to the target bitrate, > and if they are found to be too large, they are dropped and sinkWants > set to a lower resolution. The number of dropped frames should be low > (0-4 in most cases) and should not introduce a noticeable delay, or > at least should be preferrable to having the first 2-4 seconds of video > have very low quality. > > BUG=webrtc:6953 > > Review-Url: https://codereview.webrtc.org/2630333002 > Cr-Commit-Position: refs/heads/master@{#16391} > Committed: https://chromium.googlesource.com/external/webrtc/+/83399caec5762d2dad038b8e9d86163e92c18c9f TBR=perkj@webrtc.org,sprang@webrtc.org,stefan@webrtc.org,kthelgason@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:6953 Review-Url: https://codereview.webrtc.org/2666303002 Cr-Commit-Position: refs/heads/master@{#16395}
diff --git a/webrtc/api/video/video_frame.cc b/webrtc/api/video/video_frame.cc index b264746..0e3efb2 100644 --- a/webrtc/api/video/video_frame.cc +++ b/webrtc/api/video/video_frame.cc
@@ -51,10 +51,6 @@ return video_frame_buffer_ ? video_frame_buffer_->height() : 0; } -uint32_t VideoFrame::size() const { - return width() * height(); -} - rtc::scoped_refptr<VideoFrameBuffer> VideoFrame::video_frame_buffer() const { return video_frame_buffer_; }
diff --git a/webrtc/api/video/video_frame.h b/webrtc/api/video/video_frame.h index 8840782..5c57213 100644 --- a/webrtc/api/video/video_frame.h +++ b/webrtc/api/video/video_frame.h
@@ -48,10 +48,9 @@ // Get frame width. int width() const; + // Get frame height. int height() const; - // Get frame size in pixels. - uint32_t size() const; // System monotonic clock, same timebase as rtc::TimeMicros(). int64_t timestamp_us() const { return timestamp_us_; }
diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index 22606a6..ef77e9e 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc
@@ -101,12 +101,6 @@ it->second == config.rtp.ulpfec.red_rtx_payload_type); } } - -cricket::MediaConfig GetMediaConfig() { - cricket::MediaConfig media_config; - media_config.video.enable_cpu_overuse_detection = false; - return media_config; -} } // namespace namespace cricket { @@ -335,7 +329,7 @@ TEST_F(WebRtcVideoEngine2Test, SetSendFailsBeforeSettingCodecs) { engine_.Init(); std::unique_ptr<VideoMediaChannel> channel( - engine_.CreateChannel(call_.get(), GetMediaConfig(), VideoOptions())); + engine_.CreateChannel(call_.get(), MediaConfig(), VideoOptions())); EXPECT_TRUE(channel->AddSendStream(StreamParams::CreateLegacy(123))); @@ -348,7 +342,7 @@ TEST_F(WebRtcVideoEngine2Test, GetStatsWithoutSendCodecsSetDoesNotCrash) { engine_.Init(); std::unique_ptr<VideoMediaChannel> channel( - engine_.CreateChannel(call_.get(), GetMediaConfig(), VideoOptions())); + engine_.CreateChannel(call_.get(), MediaConfig(), VideoOptions())); EXPECT_TRUE(channel->AddSendStream(StreamParams::CreateLegacy(123))); VideoMediaInfo info; channel->GetStats(&info); @@ -443,7 +437,7 @@ } else { engine_.Init(); channel.reset( - engine_.CreateChannel(call_.get(), GetMediaConfig(), VideoOptions())); + engine_.CreateChannel(call_.get(), MediaConfig(), VideoOptions())); } ASSERT_TRUE( channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrc))); @@ -553,7 +547,7 @@ engine_.Init(); VideoMediaChannel* channel = - engine_.CreateChannel(call_.get(), GetMediaConfig(), VideoOptions()); + engine_.CreateChannel(call_.get(), MediaConfig(), VideoOptions()); cricket::VideoSendParameters parameters; // We need to look up the codec in the engine to get the correct payload type. for (const VideoCodec& codec : encoder_factory->supported_codecs()) @@ -571,7 +565,7 @@ engine_.Init(); VideoMediaChannel* channel = - engine_.CreateChannel(call_.get(), GetMediaConfig(), VideoOptions()); + engine_.CreateChannel(call_.get(), MediaConfig(), VideoOptions()); cricket::VideoRecvParameters parameters; parameters.codecs = codecs; EXPECT_TRUE(channel->SetRecvParameters(parameters)); @@ -644,7 +638,7 @@ engine_.Init(); std::unique_ptr<VideoMediaChannel> channel( - engine_.CreateChannel(call_.get(), GetMediaConfig(), VideoOptions())); + engine_.CreateChannel(call_.get(), MediaConfig(), VideoOptions())); cricket::VideoSendParameters parameters; parameters.codecs.push_back(GetEngineCodec("VP8")); EXPECT_TRUE(channel->SetSendParameters(parameters)); @@ -665,7 +659,7 @@ engine_.Init(); std::unique_ptr<VideoMediaChannel> channel( - engine_.CreateChannel(call_.get(), GetMediaConfig(), VideoOptions())); + engine_.CreateChannel(call_.get(), MediaConfig(), VideoOptions())); cricket::VideoSendParameters parameters; parameters.codecs.push_back(GetEngineCodec("VP8")); EXPECT_TRUE(channel->SetSendParameters(parameters)); @@ -705,7 +699,7 @@ engine_.Init(); std::unique_ptr<VideoMediaChannel> channel( - engine_.CreateChannel(call_.get(), GetMediaConfig(), VideoOptions())); + engine_.CreateChannel(call_.get(), MediaConfig(), VideoOptions())); cricket::VideoSendParameters parameters; parameters.codecs.push_back(GetEngineCodec("H264")); EXPECT_TRUE(channel->SetSendParameters(parameters)); @@ -968,8 +962,8 @@ void SetUp() override { fake_call_.reset(new FakeCall(webrtc::Call::Config(&event_log_))); engine_.Init(); - channel_.reset(engine_.CreateChannel(fake_call_.get(), GetMediaConfig(), - VideoOptions())); + channel_.reset( + engine_.CreateChannel(fake_call_.get(), MediaConfig(), VideoOptions())); channel_->OnReadyToSend(true); last_ssrc_ = 123; send_parameters_.codecs = engine_.codecs(); @@ -1777,7 +1771,7 @@ } TEST_F(WebRtcVideoChannel2Test, SetMediaConfigSuspendBelowMinBitrate) { - MediaConfig media_config = GetMediaConfig(); + MediaConfig media_config = MediaConfig(); media_config.video.suspend_below_min_bitrate = true; channel_.reset( @@ -2074,7 +2068,7 @@ cricket::VideoSendParameters parameters; parameters.codecs.push_back(codec); - MediaConfig media_config = GetMediaConfig(); + MediaConfig media_config = MediaConfig(); channel_.reset( engine_.CreateChannel(fake_call_.get(), media_config, VideoOptions())); channel_->OnReadyToSend(true); @@ -2149,8 +2143,7 @@ cricket::VideoSendParameters parameters; parameters.codecs.push_back(codec); - MediaConfig media_config = GetMediaConfig(); - media_config.video.enable_cpu_overuse_detection = true; + MediaConfig media_config = MediaConfig(); channel_.reset( engine_.CreateChannel(fake_call_.get(), media_config, VideoOptions())); channel_->OnReadyToSend(true); @@ -2215,9 +2208,9 @@ cricket::VideoSendParameters parameters; parameters.codecs.push_back(codec); - MediaConfig media_config = GetMediaConfig(); - if (enable_overuse) { - media_config.video.enable_cpu_overuse_detection = true; + MediaConfig media_config = MediaConfig(); + if (!enable_overuse) { + media_config.video.enable_cpu_overuse_detection = false; } channel_.reset( engine_.CreateChannel(fake_call_.get(), media_config, VideoOptions())); @@ -4048,7 +4041,7 @@ void SetUp() override { engine_.Init(); channel_.reset( - engine_.CreateChannel(&fake_call_, GetMediaConfig(), VideoOptions())); + engine_.CreateChannel(&fake_call_, MediaConfig(), VideoOptions())); channel_->OnReadyToSend(true); last_ssrc_ = 123; }
diff --git a/webrtc/modules/video_coding/utility/default_video_bitrate_allocator.cc b/webrtc/modules/video_coding/utility/default_video_bitrate_allocator.cc index a914e8a..482c8ec 100644 --- a/webrtc/modules/video_coding/utility/default_video_bitrate_allocator.cc +++ b/webrtc/modules/video_coding/utility/default_video_bitrate_allocator.cc
@@ -35,6 +35,7 @@ } else { allocation.SetBitrate(0, 0, total_bitrate_bps); } + return allocation; }
diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc index 5c312bc..01e64fe 100644 --- a/webrtc/video/vie_encoder.cc +++ b/webrtc/video/vie_encoder.cc
@@ -42,10 +42,6 @@ const int kMinPixelsPerFrame = 120 * 90; #endif -// The maximum number of frames to drop at beginning of stream -// to try and achieve desired bitrate. -const int kMaxInitialFramedrop = 4; - // TODO(pbos): Lower these thresholds (to closer to 100%) when we handle // pipelining encoders better (multiple input frames before something comes // out). This should effectively turn off CPU adaptations for systems that @@ -59,17 +55,6 @@ return options; } -uint32_t MaximumFrameSizeForBitrate(uint32_t kbps) { - if (kbps > 0) { - if (kbps < 300 /* qvga */) { - return 320 * 240; - } else if (kbps < 500 /* vga */) { - return 640 * 480; - } - } - return std::numeric_limits<uint32_t>::max(); -} - } // namespace class ViEEncoder::ConfigureEncoderTask : public rtc::QueuedTask { @@ -259,7 +244,6 @@ EncodedFrameObserver* encoder_timing) : shutdown_event_(true /* manual_reset */, false), number_of_cores_(number_of_cores), - initial_rampup_(0), source_proxy_(new VideoSourceProxy(this)), sink_(nullptr), settings_(settings), @@ -356,8 +340,8 @@ RTC_DCHECK_RUN_ON(&encoder_queue_); scaling_enabled_ = (degradation_preference != VideoSendStream::DegradationPreference::kMaintainResolution); - initial_rampup_ = scaling_enabled_ ? 0 : kMaxInitialFramedrop; - ConfigureQualityScaler(); + stats_proxy_->SetResolutionRestrictionStats( + scaling_enabled_, scale_counter_[kCpu] > 0, scale_counter_[kQuality]); }); } @@ -452,14 +436,8 @@ sink_->OnEncoderConfigurationChanged( std::move(streams), encoder_config_.min_transmit_bitrate_bps); - ConfigureQualityScaler(); -} - -void ViEEncoder::ConfigureQualityScaler() { - RTC_DCHECK_RUN_ON(&encoder_queue_); const auto scaling_settings = settings_.encoder->GetScalingSettings(); if (scaling_enabled_ && scaling_settings.enabled) { - // Drop frames and scale down until desired quality is achieved. if (scaling_settings.thresholds) { quality_scaler_.reset( new QualityScaler(this, *(scaling_settings.thresholds))); @@ -468,9 +446,9 @@ } } else { quality_scaler_.reset(nullptr); + stats_proxy_->SetResolutionRestrictionStats( + false, scale_counter_[kCpu] > 0, scale_counter_[kQuality]); } - stats_proxy_->SetResolutionRestrictionStats( - scaling_enabled_, scale_counter_[kCpu] > 0, scale_counter_[kQuality]); } void ViEEncoder::OnFrame(const VideoFrame& video_frame) { @@ -569,16 +547,6 @@ << ", texture=" << last_frame_info_->is_texture; } - if (initial_rampup_ < kMaxInitialFramedrop && - video_frame.size() > - MaximumFrameSizeForBitrate(encoder_start_bitrate_bps_ / 1000)) { - LOG(LS_INFO) << "Dropping frame. Too large for target bitrate."; - ScaleDown(kQuality); - ++initial_rampup_; - return; - } - initial_rampup_ = kMaxInitialFramedrop; - int64_t now_ms = clock_->TimeInMilliseconds(); if (pending_encoder_reconfiguration_) { ReconfigureEncoder();
diff --git a/webrtc/video/vie_encoder.h b/webrtc/video/vie_encoder.h index ed514c2..4a2ca1f 100644 --- a/webrtc/video/vie_encoder.h +++ b/webrtc/video/vie_encoder.h
@@ -150,8 +150,6 @@ bool nack_enabled); void ReconfigureEncoder(); - void ConfigureQualityScaler(); - // Implements VideoSinkInterface. void OnFrame(const VideoFrame& video_frame) override; @@ -177,8 +175,6 @@ rtc::Event shutdown_event_; const uint32_t number_of_cores_; - // Counts how many frames we've dropped in the initial rampup phase. - int initial_rampup_; const std::unique_ptr<VideoSourceProxy> source_proxy_; EncoderSink* sink_;
diff --git a/webrtc/video/vie_encoder_unittest.cc b/webrtc/video/vie_encoder_unittest.cc index 5dee314..dae172d 100644 --- a/webrtc/video/vie_encoder_unittest.cc +++ b/webrtc/video/vie_encoder_unittest.cc
@@ -43,9 +43,7 @@ namespace { const size_t kMaxPayloadLength = 1440; -const int kTargetBitrateBps = 1000000; -const int kLowTargetBitrateBps = kTargetBitrateBps / 10; -const int kMaxInitialFramedrop = 4; +const int kTargetBitrateBps = 100000; class TestBuffer : public webrtc::I420Buffer { public: @@ -151,7 +149,7 @@ vie_encoder_->SetSink(&sink_, false /* rotation_applied */); vie_encoder_->SetSource(&video_source_, VideoSendStream::DegradationPreference::kBalanced); - vie_encoder_->SetStartBitrate(kTargetBitrateBps); + vie_encoder_->SetStartBitrate(10000); vie_encoder_->ConfigureEncoder(std::move(video_encoder_config), kMaxPayloadLength, nack_enabled); } @@ -164,7 +162,7 @@ VideoEncoderConfig video_encoder_config; video_encoder_config.number_of_streams = num_streams; - video_encoder_config.max_bitrate_bps = kTargetBitrateBps; + video_encoder_config.max_bitrate_bps = 1000000; video_encoder_config.video_stream_factory = new rtc::RefCountedObject<VideoStreamFactory>(num_temporal_layers); ConfigureEncoder(std::move(video_encoder_config), nack_enabled); @@ -265,8 +263,6 @@ test_encoder_->CheckLastTimeStampsMatch(expected_ntp_time, timestamp); } - void ExpectDroppedFrame() { EXPECT_FALSE(encoded_frame_event_.Wait(20)); } - void SetExpectNoFrames() { rtc::CritScope lock(&crit_); expect_frames_ = false; @@ -683,6 +679,7 @@ } TEST_F(ViEEncoderTest, SwitchingSourceKeepsCpuAdaptation) { + const int kTargetBitrateBps = 100000; vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); int frame_width = 1280; @@ -752,6 +749,7 @@ } TEST_F(ViEEncoderTest, SwitchingSourceKeepsQualityAdaptation) { + const int kTargetBitrateBps = 100000; vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); int frame_width = 1280; @@ -887,6 +885,7 @@ } TEST_F(ViEEncoderTest, ScalingUpAndDownDoesNothingWithMaintainResolution) { + const int kTargetBitrateBps = 100000; int frame_width = 1280; int frame_height = 720; vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); @@ -939,6 +938,7 @@ } TEST_F(ViEEncoderTest, DoesNotScaleBelowSetLimit) { + const int kTargetBitrateBps = 100000; int frame_width = 1280; int frame_height = 720; vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); @@ -995,12 +995,12 @@ const int kDefaultFps = 30; const BitrateAllocation expected_bitrate = DefaultVideoBitrateAllocator(fake_encoder_.codec_config()) - .GetAllocation(kLowTargetBitrateBps, kDefaultFps); + .GetAllocation(kTargetBitrateBps, kDefaultFps); // First called on bitrate updated, then again on first frame. EXPECT_CALL(bitrate_observer, OnBitrateAllocationUpdated(expected_bitrate)) .Times(2); - vie_encoder_->OnBitrateUpdated(kLowTargetBitrateBps, 0, 0); + vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); const int64_t kStartTimeMs = 1; video_source_.IncomingCapturedFrame( @@ -1029,76 +1029,4 @@ vie_encoder_->Stop(); } -TEST_F(ViEEncoderTest, DropsFramesAndScalesWhenBitrateIsTooLow) { - vie_encoder_->OnBitrateUpdated(kLowTargetBitrateBps, 0, 0); - int frame_width = 640; - int frame_height = 360; - - video_source_.IncomingCapturedFrame( - CreateFrame(1, frame_width, frame_height)); - - // Expect to drop this frame, the wait should time out. - sink_.ExpectDroppedFrame(); - - // Expect the sink_wants to specify a scaled frame. - EXPECT_TRUE(video_source_.sink_wants().max_pixel_count); - EXPECT_LT(*video_source_.sink_wants().max_pixel_count, 1000 * 1000); - - int last_pixel_count = *video_source_.sink_wants().max_pixel_count; - - // Next frame is scaled - video_source_.IncomingCapturedFrame( - CreateFrame(2, frame_width * 3 / 4, frame_height * 3 / 4)); - - // Expect to drop this frame, the wait should time out. - sink_.ExpectDroppedFrame(); - - EXPECT_LT(*video_source_.sink_wants().max_pixel_count, last_pixel_count); - - vie_encoder_->Stop(); -} - -TEST_F(ViEEncoderTest, NrOfDroppedFramesLimited) { - // 1kbps. This can never be achieved. - vie_encoder_->OnBitrateUpdated(1000, 0, 0); - int frame_width = 640; - int frame_height = 360; - - // We expect the n initial frames to get dropped. - int i; - for (i = 1; i <= kMaxInitialFramedrop; ++i) { - video_source_.IncomingCapturedFrame( - CreateFrame(i, frame_width, frame_height)); - sink_.ExpectDroppedFrame(); - } - // The n+1th frame should not be dropped, even though it's size is too large. - video_source_.IncomingCapturedFrame( - CreateFrame(i, frame_width, frame_height)); - sink_.WaitForEncodedFrame(i); - - // Expect the sink_wants to specify a scaled frame. - EXPECT_TRUE(video_source_.sink_wants().max_pixel_count); - EXPECT_LT(*video_source_.sink_wants().max_pixel_count, 1000 * 1000); - - vie_encoder_->Stop(); -} - -TEST_F(ViEEncoderTest, InitialFrameDropOffWithMaintainResolutionPreference) { - int frame_width = 640; - int frame_height = 360; - vie_encoder_->OnBitrateUpdated(kLowTargetBitrateBps, 0, 0); - - // Set degradation preference. - vie_encoder_->SetSource( - &video_source_, - VideoSendStream::DegradationPreference::kMaintainResolution); - - video_source_.IncomingCapturedFrame( - CreateFrame(1, frame_width, frame_height)); - // Frame should not be dropped, even if it's too large. - sink_.WaitForEncodedFrame(1); - - vie_encoder_->Stop(); -} - } // namespace webrtc