From acabb3641b924dd0c5198a16883ae8326f35487e Mon Sep 17 00:00:00 2001 From: Florent Castelli Date: Tue, 18 Oct 2022 17:05:16 +0200 Subject: [PATCH] pc: Add asynchronous RtpSender::SetParameters() call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As the synchronous version only posts a task to recreate the encoder later, it is not possible to catch errors and state changes that could appear then. The asynchronous version of SetParameters() aims to solve this by providing a callback to wait for the completion of the encoder reconfiguration, allowing any error to be propagate and subsequent getParameters() call to have up to date information. Bug: webrtc:11607 Change-Id: I5548e75aa14a97f8d9c0c94df1e72e9cd40887b2 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278420 Reviewed-by: Harald Alvestrand Commit-Queue: Florent Castelli Reviewed-by: Erik Språng Cr-Commit-Position: refs/heads/main@{#38627} --- api/BUILD.gn | 49 ++++++- api/rtp_sender_interface.cc | 22 +++ api/rtp_sender_interface.h | 5 + api/test/mock_rtpsender.h | 4 + audio/BUILD.gn | 1 + audio/DEPS | 1 + audio/audio_send_stream.cc | 17 ++- audio/audio_send_stream.h | 7 +- audio/audio_send_stream_unittest.cc | 14 +- call/BUILD.gn | 8 +- call/audio_send_stream.h | 4 +- call/test/mock_audio_send_stream.h | 5 +- call/video_send_stream.h | 4 + examples/BUILD.gn | 1 + media/BUILD.gn | 1 + media/base/fake_media_engine.h | 16 ++- media/base/media_channel.cc | 13 ++ media/base/media_channel.h | 8 +- media/engine/fake_webrtc_call.cc | 12 +- media/engine/fake_webrtc_call.h | 6 +- media/engine/webrtc_video_engine.cc | 30 ++-- media/engine/webrtc_video_engine.h | 8 +- media/engine/webrtc_video_engine_unittest.cc | 4 +- media/engine/webrtc_voice_engine.cc | 43 +++--- media/engine/webrtc_voice_engine.h | 3 +- media/engine/webrtc_voice_engine_unittest.cc | 4 +- pc/BUILD.gn | 15 ++ pc/rtp_sender.cc | 136 ++++++++++++++++--- pc/rtp_sender.h | 11 +- pc/rtp_sender_proxy.h | 4 + pc/rtp_sender_receiver_unittest.cc | 122 ++++++++++++++++- pc/test/mock_rtp_sender_internal.h | 8 +- pc/test/mock_voice_media_channel.h | 4 +- sdk/BUILD.gn | 2 + sdk/android/BUILD.gn | 2 + video/BUILD.gn | 5 + video/test/mock_video_stream_encoder.h | 8 ++ video/video_send_stream.cc | 8 +- video/video_send_stream.h | 4 +- video/video_stream_encoder.cc | 29 +++- video/video_stream_encoder.h | 7 + video/video_stream_encoder_interface.h | 5 + video/video_stream_encoder_unittest.cc | 2 +- 43 files changed, 567 insertions(+), 95 deletions(-) create mode 100644 api/rtp_sender_interface.cc diff --git a/api/BUILD.gn b/api/BUILD.gn index 3f44bc9b86..5874b953b9 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -193,6 +193,40 @@ rtc_library("dtls_transport_interface") { absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ] } +rtc_library("dtmf_sender_interface") { + visibility = [ "*" ] + + sources = [ "dtmf_sender_interface.h" ] + deps = [ + ":media_stream_interface", + "../rtc_base:refcount", + ] +} + +rtc_library("rtp_sender_interface") { + visibility = [ "*" ] + + sources = [ + "rtp_sender_interface.cc", + "rtp_sender_interface.h", + ] + deps = [ + ":dtls_transport_interface", + ":dtmf_sender_interface", + ":frame_transformer_interface", + ":media_stream_interface", + ":rtc_error", + ":rtp_parameters", + ":scoped_refptr", + "../rtc_base:checks", + "../rtc_base:refcount", + "../rtc_base/system:rtc_export", + "crypto:frame_encryptor_interface", + "video_codecs:video_codecs_api", + ] + absl_deps = [ "//third_party/abseil-cpp/absl/functional:any_invocable" ] +} + rtc_library("libjingle_peerconnection_api") { visibility = [ "*" ] cflags = [] @@ -200,7 +234,6 @@ rtc_library("libjingle_peerconnection_api") { "crypto_params.h", "data_channel_interface.cc", "data_channel_interface.h", - "dtmf_sender_interface.h", "jsep.cc", "jsep.h", "jsep_ice_candidate.cc", @@ -212,7 +245,6 @@ rtc_library("libjingle_peerconnection_api") { "peer_connection_interface.h", "rtp_receiver_interface.cc", "rtp_receiver_interface.h", - "rtp_sender_interface.h", "rtp_transceiver_interface.cc", "rtp_transceiver_interface.h", "sctp_transport_interface.cc", @@ -221,6 +253,15 @@ rtc_library("libjingle_peerconnection_api") { "set_remote_description_observer_interface.h", "uma_metrics.h", "video_track_source_proxy_factory.h", + + # Remove when downstream has been updated + "dtmf_sender_interface.h", + "rtp_sender_interface.h", + ] + public_deps = [ # no-presubmit-check TODO(webrtc:8603) + # Remove when downstream has been updated + ":dtmf_sender_interface", + ":rtp_sender_interface", ] deps = [ ":array_view", @@ -244,6 +285,7 @@ rtc_library("libjingle_peerconnection_api") { ":rtc_stats_api", ":rtp_packet_info", ":rtp_parameters", + ":rtp_sender_interface", ":rtp_transceiver_direction", ":scoped_refptr", ":sequence_checker", @@ -294,6 +336,7 @@ rtc_library("libjingle_peerconnection_api") { absl_deps = [ "//third_party/abseil-cpp/absl/algorithm:container", "//third_party/abseil-cpp/absl/base:core_headers", + "//third_party/abseil-cpp/absl/functional:any_invocable", "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/strings", "//third_party/abseil-cpp/absl/types:optional", @@ -1014,6 +1057,7 @@ if (rtc_include_tests) { sources = [ "test/mock_dtmf_sender.h" ] deps = [ + ":dtmf_sender_interface", ":libjingle_peerconnection_api", "../test:test_support", ] @@ -1175,6 +1219,7 @@ if (rtc_include_tests) { deps = [ ":libjingle_peerconnection_api", + ":rtp_sender_interface", "../test:test_support", ] } diff --git a/api/rtp_sender_interface.cc b/api/rtp_sender_interface.cc new file mode 100644 index 0000000000..f1ca5c2203 --- /dev/null +++ b/api/rtp_sender_interface.cc @@ -0,0 +1,22 @@ +/* + * Copyright 2022 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "api/rtp_sender_interface.h" + +#include "rtc_base/checks.h" + +namespace webrtc { + +void RtpSenderInterface::SetParametersAsync(const RtpParameters& parameters, + SetParametersCallback callback) { + RTC_DCHECK_NOTREACHED() << "Default implementation called"; +} + +} // namespace webrtc diff --git a/api/rtp_sender_interface.h b/api/rtp_sender_interface.h index 7e84cd420a..2786a2ac19 100644 --- a/api/rtp_sender_interface.h +++ b/api/rtp_sender_interface.h @@ -18,6 +18,7 @@ #include #include +#include "absl/functional/any_invocable.h" #include "api/crypto/frame_encryptor_interface.h" #include "api/dtls_transport_interface.h" #include "api/dtmf_sender_interface.h" @@ -33,6 +34,8 @@ namespace webrtc { +using SetParametersCallback = absl::AnyInvocable; + class RTC_EXPORT RtpSenderInterface : public rtc::RefCountInterface { public: // Returns true if successful in setting the track. @@ -79,6 +82,8 @@ class RTC_EXPORT RtpSenderInterface : public rtc::RefCountInterface { // rtpparameters.h // The encodings are in increasing quality order for simulcast. virtual RTCError SetParameters(const RtpParameters& parameters) = 0; + virtual void SetParametersAsync(const RtpParameters& parameters, + SetParametersCallback callback); // Returns null for a video sender. virtual rtc::scoped_refptr GetDtmfSender() const = 0; diff --git a/api/test/mock_rtpsender.h b/api/test/mock_rtpsender.h index e2351f87fe..22113678b9 100644 --- a/api/test/mock_rtpsender.h +++ b/api/test/mock_rtpsender.h @@ -46,6 +46,10 @@ class MockRtpSender : public RtpSenderInterface { (const, override)); MOCK_METHOD(RtpParameters, GetParameters, (), (const, override)); MOCK_METHOD(RTCError, SetParameters, (const RtpParameters&), (override)); + MOCK_METHOD(void, + SetParametersAsync, + (const RtpParameters&, SetParametersCallback), + (override)); MOCK_METHOD(rtc::scoped_refptr, GetDtmfSender, (), diff --git a/audio/BUILD.gn b/audio/BUILD.gn index 91d66d4f8b..02bfe307e1 100644 --- a/audio/BUILD.gn +++ b/audio/BUILD.gn @@ -70,6 +70,7 @@ rtc_library("audio") { "../common_audio:common_audio_c", "../logging:rtc_event_audio", "../logging:rtc_stream_config", + "../media:rtc_media_base", "../modules/async_audio_processing", "../modules/audio_coding", "../modules/audio_coding:audio_coding_module_typedefs", diff --git a/audio/DEPS b/audio/DEPS index 9b89dc39ab..7a0c7e7ce6 100644 --- a/audio/DEPS +++ b/audio/DEPS @@ -2,6 +2,7 @@ include_rules = [ "+call", "+common_audio", "+logging/rtc_event_log", + "+media/base", "+modules/async_audio_processing", "+modules/audio_coding", "+modules/audio_device", diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc index 097ffcb835..8bc0f4b540 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc @@ -31,6 +31,7 @@ #include "common_audio/vad/include/vad.h" #include "logging/rtc_event_log/events/rtc_event_audio_send_stream_config.h" #include "logging/rtc_event_log/rtc_stream_config.h" +#include "media/base/media_channel.h" #include "modules/audio_coding/codecs/cng/audio_encoder_cng.h" #include "modules/audio_coding/codecs/red/audio_encoder_copy_red.h" #include "modules/audio_processing/include/audio_processing.h" @@ -174,7 +175,7 @@ AudioSendStream::AudioSendStream( RTC_DCHECK(rtp_rtcp_module_); RTC_DCHECK_RUN_ON(&worker_thread_checker_); - ConfigureStream(config, true); + ConfigureStream(config, true, nullptr); UpdateCachedTargetAudioBitrateConstraints(); } @@ -195,9 +196,10 @@ const webrtc::AudioSendStream::Config& AudioSendStream::GetConfig() const { } void AudioSendStream::Reconfigure( - const webrtc::AudioSendStream::Config& new_config) { + const webrtc::AudioSendStream::Config& new_config, + SetParametersCallback callback) { RTC_DCHECK_RUN_ON(&worker_thread_checker_); - ConfigureStream(new_config, false); + ConfigureStream(new_config, false, std::move(callback)); } AudioSendStream::ExtensionIds AudioSendStream::FindExtensionIds( @@ -229,7 +231,8 @@ int AudioSendStream::TransportSeqNumId(const AudioSendStream::Config& config) { void AudioSendStream::ConfigureStream( const webrtc::AudioSendStream::Config& new_config, - bool first_time) { + bool first_time, + SetParametersCallback callback) { RTC_LOG(LS_INFO) << "AudioSendStream::ConfigureStream: " << new_config.ToString(); UpdateEventLogStreamConfig(event_log_, new_config, @@ -327,6 +330,10 @@ void AudioSendStream::ConfigureStream( if (!ReconfigureSendCodec(new_config)) { RTC_LOG(LS_ERROR) << "Failed to set up send codec state."; + + webrtc::InvokeSetParametersCallback( + callback, webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR, + "Failed to set up send codec state.")); } // Set currently known overhead (used in ANA, opus only). @@ -352,6 +359,8 @@ void AudioSendStream::ConfigureStream( if (!first_time) { UpdateCachedTargetAudioBitrateConstraints(); } + + webrtc::InvokeSetParametersCallback(callback, webrtc::RTCError::OK()); } void AudioSendStream::Start() { diff --git a/audio/audio_send_stream.h b/audio/audio_send_stream.h index 4962ccd7a3..cdb7472b37 100644 --- a/audio/audio_send_stream.h +++ b/audio/audio_send_stream.h @@ -88,7 +88,8 @@ class AudioSendStream final : public webrtc::AudioSendStream, // webrtc::AudioSendStream implementation. const webrtc::AudioSendStream::Config& GetConfig() const override; - void Reconfigure(const webrtc::AudioSendStream::Config& config) override; + void Reconfigure(const webrtc::AudioSendStream::Config& config, + SetParametersCallback callback) override; void Start() override; void Stop() override; void SendAudioData(std::unique_ptr audio_frame) override; @@ -129,7 +130,9 @@ class AudioSendStream final : public webrtc::AudioSendStream, void StoreEncoderProperties(int sample_rate_hz, size_t num_channels) RTC_RUN_ON(worker_thread_checker_); - void ConfigureStream(const Config& new_config, bool first_time) + void ConfigureStream(const Config& new_config, + bool first_time, + SetParametersCallback callback) RTC_RUN_ON(worker_thread_checker_); bool SetupSendCodec(const Config& new_config) RTC_RUN_ON(worker_thread_checker_); diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc index cbf24b5e72..a81b40cbe7 100644 --- a/audio/audio_send_stream_unittest.cc +++ b/audio/audio_send_stream_unittest.cc @@ -550,7 +550,7 @@ TEST(AudioSendStreamTest, SendCodecAppliesAudioNetworkAdaptor) { auto stream_config = helper.config(); stream_config.audio_network_adaptor_config = kAnaReconfigString; - send_stream->Reconfigure(stream_config); + send_stream->Reconfigure(stream_config, nullptr); } } @@ -590,7 +590,7 @@ TEST(AudioSendStreamTest, AudioNetworkAdaptorReceivesOverhead) { auto stream_config = helper.config(); stream_config.audio_network_adaptor_config = kAnaConfigString; - send_stream->Reconfigure(stream_config); + send_stream->Reconfigure(stream_config, nullptr); } } @@ -791,7 +791,7 @@ TEST(AudioSendStreamTest, DontRecreateEncoder) { AudioSendStream::Config::SendCodecSpec(9, kG722Format); helper.config().send_codec_spec->cng_payload_type = 105; auto send_stream = helper.CreateAudioSendStream(); - send_stream->Reconfigure(helper.config()); + send_stream->Reconfigure(helper.config(), nullptr); } } @@ -816,7 +816,7 @@ TEST(AudioSendStreamTest, ReconfigureTransportCcResetsFirst) { .Times(1); } - send_stream->Reconfigure(new_config); + send_stream->Reconfigure(new_config, nullptr); } } @@ -928,11 +928,11 @@ TEST(AudioSendStreamTest, ReconfigureWithFrameEncryptor) { new_config.frame_encryptor = mock_frame_encryptor_0; EXPECT_CALL(*helper.channel_send(), SetFrameEncryptor(Ne(nullptr))) .Times(1); - send_stream->Reconfigure(new_config); + send_stream->Reconfigure(new_config, nullptr); // Not updating the frame encryptor shouldn't force it to reconfigure. EXPECT_CALL(*helper.channel_send(), SetFrameEncryptor(_)).Times(0); - send_stream->Reconfigure(new_config); + send_stream->Reconfigure(new_config, nullptr); // Updating frame encryptor to a new object should force a call to the // proxy. @@ -942,7 +942,7 @@ TEST(AudioSendStreamTest, ReconfigureWithFrameEncryptor) { new_config.crypto_options.sframe.require_frame_encryption = true; EXPECT_CALL(*helper.channel_send(), SetFrameEncryptor(Ne(nullptr))) .Times(1); - send_stream->Reconfigure(new_config); + send_stream->Reconfigure(new_config, nullptr); } } } // namespace test diff --git a/call/BUILD.gn b/call/BUILD.gn index 27a56eda90..6ed9a0423a 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -48,6 +48,7 @@ rtc_library("call_interfaces") { "../api:rtc_error", "../api:rtp_headers", "../api:rtp_parameters", + "../api:rtp_sender_interface", "../api:scoped_refptr", "../api:transport_api", "../api/adaptation:resource_adaptation_api", @@ -76,6 +77,7 @@ rtc_library("call_interfaces") { "../rtc_base/network:sent_packet", ] absl_deps = [ + "//third_party/abseil-cpp/absl/functional:any_invocable", "//third_party/abseil-cpp/absl/functional:bind_front", "//third_party/abseil-cpp/absl/strings", "//third_party/abseil-cpp/absl/types:optional", @@ -373,6 +375,7 @@ rtc_library("video_stream_api") { "../api:frame_transformer_interface", "../api:rtp_headers", "../api:rtp_parameters", + "../api:rtp_sender_interface", "../api:scoped_refptr", "../api:transport_api", "../api/adaptation:resource_adaptation_api", @@ -390,7 +393,10 @@ rtc_library("video_stream_api") { "../rtc_base:stringutils", "../video/config:encoder_config", ] - absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ] + absl_deps = [ + "//third_party/abseil-cpp/absl/functional:any_invocable", + "//third_party/abseil-cpp/absl/types:optional", + ] } rtc_library("simulated_network") { diff --git a/call/audio_send_stream.h b/call/audio_send_stream.h index 15b439c593..0f42d0fb82 100644 --- a/call/audio_send_stream.h +++ b/call/audio_send_stream.h @@ -25,6 +25,7 @@ #include "api/crypto/frame_encryptor_interface.h" #include "api/frame_transformer_interface.h" #include "api/rtp_parameters.h" +#include "api/rtp_sender_interface.h" #include "api/scoped_refptr.h" #include "call/audio_sender.h" #include "call/rtp_config.h" @@ -173,7 +174,8 @@ class AudioSendStream : public AudioSender { virtual const webrtc::AudioSendStream::Config& GetConfig() const = 0; // Reconfigure the stream according to the Configuration. - virtual void Reconfigure(const Config& config) = 0; + virtual void Reconfigure(const Config& config, + SetParametersCallback callback) = 0; // Starts stream activity. // When a stream is active, it can receive, process and deliver packets. diff --git a/call/test/mock_audio_send_stream.h b/call/test/mock_audio_send_stream.h index 4164dd550e..1993de8de0 100644 --- a/call/test/mock_audio_send_stream.h +++ b/call/test/mock_audio_send_stream.h @@ -25,7 +25,10 @@ class MockAudioSendStream : public AudioSendStream { GetConfig, (), (const, override)); - MOCK_METHOD(void, Reconfigure, (const Config& config), (override)); + MOCK_METHOD(void, + Reconfigure, + (const Config& config, SetParametersCallback callback), + (override)); MOCK_METHOD(void, Start, (), (override)); MOCK_METHOD(void, Stop, (), (override)); // GMock doesn't like move-only types, such as std::unique_ptr. diff --git a/call/video_send_stream.h b/call/video_send_stream.h index 5fd0bebe20..e54a51d901 100644 --- a/call/video_send_stream.h +++ b/call/video_send_stream.h @@ -23,6 +23,7 @@ #include "api/crypto/crypto_options.h" #include "api/frame_transformer_interface.h" #include "api/rtp_parameters.h" +#include "api/rtp_sender_interface.h" #include "api/scoped_refptr.h" #include "api/video/video_content_type.h" #include "api/video/video_frame.h" @@ -251,6 +252,9 @@ class VideoSendStream { // with the VideoStream settings. virtual void ReconfigureVideoEncoder(VideoEncoderConfig config) = 0; + virtual void ReconfigureVideoEncoder(VideoEncoderConfig config, + SetParametersCallback callback) = 0; + virtual Stats GetStats() = 0; virtual void GenerateKeyFrame(const std::vector& rids) = 0; diff --git a/examples/BUILD.gn b/examples/BUILD.gn index e683c192dc..7d87a01c77 100644 --- a/examples/BUILD.gn +++ b/examples/BUILD.gn @@ -690,6 +690,7 @@ if (is_linux || is_chromeos || is_win) { "../api:create_peerconnection_factory", "../api:libjingle_peerconnection_api", "../api:media_stream_interface", + "../api:rtp_sender_interface", "../api:scoped_refptr", "../api/audio:audio_mixer_api", "../api/audio_codecs:audio_codecs_api", diff --git a/media/BUILD.gn b/media/BUILD.gn index a6075634a1..4b7f6215f8 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -57,6 +57,7 @@ rtc_library("rtc_media_base") { "../api:media_stream_interface", "../api:rtc_error", "../api:rtp_parameters", + "../api:rtp_sender_interface", "../api:scoped_refptr", "../api:sequence_checker", "../api/audio:audio_frame_processor", diff --git a/media/base/fake_media_engine.h b/media/base/fake_media_engine.h index ece77e5174..3e5bc6adb1 100644 --- a/media/base/fake_media_engine.h +++ b/media/base/fake_media_engine.h @@ -30,6 +30,7 @@ #include "modules/audio_processing/include/audio_processing.h" #include "rtc_base/copy_on_write_buffer.h" #include "rtc_base/network_route.h" +#include "rtc_base/thread.h" using webrtc::RtpExtension; @@ -149,20 +150,25 @@ class RtpHelper : public Base { } virtual webrtc::RTCError SetRtpSendParameters( uint32_t ssrc, - const webrtc::RtpParameters& parameters) { + const webrtc::RtpParameters& parameters, + webrtc::SetParametersCallback callback) { auto parameters_iterator = rtp_send_parameters_.find(ssrc); if (parameters_iterator != rtp_send_parameters_.end()) { auto result = CheckRtpParametersInvalidModificationAndValues( parameters_iterator->second, parameters); - if (!result.ok()) - return result; + if (!result.ok()) { + return webrtc::InvokeSetParametersCallback(callback, result); + } parameters_iterator->second = parameters; - return webrtc::RTCError::OK(); + + return webrtc::InvokeSetParametersCallback(callback, + webrtc::RTCError::OK()); } // Replicate the behavior of the real media channel: return false // when setting parameters for unknown SSRCs. - return webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR); + return InvokeSetParametersCallback( + callback, webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR)); } virtual webrtc::RtpParameters GetRtpReceiveParameters(uint32_t ssrc) const { diff --git a/media/base/media_channel.cc b/media/base/media_channel.cc index e01bfb1a82..1c6802dc4c 100644 --- a/media/base/media_channel.cc +++ b/media/base/media_channel.cc @@ -12,6 +12,19 @@ #include "media/base/rtp_utils.h" +namespace webrtc { + +webrtc::RTCError InvokeSetParametersCallback(SetParametersCallback& callback, + RTCError error) { + if (callback) { + std::move(callback)(error); + callback = nullptr; + } + return error; +} + +} // namespace webrtc + namespace cricket { using webrtc::FrameDecryptorInterface; using webrtc::FrameEncryptorInterface; diff --git a/media/base/media_channel.h b/media/base/media_channel.h index 5f1d545503..5460cdb9ac 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -26,6 +26,7 @@ #include "api/media_stream_interface.h" #include "api/rtc_error.h" #include "api/rtp_parameters.h" +#include "api/rtp_sender_interface.h" #include "api/task_queue/pending_task_safety_flag.h" #include "api/transport/data_channel_transport_interface.h" #include "api/transport/rtp/rtp_source.h" @@ -61,6 +62,10 @@ class Timing; namespace webrtc { class AudioSinkInterface; class VideoFrame; + +webrtc::RTCError InvokeSetParametersCallback(SetParametersCallback& callback, + RTCError error); + } // namespace webrtc namespace cricket { @@ -277,7 +282,8 @@ class MediaChannel { virtual webrtc::RtpParameters GetRtpSendParameters(uint32_t ssrc) const = 0; virtual webrtc::RTCError SetRtpSendParameters( uint32_t ssrc, - const webrtc::RtpParameters& parameters) = 0; + const webrtc::RtpParameters& parameters, + webrtc::SetParametersCallback callback = nullptr) = 0; virtual void SetEncoderToPacketizerFrameTransformer( uint32_t ssrc, diff --git a/media/engine/fake_webrtc_call.cc b/media/engine/fake_webrtc_call.cc index 48a8b12092..063be93269 100644 --- a/media/engine/fake_webrtc_call.cc +++ b/media/engine/fake_webrtc_call.cc @@ -15,6 +15,7 @@ #include "absl/algorithm/container.h" #include "absl/strings/string_view.h" #include "api/call/audio_sink.h" +#include "media/base/media_channel.h" #include "modules/rtp_rtcp/source/rtp_util.h" #include "rtc_base/checks.h" #include "rtc_base/gunit.h" @@ -31,8 +32,10 @@ FakeAudioSendStream::FakeAudioSendStream( : id_(id), config_(config) {} void FakeAudioSendStream::Reconfigure( - const webrtc::AudioSendStream::Config& config) { + const webrtc::AudioSendStream::Config& config, + webrtc::SetParametersCallback callback) { config_ = config; + webrtc::InvokeSetParametersCallback(callback, webrtc::RTCError::OK()); } const webrtc::AudioSendStream::Config& FakeAudioSendStream::GetConfig() const { @@ -275,6 +278,12 @@ webrtc::VideoSendStream::Stats FakeVideoSendStream::GetStats() { void FakeVideoSendStream::ReconfigureVideoEncoder( webrtc::VideoEncoderConfig config) { + ReconfigureVideoEncoder(std::move(config), nullptr); +} + +void FakeVideoSendStream::ReconfigureVideoEncoder( + webrtc::VideoEncoderConfig config, + webrtc::SetParametersCallback callback) { int width, height; if (last_frame_) { width = last_frame_->width(); @@ -326,6 +335,7 @@ void FakeVideoSendStream::ReconfigureVideoEncoder( codec_settings_set_ = config.encoder_specific_settings != nullptr; encoder_config_ = std::move(config); ++num_encoder_reconfigurations_; + webrtc::InvokeSetParametersCallback(callback, webrtc::RTCError::OK()); } void FakeVideoSendStream::UpdateActiveSimulcastLayers( diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h index 1e0568e46e..a3876b3ca8 100644 --- a/media/engine/fake_webrtc_call.h +++ b/media/engine/fake_webrtc_call.h @@ -62,7 +62,8 @@ class FakeAudioSendStream final : public webrtc::AudioSendStream { private: // webrtc::AudioSendStream implementation. - void Reconfigure(const webrtc::AudioSendStream::Config& config) override; + void Reconfigure(const webrtc::AudioSendStream::Config& config, + webrtc::SetParametersCallback callback) override; void Start() override { sending_ = true; } void Stop() override { sending_ = false; } void SendAudioData(std::unique_ptr audio_frame) override { @@ -213,7 +214,10 @@ class FakeVideoSendStream final rtc::VideoSourceInterface* source, const webrtc::DegradationPreference& degradation_preference) override; webrtc::VideoSendStream::Stats GetStats() override; + void ReconfigureVideoEncoder(webrtc::VideoEncoderConfig config) override; + void ReconfigureVideoEncoder(webrtc::VideoEncoderConfig config, + webrtc::SetParametersCallback callback) override; bool sending_; webrtc::VideoSendStream::Config config_; diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index cfb15b0576..8dfc46c641 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -1042,7 +1042,8 @@ webrtc::RtpParameters WebRtcVideoChannel::GetRtpSendParameters( webrtc::RTCError WebRtcVideoChannel::SetRtpSendParameters( uint32_t ssrc, - const webrtc::RtpParameters& parameters) { + const webrtc::RtpParameters& parameters, + webrtc::SetParametersCallback callback) { RTC_DCHECK_RUN_ON(&thread_checker_); TRACE_EVENT0("webrtc", "WebRtcVideoChannel::SetRtpSendParameters"); auto it = send_streams_.find(ssrc); @@ -1050,7 +1051,8 @@ webrtc::RTCError WebRtcVideoChannel::SetRtpSendParameters( RTC_LOG(LS_ERROR) << "Attempting to set RTP send parameters for stream " "with ssrc " << ssrc << " which doesn't exist."; - return webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR); + return webrtc::InvokeSetParametersCallback( + callback, webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR)); } // TODO(deadbeef): Handle setting parameters with a list of codecs in a @@ -1059,7 +1061,8 @@ webrtc::RTCError WebRtcVideoChannel::SetRtpSendParameters( if (current_parameters.codecs != parameters.codecs) { RTC_DLOG(LS_ERROR) << "Using SetParameters to change the set of codecs " "is not currently supported."; - return webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR); + return webrtc::InvokeSetParametersCallback( + callback, webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR)); } if (!parameters.encodings.empty()) { @@ -1085,7 +1088,7 @@ webrtc::RTCError WebRtcVideoChannel::SetRtpSendParameters( SetPreferredDscp(new_dscp); } - return it->second->SetRtpParameters(parameters); + return it->second->SetRtpParameters(parameters, std::move(callback)); } webrtc::RtpParameters WebRtcVideoChannel::GetRtpReceiveParameters( @@ -2156,7 +2159,7 @@ bool WebRtcVideoChannel::WebRtcVideoSendStream::SetVideoSend( old_options.is_screencast = options->is_screencast; } if (parameters_.options != old_options) { - ReconfigureEncoder(); + ReconfigureEncoder(nullptr); } } @@ -2283,7 +2286,7 @@ void WebRtcVideoChannel::WebRtcVideoSendStream::SetSendParameters( } if (params.max_bandwidth_bps) { parameters_.max_bitrate_bps = *params.max_bandwidth_bps; - ReconfigureEncoder(); + ReconfigureEncoder(nullptr); } if (params.conference_mode) { parameters_.conference_mode = *params.conference_mode; @@ -2305,7 +2308,8 @@ void WebRtcVideoChannel::WebRtcVideoSendStream::SetSendParameters( } webrtc::RTCError WebRtcVideoChannel::WebRtcVideoSendStream::SetRtpParameters( - const webrtc::RtpParameters& new_parameters) { + const webrtc::RtpParameters& new_parameters, + webrtc::SetParametersCallback callback) { RTC_DCHECK_RUN_ON(&thread_checker_); // This is checked higher in the stack (RtpSender), so this is only checking // for users accessing the private APIs or tests, not specification @@ -2366,7 +2370,9 @@ webrtc::RTCError WebRtcVideoChannel::WebRtcVideoSendStream::SetRtpParameters( // Codecs are currently handled at the WebRtcVideoChannel level. rtp_parameters_.codecs.clear(); if (reconfigure_encoder || new_send_state) { - ReconfigureEncoder(); + // Callback responsibility is delegated to ReconfigureEncoder() + ReconfigureEncoder(std::move(callback)); + callback = nullptr; } if (new_send_state) { UpdateSendState(); @@ -2376,7 +2382,7 @@ webrtc::RTCError WebRtcVideoChannel::WebRtcVideoSendStream::SetRtpParameters( stream_->SetSource(source_, GetDegradationPreference()); } } - return webrtc::RTCError::OK(); + return webrtc::InvokeSetParametersCallback(callback, webrtc::RTCError::OK()); } webrtc::RtpParameters @@ -2564,11 +2570,13 @@ WebRtcVideoChannel::WebRtcVideoSendStream::CreateVideoEncoderConfig( return encoder_config; } -void WebRtcVideoChannel::WebRtcVideoSendStream::ReconfigureEncoder() { +void WebRtcVideoChannel::WebRtcVideoSendStream::ReconfigureEncoder( + webrtc::SetParametersCallback callback) { RTC_DCHECK_RUN_ON(&thread_checker_); if (!stream_) { // The webrtc::VideoSendStream `stream_` has not yet been created but other // parameters has changed. + webrtc::InvokeSetParametersCallback(callback, webrtc::RTCError::OK()); return; } @@ -2583,7 +2591,7 @@ void WebRtcVideoChannel::WebRtcVideoSendStream::ReconfigureEncoder() { encoder_config.encoder_specific_settings = ConfigureVideoEncoderSettings(codec_settings.codec); - stream_->ReconfigureVideoEncoder(encoder_config.Copy()); + stream_->ReconfigureVideoEncoder(encoder_config.Copy(), std::move(callback)); encoder_config.encoder_specific_settings = NULL; diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index ee5b8c3b5a..841e04dd36 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -149,7 +149,8 @@ class WebRtcVideoChannel : public VideoMediaChannel, webrtc::RtpParameters GetRtpSendParameters(uint32_t ssrc) const override; webrtc::RTCError SetRtpSendParameters( uint32_t ssrc, - const webrtc::RtpParameters& parameters) override; + const webrtc::RtpParameters& parameters, + webrtc::SetParametersCallback callback) override; webrtc::RtpParameters GetRtpReceiveParameters(uint32_t ssrc) const override; webrtc::RtpParameters GetDefaultRtpReceiveParameters() const override; bool GetSendCodec(VideoCodec* send_codec) override; @@ -363,7 +364,8 @@ class WebRtcVideoChannel : public VideoMediaChannel, ~WebRtcVideoSendStream(); void SetSendParameters(const ChangedSendParameters& send_params); - webrtc::RTCError SetRtpParameters(const webrtc::RtpParameters& parameters); + webrtc::RTCError SetRtpParameters(const webrtc::RtpParameters& parameters, + webrtc::SetParametersCallback callback); webrtc::RtpParameters GetRtpParameters() const; void SetFrameEncryptor( @@ -422,7 +424,7 @@ class WebRtcVideoChannel : public VideoMediaChannel, void RecreateWebRtcStream(); webrtc::VideoEncoderConfig CreateVideoEncoderConfig( const VideoCodec& codec) const; - void ReconfigureEncoder(); + void ReconfigureEncoder(webrtc::SetParametersCallback callback); // Calls Start or Stop according to whether or not `sending_` is true, // and whether or not the encoding in `rtp_parameters_` is active. diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 053fd173be..c151e72b79 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -5294,10 +5294,10 @@ TEST_F(WebRtcVideoChannelTest, TestSetDscpOptions) { // Various priorities map to various dscp values. parameters.encodings[0].network_priority = webrtc::Priority::kHigh; - ASSERT_TRUE(channel->SetRtpSendParameters(kSsrc, parameters).ok()); + ASSERT_TRUE(channel->SetRtpSendParameters(kSsrc, parameters, nullptr).ok()); EXPECT_EQ(rtc::DSCP_AF41, network_interface->dscp()); parameters.encodings[0].network_priority = webrtc::Priority::kVeryLow; - ASSERT_TRUE(channel->SetRtpSendParameters(kSsrc, parameters).ok()); + ASSERT_TRUE(channel->SetRtpSendParameters(kSsrc, parameters, nullptr).ok()); EXPECT_EQ(rtc::DSCP_CS1, network_interface->dscp()); // Packets should also self-identify their dscp in PacketOptions. diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index ef729f4f87..9a24b76861 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -789,19 +789,19 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream void SetSendCodecSpec( const webrtc::AudioSendStream::Config::SendCodecSpec& send_codec_spec) { UpdateSendCodecSpec(send_codec_spec); - ReconfigureAudioSendStream(); + ReconfigureAudioSendStream(nullptr); } void SetRtpExtensions(const std::vector& extensions) { RTC_DCHECK_RUN_ON(&worker_thread_checker_); config_.rtp.extensions = extensions; rtp_parameters_.header_extensions = extensions; - ReconfigureAudioSendStream(); + ReconfigureAudioSendStream(nullptr); } void SetExtmapAllowMixed(bool extmap_allow_mixed) { config_.rtp.extmap_allow_mixed = extmap_allow_mixed; - ReconfigureAudioSendStream(); + ReconfigureAudioSendStream(nullptr); } void SetMid(const std::string& mid) { @@ -810,14 +810,14 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream return; } config_.rtp.mid = mid; - ReconfigureAudioSendStream(); + ReconfigureAudioSendStream(nullptr); } void SetFrameEncryptor( rtc::scoped_refptr frame_encryptor) { RTC_DCHECK_RUN_ON(&worker_thread_checker_); config_.frame_encryptor = frame_encryptor; - ReconfigureAudioSendStream(); + ReconfigureAudioSendStream(nullptr); } void SetAudioNetworkAdaptorConfig( @@ -830,7 +830,7 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream audio_network_adaptor_config_from_options_ = audio_network_adaptor_config; UpdateAudioNetworkAdaptorConfig(); UpdateAllowedBitrateRange(); - ReconfigureAudioSendStream(); + ReconfigureAudioSendStream(nullptr); } bool SetMaxSendBitrate(int bps) { @@ -848,7 +848,7 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream if (send_rate != config_.send_codec_spec->target_bitrate_bps) { config_.send_codec_spec->target_bitrate_bps = send_rate; - ReconfigureAudioSendStream(); + ReconfigureAudioSendStream(nullptr); } return true; } @@ -958,11 +958,12 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream return rtp_parameters_; } - webrtc::RTCError SetRtpParameters(const webrtc::RtpParameters& parameters) { + webrtc::RTCError SetRtpParameters(const webrtc::RtpParameters& parameters, + webrtc::SetParametersCallback callback) { webrtc::RTCError error = CheckRtpParametersInvalidModificationAndValues( rtp_parameters_, parameters); if (!error.ok()) { - return error; + return webrtc::InvokeSetParametersCallback(callback, error); } absl::optional send_rate; @@ -971,7 +972,8 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream parameters.encodings[0].max_bitrate_bps, *audio_codec_spec_); if (!send_rate) { - return webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR); + return webrtc::InvokeSetParametersCallback( + callback, webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR)); } } @@ -1001,7 +1003,9 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream // used. UpdateAudioNetworkAdaptorConfig(); UpdateAllowedBitrateRange(); - ReconfigureAudioSendStream(); + ReconfigureAudioSendStream(std::move(callback)); + } else { + webrtc::InvokeSetParametersCallback(callback, webrtc::RTCError::OK()); } rtp_parameters_.rtcp.cname = config_.rtp.c_name; @@ -1016,7 +1020,7 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream rtc::scoped_refptr frame_transformer) { RTC_DCHECK_RUN_ON(&worker_thread_checker_); config_.frame_transformer = std::move(frame_transformer); - ReconfigureAudioSendStream(); + ReconfigureAudioSendStream(nullptr); } private: @@ -1106,10 +1110,10 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream audio_network_adaptor_config_from_options_; } - void ReconfigureAudioSendStream() { + void ReconfigureAudioSendStream(webrtc::SetParametersCallback callback) { RTC_DCHECK_RUN_ON(&worker_thread_checker_); RTC_DCHECK(stream_); - stream_->Reconfigure(config_); + stream_->Reconfigure(config_, std::move(callback)); } int NumPreferredChannels() const override { return num_encoded_channels_; } @@ -1389,14 +1393,16 @@ webrtc::RtpParameters WebRtcVoiceMediaChannel::GetRtpSendParameters( webrtc::RTCError WebRtcVoiceMediaChannel::SetRtpSendParameters( uint32_t ssrc, - const webrtc::RtpParameters& parameters) { + const webrtc::RtpParameters& parameters, + webrtc::SetParametersCallback callback) { RTC_DCHECK_RUN_ON(worker_thread_); auto it = send_streams_.find(ssrc); if (it == send_streams_.end()) { RTC_LOG(LS_WARNING) << "Attempting to set RTP send parameters for stream " "with ssrc " << ssrc << " which doesn't exist."; - return webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR); + return webrtc::InvokeSetParametersCallback( + callback, webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR)); } // TODO(deadbeef): Handle setting parameters with a list of codecs in a @@ -1405,7 +1411,8 @@ webrtc::RTCError WebRtcVoiceMediaChannel::SetRtpSendParameters( if (current_parameters.codecs != parameters.codecs) { RTC_DLOG(LS_ERROR) << "Using SetParameters to change the set of codecs " "is not currently supported."; - return webrtc::RTCError(webrtc::RTCErrorType::UNSUPPORTED_PARAMETER); + return webrtc::InvokeSetParametersCallback( + callback, webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR)); } if (!parameters.encodings.empty()) { @@ -1440,7 +1447,7 @@ webrtc::RTCError WebRtcVoiceMediaChannel::SetRtpSendParameters( // Codecs are handled at the WebRtcVoiceMediaChannel level. webrtc::RtpParameters reduced_params = parameters; reduced_params.codecs.clear(); - return it->second->SetRtpParameters(reduced_params); + return it->second->SetRtpParameters(reduced_params, std::move(callback)); } webrtc::RtpParameters WebRtcVoiceMediaChannel::GetRtpReceiveParameters( diff --git a/media/engine/webrtc_voice_engine.h b/media/engine/webrtc_voice_engine.h index 0a501bea0a..daa964a655 100644 --- a/media/engine/webrtc_voice_engine.h +++ b/media/engine/webrtc_voice_engine.h @@ -156,7 +156,8 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, webrtc::RtpParameters GetRtpSendParameters(uint32_t ssrc) const override; webrtc::RTCError SetRtpSendParameters( uint32_t ssrc, - const webrtc::RtpParameters& parameters) override; + const webrtc::RtpParameters& parameters, + webrtc::SetParametersCallback callback) override; webrtc::RtpParameters GetRtpReceiveParameters(uint32_t ssrc) const override; webrtc::RtpParameters GetDefaultRtpReceiveParameters() const override; diff --git a/media/engine/webrtc_voice_engine_unittest.cc b/media/engine/webrtc_voice_engine_unittest.cc index 9644fbdae2..0ef53bd68e 100644 --- a/media/engine/webrtc_voice_engine_unittest.cc +++ b/media/engine/webrtc_voice_engine_unittest.cc @@ -3207,10 +3207,10 @@ TEST_P(WebRtcVoiceEngineTestFake, TestSetDscpOptions) { // Various priorities map to various dscp values. parameters.encodings[0].network_priority = webrtc::Priority::kHigh; - ASSERT_TRUE(channel->SetRtpSendParameters(kSsrcZ, parameters).ok()); + ASSERT_TRUE(channel->SetRtpSendParameters(kSsrcZ, parameters, nullptr).ok()); EXPECT_EQ(rtc::DSCP_EF, network_interface.dscp()); parameters.encodings[0].network_priority = webrtc::Priority::kVeryLow; - ASSERT_TRUE(channel->SetRtpSendParameters(kSsrcZ, parameters).ok()); + ASSERT_TRUE(channel->SetRtpSendParameters(kSsrcZ, parameters, nullptr).ok()); EXPECT_EQ(rtc::DSCP_CS1, network_interface.dscp()); // Packets should also self-identify their dscp in PacketOptions. diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 8126fedb83..7395072e5d 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -415,6 +415,7 @@ rtc_source_set("rtp_sender_proxy") { deps = [ ":proxy", "../api:libjingle_peerconnection_api", + "../api:rtp_sender_interface", ] } @@ -1091,6 +1092,7 @@ rtc_source_set("sdp_offer_answer") { "../api:media_stream_interface", "../api:rtc_error", "../api:rtp_parameters", + "../api:rtp_sender_interface", "../api:rtp_transceiver_direction", "../api:scoped_refptr", "../api:sequence_checker", @@ -1181,6 +1183,7 @@ rtc_source_set("peer_connection") { "../api:rtc_error", "../api:rtc_stats_api", "../api:rtp_parameters", + "../api:rtp_sender_interface", "../api:rtp_transceiver_direction", "../api:scoped_refptr", "../api:sequence_checker", @@ -1276,6 +1279,7 @@ rtc_source_set("legacy_stats_collector") { "../api:libjingle_peerconnection_api", "../api:media_stream_interface", "../api:rtp_parameters", + "../api:rtp_sender_interface", "../api:scoped_refptr", "../api:sequence_checker", "../api/audio_codecs:audio_codecs_api", @@ -1544,6 +1548,7 @@ rtc_library("rtp_transceiver") { "../api:libjingle_peerconnection_api", "../api:rtc_error", "../api:rtp_parameters", + "../api:rtp_sender_interface", "../api:rtp_transceiver_direction", "../api:scoped_refptr", "../api:sequence_checker", @@ -1588,6 +1593,7 @@ rtc_library("rtp_transmission_manager") { "../api:media_stream_interface", "../api:rtc_error", "../api:rtp_parameters", + "../api:rtp_sender_interface", "../api:rtp_transceiver_direction", "../api:scoped_refptr", "../api:sequence_checker", @@ -1618,6 +1624,7 @@ rtc_library("transceiver_list") { "../api:libjingle_peerconnection_api", "../api:rtc_error", "../api:rtp_parameters", + "../api:rtp_sender_interface", "../api:scoped_refptr", "../api:sequence_checker", "../rtc_base:checks", @@ -1853,12 +1860,14 @@ rtc_library("rtp_sender") { ":legacy_stats_collector_interface", "../api:audio_options_api", "../api:dtls_transport_interface", + "../api:dtmf_sender_interface", "../api:frame_transformer_interface", "../api:libjingle_peerconnection_api", "../api:media_stream_interface", "../api:priority", "../api:rtc_error", "../api:rtp_parameters", + "../api:rtp_sender_interface", "../api:scoped_refptr", "../api:sequence_checker", "../api/crypto:frame_encryptor_interface", @@ -1912,6 +1921,7 @@ rtc_library("dtmf_sender") { ] deps = [ ":proxy", + "../api:dtmf_sender_interface", "../api:libjingle_peerconnection_api", "../api:scoped_refptr", "../api:sequence_checker", @@ -2179,6 +2189,7 @@ if (rtc_include_tests && !build_with_chromium) { "../api:rtc_error", "../api:rtc_stats_api", "../api:rtp_parameters", + "../api:rtp_sender_interface", "../api:scoped_refptr", "../rtc_base:checks", "../rtc_base:gunit_helpers", @@ -2193,6 +2204,7 @@ if (rtc_include_tests && !build_with_chromium) { deps = [ ":integration_test_helpers", ":pc_test_utils", + "../api:dtmf_sender_interface", "../api:libjingle_peerconnection_api", "../api:scoped_refptr", "../api/units:time_delta", @@ -2314,6 +2326,7 @@ if (rtc_include_tests && !build_with_chromium) { "../api:candidate", "../api:create_peerconnection_factory", "../api:dtls_transport_interface", + "../api:dtmf_sender_interface", "../api:fake_frame_decryptor", "../api:fake_frame_encryptor", "../api:field_trials_view", @@ -2329,6 +2342,7 @@ if (rtc_include_tests && !build_with_chromium) { "../api:packet_socket_factory", "../api:priority", "../api:rtc_error", + "../api:rtp_sender_interface", "../api:rtp_transceiver_direction", "../api:scoped_refptr", "../api/adaptation:resource_adaptation_api", @@ -2523,6 +2537,7 @@ if (rtc_include_tests && !build_with_chromium) { "../api:rtc_error", "../api:rtc_stats_api", "../api:rtp_parameters", + "../api:rtp_sender_interface", "../api:rtp_transceiver_direction", "../api:scoped_refptr", "../api/audio:audio_mixer_api", diff --git a/pc/rtp_sender.cc b/pc/rtp_sender.cc index 698d7ffe61..eb9b436fbb 100644 --- a/pc/rtp_sender.cc +++ b/pc/rtp_sender.cc @@ -88,6 +88,46 @@ RtpParameters RestoreEncodingLayers( return result; } +class SignalingThreadCallback { + public: + SignalingThreadCallback(rtc::Thread* signaling_thread, + SetParametersCallback callback) + : signaling_thread_(signaling_thread), callback_(std::move(callback)) {} + SignalingThreadCallback(SignalingThreadCallback&& other) + : signaling_thread_(other.signaling_thread_), + callback_(std::move(other.callback_)) { + other.callback_ = nullptr; + } + + ~SignalingThreadCallback() { + if (callback_) { + Resolve(RTCError(RTCErrorType::INTERNAL_ERROR)); + + RTC_CHECK_NOTREACHED(); + } + } + + void operator()(const RTCError& error) { Resolve(error); } + + private: + void Resolve(const RTCError& error) { + if (!signaling_thread_->IsCurrent()) { + signaling_thread_->PostTask( + [callback = std::move(callback_), error]() mutable { + webrtc::InvokeSetParametersCallback(callback, error); + }); + callback_ = nullptr; + return; + } + + webrtc::InvokeSetParametersCallback(callback_, error); + callback_ = nullptr; + } + + rtc::Thread* signaling_thread_; + SetParametersCallback callback_; +}; + } // namespace // Returns true if any RtpParameters member that isn't implemented contains a @@ -189,14 +229,20 @@ RtpParameters RtpSenderBase::GetParameters() const { return result; } -RTCError RtpSenderBase::SetParametersInternal(const RtpParameters& parameters) { +void RtpSenderBase::SetParametersInternal(const RtpParameters& parameters, + SetParametersCallback callback, + bool blocking) { RTC_DCHECK_RUN_ON(signaling_thread_); RTC_DCHECK(!stopped_); if (UnimplementedRtpParameterHasValue(parameters)) { - LOG_AND_RETURN_ERROR( + RTCError error( RTCErrorType::UNSUPPORTED_PARAMETER, "Attempted to set an unimplemented parameter of RtpParameters."); + RTC_LOG(LS_ERROR) << error.message() << " (" + << ::webrtc::ToString(error.type()) << ")"; + webrtc::InvokeSetParametersCallback(callback, error); + return; } if (!media_channel_ || !ssrc_) { auto result = cricket::CheckRtpParametersInvalidModificationAndValues( @@ -204,9 +250,11 @@ RTCError RtpSenderBase::SetParametersInternal(const RtpParameters& parameters) { if (result.ok()) { init_parameters_ = parameters; } - return result; + webrtc::InvokeSetParametersCallback(callback, result); + return; } - return worker_thread_->BlockingCall([&] { + auto task = [&, callback = std::move(callback), + parameters = std::move(parameters)]() mutable { RtpParameters rtp_parameters = parameters; RtpParameters old_parameters = media_channel_->GetRtpSendParameters(ssrc_); if (!disabled_rids_.empty()) { @@ -215,17 +263,26 @@ RTCError RtpSenderBase::SetParametersInternal(const RtpParameters& parameters) { old_parameters.encodings); } - auto result = cricket::CheckRtpParametersInvalidModificationAndValues( + RTCError result = cricket::CheckRtpParametersInvalidModificationAndValues( old_parameters, rtp_parameters); - if (!result.ok()) - return result; + if (!result.ok()) { + webrtc::InvokeSetParametersCallback(callback, result); + return; + } result = CheckSVCParameters(rtp_parameters); - if (!result.ok()) - return result; + if (!result.ok()) { + webrtc::InvokeSetParametersCallback(callback, result); + return; + } - return media_channel_->SetRtpSendParameters(ssrc_, rtp_parameters); - }); + media_channel_->SetRtpSendParameters(ssrc_, rtp_parameters, + std::move(callback)); + }; + if (blocking) + worker_thread_->BlockingCall(task); + else + worker_thread_->PostTask(std::move(task)); } RTCError RtpSenderBase::SetParametersInternalWithAllLayers( @@ -248,13 +305,12 @@ RTCError RtpSenderBase::SetParametersInternalWithAllLayers( } return worker_thread_->BlockingCall([&] { RtpParameters rtp_parameters = parameters; - return media_channel_->SetRtpSendParameters(ssrc_, rtp_parameters); + return media_channel_->SetRtpSendParameters(ssrc_, rtp_parameters, nullptr); }); } -RTCError RtpSenderBase::SetParameters(const RtpParameters& parameters) { +RTCError RtpSenderBase::CheckSetParameters(const RtpParameters& parameters) { RTC_DCHECK_RUN_ON(signaling_thread_); - TRACE_EVENT0("webrtc", "RtpSenderBase::SetParameters"); if (is_transceiver_stopped_) { LOG_AND_RETURN_ERROR( RTCErrorType::INVALID_STATE, @@ -264,10 +320,6 @@ RTCError RtpSenderBase::SetParameters(const RtpParameters& parameters) { LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_STATE, "Cannot set parameters on a stopped sender."); } - if (stopped_) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_STATE, - "Cannot set parameters on a stopped sender."); - } if (!last_transaction_id_) { LOG_AND_RETURN_ERROR( RTCErrorType::INVALID_STATE, @@ -281,11 +333,55 @@ RTCError RtpSenderBase::SetParameters(const RtpParameters& parameters) { " the last value returned from getParameters()"); } - RTCError result = SetParametersInternal(parameters); + return RTCError::OK(); +} + +RTCError RtpSenderBase::SetParameters(const RtpParameters& parameters) { + RTC_DCHECK_RUN_ON(signaling_thread_); + TRACE_EVENT0("webrtc", "RtpSenderBase::SetParameters"); + RTCError result = CheckSetParameters(parameters); + if (!result.ok()) + return result; + + // Some tests rely on working in single thread mode without a run loop and a + // blocking call is required to keep them working. The encoder configuration + // also involves another thread with an asynchronous task, thus we still do + // need to wait for the callback to be resolved this way. + std::unique_ptr done_event = std::make_unique(); + SetParametersInternal( + parameters, + [done = done_event.get(), &result](RTCError error) { + result = error; + done->Set(); + }, + true); + done_event->Wait(rtc::Event::kForever); last_transaction_id_.reset(); return result; } +void RtpSenderBase::SetParametersAsync(const RtpParameters& parameters, + SetParametersCallback callback) { + RTC_DCHECK_RUN_ON(signaling_thread_); + RTC_DCHECK(callback); + TRACE_EVENT0("webrtc", "RtpSenderBase::SetParametersAsync"); + RTCError result = CheckSetParameters(parameters); + if (!result.ok()) { + webrtc::InvokeSetParametersCallback(callback, result); + return; + } + + SetParametersInternal( + parameters, + SignalingThreadCallback( + signaling_thread_, + [this, callback = std::move(callback)](RTCError error) mutable { + last_transaction_id_.reset(); + webrtc::InvokeSetParametersCallback(callback, error); + }), + false); +} + void RtpSenderBase::SetStreams(const std::vector& stream_ids) { set_stream_ids(stream_ids); if (set_streams_observer_) @@ -372,7 +468,7 @@ void RtpSenderBase::SetSsrc(uint32_t ssrc) { } current_parameters.degradation_preference = init_parameters_.degradation_preference; - media_channel_->SetRtpSendParameters(ssrc_, current_parameters); + media_channel_->SetRtpSendParameters(ssrc_, current_parameters, nullptr); init_parameters_.encodings.clear(); init_parameters_.degradation_preference = absl::nullopt; }); diff --git a/pc/rtp_sender.h b/pc/rtp_sender.h index 70a9c947aa..c11b2bd7d3 100644 --- a/pc/rtp_sender.h +++ b/pc/rtp_sender.h @@ -73,7 +73,9 @@ class RtpSenderInternal : public RtpSenderInterface { // `GetParameters` and `SetParameters` operate with a transactional model. // Allow access to get/set parameters without invalidating transaction id. virtual RtpParameters GetParametersInternal() const = 0; - virtual RTCError SetParametersInternal(const RtpParameters& parameters) = 0; + virtual void SetParametersInternal(const RtpParameters& parameters, + SetParametersCallback, + bool blocking) = 0; // GetParameters and SetParameters will remove deactivated simulcast layers // and restore them on SetParameters. This is probably a Bad Idea, but we @@ -130,11 +132,16 @@ class RtpSenderBase : public RtpSenderInternal, public ObserverInterface { RtpParameters GetParameters() const override; RTCError SetParameters(const RtpParameters& parameters) override; + void SetParametersAsync(const RtpParameters& parameters, + SetParametersCallback callback) override; // `GetParameters` and `SetParameters` operate with a transactional model. // Allow access to get/set parameters without invalidating transaction id. RtpParameters GetParametersInternal() const override; - RTCError SetParametersInternal(const RtpParameters& parameters) override; + void SetParametersInternal(const RtpParameters& parameters, + SetParametersCallback callback = nullptr, + bool blocking = true) override; + RTCError CheckSetParameters(const RtpParameters& parameters); RtpParameters GetParametersInternalWithAllLayers() const override; RTCError SetParametersInternalWithAllLayers( const RtpParameters& parameters) override; diff --git a/pc/rtp_sender_proxy.h b/pc/rtp_sender_proxy.h index a38c8af715..236ac10fa2 100644 --- a/pc/rtp_sender_proxy.h +++ b/pc/rtp_sender_proxy.h @@ -35,6 +35,10 @@ PROXY_CONSTMETHOD0(std::vector, stream_ids) PROXY_CONSTMETHOD0(std::vector, init_send_encodings) PROXY_CONSTMETHOD0(RtpParameters, GetParameters) PROXY_METHOD1(RTCError, SetParameters, const RtpParameters&) +PROXY_METHOD2(void, + SetParametersAsync, + const RtpParameters&, + SetParametersCallback) PROXY_CONSTMETHOD0(rtc::scoped_refptr, GetDtmfSender) PROXY_METHOD1(void, SetFrameEncryptor, diff --git a/pc/rtp_sender_receiver_unittest.cc b/pc/rtp_sender_receiver_unittest.cc index 5df57958c8..42072dece5 100644 --- a/pc/rtp_sender_receiver_unittest.cc +++ b/pc/rtp_sender_receiver_unittest.cc @@ -855,6 +855,20 @@ TEST_F(RtpSenderReceiverTest, AudioSenderCanSetParameters) { DestroyAudioRtpSender(); } +TEST_F(RtpSenderReceiverTest, AudioSenderCanSetParametersAsync) { + CreateAudioRtpSender(); + + RtpParameters params = audio_rtp_sender_->GetParameters(); + EXPECT_EQ(1u, params.encodings.size()); + absl::optional result; + audio_rtp_sender_->SetParametersAsync( + params, [&result](webrtc::RTCError error) { result = error; }); + run_loop_.Flush(); + EXPECT_TRUE(result->ok()); + + DestroyAudioRtpSender(); +} + TEST_F(RtpSenderReceiverTest, AudioSenderCanSetParametersBeforeNegotiation) { audio_rtp_sender_ = AudioRtpSender::Create(worker_thread_, /*id=*/"", nullptr, nullptr); @@ -865,8 +879,34 @@ TEST_F(RtpSenderReceiverTest, AudioSenderCanSetParametersBeforeNegotiation) { EXPECT_TRUE(audio_rtp_sender_->SetParameters(params).ok()); params = audio_rtp_sender_->GetParameters(); - EXPECT_TRUE(audio_rtp_sender_->SetParameters(params).ok()); EXPECT_EQ(params.encodings[0].max_bitrate_bps, 90000); + EXPECT_TRUE(audio_rtp_sender_->SetParameters(params).ok()); + + DestroyAudioRtpSender(); +} + +TEST_F(RtpSenderReceiverTest, + AudioSenderCanSetParametersAsyncBeforeNegotiation) { + audio_rtp_sender_ = + AudioRtpSender::Create(worker_thread_, /*id=*/"", nullptr, nullptr); + + absl::optional result; + RtpParameters params = audio_rtp_sender_->GetParameters(); + ASSERT_EQ(1u, params.encodings.size()); + params.encodings[0].max_bitrate_bps = 90000; + + audio_rtp_sender_->SetParametersAsync( + params, [&result](webrtc::RTCError error) { result = error; }); + run_loop_.Flush(); + EXPECT_TRUE(result->ok()); + + params = audio_rtp_sender_->GetParameters(); + EXPECT_EQ(params.encodings[0].max_bitrate_bps, 90000); + + audio_rtp_sender_->SetParametersAsync( + params, [&result](webrtc::RTCError error) { result = error; }); + run_loop_.Flush(); + EXPECT_TRUE(result->ok()); DestroyAudioRtpSender(); } @@ -941,6 +981,25 @@ TEST_F(RtpSenderReceiverTest, DestroyAudioRtpSender(); } +TEST_F(RtpSenderReceiverTest, + AudioSenderSetParametersAsyncInvalidatesTransactionId) { + CreateAudioRtpSender(); + + RtpParameters params = audio_rtp_sender_->GetParameters(); + EXPECT_EQ(1u, params.encodings.size()); + absl::optional result; + audio_rtp_sender_->SetParametersAsync( + params, [&result](webrtc::RTCError error) { result = error; }); + run_loop_.Flush(); + EXPECT_TRUE(result->ok()); + audio_rtp_sender_->SetParametersAsync( + params, [&result](webrtc::RTCError error) { result = error; }); + run_loop_.Flush(); + EXPECT_EQ(RTCErrorType::INVALID_STATE, result->type()); + + DestroyAudioRtpSender(); +} + TEST_F(RtpSenderReceiverTest, AudioSenderDetectTransactionIdModification) { CreateAudioRtpSender(); @@ -1047,6 +1106,20 @@ TEST_F(RtpSenderReceiverTest, VideoSenderCanSetParameters) { DestroyVideoRtpSender(); } +TEST_F(RtpSenderReceiverTest, VideoSenderCanSetParametersAsync) { + CreateVideoRtpSender(); + + RtpParameters params = video_rtp_sender_->GetParameters(); + EXPECT_EQ(1u, params.encodings.size()); + absl::optional result; + video_rtp_sender_->SetParametersAsync( + params, [&result](webrtc::RTCError error) { result = error; }); + run_loop_.Flush(); + EXPECT_TRUE(result->ok()); + + DestroyVideoRtpSender(); +} + TEST_F(RtpSenderReceiverTest, VideoSenderCanSetParametersBeforeNegotiation) { video_rtp_sender_ = VideoRtpSender::Create(worker_thread_, /*id=*/"", nullptr); @@ -1063,6 +1136,30 @@ TEST_F(RtpSenderReceiverTest, VideoSenderCanSetParametersBeforeNegotiation) { DestroyVideoRtpSender(); } +TEST_F(RtpSenderReceiverTest, + VideoSenderCanSetParametersAsyncBeforeNegotiation) { + video_rtp_sender_ = + VideoRtpSender::Create(worker_thread_, /*id=*/"", nullptr); + + absl::optional result; + RtpParameters params = video_rtp_sender_->GetParameters(); + ASSERT_EQ(1u, params.encodings.size()); + params.encodings[0].max_bitrate_bps = 90000; + video_rtp_sender_->SetParametersAsync( + params, [&result](webrtc::RTCError error) { result = error; }); + run_loop_.Flush(); + EXPECT_TRUE(result->ok()); + + params = video_rtp_sender_->GetParameters(); + EXPECT_EQ(params.encodings[0].max_bitrate_bps, 90000); + video_rtp_sender_->SetParametersAsync( + params, [&result](webrtc::RTCError error) { result = error; }); + run_loop_.Flush(); + EXPECT_TRUE(result->ok()); + + DestroyVideoRtpSender(); +} + TEST_F(RtpSenderReceiverTest, VideoSenderInitParametersMovedAfterNegotiation) { AddVideoTrack(false); @@ -1215,6 +1312,25 @@ TEST_F(RtpSenderReceiverTest, DestroyVideoRtpSender(); } +TEST_F(RtpSenderReceiverTest, + VideoSenderSetParametersAsyncInvalidatesTransactionId) { + CreateVideoRtpSender(); + + RtpParameters params = video_rtp_sender_->GetParameters(); + EXPECT_EQ(1u, params.encodings.size()); + absl::optional result; + video_rtp_sender_->SetParametersAsync( + params, [&result](webrtc::RTCError error) { result = error; }); + run_loop_.Flush(); + EXPECT_TRUE(result->ok()); + video_rtp_sender_->SetParametersAsync( + params, [&result](webrtc::RTCError error) { result = error; }); + run_loop_.Flush(); + EXPECT_EQ(RTCErrorType::INVALID_STATE, result->type()); + + DestroyVideoRtpSender(); +} + TEST_F(RtpSenderReceiverTest, VideoSenderDetectTransactionIdModification) { CreateVideoRtpSender(); @@ -1745,9 +1861,9 @@ TEST_F(RtpSenderReceiverTest, RtpParameters parameters = video_rtp_sender_->GetParameters(); RtpParameters new_parameters = video_rtp_sender_->GetParametersInternal(); new_parameters.encodings[0].active = false; - video_rtp_sender_->SetParametersInternal(new_parameters); + video_rtp_sender_->SetParametersInternal(new_parameters, nullptr, true); new_parameters.encodings[0].active = true; - video_rtp_sender_->SetParametersInternal(new_parameters); + video_rtp_sender_->SetParametersInternal(new_parameters, nullptr, true); parameters.encodings[0].active = false; EXPECT_TRUE(video_rtp_sender_->SetParameters(parameters).ok()); } diff --git a/pc/test/mock_rtp_sender_internal.h b/pc/test/mock_rtp_sender_internal.h index 8b9e75a7fb..07d80a9a36 100644 --- a/pc/test/mock_rtp_sender_internal.h +++ b/pc/test/mock_rtp_sender_internal.h @@ -52,9 +52,13 @@ class MockRtpSenderInternal : public RtpSenderInternal { (), (const, override)); MOCK_METHOD(RTCError, SetParameters, (const RtpParameters&), (override)); - MOCK_METHOD(RTCError, + MOCK_METHOD(void, + SetParametersAsync, + (const RtpParameters&, SetParametersCallback), + (override)); + MOCK_METHOD(void, SetParametersInternal, - (const RtpParameters&), + (const RtpParameters&, SetParametersCallback, bool blocking), (override)); MOCK_METHOD(RTCError, SetParametersInternalWithAllLayers, diff --git a/pc/test/mock_voice_media_channel.h b/pc/test/mock_voice_media_channel.h index 444ca5aed6..cd80ef87c9 100644 --- a/pc/test/mock_voice_media_channel.h +++ b/pc/test/mock_voice_media_channel.h @@ -71,7 +71,9 @@ class MockVoiceMediaChannel : public VoiceMediaChannel { (const, override)); MOCK_METHOD(webrtc::RTCError, SetRtpSendParameters, - (uint32_t ssrc, const webrtc::RtpParameters& parameters), + (uint32_t ssrc, + const webrtc::RtpParameters& parameters, + webrtc::SetParametersCallback callback), (override)); MOCK_METHOD( void, diff --git a/sdk/BUILD.gn b/sdk/BUILD.gn index 759682f1d4..a01cf96ebb 100644 --- a/sdk/BUILD.gn +++ b/sdk/BUILD.gn @@ -1062,11 +1062,13 @@ if (is_ios || is_mac) { ":videorendereradapter_objc", ":videosource_objc", ":videotoolbox_objc", + "../api:dtmf_sender_interface", "../api:libjingle_peerconnection_api", "../api:media_stream_interface", "../api:rtc_event_log_output_file", "../api:rtc_stats_api", "../api:rtp_parameters", + "../api:rtp_sender_interface", "../api:scoped_refptr", "../api/audio_codecs:audio_codecs_api", "../api/audio_codecs:builtin_audio_decoder_factory", diff --git a/sdk/android/BUILD.gn b/sdk/android/BUILD.gn index 5c4280328c..8ef08a3a5b 100644 --- a/sdk/android/BUILD.gn +++ b/sdk/android/BUILD.gn @@ -765,10 +765,12 @@ if (current_os == "linux" || is_android) { ":native_api_stacktrace", "..:media_constraints", "../../api:callfactory_api", + "../../api:dtmf_sender_interface", "../../api:libjingle_peerconnection_api", "../../api:media_stream_interface", "../../api:rtc_event_log_output_file", "../../api:rtp_parameters", + "../../api:rtp_sender_interface", "../../api:turn_customizer", "../../api/crypto:options", "../../api/rtc_event_log:rtc_event_log_factory", diff --git a/video/BUILD.gn b/video/BUILD.gn index 7990d207ef..8c755b07b6 100644 --- a/video/BUILD.gn +++ b/video/BUILD.gn @@ -15,7 +15,9 @@ rtc_library("video_stream_encoder_interface") { ] deps = [ "../api:fec_controller_api", + "../api:rtc_error", "../api:rtp_parameters", + "../api:rtp_sender_interface", "../api:scoped_refptr", "../api/adaptation:resource_adaptation_api", "../api/units:data_rate", @@ -407,6 +409,7 @@ rtc_library("video_stream_encoder_impl") { ":video_stream_encoder_interface", "../api:field_trials_view", "../api:rtp_parameters", + "../api:rtp_sender_interface", "../api:sequence_checker", "../api/adaptation:resource_adaptation_api", "../api/task_queue:pending_task_safety_flag", @@ -426,6 +429,7 @@ rtc_library("video_stream_encoder_impl") { "../api/video_codecs:video_codecs_api", "../call/adaptation:resource_adaptation", "../common_video", + "../media:rtc_media_base", "../modules:module_api_public", "../modules/video_coding", "../modules/video_coding:video_codec_interface", @@ -469,6 +473,7 @@ rtc_library("video_stream_encoder_impl") { "//third_party/abseil-cpp/absl/algorithm:container", "//third_party/abseil-cpp/absl/base:core_headers", "//third_party/abseil-cpp/absl/cleanup", + "//third_party/abseil-cpp/absl/container:inlined_vector", "//third_party/abseil-cpp/absl/types:optional", ] } diff --git a/video/test/mock_video_stream_encoder.h b/video/test/mock_video_stream_encoder.h index 2f982158f0..946f45cc76 100644 --- a/video/test/mock_video_stream_encoder.h +++ b/video/test/mock_video_stream_encoder.h @@ -55,12 +55,20 @@ class MockVideoStreamEncoder : public VideoStreamEncoderInterface { MOCK_METHOD(void, MockedConfigureEncoder, (const VideoEncoderConfig&, size_t)); + MOCK_METHOD(void, + MockedConfigureEncoder, + (const VideoEncoderConfig&, size_t, SetParametersCallback)); // gtest generates implicit copy which is not allowed on VideoEncoderConfig, // so we can't mock ConfigureEncoder directly. void ConfigureEncoder(VideoEncoderConfig config, size_t max_data_payload_length) { MockedConfigureEncoder(config, max_data_payload_length); } + void ConfigureEncoder(VideoEncoderConfig config, + size_t max_data_payload_length, + SetParametersCallback) { + MockedConfigureEncoder(config, max_data_payload_length); + } }; } // namespace webrtc diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc index bf5f99aca5..30598c1103 100644 --- a/video/video_send_stream.cc +++ b/video/video_send_stream.cc @@ -301,11 +301,17 @@ void VideoSendStream::SetSource( } void VideoSendStream::ReconfigureVideoEncoder(VideoEncoderConfig config) { + ReconfigureVideoEncoder(std::move(config), nullptr); +} + +void VideoSendStream::ReconfigureVideoEncoder(VideoEncoderConfig config, + SetParametersCallback callback) { RTC_DCHECK_RUN_ON(&thread_checker_); RTC_DCHECK_EQ(content_type_, config.content_type); video_stream_encoder_->ConfigureEncoder( std::move(config), - config_.rtp.max_packet_size - CalculateMaxHeaderSize(config_.rtp)); + config_.rtp.max_packet_size - CalculateMaxHeaderSize(config_.rtp), + std::move(callback)); } VideoSendStream::Stats VideoSendStream::GetStats() { diff --git a/video/video_send_stream.h b/video/video_send_stream.h index 4174d2e5c6..a052b856ec 100644 --- a/video/video_send_stream.h +++ b/video/video_send_stream.h @@ -89,7 +89,9 @@ class VideoSendStream : public webrtc::VideoSendStream { void SetSource(rtc::VideoSourceInterface* source, const DegradationPreference& degradation_preference) override; - void ReconfigureVideoEncoder(VideoEncoderConfig) override; + void ReconfigureVideoEncoder(VideoEncoderConfig config) override; + void ReconfigureVideoEncoder(VideoEncoderConfig config, + SetParametersCallback callback) override; Stats GetStats() override; void StopPermanentlyAndGetRtpStates(RtpStateMap* rtp_state_map, diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index de40ccf348..994173228b 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -35,6 +35,7 @@ #include "call/adaptation/resource_adaptation_processor.h" #include "call/adaptation/video_source_restrictions.h" #include "call/adaptation/video_stream_adapter.h" +#include "media/base/media_channel.h" #include "modules/video_coding/include/video_codec_initializer.h" #include "modules/video_coding/svc/svc_rate_allocator.h" #include "modules/video_coding/utility/vp8_constants.h" @@ -879,9 +880,16 @@ void VideoStreamEncoder::SetStartBitrate(int start_bitrate_bps) { void VideoStreamEncoder::ConfigureEncoder(VideoEncoderConfig config, size_t max_data_payload_length) { + ConfigureEncoder(std::move(config), max_data_payload_length, nullptr); +} + +void VideoStreamEncoder::ConfigureEncoder(VideoEncoderConfig config, + size_t max_data_payload_length, + SetParametersCallback callback) { RTC_DCHECK_RUN_ON(worker_queue_); encoder_queue_.PostTask( - [this, config = std::move(config), max_data_payload_length]() mutable { + [this, config = std::move(config), max_data_payload_length, + callback = std::move(callback)]() mutable { RTC_DCHECK_RUN_ON(&encoder_queue_); RTC_DCHECK(sink_); RTC_LOG(LS_INFO) << "ConfigureEncoder requested."; @@ -912,7 +920,13 @@ void VideoStreamEncoder::ConfigureEncoder(VideoEncoderConfig config, // minimize the number of reconfigurations. The codec configuration // depends on incoming video frame size. if (last_frame_info_) { + if (callback) { + encoder_configuration_callbacks_.push_back(std::move(callback)); + } + ReconfigureEncoder(); + } else { + webrtc::InvokeSetParametersCallback(callback, webrtc::RTCError::OK()); } }); } @@ -1369,6 +1383,8 @@ void VideoStreamEncoder::ReconfigureEncoder() { stream_resource_manager_.ConfigureQualityScaler(info); stream_resource_manager_.ConfigureBandwidthQualityScaler(info); + webrtc::RTCError encoder_configuration_result = webrtc::RTCError::OK(); + if (!encoder_initialized_) { RTC_LOG(LS_WARNING) << "Failed to initialize " << CodecTypeToPayloadString(codec.codecType) @@ -1378,8 +1394,19 @@ void VideoStreamEncoder::ReconfigureEncoder() { if (switch_encoder_on_init_failures_) { RequestEncoderSwitch(); + } else { + encoder_configuration_result = + webrtc::RTCError(RTCErrorType::UNSUPPORTED_OPERATION); } } + + if (!encoder_configuration_callbacks_.empty()) { + for (auto& callback : encoder_configuration_callbacks_) { + webrtc::InvokeSetParametersCallback(callback, + encoder_configuration_result); + } + encoder_configuration_callbacks_.clear(); + } } void VideoStreamEncoder::RequestEncoderSwitch() { diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index e94c369a19..ccff3ffefd 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -17,8 +17,10 @@ #include #include +#include "absl/container/inlined_vector.h" #include "api/adaptation/resource.h" #include "api/field_trials_view.h" +#include "api/rtp_sender_interface.h" #include "api/sequence_checker.h" #include "api/task_queue/pending_task_safety_flag.h" #include "api/units/data_rate.h" @@ -106,6 +108,9 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, void ConfigureEncoder(VideoEncoderConfig config, size_t max_data_payload_length) override; + void ConfigureEncoder(VideoEncoderConfig config, + size_t max_data_payload_length, + SetParametersCallback callback) override; // Permanently stop encoding. After this method has returned, it is // guaranteed that no encoded frames will be delivered to the sink. @@ -302,6 +307,8 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, // Set when configuration must create a new encoder object, e.g., // because of a codec change. bool pending_encoder_creation_ RTC_GUARDED_BY(&encoder_queue_); + absl::InlinedVector encoder_configuration_callbacks_ + RTC_GUARDED_BY(&encoder_queue_); absl::optional last_frame_info_ RTC_GUARDED_BY(&encoder_queue_); diff --git a/video/video_stream_encoder_interface.h b/video/video_stream_encoder_interface.h index e716572e68..25190aa474 100644 --- a/video/video_stream_encoder_interface.h +++ b/video/video_stream_encoder_interface.h @@ -15,7 +15,9 @@ #include "api/adaptation/resource.h" #include "api/fec_controller_override.h" +#include "api/rtc_error.h" #include "api/rtp_parameters.h" // For DegradationPreference. +#include "api/rtp_sender_interface.h" #include "api/scoped_refptr.h" #include "api/units/data_rate.h" #include "api/video/video_bitrate_allocator.h" @@ -131,6 +133,9 @@ class VideoStreamEncoderInterface { // packetization for H.264. virtual void ConfigureEncoder(VideoEncoderConfig config, size_t max_data_payload_length) = 0; + virtual void ConfigureEncoder(VideoEncoderConfig config, + size_t max_data_payload_length, + SetParametersCallback callback) = 0; // Permanently stop encoding. After this method has returned, it is // guaranteed that no encoded frames will be delivered to the sink. diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 5271654ac9..e45b2ef887 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -885,7 +885,7 @@ class VideoStreamEncoderTest : public ::testing::Test { &video_source_, webrtc::DegradationPreference::MAINTAIN_FRAMERATE); video_stream_encoder_->SetStartBitrate(kTargetBitrate.bps()); video_stream_encoder_->ConfigureEncoder(std::move(video_encoder_config), - kMaxPayloadLength); + kMaxPayloadLength, nullptr); video_stream_encoder_->WaitUntilTaskQueueIsIdle(); }