mirror of
https://github.com/mollyim/webrtc.git
synced 2025-05-13 13:50:40 +01:00
Reland "Cleanup of video packet overhead calculation."
This is a reland of 890bc3069c
Zero bitrate caused division by zero in DCHECK for max bitrate.
Added unit tests to ensure that setting zero bitrate does not crash.
> Original change's description:
> > Cleanup of video packet overhead calculation.
> >
> > This CL updates the video packet overhead calculation to make it more
> > clear. This prepares for future work on improving the accuracy of the
> > calculation.
> >
> > Bug: webrtc:9883
> > Change-Id: I1d623a3e0de45be7b6e4a1f9e3cbe54fd2b8a45a
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/138077
> > Commit-Queue: Sebastian Jansson <srte@webrtc.org>
> > Reviewed-by: Erik Språng <sprang@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#28040}
Bug: webrtc:10674
Change-Id: I156d1ee5546ede7e43ae1d9a298dcaba6071230f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/140890
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28212}
This commit is contained in:
parent
646e096e03
commit
cf41eb1ce1
3 changed files with 54 additions and 38 deletions
|
@ -126,11 +126,11 @@ inline Frequency operator/(const DataRate rate, const DataSize size) {
|
|||
size.bytes());
|
||||
}
|
||||
inline DataRate operator*(const DataSize size, const Frequency frequency) {
|
||||
int64_t millihertz = frequency.millihertz<int64_t>();
|
||||
int64_t kMaxBeforeConversion =
|
||||
std::numeric_limits<int64_t>::max() / 8 / millihertz;
|
||||
RTC_DCHECK_LE(size.bytes(), kMaxBeforeConversion);
|
||||
int64_t millibits_per_second = size.bytes() * 8 * millihertz;
|
||||
RTC_DCHECK(frequency.IsZero() ||
|
||||
size.bytes() <= std::numeric_limits<int64_t>::max() / 8 /
|
||||
frequency.millihertz<int64_t>());
|
||||
int64_t millibits_per_second =
|
||||
size.bytes() * 8 * frequency.millihertz<int64_t>();
|
||||
return DataRate::bps((millibits_per_second + 500) / 1000);
|
||||
}
|
||||
inline DataRate operator*(const Frequency frequency, const DataSize size) {
|
||||
|
|
|
@ -189,19 +189,13 @@ std::unique_ptr<FlexfecSender> MaybeCreateFlexfecSender(
|
|||
RTPSender::FecExtensionSizes(), rtp_state, clock);
|
||||
}
|
||||
|
||||
uint32_t CalculateOverheadRateBps(int packets_per_second,
|
||||
size_t overhead_bytes_per_packet,
|
||||
uint32_t max_overhead_bps) {
|
||||
uint32_t overhead_bps =
|
||||
static_cast<uint32_t>(8 * overhead_bytes_per_packet * packets_per_second);
|
||||
return std::min(overhead_bps, max_overhead_bps);
|
||||
}
|
||||
|
||||
int CalculatePacketRate(uint32_t bitrate_bps, size_t packet_size_bytes) {
|
||||
size_t packet_size_bits = 8 * packet_size_bytes;
|
||||
// Ceil for int value of bitrate_bps / packet_size_bits.
|
||||
return static_cast<int>((bitrate_bps + packet_size_bits - 1) /
|
||||
packet_size_bits);
|
||||
DataRate CalculateOverheadRate(DataRate data_rate,
|
||||
DataSize packet_size,
|
||||
DataSize overhead_per_packet) {
|
||||
Frequency packet_rate = data_rate / packet_size;
|
||||
// TOSO(srte): We should not need to round to nearest whole packet per second
|
||||
// rate here.
|
||||
return packet_rate.RoundUpTo(Frequency::hertz(1)) * overhead_per_packet;
|
||||
}
|
||||
} // namespace
|
||||
|
||||
|
@ -694,16 +688,17 @@ void RtpVideoSender::OnBitrateUpdated(uint32_t bitrate_bps,
|
|||
int framerate) {
|
||||
// Substract overhead from bitrate.
|
||||
rtc::CritScope lock(&crit_);
|
||||
DataSize packet_overhead = DataSize::bytes(
|
||||
overhead_bytes_per_packet_ + transport_overhead_bytes_per_packet_);
|
||||
DataSize max_total_packet_size = DataSize::bytes(
|
||||
rtp_config_.max_packet_size + transport_overhead_bytes_per_packet_);
|
||||
uint32_t payload_bitrate_bps = bitrate_bps;
|
||||
if (send_side_bwe_with_overhead_) {
|
||||
uint32_t overhead_bps = CalculateOverheadRateBps(
|
||||
CalculatePacketRate(
|
||||
bitrate_bps,
|
||||
rtp_config_.max_packet_size + transport_overhead_bytes_per_packet_),
|
||||
overhead_bytes_per_packet_ + transport_overhead_bytes_per_packet_,
|
||||
bitrate_bps);
|
||||
RTC_DCHECK_LE(overhead_bps, bitrate_bps);
|
||||
payload_bitrate_bps = bitrate_bps - overhead_bps;
|
||||
DataRate overhead_rate = CalculateOverheadRate(
|
||||
DataRate::bps(bitrate_bps), max_total_packet_size, packet_overhead);
|
||||
// TODO(srte): We probably should not accept 0 payload bitrate here.
|
||||
payload_bitrate_bps =
|
||||
rtc::saturated_cast<uint32_t>(bitrate_bps - overhead_rate.bps());
|
||||
}
|
||||
|
||||
// Get the encoder target rate. It is the estimated network rate -
|
||||
|
@ -724,18 +719,20 @@ void RtpVideoSender::OnBitrateUpdated(uint32_t bitrate_bps,
|
|||
|
||||
loss_mask_vector_.clear();
|
||||
|
||||
uint32_t encoder_overhead_rate_bps =
|
||||
send_side_bwe_with_overhead_
|
||||
? CalculateOverheadRateBps(
|
||||
CalculatePacketRate(encoder_target_rate_bps_,
|
||||
rtp_config_.max_packet_size +
|
||||
transport_overhead_bytes_per_packet_ -
|
||||
overhead_bytes_per_packet_),
|
||||
overhead_bytes_per_packet_ +
|
||||
transport_overhead_bytes_per_packet_,
|
||||
bitrate_bps - encoder_target_rate_bps_)
|
||||
: 0;
|
||||
|
||||
uint32_t encoder_overhead_rate_bps = 0;
|
||||
if (send_side_bwe_with_overhead_) {
|
||||
// TODO(srte): The packet size should probably be the same as in the
|
||||
// CalculateOverheadRate call above (just max_total_packet_size), it doesn't
|
||||
// make sense to use different packet rates for different overhead
|
||||
// calculations.
|
||||
DataRate encoder_overhead_rate = CalculateOverheadRate(
|
||||
DataRate::bps(encoder_target_rate_bps_),
|
||||
max_total_packet_size - DataSize::bytes(overhead_bytes_per_packet_),
|
||||
packet_overhead);
|
||||
encoder_overhead_rate_bps =
|
||||
std::min(encoder_overhead_rate.bps<uint32_t>(),
|
||||
bitrate_bps - encoder_target_rate_bps_);
|
||||
}
|
||||
// When the field trial "WebRTC-SendSideBwe-WithOverhead" is enabled
|
||||
// protection_bitrate includes overhead.
|
||||
const uint32_t media_rate = encoder_target_rate_bps_ +
|
||||
|
|
|
@ -631,4 +631,23 @@ TEST(RtpVideoSenderTest, EarlyRetransmits) {
|
|||
test.clock().AdvanceTimeMilliseconds(33);
|
||||
ASSERT_TRUE(event.Wait(kTimeoutMs));
|
||||
}
|
||||
|
||||
TEST(RtpVideoSenderTest, CanSetZeroBitrateWithOverhead) {
|
||||
test::ScopedFieldTrials trials("WebRTC-SendSideBwe-WithOverhead/Enabled/");
|
||||
RtpVideoSenderTestFixture test({kSsrc1}, {kRtxSsrc1}, kPayloadType, {});
|
||||
|
||||
test.router()->OnBitrateUpdated(/*bitrate_bps*/ 0,
|
||||
/*fraction_loss*/ 0,
|
||||
/*rtt*/ 0,
|
||||
/*framerate*/ 0);
|
||||
}
|
||||
|
||||
TEST(RtpVideoSenderTest, CanSetZeroBitrateWithoutOverhead) {
|
||||
RtpVideoSenderTestFixture test({kSsrc1}, {kRtxSsrc1}, kPayloadType, {});
|
||||
|
||||
test.router()->OnBitrateUpdated(/*bitrate_bps*/ 0,
|
||||
/*fraction_loss*/ 0,
|
||||
/*rtt*/ 0,
|
||||
/*framerate*/ 0);
|
||||
}
|
||||
} // namespace webrtc
|
||||
|
|
Loading…
Reference in a new issue