diff --git a/api/video_codecs/video_encoder.cc b/api/video_codecs/video_encoder.cc index 3a848f39ed..d3f16a0053 100644 --- a/api/video_codecs/video_encoder.cc +++ b/api/video_codecs/video_encoder.cc @@ -116,17 +116,6 @@ VideoEncoder::RateControlParameters::RateControlParameters( framerate_fps(framerate_fps), bandwidth_allocation(bandwidth_allocation) {} -bool VideoEncoder::RateControlParameters::operator==( - const VideoEncoder::RateControlParameters& rhs) const { - return std::tie(bitrate, framerate_fps, bandwidth_allocation) == - std::tie(rhs.bitrate, rhs.framerate_fps, rhs.bandwidth_allocation); -} - -bool VideoEncoder::RateControlParameters::operator!=( - const VideoEncoder::RateControlParameters& rhs) const { - return !(rhs == *this); -} - VideoEncoder::RateControlParameters::~RateControlParameters() = default; void VideoEncoder::SetFecControllerOverride( diff --git a/api/video_codecs/video_encoder.h b/api/video_codecs/video_encoder.h index 766ea75712..0ee5521b95 100644 --- a/api/video_codecs/video_encoder.h +++ b/api/video_codecs/video_encoder.h @@ -239,9 +239,6 @@ class RTC_EXPORT VideoEncoder { // |bitrate.get_sum_bps()|, but may be higher if the application is not // network constrained. DataRate bandwidth_allocation; - - bool operator==(const RateControlParameters& rhs) const; - bool operator!=(const RateControlParameters& rhs) const; }; struct LossNotification { diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index e1ca55722d..200b4293ef 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -441,7 +441,7 @@ class VideoStreamEncoder::VideoSourceProxy { }; VideoStreamEncoder::EncoderRateSettings::EncoderRateSettings() - : rate_control(), + : VideoEncoder::RateControlParameters(), encoder_target(DataRate::Zero()), stable_encoder_target(DataRate::Zero()) {} @@ -451,13 +451,16 @@ VideoStreamEncoder::EncoderRateSettings::EncoderRateSettings( DataRate bandwidth_allocation, DataRate encoder_target, DataRate stable_encoder_target) - : rate_control(bitrate, framerate_fps, bandwidth_allocation), + : VideoEncoder::RateControlParameters(bitrate, + framerate_fps, + bandwidth_allocation), encoder_target(encoder_target), stable_encoder_target(stable_encoder_target) {} bool VideoStreamEncoder::EncoderRateSettings::operator==( const EncoderRateSettings& rhs) const { - return rate_control == rhs.rate_control && + return bitrate == rhs.bitrate && framerate_fps == rhs.framerate_fps && + bandwidth_allocation == rhs.bandwidth_allocation && encoder_target == rhs.encoder_target && stable_encoder_target == rhs.stable_encoder_target; } @@ -931,8 +934,7 @@ void VideoStreamEncoder::ReconfigureEncoder() { if (rate_allocator_ && last_encoder_rate_settings_) { // We have a new rate allocator instance and already configured target // bitrate. Update the rate allocation and notify observers. - last_encoder_rate_settings_->rate_control.framerate_fps = - GetInputFramerateFps(); + last_encoder_rate_settings_->framerate_fps = GetInputFramerateFps(); SetEncoderRates( UpdateBitrateAllocationAndNotifyObserver(*last_encoder_rate_settings_)); } @@ -1133,7 +1135,7 @@ VideoStreamEncoder::UpdateBitrateAllocationAndNotifyObserver( if (rate_allocator_ && rate_settings.encoder_target > DataRate::Zero()) { new_allocation = rate_allocator_->Allocate(VideoBitrateAllocationParameters( rate_settings.encoder_target, rate_settings.stable_encoder_target, - rate_settings.rate_control.framerate_fps)); + rate_settings.framerate_fps)); } if (bitrate_observer_ && new_allocation.get_sum_bps() > 0) { @@ -1154,27 +1156,27 @@ VideoStreamEncoder::UpdateBitrateAllocationAndNotifyObserver( } EncoderRateSettings new_rate_settings = rate_settings; - new_rate_settings.rate_control.bitrate = new_allocation; + new_rate_settings.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 // case, make sure the bandwidth allocation is at least equal the allocation // as that is part of the document contract for that field. - new_rate_settings.rate_control.bandwidth_allocation = std::max( - new_rate_settings.rate_control.bandwidth_allocation, - DataRate::bps(new_rate_settings.rate_control.bitrate.get_sum_bps())); + new_rate_settings.bandwidth_allocation = + std::max(new_rate_settings.bandwidth_allocation, + DataRate::bps(new_rate_settings.bitrate.get_sum_bps())); if (bitrate_adjuster_) { VideoBitrateAllocation adjusted_allocation = - bitrate_adjuster_->AdjustRateAllocation(new_rate_settings.rate_control); + bitrate_adjuster_->AdjustRateAllocation(new_rate_settings); RTC_LOG(LS_VERBOSE) << "Adjusting allocation, fps = " - << rate_settings.rate_control.framerate_fps << ", from " + << rate_settings.framerate_fps << ", from " << new_allocation.ToString() << ", to " << adjusted_allocation.ToString(); - new_rate_settings.rate_control.bitrate = adjusted_allocation; + new_rate_settings.bitrate = adjusted_allocation; } encoder_stats_observer_->OnBitrateAllocationUpdated( - send_codec_, new_rate_settings.rate_control.bitrate); + send_codec_, new_rate_settings.bitrate); return new_rate_settings; } @@ -1191,11 +1193,10 @@ uint32_t VideoStreamEncoder::GetInputFramerateFps() { void VideoStreamEncoder::SetEncoderRates( const EncoderRateSettings& rate_settings) { - RTC_DCHECK_GT(rate_settings.rate_control.framerate_fps, 0.0); - bool rate_control_changed = - (!last_encoder_rate_settings_.has_value() || - last_encoder_rate_settings_->rate_control != rate_settings.rate_control); - if (last_encoder_rate_settings_ != rate_settings) { + RTC_DCHECK_GT(rate_settings.framerate_fps, 0.0); + const bool settings_changes = !last_encoder_rate_settings_ || + rate_settings != *last_encoder_rate_settings_; + if (settings_changes) { last_encoder_rate_settings_ = rate_settings; } @@ -1211,16 +1212,15 @@ void VideoStreamEncoder::SetEncoderRates( // bitrate. // TODO(perkj): Make sure all known encoder implementations handle zero // target bitrate and remove this check. - if (!HasInternalSource() && - rate_settings.rate_control.bitrate.get_sum_bps() == 0) { + if (!HasInternalSource() && rate_settings.bitrate.get_sum_bps() == 0) { return; } - if (rate_control_changed) { - encoder_->SetRates(rate_settings.rate_control); + if (settings_changes) { + encoder_->SetRates(rate_settings); frame_encode_metadata_writer_.OnSetRates( - rate_settings.rate_control.bitrate, - static_cast(rate_settings.rate_control.framerate_fps + 0.5)); + rate_settings.bitrate, + static_cast(rate_settings.framerate_fps + 0.5)); } } @@ -1269,8 +1269,7 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, // |last_encoder_rate_setings_|, triggering the call to SetRate() on the // encoder. EncoderRateSettings new_rate_settings = *last_encoder_rate_settings_; - new_rate_settings.rate_control.framerate_fps = - static_cast(framerate_fps); + new_rate_settings.framerate_fps = static_cast(framerate_fps); SetEncoderRates( UpdateBitrateAllocationAndNotifyObserver(new_rate_settings)); } diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index ba9f519475..f2268678d6 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -121,7 +121,7 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, int pixel_count() const { return width * height; } }; - struct EncoderRateSettings { + struct EncoderRateSettings : public VideoEncoder::RateControlParameters { EncoderRateSettings(); EncoderRateSettings(const VideoBitrateAllocation& bitrate, double framerate_fps, @@ -131,7 +131,6 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, bool operator==(const EncoderRateSettings& rhs) const; bool operator!=(const EncoderRateSettings& rhs) const; - VideoEncoder::RateControlParameters rate_control; // This is the scalar target bitrate before the VideoBitrateAllocator, i.e. // the |target_bitrate| argument of the OnBitrateUpdated() method. This is // needed because the bitrate allocator may truncate the total bitrate and a diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 6eba930405..6f19edcbb1 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -782,11 +782,6 @@ class VideoStreamEncoderTest : public ::testing::Test { return num_encoder_initializations_; } - int GetNumSetRates() const { - rtc::CritScope lock(&local_crit_sect_); - return num_set_rates_; - } - private: int32_t Encode(const VideoFrame& input_image, const std::vector* frame_types) override { @@ -853,7 +848,6 @@ class VideoStreamEncoderTest : public ::testing::Test { void SetRates(const RateControlParameters& parameters) { rtc::CritScope lock(&local_crit_sect_); - num_set_rates_++; VideoBitrateAllocation adjusted_rate_allocation; for (size_t si = 0; si < kMaxSpatialLayers; ++si) { for (size_t ti = 0; ti < kMaxTemporalStreams; ++ti) { @@ -907,7 +901,6 @@ class VideoStreamEncoderTest : public ::testing::Test { int num_encoder_initializations_ RTC_GUARDED_BY(local_crit_sect_) = 0; std::vector resolution_bitrate_limits_ RTC_GUARDED_BY(local_crit_sect_); - int num_set_rates_ RTC_GUARDED_BY(local_crit_sect_) = 0; }; class TestSink : public VideoStreamEncoder::EncoderSink { @@ -4882,75 +4875,4 @@ TEST_F(VideoStreamEncoderTest, ResolutionEncoderSwitch) { video_stream_encoder_->Stop(); } -TEST_F(VideoStreamEncoderTest, - AllocationPropegratedToEncoderWhenTargetRateChanged) { - const int kFrameWidth = 320; - const int kFrameHeight = 180; - - // Set initial rate. - auto rate = DataRate::kbps(100); - video_stream_encoder_->OnBitrateUpdated( - /*target_bitrate=*/rate, - /*stable_target_bitrate=*/rate, - /*link_allocation=*/rate, - /*fraction_lost=*/0, - /*rtt_ms=*/0); - - // Insert a first video frame so that encoder gets configured. - int64_t timestamp_ms = fake_clock_.TimeNanos() / rtc::kNumNanosecsPerMillisec; - VideoFrame frame = CreateFrame(timestamp_ms, kFrameWidth, kFrameHeight); - frame.set_rotation(kVideoRotation_270); - video_source_.IncomingCapturedFrame(frame); - WaitForEncodedFrame(timestamp_ms); - EXPECT_EQ(1, fake_encoder_.GetNumSetRates()); - - // Change of target bitrate propagates to the encoder. - auto new_stable_rate = rate - DataRate::kbps(5); - video_stream_encoder_->OnBitrateUpdated( - /*target_bitrate=*/new_stable_rate, - /*stable_target_bitrate=*/new_stable_rate, - /*link_allocation=*/rate, - /*fraction_lost=*/0, - /*rtt_ms=*/0); - video_stream_encoder_->WaitUntilTaskQueueIsIdle(); - EXPECT_EQ(2, fake_encoder_.GetNumSetRates()); - video_stream_encoder_->Stop(); -} - -TEST_F(VideoStreamEncoderTest, - AllocationNotPropegratedToEncoderWhenTargetRateUnchanged) { - const int kFrameWidth = 320; - const int kFrameHeight = 180; - - // Set initial rate. - auto rate = DataRate::kbps(100); - video_stream_encoder_->OnBitrateUpdated( - /*target_bitrate=*/rate, - /*stable_target_bitrate=*/rate, - /*link_allocation=*/rate, - /*fraction_lost=*/0, - /*rtt_ms=*/0); - - // Insert a first video frame so that encoder gets configured. - int64_t timestamp_ms = fake_clock_.TimeNanos() / rtc::kNumNanosecsPerMillisec; - VideoFrame frame = CreateFrame(timestamp_ms, kFrameWidth, kFrameHeight); - frame.set_rotation(kVideoRotation_270); - video_source_.IncomingCapturedFrame(frame); - WaitForEncodedFrame(timestamp_ms); - EXPECT_EQ(1, fake_encoder_.GetNumSetRates()); - - // Set a higher target rate without changing the link_allocation. Should not - // reset encoder's rate. - auto new_stable_rate = rate - DataRate::kbps(5); - video_stream_encoder_->OnBitrateUpdated( - /*target_bitrate=*/rate, - /*stable_target_bitrate=*/new_stable_rate, - /*link_allocation=*/rate, - /*fraction_lost=*/0, - /*rtt_ms=*/0); - video_stream_encoder_->WaitUntilTaskQueueIsIdle(); - EXPECT_EQ(1, fake_encoder_.GetNumSetRates()); - video_stream_encoder_->Stop(); -} - } // namespace webrtc