From 6545516a14c2c45c0fcf0bbe2d4d66b1a42f889d Mon Sep 17 00:00:00 2001 From: Jonas Oreland Date: Wed, 8 Jun 2022 11:25:46 +0200 Subject: [PATCH] RtpSenderInterface::SetEncoderSelector This cl/ adds a way of setting an EncoderSelector on a specific RtpSenderInterface. This makes it possible to easily use different EncoderSelector on different streams within the same or different PeerConnections. The cl/ is almost identical to the impl. of RtpSenderInterface::SetFrameEncryptor. Iff a EncoderSelector is set on the RtpSender, it will take precedence over the VideoEncoderFactory::GetEncoderSelector. Bug: webrtc:14122 Change-Id: Ief4f7c06df7f1ef4ce3245de304a48e9de0ad587 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264542 Reviewed-by: Rasmus Brandt Commit-Queue: Jonas Oreland Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#37150} --- api/BUILD.gn | 12 ++++++ api/rtp_sender_interface.h | 8 ++++ api/test/mock_encoder_selector.h | 42 ++++++++++++++++++++ api/video_codecs/video_encoder_factory.h | 21 +++++++++- call/video_send_stream.h | 5 +++ media/BUILD.gn | 1 + media/base/media_channel.h | 8 ++++ media/engine/webrtc_video_engine.cc | 24 +++++++++++ media/engine/webrtc_video_engine.h | 12 ++++++ media/engine/webrtc_video_engine_unittest.cc | 26 ++++++++++++ pc/BUILD.gn | 1 + pc/peer_connection_integrationtest.cc | 30 ++++++++++++++ pc/rtp_sender.cc | 20 ++++++++++ pc/rtp_sender.h | 8 ++++ pc/rtp_sender_proxy.h | 4 ++ video/video_send_stream.cc | 9 +++-- video/video_stream_encoder.cc | 12 +++++- video/video_stream_encoder.h | 12 +++++- 18 files changed, 247 insertions(+), 8 deletions(-) create mode 100644 api/test/mock_encoder_selector.h diff --git a/api/BUILD.gn b/api/BUILD.gn index 2f46c6931b..afad7698ce 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -217,6 +217,7 @@ rtc_library("libjingle_peerconnection_api") { "video:video_bitrate_allocator_factory", "video:video_frame", "video:video_rtp_headers", + "video_codecs:video_codecs_api", # Basically, don't add stuff here. You might break sensitive downstream # targets like pnacl. API should not depend on anything outside of this @@ -968,6 +969,17 @@ if (rtc_include_tests) { ] } + rtc_library("mock_encoder_selector") { + visibility = [ "*" ] + testonly = true + sources = [ "test/mock_encoder_selector.h" ] + deps = [ + ":libjingle_peerconnection_api", + "../api/video_codecs:video_codecs_api", + "../test:test_support", + ] + } + rtc_library("fake_frame_encryptor") { visibility = [ "*" ] testonly = true diff --git a/api/rtp_sender_interface.h b/api/rtp_sender_interface.h index 9ffad68644..48ea864e68 100644 --- a/api/rtp_sender_interface.h +++ b/api/rtp_sender_interface.h @@ -14,6 +14,7 @@ #ifndef API_RTP_SENDER_INTERFACE_H_ #define API_RTP_SENDER_INTERFACE_H_ +#include #include #include @@ -26,6 +27,7 @@ #include "api/rtc_error.h" #include "api/rtp_parameters.h" #include "api/scoped_refptr.h" +#include "api/video_codecs/video_encoder_factory.h" #include "rtc_base/ref_count.h" #include "rtc_base/system/rtc_export.h" @@ -96,6 +98,12 @@ class RTC_EXPORT RtpSenderInterface : public rtc::RefCountInterface { virtual void SetEncoderToPacketizerFrameTransformer( rtc::scoped_refptr frame_transformer); + // Sets a user defined encoder selector. + // Overrides selector that is (optionally) provided by VideoEncoderFactory. + virtual void SetEncoderSelector( + std::unique_ptr + encoder_selector) {} + protected: ~RtpSenderInterface() override = default; }; diff --git a/api/test/mock_encoder_selector.h b/api/test/mock_encoder_selector.h new file mode 100644 index 0000000000..2e018d57ba --- /dev/null +++ b/api/test/mock_encoder_selector.h @@ -0,0 +1,42 @@ +/* + * Copyright 2018 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. + */ + +#ifndef API_TEST_MOCK_ENCODER_SELECTOR_H_ +#define API_TEST_MOCK_ENCODER_SELECTOR_H_ + +#include "api/video_codecs/video_encoder_factory.h" +#include "test/gmock.h" + +namespace webrtc { + +class MockEncoderSelector + : public VideoEncoderFactory::EncoderSelectorInterface { + public: + MOCK_METHOD(void, + OnCurrentEncoder, + (const SdpVideoFormat& format), + (override)); + + MOCK_METHOD(absl::optional, + OnAvailableBitrate, + (const DataRate& rate), + (override)); + + MOCK_METHOD(absl::optional, + OnResolutionChange, + (const RenderResolution& resolution), + (override)); + + MOCK_METHOD(absl::optional, OnEncoderBroken, (), (override)); +}; + +} // namespace webrtc + +#endif // API_TEST_MOCK_ENCODER_SELECTOR_H_ diff --git a/api/video_codecs/video_encoder_factory.h b/api/video_codecs/video_encoder_factory.h index 2914a41518..d28a2a4035 100644 --- a/api/video_codecs/video_encoder_factory.h +++ b/api/video_codecs/video_encoder_factory.h @@ -34,7 +34,10 @@ class VideoEncoderFactory { }; // An injectable class that is continuously updated with encoding conditions - // and selects the best encoder given those conditions. + // and selects the best encoder given those conditions. An implementation is + // typically stateful to avoid toggling between different encoders, which is + // costly due to recreation of objects, a new codec will always start with a + // key-frame. class EncoderSelectorInterface { public: virtual ~EncoderSelectorInterface() {} @@ -96,6 +99,22 @@ class VideoEncoderFactory { virtual std::unique_ptr CreateVideoEncoder( const SdpVideoFormat& format) = 0; + // This method creates a EncoderSelector to use for a VideoSendStream. + // (and hence should probably been called CreateEncoderSelector()). + // + // Note: This method is unsuitable if encoding several streams that + // are using same VideoEncoderFactory (either by several streams in one + // PeerConnection or streams with different PeerConnection but same + // PeerConnectionFactory). This is due to the fact that the method is not + // given any stream identifier, nor is the EncoderSelectorInterface given any + // stream identifiers, i.e one does not know which stream is being encoded + // with help of the selector. + // + // In such scenario, the `RtpSenderInterface::SetEncoderSelector` is + // recommended. + // + // TODO(bugs.webrtc.org:14122): Deprecate and remove in favor of + // `RtpSenderInterface::SetEncoderSelector`. virtual std::unique_ptr GetEncoderSelector() const { return nullptr; } diff --git a/call/video_send_stream.h b/call/video_send_stream.h index 356d8c8099..1202a23666 100644 --- a/call/video_send_stream.h +++ b/call/video_send_stream.h @@ -190,6 +190,11 @@ class VideoSendStream { // default. rtc::scoped_refptr frame_encryptor; + // An optional encoder selector provided by the user. + // Overrides VideoEncoderFactory::GetEncoderSelector(). + // Owned by RtpSenderBase. + VideoEncoderFactory::EncoderSelectorInterface* encoder_selector = nullptr; + // Per PeerConnection cryptography options. CryptoOptions crypto_options; diff --git a/media/BUILD.gn b/media/BUILD.gn index ac6c378c64..93702002f3 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -587,6 +587,7 @@ if (rtc_include_tests) { ":rtc_simulcast_encoder_adapter", "../api:create_simulcast_test_fixture_api", "../api:libjingle_peerconnection_api", + "../api:mock_encoder_selector", "../api:mock_video_bitrate_allocator", "../api:mock_video_bitrate_allocator_factory", "../api:mock_video_codec_factory", diff --git a/media/base/media_channel.h b/media/base/media_channel.h index bb07f15b9d..ec78ec8c25 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -34,6 +34,7 @@ #include "api/video/video_source_interface.h" #include "api/video/video_timing.h" #include "api/video_codecs/video_encoder_config.h" +#include "api/video_codecs/video_encoder_factory.h" #include "call/video_receive_stream.h" #include "common_video/include/quality_limitation_reason.h" #include "media/base/codec.h" @@ -244,6 +245,13 @@ class MediaChannel { // Enable network condition based codec switching. virtual void SetVideoCodecSwitchingEnabled(bool enabled); + // note: The encoder_selector object must remain valid for the lifetime of the + // MediaChannel, unless replaced. + virtual void SetEncoderSelector( + uint32_t ssrc, + webrtc::VideoEncoderFactory::EncoderSelectorInterface* encoder_selector) { + } + // Base method to send packet using NetworkInterface. bool SendPacket(rtc::CopyOnWriteBuffer* packet, const rtc::PacketOptions& options); diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 2751f94fe5..278f87bc1f 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -1942,6 +1942,18 @@ void WebRtcVideoChannel::SetFrameEncryptor( } } +void WebRtcVideoChannel::SetEncoderSelector( + uint32_t ssrc, + webrtc::VideoEncoderFactory::EncoderSelectorInterface* encoder_selector) { + RTC_DCHECK_RUN_ON(&thread_checker_); + auto matching_stream = send_streams_.find(ssrc); + if (matching_stream != send_streams_.end()) { + matching_stream->second->SetEncoderSelector(encoder_selector); + } else { + RTC_LOG(LS_ERROR) << "No stream found to attach encoder selector"; + } +} + void WebRtcVideoChannel::SetVideoCodecSwitchingEnabled(bool enabled) { RTC_DCHECK_RUN_ON(&thread_checker_); allow_codec_switching_ = enabled; @@ -2388,6 +2400,18 @@ void WebRtcVideoChannel::WebRtcVideoSendStream::SetFrameEncryptor( } } +void WebRtcVideoChannel::WebRtcVideoSendStream::SetEncoderSelector( + webrtc::VideoEncoderFactory::EncoderSelectorInterface* encoder_selector) { + RTC_DCHECK_RUN_ON(&thread_checker_); + parameters_.config.encoder_selector = encoder_selector; + if (stream_) { + RTC_LOG(LS_INFO) + << "RecreateWebRtcStream (send) because of SetEncoderSelector, ssrc=" + << parameters_.config.rtp.ssrcs[0]; + RecreateWebRtcStream(); + } +} + void WebRtcVideoChannel::WebRtcVideoSendStream::UpdateSendState() { RTC_DCHECK_RUN_ON(&thread_checker_); if (sending_) { diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index e50276ca15..b2780c4af5 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -191,6 +191,12 @@ class WebRtcVideoChannel : public VideoMediaChannel, rtc::scoped_refptr frame_encryptor) override; + // note: The encoder_selector object must remain valid for the lifetime of the + // MediaChannel, unless replaced. + void SetEncoderSelector(uint32_t ssrc, + webrtc::VideoEncoderFactory::EncoderSelectorInterface* + encoder_selector) override; + void SetVideoCodecSwitchingEnabled(bool enabled) override; bool SetBaseMinimumPlayoutDelayMs(uint32_t ssrc, int delay_ms) override; @@ -351,6 +357,12 @@ class WebRtcVideoChannel : public VideoMediaChannel, bool SetVideoSend(const VideoOptions* options, rtc::VideoSourceInterface* source); + // note: The encoder_selector object must remain valid for the lifetime of + // the MediaChannel, unless replaced. + void SetEncoderSelector( + webrtc::VideoEncoderFactory::EncoderSelectorInterface* + encoder_selector); + void SetSend(bool send); const std::vector& GetSsrcs() const; diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 52f5ca676d..b93b4f7484 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -23,6 +23,7 @@ #include "api/rtc_event_log/rtc_event_log.h" #include "api/rtp_parameters.h" #include "api/task_queue/default_task_queue_factory.h" +#include "api/test/mock_encoder_selector.h" #include "api/test/mock_video_bitrate_allocator.h" #include "api/test/mock_video_bitrate_allocator_factory.h" #include "api/test/mock_video_decoder_factory.h" @@ -8983,4 +8984,29 @@ TEST_F(WebRtcVideoChannelTest, SetsRidsOnSendStream) { EXPECT_THAT(config.rtp.rids, ElementsAreArray(rids)); } +TEST_F(WebRtcVideoChannelBaseTest, EncoderSelectorSwitchCodec) { + VideoCodec vp9 = GetEngineCodec("VP9"); + + cricket::VideoSendParameters parameters; + parameters.codecs.push_back(GetEngineCodec("VP8")); + parameters.codecs.push_back(vp9); + EXPECT_TRUE(channel_->SetSendParameters(parameters)); + channel_->SetSend(true); + + VideoCodec codec; + ASSERT_TRUE(channel_->GetSendCodec(&codec)); + EXPECT_EQ("VP8", codec.name); + + webrtc::MockEncoderSelector encoder_selector; + EXPECT_CALL(encoder_selector, OnAvailableBitrate) + .WillRepeatedly(Return(webrtc::SdpVideoFormat("VP9"))); + + channel_->SetEncoderSelector(kSsrc, &encoder_selector); + + rtc::Thread::Current()->ProcessMessages(30); + + ASSERT_TRUE(channel_->GetSendCodec(&codec)); + EXPECT_EQ("VP9", codec.name); +} + } // namespace cricket diff --git a/pc/BUILD.gn b/pc/BUILD.gn index d4acaed00d..385ecf254c 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -2343,6 +2343,7 @@ if (rtc_include_tests && !build_with_chromium) { "../api:libjingle_logging_api", "../api:libjingle_peerconnection_api", "../api:media_stream_interface", + "../api:mock_encoder_selector", "../api:mock_video_track", "../api:packet_socket_factory", "../api:priority", diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc index 9f40648214..6358bce074 100644 --- a/pc/peer_connection_integrationtest.cc +++ b/pc/peer_connection_integrationtest.cc @@ -49,6 +49,7 @@ #include "api/stats/rtc_stats.h" #include "api/stats/rtc_stats_report.h" #include "api/stats/rtcstats_objects.h" +#include "api/test/mock_encoder_selector.h" #include "api/transport/rtp/rtp_source.h" #include "api/uma_metrics.h" #include "api/units/time_delta.h" @@ -3593,6 +3594,35 @@ TEST_F(PeerConnectionIntegrationTestUnifiedPlan, callee_track->state()); } +TEST_P(PeerConnectionIntegrationTest, EndToEndRtpSenderVideoEncoderSelector) { + ASSERT_TRUE( + CreateOneDirectionalPeerConnectionWrappers(/*caller_to_callee=*/true)); + ConnectFakeSignaling(); + // Add one-directional video, from caller to callee. + rtc::scoped_refptr caller_track = + caller()->CreateLocalVideoTrack(); + auto sender = caller()->AddTrack(caller_track); + PeerConnectionInterface::RTCOfferAnswerOptions options; + options.offer_to_receive_video = 0; + caller()->SetOfferAnswerOptions(options); + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + ASSERT_EQ(callee()->pc()->GetReceivers().size(), 1u); + + std::unique_ptr encoder_selector = + std::make_unique(); + EXPECT_CALL(*encoder_selector, OnCurrentEncoder); + + sender->SetEncoderSelector(std::move(encoder_selector)); + + // Expect video to be received in one direction. + MediaExpectations media_expectations; + media_expectations.CallerExpectsNoVideo(); + media_expectations.CalleeExpectsSomeVideo(); + + EXPECT_TRUE(ExpectNewFrames(media_expectations)); +} + } // namespace } // namespace webrtc diff --git a/pc/rtp_sender.cc b/pc/rtp_sender.cc index 6c4c57893c..d8d4b520eb 100644 --- a/pc/rtp_sender.cc +++ b/pc/rtp_sender.cc @@ -132,6 +132,23 @@ void RtpSenderBase::SetFrameEncryptor( } } +void RtpSenderBase::SetEncoderSelector( + std::unique_ptr + encoder_selector) { + RTC_DCHECK_RUN_ON(signaling_thread_); + encoder_selector_ = std::move(encoder_selector); + SetEncoderSelectorOnChannel(); +} + +void RtpSenderBase::SetEncoderSelectorOnChannel() { + RTC_DCHECK_RUN_ON(signaling_thread_); + if (media_channel_ && ssrc_ && !stopped_) { + worker_thread_->Invoke(RTC_FROM_HERE, [&] { + media_channel_->SetEncoderSelector(ssrc_, encoder_selector_.get()); + }); + } +} + void RtpSenderBase::SetMediaChannel(cricket::MediaChannel* media_channel) { RTC_DCHECK(media_channel == nullptr || media_channel->media_type() == media_type()); @@ -321,6 +338,9 @@ void RtpSenderBase::SetSsrc(uint32_t ssrc) { if (frame_transformer_) { SetEncoderToPacketizerFrameTransformer(frame_transformer_); } + if (encoder_selector_) { + SetEncoderSelectorOnChannel(); + } } void RtpSenderBase::Stop() { diff --git a/pc/rtp_sender.h b/pc/rtp_sender.h index 569a6007d3..4c1d307a9a 100644 --- a/pc/rtp_sender.h +++ b/pc/rtp_sender.h @@ -185,6 +185,12 @@ class RtpSenderBase : public RtpSenderInternal, public ObserverInterface { void SetEncoderToPacketizerFrameTransformer( rtc::scoped_refptr frame_transformer) override; + void SetEncoderSelector( + std::unique_ptr + encoder_selector) override; + + void SetEncoderSelectorOnChannel(); + void SetTransceiverAsStopped() override { RTC_DCHECK_RUN_ON(signaling_thread_); is_transceiver_stopped_ = true; @@ -247,6 +253,8 @@ class RtpSenderBase : public RtpSenderInternal, public ObserverInterface { SetStreamsObserver* set_streams_observer_ = nullptr; rtc::scoped_refptr frame_transformer_; + std::unique_ptr + encoder_selector_; }; // LocalAudioSinkAdapter receives data callback as a sink to the local diff --git a/pc/rtp_sender_proxy.h b/pc/rtp_sender_proxy.h index 2f8fe2c0bf..140b5ff97e 100644 --- a/pc/rtp_sender_proxy.h +++ b/pc/rtp_sender_proxy.h @@ -11,6 +11,7 @@ #ifndef PC_RTP_SENDER_PROXY_H_ #define PC_RTP_SENDER_PROXY_H_ +#include #include #include @@ -44,6 +45,9 @@ PROXY_METHOD1(void, SetStreams, const std::vector&) PROXY_METHOD1(void, SetEncoderToPacketizerFrameTransformer, rtc::scoped_refptr) +PROXY_METHOD1(void, + SetEncoderSelector, + std::unique_ptr) END_PROXY_MAP(RtpSender) } // namespace webrtc diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc index 6f6a2d3d38..86fd0d9d6f 100644 --- a/video/video_send_stream.cc +++ b/video/video_send_stream.cc @@ -115,7 +115,8 @@ std::unique_ptr CreateVideoStreamEncoder( const VideoStreamEncoderSettings& encoder_settings, VideoStreamEncoder::BitrateAllocationCallbackType bitrate_allocation_callback_type, - const FieldTrialsView& field_trials) { + const FieldTrialsView& field_trials, + webrtc::VideoEncoderFactory::EncoderSelectorInterface* encoder_selector) { std::unique_ptr encoder_queue = task_queue_factory->CreateTaskQueue("EncoderQueue", TaskQueueFactory::Priority::NORMAL); @@ -125,7 +126,8 @@ std::unique_ptr CreateVideoStreamEncoder( std::make_unique(stats_proxy, field_trials), FrameCadenceAdapterInterface::Create(clock, encoder_queue_ptr, field_trials), - std::move(encoder_queue), bitrate_allocation_callback_type, field_trials); + std::move(encoder_queue), bitrate_allocation_callback_type, field_trials, + encoder_selector); } } // namespace @@ -160,7 +162,8 @@ VideoSendStream::VideoSendStream( &stats_proxy_, config_.encoder_settings, GetBitrateAllocationCallbackType(config_, field_trials), - field_trials)), + field_trials, + config_.encoder_selector)), encoder_feedback_( clock, config_.rtp.ssrcs, diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 31a9e25d6e..791bdad0e9 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -615,7 +615,8 @@ VideoStreamEncoder::VideoStreamEncoder( std::unique_ptr encoder_queue, BitrateAllocationCallbackType allocation_cb_type, - const FieldTrialsView& field_trials) + const FieldTrialsView& field_trials, + webrtc::VideoEncoderFactory::EncoderSelectorInterface* encoder_selector) : field_trials_(field_trials), worker_queue_(TaskQueueBase::Current()), number_of_cores_(number_of_cores), @@ -623,7 +624,14 @@ VideoStreamEncoder::VideoStreamEncoder( settings_(settings), allocation_cb_type_(allocation_cb_type), rate_control_settings_(RateControlSettings::ParseFromFieldTrials()), - encoder_selector_(settings.encoder_factory->GetEncoderSelector()), + encoder_selector_from_constructor_(encoder_selector), + encoder_selector_from_factory_( + encoder_selector_from_constructor_ + ? nullptr + : settings.encoder_factory->GetEncoderSelector()), + encoder_selector_(encoder_selector_from_constructor_ + ? encoder_selector_from_constructor_ + : encoder_selector_from_factory_.get()), encoder_stats_observer_(encoder_stats_observer), cadence_callback_(*this), frame_cadence_adapter_(std::move(frame_cadence_adapter)), diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index c68a153e24..34d7895f7d 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -81,7 +81,9 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, std::unique_ptr encoder_queue, BitrateAllocationCallbackType allocation_cb_type, - const FieldTrialsView& field_trials); + const FieldTrialsView& field_trials, + webrtc::VideoEncoderFactory::EncoderSelectorInterface* encoder_selector = + nullptr); ~VideoStreamEncoder() override; VideoStreamEncoder(const VideoStreamEncoder&) = delete; @@ -262,8 +264,14 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, const BitrateAllocationCallbackType allocation_cb_type_; const RateControlSettings rate_control_settings_; + webrtc::VideoEncoderFactory::EncoderSelectorInterface* const + encoder_selector_from_constructor_; std::unique_ptr const - encoder_selector_; + encoder_selector_from_factory_; + // Pointing to either encoder_selector_from_constructor_ or + // encoder_selector_from_factory_ but can be nullptr. + VideoEncoderFactory::EncoderSelectorInterface* const encoder_selector_; + VideoStreamEncoderObserver* const encoder_stats_observer_; // Adapter that avoids public inheritance of the cadence adapter's callback // interface.