From 6bb5ab9740d22eacfbf7b4b5bd100a1b667b2911 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Fri, 11 Jan 2019 11:11:10 +0100 Subject: [PATCH] Reland "Refactor and remove media_optimization::MediaOptimization." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 6613f8e98ab3654ade7e8f5352d8d6711b157499. Reason for revert: This change seemed innocent after all, so undoing speculative revert. Original change's description: > Revert "Refactor and remove media_optimization::MediaOptimization." > > This reverts commit 07276e4f89a93b1479d7aeefa53b4fc32daf001b. > > Reason for revert: Speculative revert due to downstream crashes. > > Original change's description: > > Refactor and remove media_optimization::MediaOptimization. > > > > This CL removes MediaOptmization and folds some of its functionality > > into VideoStreamEncoder. > > > > The FPS tracking is now handled by a RateStatistics instance. Frame > > dropping is still handled by FrameDropper. Both of these now live > > directly in VideoStreamEncoder. > > There is no intended change in behavior from this CL, but due to a new > > way of measuring frame rate, some minor perf changes can be expected. > > > > A small change in behavior is that OnBitrateUpdated is now called > > directly rather than on the next frame. Since both encoding frame and > > setting rate allocations happen on the encoder worker thread, there's > > really no reason to cache bitrates and wait until the next frame. > > An edge case though is that if a new bitrate is set before the first > > frame, we must remember that bitrate and then apply it after the video > > bitrate allocator has been first created. > > > > In addition to existing unit tests, manual tests have been used to > > confirm that frame dropping works as expected with misbehaving encoders. > > > > Bug: webrtc:10164 > > Change-Id: I7ee9c8d3c4f2bcf23c8c420310b05a4d35d94744 > > Reviewed-on: https://webrtc-review.googlesource.com/c/115620 > > Commit-Queue: Erik Språng > > Reviewed-by: Niels Moller > > Cr-Commit-Position: refs/heads/master@{#26147} > > TBR=nisse@webrtc.org,sprang@webrtc.org > > # Not skipping CQ checks because original CL landed > 1 day ago. > > Bug: webrtc:10164 > Change-Id: Ie0dae19dd012bc09e793c9661a45823fd760c20c > Reviewed-on: https://webrtc-review.googlesource.com/c/116780 > Reviewed-by: Niels Moller > Reviewed-by: Erik Språng > Commit-Queue: Niels Moller > Cr-Commit-Position: refs/heads/master@{#26191} TBR=nisse@webrtc.org,sprang@webrtc.org Change-Id: Ieda1fad301de002460bb0bf5a75267ea065176a8 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:10164 Reviewed-on: https://webrtc-review.googlesource.com/c/116960 Reviewed-by: Niels Moller Reviewed-by: Erik Språng Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#26213} --- modules/video_coding/BUILD.gn | 2 - modules/video_coding/generic_encoder.cc | 50 ++--- modules/video_coding/generic_encoder.h | 34 +--- .../video_coding/generic_encoder_unittest.cc | 10 +- modules/video_coding/media_optimization.cc | 145 --------------- modules/video_coding/media_optimization.h | 78 -------- modules/video_coding/video_coding_impl.h | 41 +---- modules/video_coding/video_sender.cc | 171 +++--------------- modules/video_coding/video_sender_unittest.cc | 28 +-- video/video_send_stream_tests.cc | 18 +- video/video_stream_encoder.cc | 170 +++++++++++++++-- video/video_stream_encoder.h | 27 +++ video/video_stream_encoder_unittest.cc | 113 +++++++++++- 13 files changed, 370 insertions(+), 517 deletions(-) delete mode 100644 modules/video_coding/media_optimization.cc delete mode 100644 modules/video_coding/media_optimization.h diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn index 6eec2f4a4c..188b6244bb 100644 --- a/modules/video_coding/BUILD.gn +++ b/modules/video_coding/BUILD.gn @@ -133,8 +133,6 @@ rtc_static_library("video_coding") { "jitter_estimator.h", "media_opt_util.cc", "media_opt_util.h", - "media_optimization.cc", - "media_optimization.h", "nack_fec_tables.h", "packet_buffer.cc", "packet_buffer.h", diff --git a/modules/video_coding/generic_encoder.cc b/modules/video_coding/generic_encoder.cc index 9dd07bc130..2d992efc36 100644 --- a/modules/video_coding/generic_encoder.cc +++ b/modules/video_coding/generic_encoder.cc @@ -22,7 +22,6 @@ #include "api/video/video_timing.h" #include "modules/include/module_common_types_public.h" #include "modules/video_coding/include/video_coding_defines.h" -#include "modules/video_coding/media_optimization.h" #include "rtc_base/checks.h" #include "rtc_base/experiments/alr_experiment.h" #include "rtc_base/logging.h" @@ -46,6 +45,7 @@ VCMGenericEncoder::VCMGenericEncoder( : encoder_(encoder), vcm_encoded_frame_callback_(encoded_frame_callback), internal_source_(internal_source), + input_frame_rate_(0), streams_or_svc_num_(0), codec_type_(VideoCodecType::kVideoCodecGeneric) {} @@ -103,38 +103,34 @@ int32_t VCMGenericEncoder::Encode(const VideoFrame& frame, return encoder_->Encode(frame, codec_specific, &frame_types); } -void VCMGenericEncoder::SetEncoderParameters(const EncoderParameters& params) { +void VCMGenericEncoder::SetEncoderParameters( + const VideoBitrateAllocation& target_bitrate, + uint32_t input_frame_rate) { RTC_DCHECK_RUNS_SERIALIZED(&race_checker_); bool rates_have_changed; { rtc::CritScope lock(¶ms_lock_); - rates_have_changed = - params.target_bitrate != encoder_params_.target_bitrate || - params.input_frame_rate != encoder_params_.input_frame_rate; - encoder_params_ = params; + rates_have_changed = target_bitrate != bitrate_allocation_ || + input_frame_rate != input_frame_rate_; + bitrate_allocation_ = target_bitrate; + input_frame_rate_ = input_frame_rate; } if (rates_have_changed) { - int res = encoder_->SetRateAllocation(params.target_bitrate, - params.input_frame_rate); + int res = encoder_->SetRateAllocation(target_bitrate, input_frame_rate); if (res != 0) { RTC_LOG(LS_WARNING) << "Error set encoder rate (total bitrate bps = " - << params.target_bitrate.get_sum_bps() - << ", framerate = " << params.input_frame_rate + << target_bitrate.get_sum_bps() + << ", framerate = " << input_frame_rate << "): " << res; } - vcm_encoded_frame_callback_->OnFrameRateChanged(params.input_frame_rate); + vcm_encoded_frame_callback_->OnFrameRateChanged(input_frame_rate); for (size_t i = 0; i < streams_or_svc_num_; ++i) { vcm_encoded_frame_callback_->OnTargetBitrateChanged( - params.target_bitrate.GetSpatialLayerSum(i) / 8, i); + target_bitrate.GetSpatialLayerSum(i) / 8, i); } } } -EncoderParameters VCMGenericEncoder::GetEncoderParameters() const { - rtc::CritScope lock(¶ms_lock_); - return encoder_params_; -} - int32_t VCMGenericEncoder::RequestFrame( const std::vector& frame_types) { RTC_DCHECK_RUNS_SERIALIZED(&race_checker_); @@ -164,11 +160,9 @@ VideoEncoder::EncoderInfo VCMGenericEncoder::GetEncoderInfo() const { } VCMEncodedFrameCallback::VCMEncodedFrameCallback( - EncodedImageCallback* post_encode_callback, - media_optimization::MediaOptimization* media_opt) + EncodedImageCallback* post_encode_callback) : internal_source_(false), post_encode_callback_(post_encode_callback), - media_opt_(media_opt), framerate_(1), last_timing_frame_time_ms_(-1), timing_frames_thresholds_({-1, 0}), @@ -406,20 +400,8 @@ EncodedImageCallback::Result VCMEncodedFrameCallback::OnEncodedImage( RTC_CHECK(videocontenttypehelpers::SetSimulcastId( &image_copy.content_type_, static_cast(spatial_idx + 1))); - Result result = post_encode_callback_->OnEncodedImage( - image_copy, codec_specific, fragmentation_header); - if (result.error != Result::OK) - return result; - - if (media_opt_) { - media_opt_->UpdateWithEncodedData(image_copy._length, - image_copy._frameType); - if (internal_source_) { - // Signal to encoder to drop next frame. - result.drop_next_frame = media_opt_->DropFrame(); - } - } - return result; + return post_encode_callback_->OnEncodedImage(image_copy, codec_specific, + fragmentation_header); } } // namespace webrtc diff --git a/modules/video_coding/generic_encoder.h b/modules/video_coding/generic_encoder.h index 1f4ade201e..463f98a749 100644 --- a/modules/video_coding/generic_encoder.h +++ b/modules/video_coding/generic_encoder.h @@ -23,33 +23,9 @@ namespace webrtc { -namespace media_optimization { -class MediaOptimization; -} // namespace media_optimization - -struct EncoderParameters { - EncoderParameters() : total_bitrate(DataRate::Zero()), input_frame_rate(0) {} - EncoderParameters(DataRate total_bitrate, - const VideoBitrateAllocation& allocation, - uint32_t framerate) - : total_bitrate(total_bitrate), - target_bitrate(allocation), - input_frame_rate(framerate) {} - - // Total bitrate allocated for this encoder. - DataRate total_bitrate; - // The bitrate allocation, across spatial and/or temporal layers. Note that - // the sum of these might be less than |total_bitrate| if the allocator has - // capped the bitrate for some configuration. - VideoBitrateAllocation target_bitrate; - // The input frame rate to the encoder in fps, measured in the video sender. - uint32_t input_frame_rate; -}; - class VCMEncodedFrameCallback : public EncodedImageCallback { public: - VCMEncodedFrameCallback(EncodedImageCallback* post_encode_callback, - media_optimization::MediaOptimization* media_opt); + explicit VCMEncodedFrameCallback(EncodedImageCallback* post_encode_callback); ~VCMEncodedFrameCallback() override; // Implements EncodedImageCallback. @@ -100,7 +76,6 @@ class VCMEncodedFrameCallback : public EncodedImageCallback { rtc::CriticalSection timing_params_lock_; bool internal_source_; EncodedImageCallback* const post_encode_callback_; - media_optimization::MediaOptimization* const media_opt_; struct EncodeStartTimeRecord { EncodeStartTimeRecord(uint32_t timestamp, @@ -153,8 +128,8 @@ class VCMGenericEncoder { const CodecSpecificInfo* codec_specific, const std::vector& frame_types); - void SetEncoderParameters(const EncoderParameters& params); - EncoderParameters GetEncoderParameters() const; + void SetEncoderParameters(const VideoBitrateAllocation& target_bitrate, + uint32_t input_frame_rate); int32_t RequestFrame(const std::vector& frame_types); bool InternalSource() const; @@ -167,7 +142,8 @@ class VCMGenericEncoder { VCMEncodedFrameCallback* const vcm_encoded_frame_callback_; const bool internal_source_; rtc::CriticalSection params_lock_; - EncoderParameters encoder_params_ RTC_GUARDED_BY(params_lock_); + VideoBitrateAllocation bitrate_allocation_ RTC_GUARDED_BY(params_lock_); + uint32_t input_frame_rate_ RTC_GUARDED_BY(params_lock_); size_t streams_or_svc_num_ RTC_GUARDED_BY(race_checker_); VideoCodecType codec_type_ RTC_GUARDED_BY(race_checker_); }; diff --git a/modules/video_coding/generic_encoder_unittest.cc b/modules/video_coding/generic_encoder_unittest.cc index 4d5df5b374..9491b5a766 100644 --- a/modules/video_coding/generic_encoder_unittest.cc +++ b/modules/video_coding/generic_encoder_unittest.cc @@ -76,7 +76,7 @@ std::vector> GetTimingFrames( const int num_streams, const int num_frames) { FakeEncodedImageCallback sink; - VCMEncodedFrameCallback callback(&sink, nullptr); + VCMEncodedFrameCallback callback(&sink); const size_t kFramerate = 30; callback.SetTimingFramesThresholds( {delay_ms, kDefaultOutlierFrameSizePercent}); @@ -192,7 +192,7 @@ TEST(TestVCMEncodedFrameCallback, NoTimingFrameIfNoEncodeStartTime) { image.SetTimestamp(static_cast(timestamp * 90)); codec_specific.codecType = kVideoCodecGeneric; FakeEncodedImageCallback sink; - VCMEncodedFrameCallback callback(&sink, nullptr); + VCMEncodedFrameCallback callback(&sink); VideoCodec::TimingFrameTriggerThresholds thresholds; thresholds.delay_ms = 1; // Make all frames timing frames. callback.SetTimingFramesThresholds(thresholds); @@ -223,7 +223,7 @@ TEST(TestVCMEncodedFrameCallback, AdjustsCaptureTimeForInternalSourceEncoder) { image.SetTimestamp(static_cast(timestamp * 90)); codec_specific.codecType = kVideoCodecGeneric; FakeEncodedImageCallback sink; - VCMEncodedFrameCallback callback(&sink, nullptr); + VCMEncodedFrameCallback callback(&sink); callback.SetInternalSource(true); VideoCodec::TimingFrameTriggerThresholds thresholds; thresholds.delay_ms = 1; // Make all frames timing frames. @@ -258,7 +258,7 @@ TEST(TestVCMEncodedFrameCallback, NotifiesAboutDroppedFrames) { const int64_t kTimestampMs4 = 47721870; codec_specific.codecType = kVideoCodecGeneric; FakeEncodedImageCallback sink; - VCMEncodedFrameCallback callback(&sink, nullptr); + VCMEncodedFrameCallback callback(&sink); // Any non-zero bitrate needed to be set before the first frame. callback.OnTargetBitrateChanged(500, 0); image.capture_time_ms_ = kTimestampMs1; @@ -293,7 +293,7 @@ TEST(TestVCMEncodedFrameCallback, RestoresCaptureTimestamps) { const int64_t kTimestampMs = 123456; codec_specific.codecType = kVideoCodecGeneric; FakeEncodedImageCallback sink; - VCMEncodedFrameCallback callback(&sink, nullptr); + VCMEncodedFrameCallback callback(&sink); // Any non-zero bitrate needed to be set before the first frame. callback.OnTargetBitrateChanged(500, 0); image.capture_time_ms_ = kTimestampMs; // Incorrect timesetamp. diff --git a/modules/video_coding/media_optimization.cc b/modules/video_coding/media_optimization.cc deleted file mode 100644 index f74e5eba85..0000000000 --- a/modules/video_coding/media_optimization.cc +++ /dev/null @@ -1,145 +0,0 @@ -/* - * Copyright (c) 2012 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "modules/video_coding/media_optimization.h" - -#include -#include -#include - -#include "modules/video_coding/utility/frame_dropper.h" -#include "system_wrappers/include/clock.h" - -namespace webrtc { -namespace media_optimization { -namespace { -const int64_t kFrameHistoryWinMs = 2000; -} // namespace - -MediaOptimization::MediaOptimization(Clock* clock) - : clock_(clock), - max_bit_rate_(0), - max_frame_rate_(0), - frame_dropper_(), - incoming_frame_rate_(0) { - memset(incoming_frame_times_, -1, sizeof(incoming_frame_times_)); - frame_dropper_.SetRates(0, 0); -} - -MediaOptimization::~MediaOptimization() = default; - -void MediaOptimization::SetEncodingData(int32_t max_bit_rate, - uint32_t target_bitrate, - uint32_t max_frame_rate) { - rtc::CritScope lock(&crit_sect_); - // Everything codec specific should be reset here since the codec has changed. - max_bit_rate_ = max_bit_rate; - max_frame_rate_ = static_cast(max_frame_rate); - float target_bitrate_kbps = static_cast(target_bitrate) / 1000.0f; - frame_dropper_.Reset(); - frame_dropper_.SetRates(target_bitrate_kbps, max_frame_rate_); -} - -uint32_t MediaOptimization::SetTargetRates(uint32_t target_bitrate) { - rtc::CritScope lock(&crit_sect_); - - // Cap target video bitrate to codec maximum. - int video_target_bitrate = target_bitrate; - if (max_bit_rate_ > 0 && video_target_bitrate > max_bit_rate_) { - video_target_bitrate = max_bit_rate_; - } - float target_video_bitrate_kbps = - static_cast(video_target_bitrate) / 1000.0f; - - float framerate = incoming_frame_rate_; - if (framerate == 0.0) { - // No framerate estimate available, use configured max framerate instead. - framerate = max_frame_rate_; - } - - frame_dropper_.SetRates(target_video_bitrate_kbps, framerate); - - return video_target_bitrate; -} - -uint32_t MediaOptimization::InputFrameRate() { - rtc::CritScope lock(&crit_sect_); - return InputFrameRateInternal(); -} - -uint32_t MediaOptimization::InputFrameRateInternal() { - ProcessIncomingFrameRate(clock_->TimeInMilliseconds()); - uint32_t framerate = static_cast(std::min( - std::numeric_limits::max(), incoming_frame_rate_ + 0.5f)); - return framerate; -} - -void MediaOptimization::UpdateWithEncodedData( - const size_t encoded_image_length, - const FrameType encoded_image_frametype) { - size_t encoded_length = encoded_image_length; - rtc::CritScope lock(&crit_sect_); - if (encoded_length > 0) { - const bool delta_frame = encoded_image_frametype != kVideoFrameKey; - frame_dropper_.Fill(encoded_length, delta_frame); - } -} - -void MediaOptimization::EnableFrameDropper(bool enable) { - rtc::CritScope lock(&crit_sect_); - frame_dropper_.Enable(enable); -} - -bool MediaOptimization::DropFrame() { - rtc::CritScope lock(&crit_sect_); - UpdateIncomingFrameRate(); - // Leak appropriate number of bytes. - frame_dropper_.Leak(static_cast(InputFrameRateInternal() + 0.5f)); - return frame_dropper_.DropFrame(); -} - -void MediaOptimization::UpdateIncomingFrameRate() { - int64_t now = clock_->TimeInMilliseconds(); - if (incoming_frame_times_[0] == 0) { - // No shifting if this is the first time. - } else { - // Shift all times one step. - for (int32_t i = (kFrameCountHistorySize - 2); i >= 0; i--) { - incoming_frame_times_[i + 1] = incoming_frame_times_[i]; - } - } - incoming_frame_times_[0] = now; - ProcessIncomingFrameRate(now); -} - -// Allowing VCM to keep track of incoming frame rate. -void MediaOptimization::ProcessIncomingFrameRate(int64_t now) { - int32_t num = 0; - int32_t nr_of_frames = 0; - for (num = 1; num < (kFrameCountHistorySize - 1); ++num) { - if (incoming_frame_times_[num] <= 0 || - // Don't use data older than 2 s. - now - incoming_frame_times_[num] > kFrameHistoryWinMs) { - break; - } else { - nr_of_frames++; - } - } - if (num > 1) { - const int64_t diff = - incoming_frame_times_[0] - incoming_frame_times_[num - 1]; - incoming_frame_rate_ = 0.0; // No frame rate estimate available. - if (diff > 0) { - incoming_frame_rate_ = nr_of_frames * 1000.0f / static_cast(diff); - } - } -} -} // namespace media_optimization -} // namespace webrtc diff --git a/modules/video_coding/media_optimization.h b/modules/video_coding/media_optimization.h deleted file mode 100644 index 0b6b82f8de..0000000000 --- a/modules/video_coding/media_optimization.h +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Copyright (c) 2012 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef MODULES_VIDEO_CODING_MEDIA_OPTIMIZATION_H_ -#define MODULES_VIDEO_CODING_MEDIA_OPTIMIZATION_H_ - -#include - -#include "modules/include/module_common_types.h" -#include "modules/video_coding/include/video_coding.h" -#include "modules/video_coding/media_opt_util.h" -#include "modules/video_coding/utility/frame_dropper.h" -#include "rtc_base/criticalsection.h" - -namespace webrtc { - -class Clock; - -namespace media_optimization { - -class MediaOptimization { - public: - explicit MediaOptimization(Clock* clock); - ~MediaOptimization(); - - // Informs media optimization of initial encoding state. - // TODO(perkj): Deprecate SetEncodingData once its not used for stats in - // VideoStreamEncoder. - void SetEncodingData(int32_t max_bit_rate, - uint32_t bit_rate, - uint32_t max_frame_rate); - - // Sets target rates for the encoder given the channel parameters. - // Input: |target bitrate| - the encoder target bitrate in bits/s. - uint32_t SetTargetRates(uint32_t target_bitrate); - - void EnableFrameDropper(bool enable); - bool DropFrame(); - - // Informs Media Optimization of encoded output. - // TODO(perkj): Deprecate SetEncodingData once its not used for stats in - // VideoStreamEncoder. - void UpdateWithEncodedData(const size_t encoded_image_length, - const FrameType encoded_image_frametype); - - // InputFrameRate 0 = no frame rate estimate available. - uint32_t InputFrameRate(); - - private: - enum { kFrameCountHistorySize = 90 }; - - void UpdateIncomingFrameRate() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); - void ProcessIncomingFrameRate(int64_t now) - RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); - uint32_t InputFrameRateInternal() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); - - // Protect all members. - rtc::CriticalSection crit_sect_; - - Clock* const clock_ RTC_GUARDED_BY(crit_sect_); - int32_t max_bit_rate_ RTC_GUARDED_BY(crit_sect_); - float max_frame_rate_ RTC_GUARDED_BY(crit_sect_); - FrameDropper frame_dropper_ RTC_GUARDED_BY(crit_sect_); - float incoming_frame_rate_ RTC_GUARDED_BY(crit_sect_); - int64_t incoming_frame_times_[kFrameCountHistorySize] RTC_GUARDED_BY( - crit_sect_); -}; -} // namespace media_optimization -} // namespace webrtc - -#endif // MODULES_VIDEO_CODING_MEDIA_OPTIMIZATION_H_ diff --git a/modules/video_coding/video_coding_impl.h b/modules/video_coding/video_coding_impl.h index e7fb74c48d..08b0753271 100644 --- a/modules/video_coding/video_coding_impl.h +++ b/modules/video_coding/video_coding_impl.h @@ -24,7 +24,6 @@ #include "modules/video_coding/generic_decoder.h" #include "modules/video_coding/generic_encoder.h" #include "modules/video_coding/jitter_buffer.h" -#include "modules/video_coding/media_optimization.h" #include "modules/video_coding/receiver.h" #include "modules/video_coding/timing.h" #include "rtc_base/onetimeevent.h" @@ -63,7 +62,6 @@ class VideoSender { typedef VideoCodingModule::SenderNackMode SenderNackMode; VideoSender(Clock* clock, EncodedImageCallback* post_encode_callback); - ~VideoSender(); // Register the send codec to be used. @@ -75,60 +73,27 @@ class VideoSender { void RegisterExternalEncoder(VideoEncoder* externalEncoder, bool internalSource); - // Update the channel parameters based on new rates and rtt. This will also - // cause an immediate call to VideoEncoder::SetRateAllocation(). - int32_t SetChannelParameters( - uint32_t target_bitrate_bps, - VideoBitrateAllocator* bitrate_allocator, - VideoBitrateAllocationObserver* bitrate_updated_callback); - - // Updates the channel parameters with a new bitrate allocation, but using the - // current targit_bitrate, loss rate and rtt. That is, the distribution or - // caps may be updated to a change to a new VideoCodec or allocation mode. - // The new parameters will be stored as pending EncoderParameters, and the - // encoder will only be updated on the next frame. - void UpdateChannelParameters( - VideoBitrateAllocator* bitrate_allocator, - VideoBitrateAllocationObserver* bitrate_updated_callback); + // Update the the encoder with new bitrate allocation and framerate. + int32_t SetChannelParameters(const VideoBitrateAllocation& bitrate_allocation, + uint32_t framerate_fps); int32_t AddVideoFrame(const VideoFrame& videoFrame, const CodecSpecificInfo* codecSpecificInfo, absl::optional encoder_info); int32_t IntraFrameRequest(size_t stream_index); - int32_t EnableFrameDropper(bool enable); private: - EncoderParameters UpdateEncoderParameters( - const EncoderParameters& params, - VideoBitrateAllocator* bitrate_allocator, - uint32_t target_bitrate_bps); - void SetEncoderParameters(EncoderParameters params, bool has_internal_source) - RTC_EXCLUSIVE_LOCKS_REQUIRED(encoder_crit_); - VideoBitrateAllocation GetAllocation( - uint32_t bitrate_bps, - uint32_t framerate_fps, - VideoBitrateAllocator* bitrate_allocator) const; - rtc::CriticalSection encoder_crit_; VCMGenericEncoder* _encoder; - media_optimization::MediaOptimization _mediaOpt; VCMEncodedFrameCallback _encodedFrameCallback RTC_GUARDED_BY(encoder_crit_); - EncodedImageCallback* const post_encode_callback_; VCMEncoderDataBase _codecDataBase RTC_GUARDED_BY(encoder_crit_); - // If frame dropper is not force disabled, frame dropping might still be - // disabled if VideoEncoder::GetEncoderInfo() indicates that the encoder has a - // trusted rate controller. This is determined on a per-frame basis, as the - // encoder behavior might dynamically change. - bool force_disable_frame_dropper_ RTC_GUARDED_BY(encoder_crit_); - // Must be accessed on the construction thread of VideoSender. VideoCodec current_codec_; rtc::SequencedTaskChecker sequenced_checker_; rtc::CriticalSection params_crit_; - EncoderParameters encoder_params_ RTC_GUARDED_BY(params_crit_); bool encoder_has_internal_source_ RTC_GUARDED_BY(params_crit_); std::vector next_frame_types_ RTC_GUARDED_BY(params_crit_); }; diff --git a/modules/video_coding/video_sender.cc b/modules/video_coding/video_sender.cc index aea26f7f56..fe959b8cd4 100644 --- a/modules/video_coding/video_sender.cc +++ b/modules/video_coding/video_sender.cc @@ -25,7 +25,6 @@ #include "modules/video_coding/include/video_coding_defines.h" #include "modules/video_coding/include/video_error_codes.h" #include "modules/video_coding/internal_defines.h" -#include "modules/video_coding/media_optimization.h" #include "modules/video_coding/utility/default_video_bitrate_allocator.h" #include "modules/video_coding/video_coding_impl.h" #include "rtc_base/checks.h" @@ -39,20 +38,11 @@ namespace webrtc { namespace vcm { -namespace { - -constexpr char kFrameDropperFieldTrial[] = "WebRTC-FrameDropper"; - -} // namespace - VideoSender::VideoSender(Clock* clock, EncodedImageCallback* post_encode_callback) : _encoder(nullptr), - _mediaOpt(clock), - _encodedFrameCallback(post_encode_callback, &_mediaOpt), - post_encode_callback_(post_encode_callback), + _encodedFrameCallback(post_encode_callback), _codecDataBase(&_encodedFrameCallback), - force_disable_frame_dropper_(false), current_codec_(), encoder_has_internal_source_(false), next_frame_types_(1, kVideoFrameDelta) { @@ -93,27 +83,6 @@ int32_t VideoSender::RegisterSendCodec(const VideoCodec* sendCodec, // SetSendCodec succeeded, _encoder should be set. RTC_DCHECK(_encoder); - int numLayers; - if (sendCodec->codecType == kVideoCodecVP8) { - numLayers = sendCodec->VP8().numberOfTemporalLayers; - } else if (sendCodec->codecType == kVideoCodecVP9) { - numLayers = sendCodec->VP9().numberOfTemporalLayers; - } else if (sendCodec->codecType == kVideoCodecGeneric && - sendCodec->numberOfSimulcastStreams > 0) { - // This is mainly for unit testing, disabling frame dropping. - // TODO(sprang): Add a better way to disable frame dropping. - numLayers = sendCodec->simulcastStream[0].numberOfTemporalLayers; - } else { - numLayers = 1; - } - - // Force-disable frame dropper if either: - // * We have screensharing with layers. - // * "WebRTC-FrameDropper" field trial is "Disabled". - force_disable_frame_dropper_ = - field_trial::IsDisabled(kFrameDropperFieldTrial) || - (numLayers > 1 && sendCodec->mode == VideoCodecMode::kScreensharing); - { rtc::CritScope cs(¶ms_crit_); next_frame_types_.clear(); @@ -128,9 +97,6 @@ int32_t VideoSender::RegisterSendCodec(const VideoCodec* sendCodec, << " start bitrate " << sendCodec->startBitrate << " max frame rate " << sendCodec->maxFramerate << " max payload size " << maxPayloadSize; - _mediaOpt.SetEncodingData(sendCodec->maxBitrate * 1000, - sendCodec->startBitrate * 1000, - sendCodec->maxFramerate); return VCM_OK; } @@ -156,149 +122,58 @@ void VideoSender::RegisterExternalEncoder(VideoEncoder* externalEncoder, internalSource); } -EncoderParameters VideoSender::UpdateEncoderParameters( - const EncoderParameters& params, - VideoBitrateAllocator* bitrate_allocator, - uint32_t target_bitrate_bps) { - uint32_t video_target_rate_bps = _mediaOpt.SetTargetRates(target_bitrate_bps); - uint32_t input_frame_rate = _mediaOpt.InputFrameRate(); - if (input_frame_rate == 0) - input_frame_rate = current_codec_.maxFramerate; - - EncoderParameters new_encoder_params = { - DataRate::bps(target_bitrate_bps), - GetAllocation(video_target_rate_bps, input_frame_rate, bitrate_allocator), - input_frame_rate}; - return new_encoder_params; -} - -VideoBitrateAllocation VideoSender::GetAllocation( - uint32_t bitrate_bps, - uint32_t framerate_fps, - VideoBitrateAllocator* bitrate_allocator) const { - VideoBitrateAllocation bitrate_allocation; - // Only call allocators if bitrate > 0 (ie, not suspended), otherwise they - // might cap the bitrate to the min bitrate configured. - if (bitrate_bps > 0) { - if (bitrate_allocator) { - bitrate_allocation = - bitrate_allocator->GetAllocation(bitrate_bps, framerate_fps); - } else { - DefaultVideoBitrateAllocator default_allocator(current_codec_); - bitrate_allocation = - default_allocator.GetAllocation(bitrate_bps, framerate_fps); - } - } - return bitrate_allocation; -} - -void VideoSender::UpdateChannelParameters( - VideoBitrateAllocator* bitrate_allocator, - VideoBitrateAllocationObserver* bitrate_updated_callback) { - VideoBitrateAllocation target_rate; - { - rtc::CritScope cs(¶ms_crit_); - encoder_params_ = - UpdateEncoderParameters(encoder_params_, bitrate_allocator, - encoder_params_.total_bitrate.bps()); - target_rate = encoder_params_.target_bitrate; - } - if (bitrate_updated_callback && target_rate.get_sum_bps() > 0) { - bitrate_updated_callback->OnBitrateAllocationUpdated(target_rate); - } -} - int32_t VideoSender::SetChannelParameters( - uint32_t target_bitrate_bps, - VideoBitrateAllocator* bitrate_allocator, - VideoBitrateAllocationObserver* bitrate_updated_callback) { - EncoderParameters encoder_params; - encoder_params = UpdateEncoderParameters(encoder_params, bitrate_allocator, - target_bitrate_bps); - if (bitrate_updated_callback && target_bitrate_bps > 0) { - bitrate_updated_callback->OnBitrateAllocationUpdated( - encoder_params.target_bitrate); - } - + const VideoBitrateAllocation& bitrate_allocation, + uint32_t framerate_fps) { bool encoder_has_internal_source; { rtc::CritScope cs(¶ms_crit_); - encoder_params_ = encoder_params; encoder_has_internal_source = encoder_has_internal_source_; } - // For encoders with internal sources, we need to tell the encoder directly, - // instead of waiting for an AddVideoFrame that will never come (internal - // source encoders don't get input frames). - if (encoder_has_internal_source) { + { rtc::CritScope cs(&encoder_crit_); if (_encoder) { - SetEncoderParameters(encoder_params, encoder_has_internal_source); + // |target_bitrate == 0 | means that the network is down or the send pacer + // is full. We currently only report this if the encoder has an internal + // source. If the encoder does not have an internal source, higher levels + // are expected to not call AddVideoFrame. We do this since its unclear + // how current encoder implementations behave when given a zero target + // bitrate. + // TODO(perkj): Make sure all known encoder implementations handle zero + // target bitrate and remove this check. + if (!encoder_has_internal_source && + bitrate_allocation.get_sum_bps() == 0) { + return VCM_OK; + } + + if (framerate_fps == 0) { + // No frame rate estimate available, use default. + framerate_fps = current_codec_.maxFramerate; + } + if (_encoder != nullptr) + _encoder->SetEncoderParameters(bitrate_allocation, framerate_fps); } } return VCM_OK; } -void VideoSender::SetEncoderParameters(EncoderParameters params, - bool has_internal_source) { - // |target_bitrate == 0 | means that the network is down or the send pacer is - // full. We currently only report this if the encoder has an internal source. - // If the encoder does not have an internal source, higher levels are expected - // to not call AddVideoFrame. We do this since its unclear how current - // encoder implementations behave when given a zero target bitrate. - // TODO(perkj): Make sure all known encoder implementations handle zero - // target bitrate and remove this check. - if (!has_internal_source && params.target_bitrate.get_sum_bps() == 0) - return; - - if (params.input_frame_rate == 0) { - // No frame rate estimate available, use default. - params.input_frame_rate = current_codec_.maxFramerate; - } - if (_encoder != nullptr) - _encoder->SetEncoderParameters(params); -} - // Add one raw video frame to the encoder, blocking. int32_t VideoSender::AddVideoFrame( const VideoFrame& videoFrame, const CodecSpecificInfo* codecSpecificInfo, absl::optional encoder_info) { - EncoderParameters encoder_params; std::vector next_frame_types; bool encoder_has_internal_source = false; { rtc::CritScope lock(¶ms_crit_); - encoder_params = encoder_params_; next_frame_types = next_frame_types_; encoder_has_internal_source = encoder_has_internal_source_; } rtc::CritScope lock(&encoder_crit_); if (_encoder == nullptr) return VCM_UNINITIALIZED; - SetEncoderParameters(encoder_params, encoder_has_internal_source); - if (!encoder_info) { - encoder_info = _encoder->GetEncoderInfo(); - } - - // Frame dropping is enabled iff frame dropping has been requested, and - // frame dropping is not force-disabled, and rate controller is not trusted. - const bool frame_dropping_enabled = - !force_disable_frame_dropper_ && - !encoder_info->has_trusted_rate_controller; - _mediaOpt.EnableFrameDropper(frame_dropping_enabled); - - if (_mediaOpt.DropFrame()) { - RTC_LOG(LS_VERBOSE) << "Drop Frame: " - << "target bitrate " - << encoder_params.target_bitrate.get_sum_bps() - << ", input frame rate " - << encoder_params.input_frame_rate; - post_encode_callback_->OnDroppedFrame( - EncodedImageCallback::DropReason::kDroppedByMediaOptimizations); - return VCM_OK; - } // TODO(pbos): Make sure setting send codec is synchronized with video // processing so frame size always matches. if (!_codecDataBase.MatchesCurrentResolution(videoFrame.width(), diff --git a/modules/video_coding/video_sender_unittest.cc b/modules/video_coding/video_sender_unittest.cc index 986bd3fc48..a9c9e455e2 100644 --- a/modules/video_coding/video_sender_unittest.cc +++ b/modules/video_coding/video_sender_unittest.cc @@ -319,14 +319,14 @@ TEST_F(TestVideoSenderWithMockEncoder, TestSetRate) { SetRateAllocation(new_rate_allocation, settings_.maxFramerate)) .Times(1) .WillOnce(Return(0)); - sender_->SetChannelParameters(new_bitrate_kbps * 1000, rate_allocator_.get(), - nullptr); + sender_->SetChannelParameters(new_rate_allocation, settings_.maxFramerate); AddFrame(); clock_.AdvanceTimeMilliseconds(kFrameIntervalMs); // Expect no call to encoder_.SetRates if the new bitrate is zero. EXPECT_CALL(encoder_, SetRateAllocation(_, _)).Times(0); - sender_->SetChannelParameters(0, rate_allocator_.get(), nullptr); + sender_->SetChannelParameters(VideoBitrateAllocation(), + settings_.maxFramerate); AddFrame(); } @@ -363,8 +363,7 @@ TEST_F(TestVideoSenderWithMockEncoder, TestEncoderParametersForInternalSource) { EXPECT_CALL(encoder_, SetRateAllocation(new_rate_allocation, _)) .Times(1) .WillOnce(Return(0)); - sender_->SetChannelParameters(new_bitrate_kbps * 1000, rate_allocator_.get(), - nullptr); + sender_->SetChannelParameters(new_rate_allocation, settings_.maxFramerate); } TEST_F(TestVideoSenderWithMockEncoder, @@ -375,8 +374,9 @@ TEST_F(TestVideoSenderWithMockEncoder, // Expect initial call to SetChannelParameters. Rates are initialized through // InitEncode and expects no additional call before the framerate (or bitrate) // updates. - sender_->SetChannelParameters(settings_.startBitrate * 1000, - rate_allocator_.get(), nullptr); + sender_->SetChannelParameters( + rate_allocator_->GetAllocation(settings_.startBitrate * 1000, kInputFps), + kInputFps); while (clock_.TimeInMilliseconds() < start_time + kRateStatsWindowMs) { AddFrame(); clock_.AdvanceTimeMilliseconds(1000 / kInputFps); @@ -390,8 +390,7 @@ TEST_F(TestVideoSenderWithMockEncoder, EXPECT_CALL(encoder_, SetRateAllocation(new_rate_allocation, kInputFps)) .Times(1) .WillOnce(Return(0)); - sender_->SetChannelParameters(new_bitrate_bps, rate_allocator_.get(), - nullptr); + sender_->SetChannelParameters(new_rate_allocation, kInputFps); AddFrame(); } @@ -439,12 +438,13 @@ class TestVideoSenderWithVp8 : public TestVideoSender { AddFrame(); // SetChannelParameters needs to be called frequently to propagate // framerate from the media optimization into the encoder. - // Note: SetChannelParameters fails if less than 2 frames are in the - // buffer since it will fail to calculate the framerate. + const VideoBitrateAllocation bitrate_allocation = + rate_allocator_->GetAllocation(available_bitrate_kbps_ * 1000, + static_cast(framerate)); if (i != 0) { - EXPECT_EQ(VCM_OK, sender_->SetChannelParameters( - available_bitrate_kbps_ * 1000, - rate_allocator_.get(), nullptr)); + EXPECT_EQ(VCM_OK, + sender_->SetChannelParameters( + bitrate_allocation, static_cast(framerate))); } } } diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc index 1a7f9c419f..c306e84275 100644 --- a/video/video_send_stream_tests.cc +++ b/video/video_send_stream_tests.cc @@ -2829,11 +2829,21 @@ TEST_P(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { } void WaitForSetRates(uint32_t expected_bitrate) { - EXPECT_TRUE( - bitrate_changed_event_.Wait(VideoSendStreamTest::kDefaultTimeoutMs)) - << "Timed out while waiting encoder rate to be set."; + // Wait for the expected rate to be set. In some cases there can be + // more than one update pending, in which case we keep waiting + // until the correct value has been observed. + const int64_t start_time = rtc::TimeMillis(); + do { + rtc::CritScope lock(&crit_); + if (target_bitrate_ == expected_bitrate) { + return; + } + } while (bitrate_changed_event_.Wait( + std::max(int64_t{1}, VideoSendStreamTest::kDefaultTimeoutMs - + (rtc::TimeMillis() - start_time)))); rtc::CritScope lock(&crit_); - EXPECT_EQ(expected_bitrate, target_bitrate_); + EXPECT_EQ(target_bitrate_, expected_bitrate) + << "Timed out while waiting encoder rate to be set."; } void ModifySenderBitrateConfig( diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 8484708247..0cb6c3b2e0 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -15,12 +15,14 @@ #include #include +#include "absl/memory/memory.h" #include "api/video/encoded_image.h" #include "api/video/i420_buffer.h" #include "api/video/video_bitrate_allocator_factory.h" #include "modules/video_coding/codecs/vp9/svc_rate_allocator.h" #include "modules/video_coding/include/video_codec_initializer.h" #include "modules/video_coding/include/video_coding.h" +#include "modules/video_coding/utility/default_video_bitrate_allocator.h" #include "rtc_base/arraysize.h" #include "rtc_base/checks.h" #include "rtc_base/experiments/quality_scaling_experiment.h" @@ -45,6 +47,7 @@ const int kMinFramerateFps = 2; const int64_t kPendingFrameTimeoutMs = 1000; const char kInitialFramedropFieldTrial[] = "WebRTC-InitialFramedrop"; +constexpr char kFrameDropperFieldTrial[] = "WebRTC-FrameDropper"; // The maximum number of frames to drop at beginning of stream // to try and achieve desired bitrate. @@ -53,6 +56,10 @@ const int kMaxInitialFramedrop = 4; // enable DropFrameDueToSize logic. const float kFramedropThreshold = 0.3; +// Averaging window spanning 90 frames at default 30fps, matching old media +// optimization module defaults. +const int64_t kFrameRateAvergingWindowSizeMs = (1000 / 30) * 90; + // Initial limits for BALANCED degradation preference. int MinFps(int pixels) { if (pixels <= 320 * 240) { @@ -380,6 +387,9 @@ VideoStreamEncoder::VideoStreamEncoder( dropped_frame_count_(0), pending_frame_post_time_us_(0), bitrate_observer_(nullptr), + force_disable_frame_dropper_(false), + input_framerate_(kFrameRateAvergingWindowSizeMs, 1000), + pending_frame_drops_(0), encoder_queue_("EncoderQueue") { RTC_DCHECK(encoder_stats_observer); RTC_DCHECK(overuse_detector_); @@ -545,7 +555,6 @@ void VideoStreamEncoder::ReconfigureEncoder() { rate_allocator_ = settings_.bitrate_allocator_factory->CreateVideoBitrateAllocator(codec); - RTC_CHECK(rate_allocator_) << "Failed to create bitrate allocator."; // Set min_bitrate_bps, max_bitrate_bps, and max padding bit rate for VP9. if (encoder_config_.codec_type == kVideoCodecVP9) { @@ -605,8 +614,39 @@ void VideoStreamEncoder::ReconfigureEncoder() { rate_allocator_.reset(); } - video_sender_.UpdateChannelParameters(rate_allocator_.get(), - bitrate_observer_); + int num_layers; + if (codec.codecType == kVideoCodecVP8) { + num_layers = codec.VP8()->numberOfTemporalLayers; + } else if (codec.codecType == kVideoCodecVP9) { + num_layers = codec.VP9()->numberOfTemporalLayers; + } else if (codec.codecType == kVideoCodecGeneric && + codec.numberOfSimulcastStreams > 0) { + // This is mainly for unit testing, disabling frame dropping. + // TODO(sprang): Add a better way to disable frame dropping. + num_layers = codec.simulcastStream[0].numberOfTemporalLayers; + } else { + num_layers = 1; + } + + frame_dropper_.Reset(); + frame_dropper_.SetRates(codec.startBitrate, max_framerate_); + uint32_t framerate_fps = GetInputFramerateFps(); + // Force-disable frame dropper if either: + // * We have screensharing with layers. + // * "WebRTC-FrameDropper" field trial is "Disabled". + force_disable_frame_dropper_ = + field_trial::IsDisabled(kFrameDropperFieldTrial) || + (num_layers > 1 && codec.mode == VideoCodecMode::kScreensharing); + + if (rate_allocator_ && last_observed_bitrate_bps_ > 0) { + // We have a new rate allocator instance and already configured target + // bitrate. Update the rate allocation and notify observsers. + VideoBitrateAllocation bitrate_allocation = + GetBitrateAllocationAndNotifyObserver(last_observed_bitrate_bps_, + framerate_fps); + + video_sender_.SetChannelParameters(bitrate_allocation, framerate_fps); + } encoder_stats_observer_->OnEncoderReconfigured(encoder_config_, streams); @@ -776,6 +816,31 @@ void VideoStreamEncoder::TraceFrameDropEnd() { encoder_paused_and_dropped_frame_ = false; } +VideoBitrateAllocation +VideoStreamEncoder::GetBitrateAllocationAndNotifyObserver( + const uint32_t target_bitrate_bps, + uint32_t framerate_fps) { + // Only call allocators if bitrate > 0 (ie, not suspended), otherwise they + // might cap the bitrate to the min bitrate configured. + VideoBitrateAllocation bitrate_allocation; + if (rate_allocator_ && target_bitrate_bps > 0) { + bitrate_allocation = + rate_allocator_->GetAllocation(target_bitrate_bps, framerate_fps); + } + + if (bitrate_observer_ && bitrate_allocation.get_sum_bps() > 0) { + bitrate_observer_->OnBitrateAllocationUpdated(bitrate_allocation); + } + + return bitrate_allocation; +} + +uint32_t VideoStreamEncoder::GetInputFramerateFps() { + const uint32_t default_fps = max_framerate_ != -1 ? max_framerate_ : 30; + return input_framerate_.Rate(clock_->TimeInMilliseconds()) + .value_or(default_fps); +} + void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, int64_t time_when_posted_us) { RTC_DCHECK_RUN_ON(&encoder_queue_); @@ -798,6 +863,8 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, // InitialFrameDropOffWhenEncoderDisabledScaling, the return value // from GetScalingSettings should enable or disable the frame drop. + uint32_t framerate_fps = GetInputFramerateFps(); + int64_t now_ms = clock_->TimeInMilliseconds(); if (pending_encoder_reconfiguration_) { ReconfigureEncoder(); @@ -805,8 +872,10 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, } else if (!last_parameters_update_ms_ || now_ms - *last_parameters_update_ms_ >= vcm::VCMProcessTimer::kDefaultProcessIntervalMs) { - video_sender_.UpdateChannelParameters(rate_allocator_.get(), - bitrate_observer_); + video_sender_.SetChannelParameters( + GetBitrateAllocationAndNotifyObserver(last_observed_bitrate_bps_, + framerate_fps), + framerate_fps); last_parameters_update_ms_.emplace(now_ms); } @@ -848,6 +917,23 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, } pending_frame_.reset(); + + frame_dropper_.Leak(framerate_fps); + // Frame dropping is enabled iff frame dropping is not force-disabled, and + // rate controller is not trusted. + const bool frame_dropping_enabled = + !force_disable_frame_dropper_ && + !encoder_info_.has_trusted_rate_controller; + frame_dropper_.Enable(frame_dropping_enabled); + if (frame_dropping_enabled && frame_dropper_.DropFrame()) { + RTC_LOG(LS_VERBOSE) << "Drop Frame: " + << "target bitrate " << last_observed_bitrate_bps_ + << ", input frame rate " << framerate_fps; + OnDroppedFrame( + EncodedImageCallback::DropReason::kDroppedByMediaOptimizations); + return; + } + EncodeVideoFrame(video_frame, time_when_posted_us); } @@ -896,6 +982,7 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame, } encoder_info_ = info; + input_framerate_.Update(1u, clock_->TimeInMilliseconds()); video_sender_.AddVideoFrame(out_frame, nullptr, encoder_info_); } @@ -937,14 +1024,25 @@ EncodedImageCallback::Result VideoStreamEncoder::OnEncodedImage( encoded_image.timing_.encode_start_ms)); } - encoder_queue_.PostTask( - [this, timestamp, time_sent_us, qp, capture_time_us, encode_duration_us] { - RTC_DCHECK_RUN_ON(&encoder_queue_); - overuse_detector_->FrameSent(timestamp, time_sent_us, capture_time_us, - encode_duration_us); - if (quality_scaler_ && qp >= 0) - quality_scaler_->ReportQp(qp); - }); + // Run post encode tasks, such as overuse detection and frame rate/drop + // stats for internal encoders. + const size_t frame_size = encoded_image.size(); + const bool keyframe = encoded_image._frameType == FrameType::kVideoFrameKey; + RunPostEncode(timestamp, time_sent_us, capture_time_us, encode_duration_us, + qp, frame_size, keyframe); + + if (result.error == Result::OK) { + // In case of an internal encoder running on a separate thread, the + // decision to drop a frame might be a frame late and signaled via + // atomic flag. This is because we can't easily wait for the worker thread + // without risking deadlocks, eg during shutdown when the worker thread + // might be waiting for the internal encoder threads to stop. + if (pending_frame_drops_.load() > 0) { + int pending_drops = pending_frame_drops_.fetch_sub(1); + RTC_DCHECK_GT(pending_drops, 0); + result.drop_next_frame = true; + } + } return result; } @@ -1002,8 +1100,12 @@ void VideoStreamEncoder::OnBitrateUpdated(uint32_t bitrate_bps, has_seen_first_significant_bwe_change_ = true; } - video_sender_.SetChannelParameters(bitrate_bps, rate_allocator_.get(), - bitrate_observer_); + uint32_t framerate_fps = GetInputFramerateFps(); + frame_dropper_.SetRates((bitrate_bps + 500) / 1000, framerate_fps); + + VideoBitrateAllocation bitrate_allocation = + GetBitrateAllocationAndNotifyObserver(bitrate_bps, framerate_fps); + video_sender_.SetChannelParameters(bitrate_allocation, framerate_fps); encoder_start_bitrate_bps_ = bitrate_bps != 0 ? bitrate_bps : encoder_start_bitrate_bps_; @@ -1262,6 +1364,44 @@ VideoStreamEncoder::GetConstAdaptCounter() { return adapt_counters_[degradation_preference_]; } +void VideoStreamEncoder::RunPostEncode(uint32_t frame_timestamp, + int64_t time_sent_us, + int64_t capture_time_us, + absl::optional encode_durations_us, + int qp, + size_t frame_size_bytes, + bool keyframe) { + if (!encoder_queue_.IsCurrent()) { + encoder_queue_.PostTask([this, frame_timestamp, time_sent_us, qp, + capture_time_us, encode_durations_us, + frame_size_bytes, keyframe] { + RunPostEncode(frame_timestamp, time_sent_us, capture_time_us, + encode_durations_us, qp, frame_size_bytes, keyframe); + }); + return; + } + + RTC_DCHECK_RUN_ON(&encoder_queue_); + if (frame_size_bytes > 0) { + frame_dropper_.Fill(frame_size_bytes, !keyframe); + } + + if (encoder_info_.has_internal_source) { + // Update frame dropper after the fact for internal sources. + input_framerate_.Update(1u, clock_->TimeInMilliseconds()); + frame_dropper_.Leak(GetInputFramerateFps()); + // Signal to encoder to drop next frame. + if (frame_dropper_.DropFrame()) { + pending_frame_drops_.fetch_add(1); + } + } + + overuse_detector_->FrameSent(frame_timestamp, time_sent_us, capture_time_us, + encode_durations_us); + if (quality_scaler_ && qp >= 0) + quality_scaler_->ReportQp(qp); +} + // Class holding adaptation information. VideoStreamEncoder::AdaptCounter::AdaptCounter() { fps_counters_.resize(kScaleReasonSize); diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index e5ef981de1..e522d64072 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -24,10 +24,12 @@ #include "api/video/video_stream_encoder_observer.h" #include "api/video/video_stream_encoder_settings.h" #include "api/video_codecs/video_encoder.h" +#include "modules/video_coding/utility/frame_dropper.h" #include "modules/video_coding/utility/quality_scaler.h" #include "modules/video_coding/video_coding_impl.h" #include "rtc_base/criticalsection.h" #include "rtc_base/event.h" +#include "rtc_base/rate_statistics.h" #include "rtc_base/sequenced_task_checker.h" #include "rtc_base/task_queue.h" #include "video/overuse_frame_detector.h" @@ -131,6 +133,11 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, void TraceFrameDropStart(); void TraceFrameDropEnd(); + VideoBitrateAllocation GetBitrateAllocationAndNotifyObserver( + const uint32_t target_bitrate_bps, + uint32_t framerate_fps) RTC_RUN_ON(&encoder_queue_); + uint32_t GetInputFramerateFps() RTC_RUN_ON(&encoder_queue_); + // Class holding adaptation information. class AdaptCounter final { public: @@ -173,6 +180,13 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, void UpdateAdaptationStats(AdaptReason reason) RTC_RUN_ON(&encoder_queue_); VideoStreamEncoderObserver::AdaptationSteps GetActiveCounts( AdaptReason reason) RTC_RUN_ON(&encoder_queue_); + void RunPostEncode(uint32_t frame_timestamp, + int64_t time_sent_us, + int64_t capture_time_us, + absl::optional encode_durations_us, + int qp, + size_t frame_size_bytes, + bool keyframe); rtc::Event shutdown_event_; @@ -267,6 +281,19 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, RTC_GUARDED_BY(&encoder_queue_); VideoEncoder::EncoderInfo encoder_info_ RTC_GUARDED_BY(&encoder_queue_); + FrameDropper frame_dropper_ RTC_GUARDED_BY(&encoder_queue_); + + // If frame dropper is not force disabled, frame dropping might still be + // disabled if VideoEncoder::GetEncoderInfo() indicates that the encoder has a + // trusted rate controller. This is determined on a per-frame basis, as the + // encoder behavior might dynamically change. + bool force_disable_frame_dropper_ RTC_GUARDED_BY(&encoder_queue_); + RateStatistics input_framerate_ RTC_GUARDED_BY(&encoder_queue_); + // Incremented on worker thread whenever |frame_dropper_| determines that a + // frame should be dropped. Decremented on whichever thread runs + // OnEncodedImage(), which is only called by one thread but not necessarily + // the worker thread. + std::atomic pending_frame_drops_; // All public methods are proxied to |encoder_queue_|. It must must be // destroyed first to make sure no tasks are run that use other members. diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 84521fb0c7..7cc1e64efb 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -553,6 +553,16 @@ class VideoStreamEncoderTest : public ::testing::Test { force_init_encode_failed_ = force_failure; } + void SimulateOvershoot(double rate_factor) { + rtc::CritScope lock(&local_crit_sect_); + rate_factor_ = rate_factor; + } + + uint32_t GetLastFramerate() { + rtc::CritScope lock(&local_crit_sect_); + return last_framerate_; + } + private: int32_t Encode(const VideoFrame& input_image, const CodecSpecificInfo* codec_specific_info, @@ -599,6 +609,25 @@ class VideoStreamEncoderTest : public ::testing::Test { return res; } + int32_t SetRateAllocation(const VideoBitrateAllocation& rate_allocation, + uint32_t framerate) { + rtc::CritScope lock(&local_crit_sect_); + VideoBitrateAllocation adjusted_rate_allocation; + for (size_t si = 0; si < kMaxSpatialLayers; ++si) { + for (size_t ti = 0; ti < kMaxTemporalStreams; ++ti) { + if (rate_allocation.HasBitrate(si, ti)) { + adjusted_rate_allocation.SetBitrate( + si, ti, + static_cast(rate_allocation.GetBitrate(si, ti) * + rate_factor_)); + } + } + } + last_framerate_ = framerate; + return FakeEncoder::SetRateAllocation(adjusted_rate_allocation, + framerate); + } + rtc::CriticalSection local_crit_sect_; bool block_next_encode_ RTC_GUARDED_BY(local_crit_sect_) = false; rtc::Event continue_encode_event_; @@ -610,6 +639,8 @@ class VideoStreamEncoderTest : public ::testing::Test { std::vector> allocated_temporal_layers_ RTC_GUARDED_BY(local_crit_sect_); bool force_init_encode_failed_ RTC_GUARDED_BY(local_crit_sect_) = false; + double rate_factor_ RTC_GUARDED_BY(local_crit_sect_) = 1.0; + uint32_t last_framerate_ RTC_GUARDED_BY(local_crit_sect_) = 0; }; class TestSink : public VideoStreamEncoder::EncoderSink { @@ -2093,9 +2124,8 @@ TEST_F(VideoStreamEncoderTest, CallsBitrateObserver) { DefaultVideoBitrateAllocator(fake_encoder_.codec_config()) .GetAllocation(kLowTargetBitrateBps, kDefaultFps); - // First called on bitrate updated, then again on first frame. EXPECT_CALL(bitrate_observer, OnBitrateAllocationUpdated(expected_bitrate)) - .Times(2); + .Times(1); video_stream_encoder_->OnBitrateUpdated(kLowTargetBitrateBps, 0, 0); const int64_t kStartTimeMs = 1; @@ -3156,9 +3186,6 @@ TEST_F(VideoStreamEncoderTest, DoesNotUpdateBitrateAllocationWhenSuspended) { MockBitrateObserver bitrate_observer; video_stream_encoder_->SetBitrateAllocationObserver(&bitrate_observer); - - EXPECT_CALL(bitrate_observer, OnBitrateAllocationUpdated(_)).Times(1); - // Initial bitrate update. video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); video_stream_encoder_->WaitUntilTaskQueueIsIdle(); @@ -3227,4 +3254,80 @@ TEST_F(VideoStreamEncoderTest, video_stream_encoder_->Stop(); } +TEST_F(VideoStreamEncoderTest, DropsFramesWhenEncoderOvershoots) { + const int kFrameWidth = 320; + const int kFrameHeight = 240; + const int kFps = 30; + const int kTargetBitrateBps = 120000; + const int kNumFramesInRun = kFps * 5; // Runs of five seconds. + + video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); + + int64_t timestamp_ms = fake_clock_.TimeNanos() / rtc::kNumNanosecsPerMillisec; + max_framerate_ = kFps; + + // Insert 3 seconds of video, verify number of drops with normal bitrate. + fake_encoder_.SimulateOvershoot(1.0); + int num_dropped = 0; + for (int i = 0; i < kNumFramesInRun; ++i) { + video_source_.IncomingCapturedFrame( + CreateFrame(timestamp_ms, kFrameWidth, kFrameHeight)); + // Wait up to two frame durations for a frame to arrive. + if (!TimedWaitForEncodedFrame(timestamp_ms, 2 * 1000 / kFps)) { + ++num_dropped; + } + timestamp_ms += 1000 / kFps; + } + + // Frame drops should be less than 5% + EXPECT_LT(num_dropped, 5 * kNumFramesInRun / 100); + + // Make encoder produce frames at double the expected bitrate during 3 seconds + // of video, verify number of drops. Rate needs to be slightly changed in + // order to force the rate to be reconfigured. + fake_encoder_.SimulateOvershoot(2.0); + video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps + 1000, 0, 0); + num_dropped = 0; + for (int i = 0; i < kNumFramesInRun; ++i) { + video_source_.IncomingCapturedFrame( + CreateFrame(timestamp_ms, kFrameWidth, kFrameHeight)); + // Wait up to two frame durations for a frame to arrive. + if (!TimedWaitForEncodedFrame(timestamp_ms, 2 * 1000 / kFps)) { + ++num_dropped; + } + timestamp_ms += 1000 / kFps; + } + + // Frame drops should be more than 40%. + EXPECT_GT(num_dropped, 40 * kNumFramesInRun / 100); + + video_stream_encoder_->Stop(); +} + +TEST_F(VideoStreamEncoderTest, ConfiguresCorrectFrameRate) { + const int kFrameWidth = 320; + const int kFrameHeight = 240; + const int kActualInputFps = 24; + const int kTargetBitrateBps = 120000; + + ASSERT_GT(max_framerate_, kActualInputFps); + + int64_t timestamp_ms = fake_clock_.TimeNanos() / rtc::kNumNanosecsPerMillisec; + max_framerate_ = kActualInputFps; + video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); + + // Insert 3 seconds of video, with an input fps lower than configured max. + for (int i = 0; i < kActualInputFps * 3; ++i) { + video_source_.IncomingCapturedFrame( + CreateFrame(timestamp_ms, kFrameWidth, kFrameHeight)); + // Wait up to two frame durations for a frame to arrive. + WaitForEncodedFrame(timestamp_ms); + timestamp_ms += 1000 / kActualInputFps; + } + + EXPECT_NEAR(kActualInputFps, fake_encoder_.GetLastFramerate(), 1); + + video_stream_encoder_->Stop(); +} + } // namespace webrtc