Fix perf regression in screenshare temporal layer bitrate allocation
A recent cl (https://codereview.webrtc.org/2510583002) introduced an issue where temporal layers may return incorrect bitrates, given that they are stateful and that the GetPreferredBitrateBps is called. The fix is to use a temporary simulcast rate allocator instance, without temporal layers, and get the preferred bitrate from that. Additionally, some regression in bitrate allocated stems from overly often reconfiguring the encoder, which yields suboptimal rate control. The fix here is to limit encoder updates to when values have actually changed. As a bonus, dchecks added by this cl found a bug in the (unused) RealtimeTemporalLayers implementation. Fixed that as well. BUG=webrtc:6301, chromium:666654 Review-Url: https://codereview.webrtc.org/2529073003 Cr-Commit-Position: refs/heads/master@{#15250}
This commit is contained in:
@ -99,16 +99,18 @@ BitrateAllocation SimulcastRateAllocator::GetAllocation(
|
||||
|
||||
uint32_t target_bitrate_kbps =
|
||||
allocated_bitrates_bps.GetBitrate(simulcast_id, 0) / 1000;
|
||||
const uint32_t expected_allocated_bitrate_kbps = target_bitrate_kbps;
|
||||
RTC_DCHECK_EQ(
|
||||
target_bitrate_kbps,
|
||||
allocated_bitrates_bps.GetSpatialLayerSum(simulcast_id) / 1000);
|
||||
const int num_temporal_streams = std::max<uint8_t>(
|
||||
1, codec_.numberOfSimulcastStreams == 0
|
||||
? codec_.VP8().numberOfTemporalLayers
|
||||
: codec_.simulcastStream[0].numberOfTemporalLayers);
|
||||
|
||||
uint32_t max_bitrate_kbps;
|
||||
if (num_spatial_streams == 1) {
|
||||
max_bitrate_kbps = codec_.maxBitrate;
|
||||
const int num_temporal_streams =
|
||||
codec_.numberOfSimulcastStreams == 0
|
||||
? codec_.VP8().numberOfTemporalLayers
|
||||
: codec_.simulcastStream[0].numberOfTemporalLayers;
|
||||
|
||||
// TODO(holmer): This is a temporary hack for screensharing, where we
|
||||
// interpret the startBitrate as the encoder target bitrate. This is
|
||||
@ -127,19 +129,28 @@ BitrateAllocation SimulcastRateAllocator::GetAllocation(
|
||||
|
||||
std::vector<uint32_t> tl_allocation = tl_it->second->OnRatesUpdated(
|
||||
target_bitrate_kbps, max_bitrate_kbps, framerate);
|
||||
RTC_DCHECK_GT(tl_allocation.size(), 0);
|
||||
RTC_DCHECK_LE(tl_allocation.size(), num_temporal_streams);
|
||||
|
||||
uint64_t tl_allocation_sum_kbps = 0;
|
||||
for (size_t tl_index = 0; tl_index < tl_allocation.size(); ++tl_index) {
|
||||
uint32_t layer_rate_kbps = tl_allocation[tl_index];
|
||||
allocated_bitrates_bps.SetBitrate(simulcast_id, tl_index,
|
||||
tl_allocation[tl_index] * 1000);
|
||||
layer_rate_kbps * 1000);
|
||||
tl_allocation_sum_kbps += layer_rate_kbps;
|
||||
}
|
||||
RTC_DCHECK_LE(tl_allocation_sum_kbps, expected_allocated_bitrate_kbps);
|
||||
}
|
||||
|
||||
return allocated_bitrates_bps;
|
||||
}
|
||||
|
||||
uint32_t SimulcastRateAllocator::GetPreferredBitrateBps(uint32_t framerate) {
|
||||
// Create a temporary instance without temporal layers, as they may be
|
||||
// stateful, and updating the bitrate to max here can cause side effects.
|
||||
SimulcastRateAllocator temp_allocator(codec_, nullptr);
|
||||
BitrateAllocation allocation =
|
||||
GetAllocation(codec_.maxBitrate * 1000, framerate);
|
||||
temp_allocator.GetAllocation(codec_.maxBitrate * 1000, framerate);
|
||||
return allocation.get_sum_bps();
|
||||
}
|
||||
|
||||
|
||||
@ -15,14 +15,28 @@
|
||||
#include <utility>
|
||||
#include <vector>
|
||||
|
||||
#include "webrtc/test/gmock.h"
|
||||
#include "webrtc/test/gtest.h"
|
||||
|
||||
namespace webrtc {
|
||||
namespace {
|
||||
using ::testing::_;
|
||||
|
||||
constexpr uint32_t kMinBitrateKbps = 50;
|
||||
constexpr uint32_t kTargetBitrateKbps = 100;
|
||||
constexpr uint32_t kMaxBitrateKbps = 1000;
|
||||
constexpr uint32_t kFramerateFps = 5;
|
||||
|
||||
class MockTemporalLayers : public TemporalLayers {
|
||||
public:
|
||||
MOCK_METHOD1(EncodeFlags, int(uint32_t));
|
||||
MOCK_METHOD3(OnRatesUpdated, std::vector<uint32_t>(int, int, int));
|
||||
MOCK_METHOD1(UpdateConfiguration, bool(vpx_codec_enc_cfg_t*));
|
||||
MOCK_METHOD3(PopulateCodecSpecific,
|
||||
void(bool, CodecSpecificInfoVP8*, uint32_t));
|
||||
MOCK_METHOD3(FrameEncoded, void(unsigned int, uint32_t, int));
|
||||
MOCK_CONST_METHOD0(CurrentLayerId, int());
|
||||
};
|
||||
} // namespace
|
||||
|
||||
class SimulcastRateAllocatorTest : public ::testing::TestWithParam<bool> {
|
||||
@ -251,6 +265,10 @@ TEST_F(SimulcastRateAllocatorTest, OneToThreeStreams) {
|
||||
}
|
||||
|
||||
TEST_F(SimulcastRateAllocatorTest, GetPreferredBitrateBps) {
|
||||
MockTemporalLayers mock_layers;
|
||||
allocator_.reset(new SimulcastRateAllocator(codec_, nullptr));
|
||||
allocator_->OnTemporalLayersCreated(0, &mock_layers);
|
||||
EXPECT_CALL(mock_layers, OnRatesUpdated(_, _, _)).Times(0);
|
||||
EXPECT_EQ(codec_.maxBitrate * 1000,
|
||||
allocator_->GetPreferredBitrateBps(codec_.maxFramerate));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user