Fix bug where min transmit bitrate wasn't working
A recent refactoring (r13192) introduced a bug where the min transmit config wasn't being respected. Specifically, if a VideoSendStream was created without it and the reconfigured, the min transmit bitrate would not take effect. Probably the other way around as well. BUG=webrtc::5687 Review-Url: https://codereview.webrtc.org/2106183002 Cr-Commit-Position: refs/heads/master@{#13390}
This commit is contained in:
@ -92,8 +92,9 @@ class Call {
|
||||
};
|
||||
|
||||
struct Stats {
|
||||
int send_bandwidth_bps = 0;
|
||||
int recv_bandwidth_bps = 0;
|
||||
int send_bandwidth_bps = 0; // Estimated available send bandwidth.
|
||||
int max_padding_bitrate_bps = 0; // Cumulative configured max padding.
|
||||
int recv_bandwidth_bps = 0; // Estimated available receive bandwidth.
|
||||
int64_t pacer_delay_ms = 0;
|
||||
int64_t rtt_ms = -1;
|
||||
};
|
||||
|
||||
@ -189,6 +189,7 @@ class Call : public webrtc::Call,
|
||||
int64_t pacer_bitrate_sum_kbits_ GUARDED_BY(&bitrate_crit_);
|
||||
uint32_t min_allocated_send_bitrate_bps_ GUARDED_BY(&bitrate_crit_);
|
||||
int64_t num_bitrate_updates_ GUARDED_BY(&bitrate_crit_);
|
||||
uint32_t configured_max_padding_bitrate_bps_ GUARDED_BY(&bitrate_crit_);
|
||||
|
||||
std::map<std::string, rtc::NetworkRoute> network_routes_;
|
||||
|
||||
@ -229,6 +230,8 @@ Call::Call(const Call::Config& config)
|
||||
pacer_bitrate_sum_kbits_(0),
|
||||
min_allocated_send_bitrate_bps_(0),
|
||||
num_bitrate_updates_(0),
|
||||
configured_max_padding_bitrate_bps_(0),
|
||||
|
||||
remb_(clock_),
|
||||
congestion_controller_(
|
||||
new CongestionController(clock_, this, &remb_, event_log_.get())),
|
||||
@ -552,6 +555,10 @@ Call::Stats Call::GetStats() const {
|
||||
stats.recv_bandwidth_bps = recv_bandwidth;
|
||||
stats.pacer_delay_ms = congestion_controller_->GetPacerQueuingDelayMs();
|
||||
stats.rtt_ms = call_stats_->rtcp_rtt_stats()->LastProcessedRtt();
|
||||
{
|
||||
rtc::CritScope cs(&bitrate_crit_);
|
||||
stats.max_padding_bitrate_bps = configured_max_padding_bitrate_bps_;
|
||||
}
|
||||
return stats;
|
||||
}
|
||||
|
||||
@ -714,6 +721,7 @@ void Call::OnAllocationLimitsChanged(uint32_t min_send_bitrate_bps,
|
||||
min_send_bitrate_bps, max_padding_bitrate_bps);
|
||||
rtc::CritScope lock(&bitrate_crit_);
|
||||
min_allocated_send_bitrate_bps_ = min_send_bitrate_bps;
|
||||
configured_max_padding_bitrate_bps_ = max_padding_bitrate_bps;
|
||||
}
|
||||
|
||||
void Call::ConfigureSync(const std::string& sync_group) {
|
||||
|
||||
@ -589,6 +589,16 @@ void VideoSendStream::EncoderProcess() {
|
||||
current_encoder_settings_->video_codec.startBitrate = std::max(
|
||||
bitrate_allocator_->GetStartBitrate(this) / 1000,
|
||||
static_cast<int>(current_encoder_settings_->video_codec.minBitrate));
|
||||
|
||||
if (state_ == State::kStarted) {
|
||||
bitrate_allocator_->AddObserver(
|
||||
this, current_encoder_settings_->video_codec.minBitrate * 1000,
|
||||
current_encoder_settings_->video_codec.maxBitrate * 1000,
|
||||
CalulcateMaxPadBitrateBps(current_encoder_settings_->config,
|
||||
config_.suspend_below_min_bitrate),
|
||||
!config_.suspend_below_min_bitrate);
|
||||
}
|
||||
|
||||
payload_router_.SetSendStreams(current_encoder_settings_->config.streams);
|
||||
vie_encoder_.SetEncoder(current_encoder_settings_->video_codec,
|
||||
payload_router_.MaxPayloadLength());
|
||||
|
||||
@ -1121,6 +1121,97 @@ TEST_F(VideoSendStreamTest, MinTransmitBitrateRespectsRemb) {
|
||||
RunBaseTest(&test);
|
||||
}
|
||||
|
||||
class MaxPaddingSetTest : public test::SendTest {
|
||||
public:
|
||||
static const uint32_t kMinTransmitBitrateBps = 400000;
|
||||
static const uint32_t kActualEncodeBitrateBps = 40000;
|
||||
static const uint32_t kMinPacketsToSend = 50;
|
||||
|
||||
explicit MaxPaddingSetTest(bool test_switch_content_type)
|
||||
: SendTest(test::CallTest::kDefaultTimeoutMs),
|
||||
call_(nullptr),
|
||||
send_stream_(nullptr),
|
||||
packets_sent_(0),
|
||||
running_without_padding_(test_switch_content_type) {}
|
||||
|
||||
void OnVideoStreamsCreated(
|
||||
VideoSendStream* send_stream,
|
||||
const std::vector<VideoReceiveStream*>& receive_streams) override {
|
||||
send_stream_ = send_stream;
|
||||
}
|
||||
|
||||
void ModifyVideoConfigs(
|
||||
VideoSendStream::Config* send_config,
|
||||
std::vector<VideoReceiveStream::Config>* receive_configs,
|
||||
VideoEncoderConfig* encoder_config) override {
|
||||
RTC_DCHECK_EQ(1u, encoder_config->streams.size());
|
||||
if (running_without_padding_) {
|
||||
encoder_config->min_transmit_bitrate_bps = 0;
|
||||
encoder_config->content_type =
|
||||
VideoEncoderConfig::ContentType::kRealtimeVideo;
|
||||
} else {
|
||||
encoder_config->min_transmit_bitrate_bps = kMinTransmitBitrateBps;
|
||||
encoder_config->content_type = VideoEncoderConfig::ContentType::kScreen;
|
||||
}
|
||||
encoder_config_ = *encoder_config;
|
||||
}
|
||||
|
||||
void OnCallsCreated(Call* sender_call, Call* receiver_call) override {
|
||||
call_ = sender_call;
|
||||
}
|
||||
|
||||
Action OnSendRtp(const uint8_t* packet, size_t length) override {
|
||||
rtc::CritScope lock(&crit_);
|
||||
|
||||
if (running_without_padding_)
|
||||
EXPECT_EQ(0, call_->GetStats().max_padding_bitrate_bps);
|
||||
|
||||
// Wait until at least kMinPacketsToSend frames have been encoded, so that
|
||||
// we have reliable data.
|
||||
if (++packets_sent_ < kMinPacketsToSend)
|
||||
return SEND_PACKET;
|
||||
|
||||
if (running_without_padding_) {
|
||||
// We've sent kMinPacketsToSend packets with default configuration, switch
|
||||
// to enabling screen content and setting min transmit bitrate.
|
||||
packets_sent_ = 0;
|
||||
encoder_config_.min_transmit_bitrate_bps = kMinTransmitBitrateBps;
|
||||
encoder_config_.content_type = VideoEncoderConfig::ContentType::kScreen;
|
||||
send_stream_->ReconfigureVideoEncoder(encoder_config_);
|
||||
running_without_padding_ = false;
|
||||
return SEND_PACKET;
|
||||
}
|
||||
|
||||
// Make sure the pacer has been configured with a min transmit bitrate.
|
||||
if (call_->GetStats().max_padding_bitrate_bps > 0)
|
||||
observation_complete_.Set();
|
||||
|
||||
return SEND_PACKET;
|
||||
}
|
||||
|
||||
void PerformTest() override {
|
||||
ASSERT_TRUE(Wait()) << "Timed out waiting for a valid padding bitrate.";
|
||||
}
|
||||
|
||||
private:
|
||||
rtc::CriticalSection crit_;
|
||||
Call* call_;
|
||||
VideoSendStream* send_stream_;
|
||||
VideoEncoderConfig encoder_config_;
|
||||
uint32_t packets_sent_ GUARDED_BY(crit_);
|
||||
bool running_without_padding_;
|
||||
};
|
||||
|
||||
TEST_F(VideoSendStreamTest, RespectsMinTransmitBitrate) {
|
||||
MaxPaddingSetTest test(false);
|
||||
RunBaseTest(&test);
|
||||
}
|
||||
|
||||
TEST_F(VideoSendStreamTest, RespectsMinTransmitBitrateAfterContentSwitch) {
|
||||
MaxPaddingSetTest test(true);
|
||||
RunBaseTest(&test);
|
||||
}
|
||||
|
||||
TEST_F(VideoSendStreamTest, CanReconfigureToUseStartBitrateAbovePreviousMax) {
|
||||
class StartBitrateObserver : public test::FakeEncoder {
|
||||
public:
|
||||
|
||||
Reference in New Issue
Block a user