zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix minor regression caused by a8336d3

Browse Source 
VideoEncoder::SetRates was being called unnessesarily when the fields
appended to RateControlParameters were changed. Since SetRates only
cares about RateControlParameters, it should have only been called if
the RateControlParameters themselves were actually changed.

Bug: webrtc:10126
Change-Id: Ic47d67e642a3043307fec950e5fba970d9f95167
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/152829
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@google.com>
Cr-Commit-Position: refs/heads/master@{#29208}
This commit is contained in:
Evan Shrubsole
2019-09-17 13:00:04 +02:00
committed by Commit Bot
parent 7d00342f66
commit 809198edff
5 changed files with 121 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
api/video_codecs/video_encoder.cc
View File

@ -116,6 +116,17 @@ 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(

3
api/video_codecs/video_encoder.h
View File

@ -239,6 +239,9 @@ 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 {

53
video/video_stream_encoder.cc
View File

@ -441,7 +441,7 @@ class VideoStreamEncoder::VideoSourceProxy {
};
VideoStreamEncoder::EncoderRateSettings::EncoderRateSettings()
: VideoEncoder::RateControlParameters(),
: rate_control(),
encoder_target(DataRate::Zero()),
stable_encoder_target(DataRate::Zero()) {}
@ -451,16 +451,13 @@ VideoStreamEncoder::EncoderRateSettings::EncoderRateSettings(
DataRate bandwidth_allocation,
DataRate encoder_target,
DataRate stable_encoder_target)
: VideoEncoder::RateControlParameters(bitrate,
framerate_fps,
bandwidth_allocation),
: rate_control(bitrate, framerate_fps, bandwidth_allocation),
encoder_target(encoder_target),
stable_encoder_target(stable_encoder_target) {}
bool VideoStreamEncoder::EncoderRateSettings::operator==(
const EncoderRateSettings& rhs) const {
return bitrate == rhs.bitrate && framerate_fps == rhs.framerate_fps &&
bandwidth_allocation == rhs.bandwidth_allocation &&
return rate_control == rhs.rate_control &&
encoder_target == rhs.encoder_target &&
stable_encoder_target == rhs.stable_encoder_target;
}
@ -948,7 +945,8 @@ 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_->framerate_fps = GetInputFramerateFps();
last_encoder_rate_settings_->rate_control.framerate_fps =
GetInputFramerateFps();
SetEncoderRates(
UpdateBitrateAllocationAndNotifyObserver(*last_encoder_rate_settings_));
}
@ -1149,7 +1147,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.framerate_fps));
rate_settings.rate_control.framerate_fps));
}
if (bitrate_observer_ && new_allocation.get_sum_bps() > 0) {
@ -1170,27 +1168,27 @@ VideoStreamEncoder::UpdateBitrateAllocationAndNotifyObserver(
}
EncoderRateSettings new_rate_settings = rate_settings;
new_rate_settings.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
// 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.bandwidth_allocation =
std::max(new_rate_settings.bandwidth_allocation,
DataRate::bps(new_rate_settings.bitrate.get_sum_bps()));
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()));
if (bitrate_adjuster_) {
VideoBitrateAllocation adjusted_allocation =
bitrate_adjuster_->AdjustRateAllocation(new_rate_settings);
bitrate_adjuster_->AdjustRateAllocation(new_rate_settings.rate_control);
RTC_LOG(LS_VERBOSE) << "Adjusting allocation, fps = "
<< rate_settings.framerate_fps << ", from "
<< rate_settings.rate_control.framerate_fps << ", from "
<< new_allocation.ToString() << ", to "
<< adjusted_allocation.ToString();
new_rate_settings.bitrate = adjusted_allocation;
new_rate_settings.rate_control.bitrate = adjusted_allocation;
}
encoder_stats_observer_->OnBitrateAllocationUpdated(
send_codec_, new_rate_settings.bitrate);
send_codec_, new_rate_settings.rate_control.bitrate);
return new_rate_settings;
}
@ -1207,10 +1205,11 @@ uint32_t VideoStreamEncoder::GetInputFramerateFps() {
void VideoStreamEncoder::SetEncoderRates(
const EncoderRateSettings& 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) {
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) {
last_encoder_rate_settings_ = rate_settings;
}
@ -1226,15 +1225,16 @@ void VideoStreamEncoder::SetEncoderRates(
// bitrate.
// TODO(perkj): Make sure all known encoder implementations handle zero
// target bitrate and remove this check.
if (!HasInternalSource() && rate_settings.bitrate.get_sum_bps() == 0) {
if (!HasInternalSource() &&
rate_settings.rate_control.bitrate.get_sum_bps() == 0) {
return;
}
if (settings_changes) {
encoder_->SetRates(rate_settings);
if (rate_control_changed) {
encoder_->SetRates(rate_settings.rate_control);
frame_encode_metadata_writer_.OnSetRates(
rate_settings.bitrate,
static_cast<uint32_t>(rate_settings.framerate_fps + 0.5));
rate_settings.rate_control.bitrate,
static_cast<uint32_t>(rate_settings.rate_control.framerate_fps + 0.5));
}
}
@ -1283,7 +1283,8 @@ 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.framerate_fps = static_cast<double>(framerate_fps);
new_rate_settings.rate_control.framerate_fps =
static_cast<double>(framerate_fps);
SetEncoderRates(
UpdateBitrateAllocationAndNotifyObserver(new_rate_settings));
}

3
video/video_stream_encoder.h
View File

@ -121,7 +121,7 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
int pixel_count() const { return width * height; }
};
struct EncoderRateSettings : public VideoEncoder::RateControlParameters {
struct EncoderRateSettings {
EncoderRateSettings();
EncoderRateSettings(const VideoBitrateAllocation& bitrate,
double framerate_fps,
@ -131,6 +131,7 @@ 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

78
video/video_stream_encoder_unittest.cc
View File

@ -782,6 +782,11 @@ 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<VideoFrameType>* frame_types) override {
@ -848,6 +853,7 @@ 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) {
@ -901,6 +907,7 @@ class VideoStreamEncoderTest : public ::testing::Test {
int num_encoder_initializations_ RTC_GUARDED_BY(local_crit_sect_) = 0;
std::vector<ResolutionBitrateLimits> 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 {
@ -4875,4 +4882,75 @@ 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