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));