From b8781796d496ab54397e7c6c28c22ba9a7b2e38d Mon Sep 17 00:00:00 2001 From: Per Kjellander Date: Tue, 6 Oct 2020 19:56:17 +0000 Subject: [PATCH] Revert "Refactor reporting of VideoBitrateAllocation" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 39a31afb77e3ce5c4ff53b8bab06364712cae7ce. Reason for revert: Will cause RTCP Target bitrate messages to not be sent unless BWE changes. Original change's description: > Refactor reporting of VideoBitrateAllocation > > 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} TBR=sprang@webrtc.org,perkj@webrtc.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: webrtc:12000 Change-Id: Icf21e6ae28dc17c61b9243c037ffef9b623894ee Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/186945 Reviewed-by: Per Kjellander Reviewed-by: Erik Språng Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/master@{#32337} --- api/video_codecs/video_encoder.h | 3 --- video/video_stream_encoder.cc | 45 ++++++++++++++++++++------------ video/video_stream_encoder.h | 5 ++-- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/api/video_codecs/video_encoder.h b/api/video_codecs/video_encoder.h index ed46691023..f704e3df33 100644 --- a/api/video_codecs/video_encoder.h +++ b/api/video_codecs/video_encoder.h @@ -267,9 +267,6 @@ 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 e7bc15bb0a..2a77219f97 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -874,7 +874,7 @@ void VideoStreamEncoder::ReconfigureEncoder() { last_encoder_rate_settings_.reset(); rate_settings.rate_control.framerate_fps = GetInputFramerateFps(); - SetEncoderRates(UpdateBitrateAllocation(rate_settings)); + SetEncoderRates(UpdateBitrateAllocationAndNotifyObserver(rate_settings)); } encoder_stats_observer_->OnEncoderReconfigured(encoder_config_, streams); @@ -1053,7 +1053,7 @@ void VideoStreamEncoder::TraceFrameDropEnd() { } VideoStreamEncoder::EncoderRateSettings -VideoStreamEncoder::UpdateBitrateAllocation( +VideoStreamEncoder::UpdateBitrateAllocationAndNotifyObserver( const EncoderRateSettings& rate_settings) { VideoBitrateAllocation new_allocation; // Only call allocators if bitrate > 0 (ie, not suspended), otherwise they @@ -1064,8 +1064,24 @@ VideoStreamEncoder::UpdateBitrateAllocation( 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 @@ -1086,6 +1102,9 @@ VideoStreamEncoder::UpdateBitrateAllocation( new_rate_settings.rate_control.bitrate = adjusted_allocation; } + encoder_stats_observer_->OnBitrateAllocationUpdated( + send_codec_, new_rate_settings.rate_control.bitrate); + return new_rate_settings; } @@ -1109,7 +1128,7 @@ void VideoStreamEncoder::SetEncoderRates( last_encoder_rate_settings_ = rate_settings; } - if (!encoder_ || !rate_control_changed) { + if (!encoder_) { return; } @@ -1126,22 +1145,13 @@ 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, @@ -1192,7 +1202,8 @@ 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(UpdateBitrateAllocation(new_rate_settings)); + SetEncoderRates( + UpdateBitrateAllocationAndNotifyObserver(new_rate_settings)); } last_parameters_update_ms_.emplace(now_ms); } @@ -1727,7 +1738,7 @@ void VideoStreamEncoder::OnBitrateUpdated(DataRate target_bitrate, EncoderRateSettings new_rate_settings{ VideoBitrateAllocation(), static_cast(framerate_fps), link_allocation, target_bitrate, stable_target_bitrate}; - SetEncoderRates(UpdateBitrateAllocation(new_rate_settings)); + SetEncoderRates(UpdateBitrateAllocationAndNotifyObserver(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 4c0f84b105..e80d1203c7 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -192,8 +192,9 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, void TraceFrameDropEnd(); // Returns a copy of |rate_settings| with the |bitrate| field updated using - // the current VideoBitrateAllocator. - EncoderRateSettings UpdateBitrateAllocation( + // the current VideoBitrateAllocator, and notifies any listeners of the new + // allocation. + EncoderRateSettings UpdateBitrateAllocationAndNotifyObserver( const EncoderRateSettings& rate_settings) RTC_RUN_ON(&encoder_queue_); uint32_t GetInputFramerateFps() RTC_RUN_ON(&encoder_queue_);