From 62aee9379c03bf0573d2549bc75571784522e736 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Wed, 2 Oct 2019 12:27:06 +0200 Subject: [PATCH] Adds trial to calculate audio overhead based on available data. This adds the ability to disable legacy overhead calculation so we'll use the available data on per packet over head and frame length range to set the min and max total allocatable bitrate. Bug: webrtc:11001 Change-Id: I2a94499433e15bad11a08f81fe7f1dfc27982cdf Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/155175 Reviewed-by: Oskar Sundbom Commit-Queue: Sebastian Jansson Cr-Commit-Position: refs/heads/master@{#29368} --- api/audio_codecs/BUILD.gn | 1 + api/audio_codecs/audio_encoder.cc | 5 +++ api/audio_codecs/audio_encoder.h | 8 ++++ audio/audio_send_stream.cc | 39 +++++++++++------ audio/audio_send_stream.h | 7 ++- audio/audio_send_stream_unittest.cc | 43 +++++++++++++------ .../codecs/opus/audio_encoder_opus.cc | 13 ++++++ .../codecs/opus/audio_encoder_opus.h | 2 + test/mock_audio_encoder.h | 3 ++ 9 files changed, 93 insertions(+), 28 deletions(-) diff --git a/api/audio_codecs/BUILD.gn b/api/audio_codecs/BUILD.gn index 80e2534374..65da28fdb8 100644 --- a/api/audio_codecs/BUILD.gn +++ b/api/audio_codecs/BUILD.gn @@ -37,6 +37,7 @@ rtc_source_set("audio_codecs_api") { "../../rtc_base:rtc_base_approved", "../../rtc_base:sanitizer", "../../rtc_base/system:rtc_export", + "../units:time_delta", "//third_party/abseil-cpp/absl/strings", "//third_party/abseil-cpp/absl/types:optional", ] diff --git a/api/audio_codecs/audio_encoder.cc b/api/audio_codecs/audio_encoder.cc index 1d885f9cec..ae251333e2 100644 --- a/api/audio_codecs/audio_encoder.cc +++ b/api/audio_codecs/audio_encoder.cc @@ -108,4 +108,9 @@ ANAStats AudioEncoder::GetANAStats() const { return ANAStats(); } +absl::optional> +AudioEncoder::GetFrameLengthRange() const { + return absl::nullopt; +} + } // namespace webrtc diff --git a/api/audio_codecs/audio_encoder.h b/api/audio_codecs/audio_encoder.h index c908518063..c6efa47ffc 100644 --- a/api/audio_codecs/audio_encoder.h +++ b/api/audio_codecs/audio_encoder.h @@ -13,11 +13,13 @@ #include #include +#include #include #include "absl/types/optional.h" #include "api/array_view.h" #include "api/call/bitrate_allocation.h" +#include "api/units/time_delta.h" #include "rtc_base/buffer.h" #include "rtc_base/deprecation.h" @@ -241,6 +243,12 @@ class AudioEncoder { // Get statistics related to audio network adaptation. virtual ANAStats GetANAStats() const; + // The range of frame lengths that are supported or nullopt if there's no sch + // information. This is used to calculated the full bitrate range, including + // overhead. + virtual absl::optional> GetFrameLengthRange() + const; + protected: // Subclasses implement this to perform the actual encoding. Called by // Encode(). diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc index c6fe7785f6..1ec2766617 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc @@ -133,6 +133,8 @@ AudioSendStream::AudioSendStream( audio_state_(audio_state), channel_send_(std::move(channel_send)), event_log_(event_log), + use_legacy_overhead_calculation_( + !field_trial::IsDisabled("WebRTC-Audio-LegacyOverhead")), bitrate_allocator_(bitrate_allocator), rtp_transport_(rtp_transport), packet_loss_tracker_(kPacketLossTrackerMaxWindowSizeMs, @@ -481,6 +483,7 @@ void AudioSendStream::DeliverRtcp(const uint8_t* packet, size_t length) { } 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. @@ -666,6 +669,11 @@ bool AudioSendStream::SetupSendCodec(AudioSendStream* stream, encoder->OnReceivedOverhead(stream->GetPerPacketOverheadBytes()); } } + stream->worker_queue_->PostTask( + [stream, length_range = encoder->GetFrameLengthRange()] { + RTC_DCHECK_RUN_ON(stream->worker_queue_); + stream->frame_length_range_ = length_range; + }); stream->StoreEncoderProperties(encoder->SampleRateHz(), encoder->NumChannels()); @@ -872,20 +880,25 @@ AudioSendStream::GetMinMaxBitrateConstraints() const { if (allocation_settings_.MaxBitrate()) constraints.max = *allocation_settings_.MaxBitrate(); - RTC_DCHECK_GE(constraints.min.bps(), 0); - RTC_DCHECK_GE(constraints.max.bps(), 0); - RTC_DCHECK_GE(constraints.max.bps(), constraints.min.bps()); - - // TODO(srte,dklee): Replace these with proper overhead calculations. + RTC_DCHECK_GE(constraints.min, DataRate::Zero()); + RTC_DCHECK_GE(constraints.max, DataRate::Zero()); + RTC_DCHECK_GE(constraints.max, constraints.min); if (allocation_settings_.IncludeOverheadInAudioAllocation()) { - // OverheadPerPacket = Ipv4(20B) + UDP(8B) + SRTP(10B) + RTP(12) - const DataSize kOverheadPerPacket = DataSize::bytes(20 + 8 + 10 + 12); - const TimeDelta kMaxFrameLength = TimeDelta::ms(60); // Based on Opus spec - const DataRate kMinOverhead = kOverheadPerPacket / kMaxFrameLength; - constraints.min += kMinOverhead; - // TODO(dklee): This is obviously overly conservative to avoid exceeding max - // bitrate. Carefully reconsider the logic when addressing todo above. - constraints.max += kMinOverhead; + if (use_legacy_overhead_calculation_) { + // OverheadPerPacket = Ipv4(20B) + UDP(8B) + SRTP(10B) + RTP(12) + const DataSize kOverheadPerPacket = DataSize::bytes(20 + 8 + 10 + 12); + const TimeDelta kMaxFrameLength = + TimeDelta::ms(60); // Based on Opus spec + const DataRate kMinOverhead = kOverheadPerPacket / kMaxFrameLength; + constraints.min += kMinOverhead; + constraints.max += kMinOverhead; + } else { + RTC_DCHECK(frame_length_range_); + const DataSize kOverheadPerPacket = + DataSize::bytes(total_packet_overhead_bytes_); + constraints.min += kOverheadPerPacket / frame_length_range_->second; + constraints.max += kOverheadPerPacket / frame_length_range_->first; + } } return constraints; } diff --git a/audio/audio_send_stream.h b/audio/audio_send_stream.h index e063849f1a..80b508717f 100644 --- a/audio/audio_send_stream.h +++ b/audio/audio_send_stream.h @@ -12,6 +12,7 @@ #define AUDIO_AUDIO_SEND_STREAM_H_ #include +#include #include #include "audio/audio_level.h" @@ -133,7 +134,8 @@ class AudioSendStream final : public webrtc::AudioSendStream, // Returns bitrate constraints, maybe including overhead when enabled by // field trial. - TargetAudioBitrateConstraints GetMinMaxBitrateConstraints() const; + TargetAudioBitrateConstraints GetMinMaxBitrateConstraints() const + RTC_RUN_ON(worker_queue_); // Sets per-packet overhead on encoded (for ANA) based on current known values // of transport and packetization overheads. @@ -157,6 +159,7 @@ class AudioSendStream final : public webrtc::AudioSendStream, rtc::scoped_refptr audio_state_; const std::unique_ptr channel_send_; RtcEventLog* const event_log_; + const bool use_legacy_overhead_calculation_; int encoder_sample_rate_hz_ = 0; size_t encoder_num_channels_ = 0; @@ -204,6 +207,8 @@ class AudioSendStream final : public webrtc::AudioSendStream, bool registered_with_allocator_ RTC_GUARDED_BY(worker_queue_) = false; size_t total_packet_overhead_bytes_ RTC_GUARDED_BY(worker_queue_) = 0; + absl::optional> frame_length_range_ + RTC_GUARDED_BY(worker_queue_); RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(AudioSendStream); }; diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc index 948fcfed72..d787a8adbd 100644 --- a/audio/audio_send_stream_unittest.cc +++ b/audio/audio_send_stream_unittest.cc @@ -83,8 +83,10 @@ const AudioCodecSpec kCodecSpecs[] = { // should be made more precise in the future. This can be changed when that // logic is more accurate. const DataSize kOverheadPerPacket = DataSize::bytes(20 + 8 + 10 + 12); -const TimeDelta kMaxFrameLength = TimeDelta::ms(60); -const DataRate kOverheadRate = kOverheadPerPacket / kMaxFrameLength; +const TimeDelta kMinFrameLength = TimeDelta::ms(20); +const TimeDelta kMaxFrameLength = TimeDelta::ms(120); +const DataRate kMinOverheadRate = kOverheadPerPacket / kMaxFrameLength; +const DataRate kMaxOverheadRate = kOverheadPerPacket / kMinFrameLength; class MockLimitObserver : public BitrateAllocator::LimitObserver { public: @@ -104,6 +106,9 @@ std::unique_ptr SetupAudioEncoderMock( .WillByDefault(Return(spec.info.num_channels)); ON_CALL(*encoder.get(), RtpTimestampRateHz()) .WillByDefault(Return(spec.format.clockrate_hz)); + ON_CALL(*encoder.get(), GetFrameLengthRange()) + .WillByDefault(Return(absl::optional>{ + {TimeDelta::ms(20), TimeDelta::ms(120)}})); return encoder; } } @@ -299,6 +304,7 @@ struct ConfigHelper { EXPECT_CALL(*audio_processing_, GetStatistics(true)) .WillRepeatedly(Return(audio_processing_stats_)); } + TaskQueueForTest* worker() { return &worker_queue_; } private: SimulatedClock clock_; @@ -546,7 +552,7 @@ TEST(AudioSendStreamTest, DoesNotPassHigherBitrateThanMaxBitrate) { update.packet_loss_ratio = 0; update.round_trip_time = TimeDelta::ms(50); update.bwe_period = TimeDelta::ms(6000); - send_stream->OnBitrateUpdated(update); + helper.worker()->SendTask([&] { send_stream->OnBitrateUpdated(update); }); } TEST(AudioSendStreamTest, SSBweTargetInRangeRespected) { @@ -559,7 +565,7 @@ TEST(AudioSendStreamTest, SSBweTargetInRangeRespected) { Eq(DataRate::bps(helper.config().max_bitrate_bps - 5000))))); BitrateAllocationUpdate update; update.target_bitrate = DataRate::bps(helper.config().max_bitrate_bps - 5000); - send_stream->OnBitrateUpdated(update); + helper.worker()->SendTask([&] { send_stream->OnBitrateUpdated(update); }); } TEST(AudioSendStreamTest, SSBweFieldTrialMinRespected) { @@ -574,7 +580,7 @@ TEST(AudioSendStreamTest, SSBweFieldTrialMinRespected) { Eq(DataRate::kbps(6))))); BitrateAllocationUpdate update; update.target_bitrate = DataRate::kbps(1); - send_stream->OnBitrateUpdated(update); + helper.worker()->SendTask([&] { send_stream->OnBitrateUpdated(update); }); } TEST(AudioSendStreamTest, SSBweFieldTrialMaxRespected) { @@ -589,55 +595,64 @@ TEST(AudioSendStreamTest, SSBweFieldTrialMaxRespected) { Eq(DataRate::kbps(64))))); BitrateAllocationUpdate update; update.target_bitrate = DataRate::kbps(128); - send_stream->OnBitrateUpdated(update); + helper.worker()->SendTask([&] { send_stream->OnBitrateUpdated(update); }); } TEST(AudioSendStreamTest, SSBweWithOverhead) { ScopedFieldTrials field_trials( "WebRTC-Audio-SendSideBwe/Enabled/" - "WebRTC-SendSideBwe-WithOverhead/Enabled/"); + "WebRTC-SendSideBwe-WithOverhead/Enabled/" + "WebRTC-Audio-LegacyOverhead/Disabled/"); ConfigHelper helper(true, true); auto send_stream = helper.CreateAudioSendStream(); + EXPECT_CALL(*helper.channel_send(), CallEncoder(_)).Times(1); + send_stream->OnOverheadChanged(kOverheadPerPacket.bytes()); const DataRate bitrate = - DataRate::bps(helper.config().max_bitrate_bps) + kOverheadRate; + DataRate::bps(helper.config().max_bitrate_bps) + kMaxOverheadRate; EXPECT_CALL(*helper.channel_send(), OnBitrateAllocation(Field( &BitrateAllocationUpdate::target_bitrate, Eq(bitrate)))); BitrateAllocationUpdate update; update.target_bitrate = bitrate; - send_stream->OnBitrateUpdated(update); + helper.worker()->SendTask([&] { send_stream->OnBitrateUpdated(update); }); } TEST(AudioSendStreamTest, SSBweWithOverheadMinRespected) { ScopedFieldTrials field_trials( "WebRTC-Audio-SendSideBwe/Enabled/" "WebRTC-SendSideBwe-WithOverhead/Enabled/" + "WebRTC-Audio-LegacyOverhead/Disabled/" "WebRTC-Audio-Allocation/min:6kbps,max:64kbps/"); ConfigHelper helper(true, true); auto send_stream = helper.CreateAudioSendStream(); - const DataRate bitrate = DataRate::kbps(6) + kOverheadRate; + EXPECT_CALL(*helper.channel_send(), CallEncoder(_)).Times(1); + send_stream->OnOverheadChanged(kOverheadPerPacket.bytes()); + const DataRate bitrate = DataRate::kbps(6) + kMinOverheadRate; EXPECT_CALL(*helper.channel_send(), OnBitrateAllocation(Field( &BitrateAllocationUpdate::target_bitrate, Eq(bitrate)))); BitrateAllocationUpdate update; update.target_bitrate = DataRate::kbps(1); - send_stream->OnBitrateUpdated(update); + helper.worker()->SendTask([&] { send_stream->OnBitrateUpdated(update); }); } TEST(AudioSendStreamTest, SSBweWithOverheadMaxRespected) { ScopedFieldTrials field_trials( "WebRTC-Audio-SendSideBwe/Enabled/" "WebRTC-SendSideBwe-WithOverhead/Enabled/" + "WebRTC-Audio-LegacyOverhead/Disabled/" "WebRTC-Audio-Allocation/min:6kbps,max:64kbps/"); ConfigHelper helper(true, true); auto send_stream = helper.CreateAudioSendStream(); - const DataRate bitrate = DataRate::kbps(64) + kOverheadRate; + EXPECT_CALL(*helper.channel_send(), CallEncoder(_)).Times(1); + send_stream->OnOverheadChanged(kOverheadPerPacket.bytes()); + const DataRate bitrate = DataRate::kbps(64) + kMaxOverheadRate; EXPECT_CALL(*helper.channel_send(), OnBitrateAllocation(Field( &BitrateAllocationUpdate::target_bitrate, Eq(bitrate)))); BitrateAllocationUpdate update; update.target_bitrate = DataRate::kbps(128); - send_stream->OnBitrateUpdated(update); + helper.worker()->SendTask([&] { send_stream->OnBitrateUpdated(update); }); } TEST(AudioSendStreamTest, ProbingIntervalOnBitrateUpdated) { @@ -652,7 +667,7 @@ TEST(AudioSendStreamTest, ProbingIntervalOnBitrateUpdated) { update.packet_loss_ratio = 0; update.round_trip_time = TimeDelta::ms(50); update.bwe_period = TimeDelta::ms(5000); - send_stream->OnBitrateUpdated(update); + helper.worker()->SendTask([&] { send_stream->OnBitrateUpdated(update); }); } // Test that AudioSendStream doesn't recreate the encoder unnecessarily. diff --git a/modules/audio_coding/codecs/opus/audio_encoder_opus.cc b/modules/audio_coding/codecs/opus/audio_encoder_opus.cc index 60af6075aa..831bc23b92 100644 --- a/modules/audio_coding/codecs/opus/audio_encoder_opus.cc +++ b/modules/audio_coding/codecs/opus/audio_encoder_opus.cc @@ -879,4 +879,17 @@ ANAStats AudioEncoderOpusImpl::GetANAStats() const { return ANAStats(); } +absl::optional > +AudioEncoderOpusImpl::GetFrameLengthRange() const { + if (config_.supported_frame_lengths_ms.empty()) { + return absl::nullopt; + } else if (audio_network_adaptor_) { + return {{TimeDelta::ms(config_.supported_frame_lengths_ms.front()), + TimeDelta::ms(config_.supported_frame_lengths_ms.back())}}; + } else { + return {{TimeDelta::ms(config_.frame_size_ms), + TimeDelta::ms(config_.frame_size_ms)}}; + } +} + } // namespace webrtc diff --git a/modules/audio_coding/codecs/opus/audio_encoder_opus.h b/modules/audio_coding/codecs/opus/audio_encoder_opus.h index 1f785a446e..de2d956cb0 100644 --- a/modules/audio_coding/codecs/opus/audio_encoder_opus.h +++ b/modules/audio_coding/codecs/opus/audio_encoder_opus.h @@ -115,6 +115,8 @@ class AudioEncoderOpusImpl final : public AudioEncoder { void SetReceiverFrameLengthRange(int min_frame_length_ms, int max_frame_length_ms) override; ANAStats GetANAStats() const override; + absl::optional > GetFrameLengthRange() + const override; rtc::ArrayView supported_frame_lengths_ms() const { return config_.supported_frame_lengths_ms; } diff --git a/test/mock_audio_encoder.h b/test/mock_audio_encoder.h index 60425e0613..2dfd15ca98 100644 --- a/test/mock_audio_encoder.h +++ b/test/mock_audio_encoder.h @@ -34,6 +34,9 @@ class MockAudioEncoder : public AudioEncoder { MOCK_CONST_METHOD0(Num10MsFramesInNextPacket, size_t()); MOCK_CONST_METHOD0(Max10MsFramesInAPacket, size_t()); MOCK_CONST_METHOD0(GetTargetBitrate, int()); + MOCK_CONST_METHOD0(GetFrameLengthRange, + absl::optional>()); + MOCK_METHOD0(Reset, void()); MOCK_METHOD1(SetFec, bool(bool enable)); MOCK_METHOD1(SetDtx, bool(bool enable));