mollyim/webrtc
1
0
Fork 0
mirror of https://github.com/mollyim/webrtc.git synced 2025-05-14 06:10:40 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

Revert "Fix minor regression caused by a8336d3"

Browse source 
This reverts commit 809198edff.

Reason for revert: Performance regressions that need to be addressed.

Original change's description:
> Fix minor regression caused by a8336d3
> 
> 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}

TBR=sprang@webrtc.org,eshr@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: webrtc:10126
Change-Id: I133cbe5d8cb894ed944ae8a2d0f63a78bbed72ee
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/153484
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29221}
This commit is contained in:
Evan Shrubsole 2019-09-18 13:46:06 +00:00 committed by Commit Bot
parent 53227ccba9
commit b6a45dda4c
5 changed files with 27 additions and 121 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
api/video_codecs/video_encoder.cc
View file

@ -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(

3
api/video_codecs/video_encoder.h
View file

@ -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 {

53
video/video_stream_encoder.cc
View file

@ -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<uint32_t>(rate_settings.rate_control.framerate_fps + 0.5));
rate_settings.bitrate,
static_cast<uint32_t>(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<double>(framerate_fps);
new_rate_settings.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 {
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

78
video/video_stream_encoder_unittest.cc
View file

@ -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<VideoFrameType>* 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<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 {
@ -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