From 39a31afb77e3ce5c4ff53b8bab06364712cae7ce Mon Sep 17 00:00:00 2001 From: Per Kjellander Date: Thu, 1 Oct 2020 15:19:31 +0200 Subject: [PATCH] Refactor reporting of VideoBitrateAllocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move reporting of target bitrate to just after the encoder has been updated. Bug: webrtc:12000 Change-Id: I3e7c5bd44c2f64e5f7e32d6451861b80e0b779ca Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/186041 Commit-Queue: Per Kjellander Reviewed-by: Erik Språng Cr-Commit-Position: refs/heads/master@{#32275} --- api/video_codecs/video_encoder.h | 3 +++ video/video_stream_encoder.cc | 45 ++++++++++++-------------------- video/video_stream_encoder.h | 5 ++-- 3 files changed, 22 insertions(+), 31 deletions(-) diff --git a/api/video_codecs/video_encoder.h b/api/video_codecs/video_encoder.h index d73c9f0dcb..9ef8021190 100644 --- a/api/video_codecs/video_encoder.h +++ b/api/video_codecs/video_encoder.h @@ -267,6 +267,9 @@ class RTC_EXPORT VideoEncoder { // Target bitrate, per spatial/temporal layer. // A target bitrate of 0bps indicates a layer should not be encoded at all. + VideoBitrateAllocation target_bitrate; + // Adjusted target bitrate, per spatial/temporal layer. May be lower or + // higher than the target depending on encoder behaviour. VideoBitrateAllocation bitrate; // Target framerate, in fps. A value <= 0.0 is invalid and should be // interpreted as framerate target not available. In this case the encoder diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index f229a6097e..1ecbef108c 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -875,7 +875,7 @@ void VideoStreamEncoder::ReconfigureEncoder() { last_encoder_rate_settings_.reset(); rate_settings.rate_control.framerate_fps = GetInputFramerateFps(); - SetEncoderRates(UpdateBitrateAllocationAndNotifyObserver(rate_settings)); + SetEncoderRates(UpdateBitrateAllocation(rate_settings)); } encoder_stats_observer_->OnEncoderReconfigured(encoder_config_, streams); @@ -1054,7 +1054,7 @@ void VideoStreamEncoder::TraceFrameDropEnd() { } VideoStreamEncoder::EncoderRateSettings -VideoStreamEncoder::UpdateBitrateAllocationAndNotifyObserver( +VideoStreamEncoder::UpdateBitrateAllocation( const EncoderRateSettings& rate_settings) { VideoBitrateAllocation new_allocation; // Only call allocators if bitrate > 0 (ie, not suspended), otherwise they @@ -1065,24 +1065,8 @@ VideoStreamEncoder::UpdateBitrateAllocationAndNotifyObserver( rate_settings.rate_control.framerate_fps)); } - if (bitrate_observer_ && new_allocation.get_sum_bps() > 0) { - if (encoder_ && encoder_initialized_) { - // Avoid too old encoder_info_. - const int64_t kMaxDiffMs = 100; - const bool updated_recently = - (last_encode_info_ms_ && ((clock_->TimeInMilliseconds() - - *last_encode_info_ms_) < kMaxDiffMs)); - // Update allocation according to info from encoder. - bitrate_observer_->OnBitrateAllocationUpdated( - UpdateAllocationFromEncoderInfo( - new_allocation, - updated_recently ? encoder_info_ : encoder_->GetEncoderInfo())); - } else { - bitrate_observer_->OnBitrateAllocationUpdated(new_allocation); - } - } - EncoderRateSettings new_rate_settings = rate_settings; + new_rate_settings.rate_control.target_bitrate = new_allocation; new_rate_settings.rate_control.bitrate = new_allocation; // VideoBitrateAllocator subclasses may allocate a bitrate higher than the // target in order to sustain the min bitrate of the video codec. In this @@ -1103,9 +1087,6 @@ VideoStreamEncoder::UpdateBitrateAllocationAndNotifyObserver( new_rate_settings.rate_control.bitrate = adjusted_allocation; } - encoder_stats_observer_->OnBitrateAllocationUpdated( - send_codec_, new_rate_settings.rate_control.bitrate); - return new_rate_settings; } @@ -1129,7 +1110,7 @@ void VideoStreamEncoder::SetEncoderRates( last_encoder_rate_settings_ = rate_settings; } - if (!encoder_) { + if (!encoder_ || !rate_control_changed) { return; } @@ -1146,13 +1127,22 @@ void VideoStreamEncoder::SetEncoderRates( return; } - if (rate_control_changed) { encoder_->SetRates(rate_settings.rate_control); + + encoder_stats_observer_->OnBitrateAllocationUpdated( + send_codec_, rate_settings.rate_control.bitrate); frame_encode_metadata_writer_.OnSetRates( rate_settings.rate_control.bitrate, static_cast(rate_settings.rate_control.framerate_fps + 0.5)); stream_resource_manager_.SetEncoderRates(rate_settings.rate_control); - } + if (bitrate_observer_) { + bitrate_observer_->OnBitrateAllocationUpdated( + // Update allocation according to info from encoder. An encoder may + // choose to not use all layers due to for example HW. + UpdateAllocationFromEncoderInfo( + rate_settings.rate_control.target_bitrate, + encoder_->GetEncoderInfo())); + } } void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, @@ -1203,8 +1193,7 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, EncoderRateSettings new_rate_settings = *last_encoder_rate_settings_; new_rate_settings.rate_control.framerate_fps = static_cast(framerate_fps); - SetEncoderRates( - UpdateBitrateAllocationAndNotifyObserver(new_rate_settings)); + SetEncoderRates(UpdateBitrateAllocation(new_rate_settings)); } last_parameters_update_ms_.emplace(now_ms); } @@ -1745,7 +1734,7 @@ void VideoStreamEncoder::OnBitrateUpdated(DataRate target_bitrate, EncoderRateSettings new_rate_settings{ VideoBitrateAllocation(), static_cast(framerate_fps), link_allocation, target_bitrate, stable_target_bitrate}; - SetEncoderRates(UpdateBitrateAllocationAndNotifyObserver(new_rate_settings)); + SetEncoderRates(UpdateBitrateAllocation(new_rate_settings)); if (target_bitrate.bps() != 0) encoder_target_bitrate_bps_ = target_bitrate.bps(); diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index e80d1203c7..4c0f84b105 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -192,9 +192,8 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, void TraceFrameDropEnd(); // Returns a copy of |rate_settings| with the |bitrate| field updated using - // the current VideoBitrateAllocator, and notifies any listeners of the new - // allocation. - EncoderRateSettings UpdateBitrateAllocationAndNotifyObserver( + // the current VideoBitrateAllocator. + EncoderRateSettings UpdateBitrateAllocation( const EncoderRateSettings& rate_settings) RTC_RUN_ON(&encoder_queue_); uint32_t GetInputFramerateFps() RTC_RUN_ON(&encoder_queue_);