Remove packet overhead lock and cached bitrate constraints.

These are no longer needed since the RTP transport runs on the worker
thread now.

Some tests that were too strict on ordering needed change.

Bug: none
Change-Id: I4265cb1a4fd3355208f19aefdbb7abeb45b6cadf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/335700
Reviewed-by: Tomas Lundqvist <tomasl@google.com>
Commit-Queue: Jakob Ivarsson‎ <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41599}
This commit is contained in:
Jakob Ivarsson 2024-01-22 14:49:15 +01:00 committed by WebRTC LUCI CQ
parent eb76f193f3
commit 340d6c0236
3 changed files with 38 additions and 98 deletions

View file

@ -171,7 +171,6 @@ AudioSendStream::AudioSendStream(
RTC_DCHECK_RUN_ON(&worker_thread_checker_);
ConfigureStream(config, true, nullptr);
UpdateCachedTargetAudioBitrateConstraints();
}
AudioSendStream::~AudioSendStream() {
@ -324,10 +323,7 @@ void AudioSendStream::ConfigureStream(
}
// Set currently known overhead (used in ANA, opus only).
{
MutexLock lock(&overhead_per_packet_lock_);
UpdateOverheadForEncoder();
}
UpdateOverheadPerPacket();
channel_send_->CallEncoder([this](AudioEncoder* encoder) {
RTC_DCHECK_RUN_ON(&worker_thread_checker_);
@ -335,7 +331,6 @@ void AudioSendStream::ConfigureStream(
return;
}
frame_length_range_ = encoder->GetFrameLengthRange();
UpdateCachedTargetAudioBitrateConstraints();
});
if (sending_) {
@ -343,9 +338,6 @@ void AudioSendStream::ConfigureStream(
}
config_ = new_config;
if (!first_time) {
UpdateCachedTargetAudioBitrateConstraints();
}
webrtc::InvokeSetParametersCallback(callback, webrtc::RTCError::OK());
}
@ -488,30 +480,23 @@ webrtc::AudioSendStream::Stats AudioSendStream::GetStats(
void AudioSendStream::DeliverRtcp(const uint8_t* packet, size_t length) {
RTC_DCHECK_RUN_ON(&worker_thread_checker_);
channel_send_->ReceivedRTCPPacket(packet, length);
{
// Poll if overhead has changed, which it can do if ack triggers us to stop
// sending mid/rid.
MutexLock lock(&overhead_per_packet_lock_);
UpdateOverheadForEncoder();
}
UpdateCachedTargetAudioBitrateConstraints();
// Poll if overhead has changed, which it can do if ack triggers us to stop
// sending mid/rid.
UpdateOverheadPerPacket();
}
uint32_t AudioSendStream::OnBitrateUpdated(BitrateAllocationUpdate update) {
RTC_DCHECK_RUN_ON(&worker_thread_checker_);
// 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.
RTC_DCHECK(cached_constraints_.has_value());
update.target_bitrate.Clamp(cached_constraints_->min,
cached_constraints_->max);
update.stable_target_bitrate.Clamp(cached_constraints_->min,
cached_constraints_->max);
absl::optional<TargetAudioBitrateConstraints> constraints =
GetMinMaxBitrateConstraints();
if (constraints) {
update.target_bitrate.Clamp(constraints->min, constraints->max);
update.stable_target_bitrate.Clamp(constraints->min, constraints->max);
}
channel_send_->OnBitrateAllocation(update);
// The amount of audio protection is not exposed by the encoder, hence
// always returning 0.
return 0;
@ -520,41 +505,30 @@ uint32_t AudioSendStream::OnBitrateUpdated(BitrateAllocationUpdate update) {
void AudioSendStream::SetTransportOverhead(
int transport_overhead_per_packet_bytes) {
RTC_DCHECK_RUN_ON(&worker_thread_checker_);
{
MutexLock lock(&overhead_per_packet_lock_);
transport_overhead_per_packet_bytes_ = transport_overhead_per_packet_bytes;
UpdateOverheadForEncoder();
}
UpdateCachedTargetAudioBitrateConstraints();
transport_overhead_per_packet_bytes_ = transport_overhead_per_packet_bytes;
UpdateOverheadPerPacket();
}
void AudioSendStream::UpdateOverheadForEncoder() {
void AudioSendStream::UpdateOverheadPerPacket() {
RTC_DCHECK_RUN_ON(&worker_thread_checker_);
size_t overhead_per_packet_bytes = GetPerPacketOverheadBytes();
size_t overhead_per_packet_bytes =
transport_overhead_per_packet_bytes_ +
rtp_rtcp_module_->ExpectedPerPacketOverhead();
if (overhead_per_packet_ == overhead_per_packet_bytes) {
return;
}
overhead_per_packet_ = overhead_per_packet_bytes;
channel_send_->CallEncoder([&](AudioEncoder* encoder) {
encoder->OnReceivedOverhead(overhead_per_packet_bytes);
});
if (total_packet_overhead_bytes_ != overhead_per_packet_bytes) {
total_packet_overhead_bytes_ = overhead_per_packet_bytes;
if (registered_with_allocator_) {
ConfigureBitrateObserver();
}
if (registered_with_allocator_) {
ConfigureBitrateObserver();
}
}
size_t AudioSendStream::TestOnlyGetPerPacketOverheadBytes() const {
MutexLock lock(&overhead_per_packet_lock_);
return GetPerPacketOverheadBytes();
}
size_t AudioSendStream::GetPerPacketOverheadBytes() const {
return transport_overhead_per_packet_bytes_ +
rtp_rtcp_module_->ExpectedPerPacketOverhead();
RTC_DCHECK_RUN_ON(&worker_thread_checker_);
return overhead_per_packet_;
}
RtpState AudioSendStream::GetRtpState() const {
@ -648,13 +622,9 @@ bool AudioSendStream::SetupSendCodec(const Config& new_config) {
}
// Set currently known overhead (used in ANA, opus only).
// If overhead changes later, it will be updated in UpdateOverheadForEncoder.
{
MutexLock lock(&overhead_per_packet_lock_);
size_t overhead = GetPerPacketOverheadBytes();
if (overhead > 0) {
encoder->OnReceivedOverhead(overhead);
}
// If overhead changes later, it will be updated in UpdateOverheadPerPacket.
if (overhead_per_packet_ > 0) {
encoder->OnReceivedOverhead(overhead_per_packet_);
}
StoreEncoderProperties(encoder->SampleRateHz(), encoder->NumChannels());
@ -716,18 +686,14 @@ void AudioSendStream::ReconfigureANA(const Config& new_config) {
return;
}
if (new_config.audio_network_adaptor_config) {
// This lock needs to be acquired before CallEncoder, since it aquires
// another lock and we need to maintain the same order at all call sites to
// avoid deadlock.
MutexLock lock(&overhead_per_packet_lock_);
size_t overhead = GetPerPacketOverheadBytes();
channel_send_->CallEncoder([&](AudioEncoder* encoder) {
RTC_DCHECK_RUN_ON(&worker_thread_checker_);
if (encoder->EnableAudioNetworkAdaptor(
*new_config.audio_network_adaptor_config, event_log_)) {
RTC_LOG(LS_INFO) << "Audio network adaptor enabled on SSRC "
<< new_config.rtp.ssrc;
if (overhead > 0) {
encoder->OnReceivedOverhead(overhead);
if (overhead_per_packet_ > 0) {
encoder->OnReceivedOverhead(overhead_per_packet_);
}
} else {
RTC_LOG(LS_INFO) << "Failed to enable Audio network adaptor on SSRC "
@ -832,8 +798,7 @@ void AudioSendStream::ConfigureBitrateObserver() {
priority_bitrate += max_overhead;
} else {
RTC_DCHECK(frame_length_range_);
const DataSize overhead_per_packet =
DataSize::Bytes(total_packet_overhead_bytes_);
const DataSize overhead_per_packet = DataSize::Bytes(overhead_per_packet_);
DataRate min_overhead = overhead_per_packet / frame_length_range_->second;
priority_bitrate += min_overhead;
}
@ -897,10 +862,9 @@ AudioSendStream::GetMinMaxBitrateConstraints() const {
RTC_LOG(LS_WARNING) << "frame_length_range_ is not set";
return absl::nullopt;
}
const DataSize kOverheadPerPacket =
DataSize::Bytes(total_packet_overhead_bytes_);
constraints.min += kOverheadPerPacket / frame_length_range_->second;
constraints.max += kOverheadPerPacket / frame_length_range_->first;
const DataSize overhead_per_packet = DataSize::Bytes(overhead_per_packet_);
constraints.min += overhead_per_packet / frame_length_range_->second;
constraints.max += overhead_per_packet / frame_length_range_->first;
}
return constraints;
}
@ -910,14 +874,5 @@ void AudioSendStream::RegisterCngPayloadType(int payload_type,
channel_send_->RegisterCngPayloadType(payload_type, clockrate_hz);
}
void AudioSendStream::UpdateCachedTargetAudioBitrateConstraints() {
absl::optional<AudioSendStream::TargetAudioBitrateConstraints>
new_constraints = GetMinMaxBitrateConstraints();
if (!new_constraints.has_value()) {
return;
}
cached_constraints_ = new_constraints;
}
} // namespace internal
} // namespace webrtc

View file

@ -110,8 +110,7 @@ class AudioSendStream final : public webrtc::AudioSendStream,
const voe::ChannelSendInterface* GetChannel() const;
// Returns combined per-packet overhead.
size_t TestOnlyGetPerPacketOverheadBytes() const
RTC_LOCKS_EXCLUDED(overhead_per_packet_lock_);
size_t TestOnlyGetPerPacketOverheadBytes() const;
private:
class TimedTransport;
@ -152,19 +151,11 @@ class AudioSendStream final : public webrtc::AudioSendStream,
// Sets per-packet overhead on encoded (for ANA) based on current known values
// of transport and packetization overheads.
void UpdateOverheadForEncoder()
RTC_EXCLUSIVE_LOCKS_REQUIRED(overhead_per_packet_lock_);
// Returns combined per-packet overhead.
size_t GetPerPacketOverheadBytes() const
RTC_EXCLUSIVE_LOCKS_REQUIRED(overhead_per_packet_lock_);
void UpdateOverheadPerPacket();
void RegisterCngPayloadType(int payload_type, int clockrate_hz)
RTC_RUN_ON(worker_thread_checker_);
void UpdateCachedTargetAudioBitrateConstraints()
RTC_RUN_ON(worker_thread_checker_);
Clock* clock_;
const FieldTrialsView& field_trials_;
@ -193,9 +184,6 @@ class AudioSendStream final : public webrtc::AudioSendStream,
BitrateAllocatorInterface* const bitrate_allocator_
RTC_GUARDED_BY(worker_thread_checker_);
absl::optional<AudioSendStream::TargetAudioBitrateConstraints>
cached_constraints_ RTC_GUARDED_BY(worker_thread_checker_) =
absl::nullopt;
RtpTransportControllerSendInterface* const rtp_transport_;
RtpRtcpInterface* const rtp_rtcp_module_;
@ -217,17 +205,14 @@ class AudioSendStream final : public webrtc::AudioSendStream,
const std::vector<RtpExtension>& extensions);
static int TransportSeqNumId(const Config& config);
mutable Mutex overhead_per_packet_lock_;
size_t overhead_per_packet_ RTC_GUARDED_BY(overhead_per_packet_lock_) = 0;
// Current transport overhead (ICE, TURN, etc.)
size_t transport_overhead_per_packet_bytes_
RTC_GUARDED_BY(overhead_per_packet_lock_) = 0;
RTC_GUARDED_BY(worker_thread_checker_) = 0;
// Total overhead, including transport and RTP headers.
size_t overhead_per_packet_ RTC_GUARDED_BY(worker_thread_checker_) = 0;
bool registered_with_allocator_ RTC_GUARDED_BY(worker_thread_checker_) =
false;
size_t total_packet_overhead_bytes_ RTC_GUARDED_BY(worker_thread_checker_) =
0;
absl::optional<std::pair<TimeDelta, TimeDelta>> frame_length_range_
RTC_GUARDED_BY(worker_thread_checker_);
};

View file

@ -560,8 +560,7 @@ TEST(AudioSendStreamTest, AudioNetworkAdaptorReceivesOverhead) {
InSequence s;
EXPECT_CALL(
*mock_encoder,
OnReceivedOverhead(Eq(kOverheadPerPacket.bytes<size_t>())))
.Times(2);
OnReceivedOverhead(Eq(kOverheadPerPacket.bytes<size_t>())));
EXPECT_CALL(*mock_encoder,
EnableAudioNetworkAdaptor(StrEq(kAnaConfigString), _))
.WillOnce(Return(true));
@ -847,7 +846,6 @@ TEST(AudioSendStreamTest, AudioOverheadChanged) {
EXPECT_CALL(*helper.rtp_rtcp(), ExpectedPerPacketOverhead)
.WillRepeatedly(Return(audio_overhead_per_packet_bytes));
auto send_stream = helper.CreateAudioSendStream();
auto new_config = helper.config();
BitrateAllocationUpdate update;
update.target_bitrate =
@ -861,6 +859,8 @@ TEST(AudioSendStreamTest, AudioOverheadChanged) {
EXPECT_CALL(*helper.rtp_rtcp(), ExpectedPerPacketOverhead)
.WillRepeatedly(Return(audio_overhead_per_packet_bytes + 20));
// RTP overhead can only change in response to RTCP or configuration change.
send_stream->Reconfigure(helper.config(), nullptr);
EXPECT_CALL(*helper.channel_send(), OnBitrateAllocation);
send_stream->OnBitrateUpdated(update);