Invoke Audio Injector callback on ProxyAudioEncoder::OnReceivedTargetAudioBitrate calls. Remove empty override of the OnReceivedTargetAudioBitrate method overload in ProxyAudioEncoder and handle invocation from multiple threads. Bug: chromium:532190488 Change-Id: I28409904b57f7341120a9e37c2fd0a5ec948dcee Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/501580 Commit-Queue: Henrik Boström <hbos@webrtc.org> Reviewed-by: Henrik Boström <hbos@webrtc.org> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org> Cr-Commit-Position: refs/heads/main@{#48558}
diff --git a/api/encoded_audio_frame_injector_interface.h b/api/encoded_audio_frame_injector_interface.h index 42ea805..16264df 100644 --- a/api/encoded_audio_frame_injector_interface.h +++ b/api/encoded_audio_frame_injector_interface.h
@@ -21,7 +21,8 @@ namespace webrtc { // Callback invoked by webrtc to notify of an update to target bitrate. -using TargetBitrateCallback = absl::AnyInvocable<void(int32_t target_bitrate)>; +using TargetBitrateCallback = + absl::AnyInvocable<void(int32_t target_bitrate) const>; // Interface that allows injecting encoded audio frames on a sender. class EncodedAudioFrameInjectorInterface : public RefCountInterface {
diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 785508e..2515d40 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn
@@ -3562,6 +3562,7 @@ ":video_rtp_receiver", ":video_track", "../api:audio_options_api", + "../api:bitrate_allocation", "../api:dtmf_sender_interface", "../api:fake_frame_decryptor", "../api:fake_frame_encryptor", @@ -3585,6 +3586,7 @@ "../api/crypto:options", "../api/environment", "../api/task_queue", + "../api/units:data_rate", "../api/units:timestamp", "../api/video:builtin_video_bitrate_allocator_factory", "../api/video:encoded_image",
diff --git a/pc/encoded_audio_frame_injector.cc b/pc/encoded_audio_frame_injector.cc index fbfe80f..c5f768e 100644 --- a/pc/encoded_audio_frame_injector.cc +++ b/pc/encoded_audio_frame_injector.cc
@@ -193,14 +193,14 @@ return std::make_pair(TimeDelta::Millis(20), TimeDelta::Millis(20)); } + // Called on any thread void OnReceivedUplinkAllocation(BitrateAllocationUpdate update) override { - RTC_DCHECK_RUN_ON(&worker_sequence_checker_); injector_->InvokeBitrateInfoCallback(update.target_bitrate.bps()); } - // empty override to avoid default implementation of the method to invoke - // OnReceivedUplinkAllocation on the wrong thread. - void OnReceivedTargetAudioBitrate(int target_bps) override {} + void OnReceivedTargetAudioBitrate(int target_bps) override { + injector_->InvokeBitrateInfoCallback(target_bps); + } EncodedInfo EncodeImpl(uint32_t rtp_timestamp, std::span<const int16_t> audio, @@ -321,9 +321,9 @@ scoped_refptr<EncodedAudioFrameInjector>(this))); } +// Called on any thread void EncodedAudioFrameInjector::InvokeBitrateInfoCallback( int32_t allocated_bitrate) { - RTC_DCHECK_RUN_ON(&encoder_sequence_checker_); if (bitrate_callback_) { bitrate_callback_(allocated_bitrate); }
diff --git a/pc/encoded_audio_frame_injector.h b/pc/encoded_audio_frame_injector.h index e66744a..4d38686 100644 --- a/pc/encoded_audio_frame_injector.h +++ b/pc/encoded_audio_frame_injector.h
@@ -69,7 +69,7 @@ RTC_NO_UNIQUE_ADDRESS SequenceChecker encoder_sequence_checker_{ SequenceChecker::kDetached}; - TargetBitrateCallback bitrate_callback_; + const TargetBitrateCallback bitrate_callback_; }; } // namespace webrtc
diff --git a/pc/rtp_sender_receiver_unittest.cc b/pc/rtp_sender_receiver_unittest.cc index d8192f4..9070204 100644 --- a/pc/rtp_sender_receiver_unittest.cc +++ b/pc/rtp_sender_receiver_unittest.cc
@@ -8,6 +8,7 @@ * be found in the AUTHORS file in the root of the source tree. */ +#include <atomic> #include <cstddef> #include <cstdint> #include <iterator> @@ -21,7 +22,9 @@ #include "absl/algorithm/container.h" #include "absl/functional/any_invocable.h" #include "api/audio_codecs/audio_encoder_factory.h" +#include "api/audio_codecs/audio_format.h" #include "api/audio_options.h" +#include "api/call/bitrate_allocation.h" #include "api/crypto/crypto_options.h" #include "api/crypto/frame_decryptor_interface.h" #include "api/crypto/frame_encryptor_interface.h" @@ -44,6 +47,7 @@ #include "api/test/fake_frame_encryptor.h" #include "api/test/mock_transformable_video_frame.h" #include "api/test/rtc_error_matchers.h" +#include "api/units/data_rate.h" #include "api/units/timestamp.h" #include "api/video/builtin_video_bitrate_allocator_factory.h" #include "api/video/encoded_image.h" @@ -1532,6 +1536,61 @@ DestroyAudioRtpSender(); } +TEST_F(RtpSenderReceiverTest, ProxyAudioEncoderInvokesBitrateCallback) { + CreateAudioRtpSenderWithNoTrack(); + + std::atomic<int32_t> last_allocated_bitrate = 0; + TargetBitrateCallback bitrate_callback = + [&last_allocated_bitrate](int32_t allocated_bitrate) { + last_allocated_bitrate = allocated_bitrate; + }; + + auto injector = audio_rtp_sender_->CreateEncodedAudioFrameInjector( + std::move(bitrate_callback)); + ASSERT_TRUE(injector); + + auto* injector_impl = static_cast<EncodedAudioFrameInjector*>(injector.get()); + auto encoder_factory = injector_impl->CreateEncoderFactory(); + ASSERT_TRUE(encoder_factory); + auto encoder = + encoder_factory->Create(env_, SdpAudioFormat("opus", 48000, 2), {}); + ASSERT_TRUE(encoder); + + // Verify that the callback is invoked no matter which method is called on the + // proxy encoder, including when called from different threads. + encoder->OnReceivedTargetAudioBitrate(32000); + EXPECT_EQ(32000, last_allocated_bitrate.load()); + + BitrateAllocationUpdate update; + update.target_bitrate = DataRate::BitsPerSec(64000); + encoder->OnReceivedUplinkAllocation(update); + EXPECT_EQ(64000, last_allocated_bitrate.load()); + + worker_thread_->BlockingCall( + [&] { encoder->OnReceivedTargetAudioBitrate(48000); }); + EXPECT_EQ(48000, last_allocated_bitrate.load()); + + worker_thread_->BlockingCall([&] { + BitrateAllocationUpdate worker_update; + worker_update.target_bitrate = DataRate::BitsPerSec(96000); + encoder->OnReceivedUplinkAllocation(worker_update); + }); + EXPECT_EQ(96000, last_allocated_bitrate.load()); + + network_thread_->BlockingCall( + [&] { encoder->OnReceivedTargetAudioBitrate(50000); }); + EXPECT_EQ(50000, last_allocated_bitrate.load()); + + network_thread_->BlockingCall([&] { + BitrateAllocationUpdate network_update; + network_update.target_bitrate = DataRate::BitsPerSec(100000); + encoder->OnReceivedUplinkAllocation(network_update); + }); + EXPECT_EQ(100000, last_allocated_bitrate.load()); + + DestroyAudioRtpSender(); +} + TEST_F(RtpSenderReceiverTest, CreateAudioFrameInjectorInvalidStates) { CreateAudioRtpSender();