Refactor EncodeParameters usage, remove unused rtt/loss
Bug: webrtc:10126
Change-Id: Ib93f5e65b25540576c026197f72a5902cf43fc16
Reviewed-on: https://webrtc-review.googlesource.com/c/114281
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26001}diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn
index 2ec46e6..196880d 100644
--- a/modules/video_coding/BUILD.gn
+++ b/modules/video_coding/BUILD.gn
@@ -173,6 +173,7 @@
"..:module_api_public",
"../..:webrtc_common",
"../../api:fec_controller_api",
+ "../../api/units:data_rate",
"../../api/video:builtin_video_bitrate_allocator_factory",
"../../api/video:encoded_frame",
"../../api/video:video_bitrate_allocator",
diff --git a/modules/video_coding/generic_encoder.cc b/modules/video_coding/generic_encoder.cc
index 807ccbe..1f24159 100644
--- a/modules/video_coding/generic_encoder.cc
+++ b/modules/video_coding/generic_encoder.cc
@@ -46,7 +46,6 @@
: encoder_(encoder),
vcm_encoded_frame_callback_(encoded_frame_callback),
internal_source_(internal_source),
- encoder_params_({VideoBitrateAllocation(), 0, 0, 0}),
streams_or_svc_num_(0),
codec_type_(VideoCodecType::kVideoCodecGeneric) {}
diff --git a/modules/video_coding/generic_encoder.h b/modules/video_coding/generic_encoder.h
index 2f841b3..1f4ade2 100644
--- a/modules/video_coding/generic_encoder.h
+++ b/modules/video_coding/generic_encoder.h
@@ -15,9 +15,9 @@
#include <list>
#include <vector>
+#include "api/units/data_rate.h"
#include "modules/video_coding/include/video_codec_interface.h"
#include "modules/video_coding/include/video_coding_defines.h"
-
#include "rtc_base/criticalsection.h"
#include "rtc_base/race_checker.h"
@@ -28,9 +28,21 @@
} // 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;
- uint8_t loss_rate;
- int64_t rtt;
+ // The input frame rate to the encoder in fps, measured in the video sender.
uint32_t input_frame_rate;
};
diff --git a/modules/video_coding/video_coding_impl.cc b/modules/video_coding/video_coding_impl.cc
index 39eccb5..dffebd7 100644
--- a/modules/video_coding/video_coding_impl.cc
+++ b/modules/video_coding/video_coding_impl.cc
@@ -121,8 +121,8 @@
int32_t SetChannelParameters(uint32_t target_bitrate, // bits/s.
uint8_t lossRate,
int64_t rtt) override {
- return sender_.SetChannelParameters(target_bitrate, lossRate, rtt,
- rate_allocator_.get(), nullptr);
+ return sender_.SetChannelParameters(target_bitrate, rate_allocator_.get(),
+ nullptr);
}
int32_t SetVideoProtection(VCMVideoProtection videoProtection,
diff --git a/modules/video_coding/video_coding_impl.h b/modules/video_coding/video_coding_impl.h
index e27a2fe..daf21d8 100644
--- a/modules/video_coding/video_coding_impl.h
+++ b/modules/video_coding/video_coding_impl.h
@@ -79,8 +79,6 @@
// cause an immediate call to VideoEncoder::SetRateAllocation().
int32_t SetChannelParameters(
uint32_t target_bitrate_bps,
- uint8_t loss_rate,
- int64_t rtt,
VideoBitrateAllocator* bitrate_allocator,
VideoBitrateAllocationObserver* bitrate_updated_callback);
@@ -107,6 +105,10 @@
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;
diff --git a/modules/video_coding/video_sender.cc b/modules/video_coding/video_sender.cc
index f8e2106..da59fe9 100644
--- a/modules/video_coding/video_sender.cc
+++ b/modules/video_coding/video_sender.cc
@@ -55,7 +55,6 @@
frame_dropper_requested_(true),
force_disable_frame_dropper_(false),
current_codec_(),
- encoder_params_({VideoBitrateAllocation(), 0, 0, 0}),
encoder_has_internal_source_(false),
next_frame_types_(1, kVideoFrameDelta) {
// Allow VideoSender to be created on one thread but used on another, post
@@ -167,22 +166,31 @@
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 (target_bitrate_bps > 0) {
+ if (bitrate_bps > 0) {
if (bitrate_allocator) {
- bitrate_allocation = bitrate_allocator->GetAllocation(
- video_target_rate_bps, input_frame_rate);
+ bitrate_allocation =
+ bitrate_allocator->GetAllocation(bitrate_bps, framerate_fps);
} else {
DefaultVideoBitrateAllocator default_allocator(current_codec_);
- bitrate_allocation = default_allocator.GetAllocation(
- video_target_rate_bps, input_frame_rate);
+ bitrate_allocation =
+ default_allocator.GetAllocation(bitrate_bps, framerate_fps);
}
}
- EncoderParameters new_encoder_params = {bitrate_allocation, params.loss_rate,
- params.rtt, input_frame_rate};
- return new_encoder_params;
+ return bitrate_allocation;
}
void VideoSender::UpdateChannelParameters(
@@ -193,22 +201,19 @@
rtc::CritScope cs(¶ms_crit_);
encoder_params_ =
UpdateEncoderParameters(encoder_params_, bitrate_allocator,
- encoder_params_.target_bitrate.get_sum_bps());
+ encoder_params_.total_bitrate.bps());
target_rate = encoder_params_.target_bitrate;
}
- if (bitrate_updated_callback && target_rate.get_sum_bps() > 0)
+ 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,
- uint8_t loss_rate,
- int64_t rtt,
VideoBitrateAllocator* bitrate_allocator,
VideoBitrateAllocationObserver* bitrate_updated_callback) {
EncoderParameters encoder_params;
- encoder_params.loss_rate = loss_rate;
- encoder_params.rtt = rtt;
encoder_params = UpdateEncoderParameters(encoder_params, bitrate_allocator,
target_bitrate_bps);
if (bitrate_updated_callback && target_bitrate_bps > 0) {
@@ -286,11 +291,10 @@
_mediaOpt.EnableFrameDropper(frame_dropping_enabled);
if (_mediaOpt.DropFrame()) {
- RTC_LOG(LS_VERBOSE) << "Drop Frame "
+ RTC_LOG(LS_VERBOSE) << "Drop Frame: "
<< "target bitrate "
<< encoder_params.target_bitrate.get_sum_bps()
- << " loss rate " << encoder_params.loss_rate << " rtt "
- << encoder_params.rtt << " input frame rate "
+ << ", input frame rate "
<< encoder_params.input_frame_rate;
post_encode_callback_->OnDroppedFrame(
EncodedImageCallback::DropReason::kDroppedByMediaOptimizations);
diff --git a/modules/video_coding/video_sender_unittest.cc b/modules/video_coding/video_sender_unittest.cc
index c2ff0f6..8e727df 100644
--- a/modules/video_coding/video_sender_unittest.cc
+++ b/modules/video_coding/video_sender_unittest.cc
@@ -314,14 +314,14 @@
SetRateAllocation(new_rate_allocation, settings_.maxFramerate))
.Times(1)
.WillOnce(Return(0));
- sender_->SetChannelParameters(new_bitrate_kbps * 1000, 0, 200,
- rate_allocator_.get(), nullptr);
+ sender_->SetChannelParameters(new_bitrate_kbps * 1000, rate_allocator_.get(),
+ nullptr);
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, 0, 200, rate_allocator_.get(), nullptr);
+ sender_->SetChannelParameters(0, rate_allocator_.get(), nullptr);
AddFrame();
}
@@ -358,21 +358,19 @@
EXPECT_CALL(encoder_, SetRateAllocation(new_rate_allocation, _))
.Times(1)
.WillOnce(Return(0));
- sender_->SetChannelParameters(new_bitrate_kbps * 1000, 0, 200,
- rate_allocator_.get(), nullptr);
+ sender_->SetChannelParameters(new_bitrate_kbps * 1000, rate_allocator_.get(),
+ nullptr);
}
TEST_F(TestVideoSenderWithMockEncoder,
NoRedundantSetChannelParameterOrSetRatesCalls) {
- const uint8_t kLossRate = 4;
- const uint8_t kRtt = 200;
const int64_t kRateStatsWindowMs = 2000;
const uint32_t kInputFps = 20;
int64_t start_time = clock_.TimeInMilliseconds();
// 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, kLossRate, kRtt,
+ sender_->SetChannelParameters(settings_.startBitrate * 1000,
rate_allocator_.get(), nullptr);
while (clock_.TimeInMilliseconds() < start_time + kRateStatsWindowMs) {
AddFrame();
@@ -387,8 +385,8 @@
EXPECT_CALL(encoder_, SetRateAllocation(new_rate_allocation, kInputFps))
.Times(1)
.WillOnce(Return(0));
- sender_->SetChannelParameters(new_bitrate_bps, kLossRate, kRtt,
- rate_allocator_.get(), nullptr);
+ sender_->SetChannelParameters(new_bitrate_bps, rate_allocator_.get(),
+ nullptr);
AddFrame();
}
@@ -440,7 +438,7 @@
// buffer since it will fail to calculate the framerate.
if (i != 0) {
EXPECT_EQ(VCM_OK, sender_->SetChannelParameters(
- available_bitrate_kbps_ * 1000, 0, 200,
+ available_bitrate_kbps_ * 1000,
rate_allocator_.get(), nullptr));
}
}
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc
index 8cabc03..235e72b 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -1003,8 +1003,7 @@
has_seen_first_significant_bwe_change_ = true;
}
- video_sender_.SetChannelParameters(bitrate_bps, fraction_lost,
- round_trip_time_ms, rate_allocator_.get(),
+ video_sender_.SetChannelParameters(bitrate_bps, rate_allocator_.get(),
bitrate_observer_);
encoder_start_bitrate_bps_ =
