Replaces OverheadObserver with simple getter.
This interface has a couple of issues. Primarily for me, it makes it
difficult work with the paced sender as we need to either temporarily
release a lock or force a thread-handover in order to avoid a cyclic
lock order.
For video in particular, its behavior is also falky since header sizes
can vary not only form frame to frame, but from packet to packet within
a frame (e.g. TimingInfo extension is only on the last packet, if set).
On bitrate allocation, the last reported value is picked, leading to
timing issues affecting the bitrate set.
This CL removes the callback interface and instead we simply poll the
RTP module for a packet overhead. This consists of an expected overhead
based on which non-volatile header extensions are registered (so for
instance AbsoluteCaptureTime is disregarded since it's only populated
once per second). The overhead estimation is a little less accurate but
instead simpler and deterministic.
Bug: webrtc:10809
Change-Id: I2c3d3fcca6ad35704c4c1b6b9e0a39227aada1ea
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/173704
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Ali Tofigh <alito@webrtc.org>
Reviewed-by: Jakob Ivarsson <jakobi@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31185}
diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc
index 36010d8..8730c45 100644
--- a/audio/audio_send_stream.cc
+++ b/audio/audio_send_stream.cc
@@ -118,7 +118,6 @@
voe::CreateChannelSend(clock,
task_queue_factory,
module_process_thread,
- /*overhead_observer=*/this,
config.send_transport,
rtcp_rtt_stats,
event_log,
@@ -343,6 +342,12 @@
RTC_LOG(LS_ERROR) << "Failed to set up send codec state.";
}
+ // Set currently known overhead (used in ANA, opus only).
+ {
+ rtc::CritScope cs(&overhead_per_packet_lock_);
+ UpdateOverheadForEncoder();
+ }
+
channel_send_->CallEncoder([this](AudioEncoder* encoder) {
if (!encoder) {
return;
@@ -505,10 +510,17 @@
// thread. Then this check can be enabled.
// RTC_DCHECK(!worker_thread_checker_.IsCurrent());
channel_send_->ReceivedRTCPPacket(packet, length);
+ worker_queue_->PostTask([&]() {
+ // Poll if overhead has changed, which it can do if ack triggers us to stop
+ // sending mid/rid.
+ rtc::CritScope cs(&overhead_per_packet_lock_);
+ UpdateOverheadForEncoder();
+ });
}
uint32_t AudioSendStream::OnBitrateUpdated(BitrateAllocationUpdate update) {
RTC_DCHECK_RUN_ON(worker_queue_);
+
// Pick a target bitrate between the constraints. Overrules the allocator if
// it 1) allocated a bitrate of zero to disable the stream or 2) allocated a
// higher than max to allow for e.g. extra FEC.
@@ -531,13 +543,6 @@
UpdateOverheadForEncoder();
}
-void AudioSendStream::OnOverheadChanged(
- size_t overhead_bytes_per_packet_bytes) {
- rtc::CritScope cs(&overhead_per_packet_lock_);
- audio_overhead_per_packet_bytes_ = overhead_bytes_per_packet_bytes;
- UpdateOverheadForEncoder();
-}
-
void AudioSendStream::UpdateOverheadForEncoder() {
const size_t overhead_per_packet_bytes = GetPerPacketOverheadBytes();
if (overhead_per_packet_bytes == 0) {
@@ -546,7 +551,7 @@
channel_send_->CallEncoder([&](AudioEncoder* encoder) {
encoder->OnReceivedOverhead(overhead_per_packet_bytes);
});
- worker_queue_->PostTask([this, overhead_per_packet_bytes] {
+ auto update_task = [this, overhead_per_packet_bytes] {
RTC_DCHECK_RUN_ON(worker_queue_);
if (total_packet_overhead_bytes_ != overhead_per_packet_bytes) {
total_packet_overhead_bytes_ = overhead_per_packet_bytes;
@@ -554,7 +559,12 @@
ConfigureBitrateObserver();
}
}
- });
+ };
+ if (worker_queue_->IsCurrent()) {
+ update_task();
+ } else {
+ worker_queue_->PostTask(update_task);
+ }
}
size_t AudioSendStream::TestOnlyGetPerPacketOverheadBytes() const {
@@ -564,7 +574,7 @@
size_t AudioSendStream::GetPerPacketOverheadBytes() const {
return transport_overhead_per_packet_bytes_ +
- audio_overhead_per_packet_bytes_;
+ rtp_rtcp_module_->ExpectedPerPacketOverhead();
}
RtpState AudioSendStream::GetRtpState() const {
@@ -651,8 +661,9 @@
// If overhead changes later, it will be updated in UpdateOverheadForEncoder.
{
rtc::CritScope cs(&overhead_per_packet_lock_);
- if (GetPerPacketOverheadBytes() > 0) {
- encoder->OnReceivedOverhead(GetPerPacketOverheadBytes());
+ size_t overhead = GetPerPacketOverheadBytes();
+ if (overhead > 0) {
+ encoder->OnReceivedOverhead(overhead);
}
}
@@ -704,12 +715,6 @@
ReconfigureANA(new_config);
ReconfigureCNG(new_config);
- // Set currently known overhead (used in ANA, opus only).
- {
- rtc::CritScope cs(&overhead_per_packet_lock_);
- UpdateOverheadForEncoder();
- }
-
return true;
}
@@ -836,9 +841,9 @@
priority_bitrate += max_overhead;
} else {
RTC_DCHECK(frame_length_range_);
- const DataSize kOverheadPerPacket =
+ const DataSize overhead_per_packet =
DataSize::Bytes(total_packet_overhead_bytes_);
- DataRate min_overhead = kOverheadPerPacket / frame_length_range_->second;
+ DataRate min_overhead = overhead_per_packet / frame_length_range_->second;
priority_bitrate += min_overhead;
}
}
