From 8f01c4e1b654028fd69e5d21721db47c99c7ae4f Mon Sep 17 00:00:00 2001 From: Elad Alon Date: Fri, 28 Jun 2019 15:19:43 +0200 Subject: [PATCH] Define FecControllerOverride and plumb it down to VideoEncoder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The purpose of this interface is to allow VideoEncoder to override the bandwidth allocation set by FecController in RtpVideoSender. This CL defines the interface and sends it down to VideoSender. Two upcoming CLs will: 1. Make LibvpxVp8Encoder pass it on to the (injectable) FrameBufferController, where it might be put to good use. 2. Modify RtpVideoSender to respond to the message sent to it via this API. TBR=kwiberg@webrtc.org Bug: webrtc:10769 Change-Id: I2ef82f0ddcde7fd078e32d8aabf6efe43e0f7f8a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/143962 Commit-Queue: Elad Alon Reviewed-by: Erik Språng Reviewed-by: Rasmus Brandt Cr-Commit-Position: refs/heads/master@{#28416} --- api/BUILD.gn | 1 + api/fec_controller_override.h | 28 +++++++++++++++++++ api/test/mock_video_encoder.h | 2 ++ api/video/BUILD.gn | 1 + api/video/video_stream_encoder_interface.h | 8 ++++++ api/video_codecs/BUILD.gn | 2 ++ api/video_codecs/test/BUILD.gn | 1 + ...oder_software_fallback_wrapper_unittest.cc | 6 ++++ api/video_codecs/video_encoder.cc | 3 ++ api/video_codecs/video_encoder.h | 7 +++++ ...video_encoder_software_fallback_wrapper.cc | 16 +++++++++++ call/rtp_video_sender.cc | 4 +++ call/rtp_video_sender.h | 4 +++ call/rtp_video_sender_interface.h | 7 ++++- media/BUILD.gn | 2 ++ media/engine/encoder_simulcast_proxy.cc | 5 ++++ media/engine/encoder_simulcast_proxy.h | 2 ++ media/engine/fake_webrtc_video_engine.cc | 5 ++++ media/engine/fake_webrtc_video_engine.h | 3 ++ media/engine/simulcast_encoder_adapter.cc | 5 ++++ media/engine/simulcast_encoder_adapter.h | 5 ++++ .../simulcast_encoder_adapter_unittest.cc | 3 ++ modules/video_coding/BUILD.gn | 3 ++ .../include/multiplex_encoder_adapter.h | 5 ++++ .../multiplex/multiplex_encoder_adapter.cc | 5 ++++ .../codecs/vp8/libvpx_vp8_encoder.cc | 7 ++++- .../codecs/vp8/libvpx_vp8_encoder.h | 4 +++ modules/video_coding/codecs/vp9/vp9_impl.cc | 5 ++++ modules/video_coding/codecs/vp9/vp9_impl.h | 6 +++- test/BUILD.gn | 1 + test/configurable_frame_size_encoder.cc | 5 ++++ test/configurable_frame_size_encoder.h | 3 ++ test/fake_encoder.cc | 5 ++++ test/fake_encoder.h | 4 +++ .../video/quality_analyzing_video_encoder.cc | 5 ++++ .../video/quality_analyzing_video_encoder.h | 2 ++ test/video_encoder_proxy_factory.h | 10 +++++++ video/test/mock_video_stream_encoder.h | 1 + video/video_quality_test.cc | 6 ++++ video/video_send_stream_impl.cc | 1 + video/video_send_stream_impl_unittest.cc | 5 ++-- video/video_send_stream_tests.cc | 5 ++++ video/video_stream_encoder.cc | 16 +++++++++++ video/video_stream_encoder.h | 5 ++++ 44 files changed, 224 insertions(+), 5 deletions(-) create mode 100644 api/fec_controller_override.h diff --git a/api/BUILD.gn b/api/BUILD.gn index 8e9767d40f..6699b146ce 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -472,6 +472,7 @@ rtc_source_set("fec_controller_api") { visibility = [ "*" ] sources = [ "fec_controller.h", + "fec_controller_override.h", ] deps = [ diff --git a/api/fec_controller_override.h b/api/fec_controller_override.h new file mode 100644 index 0000000000..233812fb0a --- /dev/null +++ b/api/fec_controller_override.h @@ -0,0 +1,28 @@ +/* Copyright 2019 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. + */ + +// This is an EXPERIMENTAL interface. + +#ifndef API_FEC_CONTROLLER_OVERRIDE_H_ +#define API_FEC_CONTROLLER_OVERRIDE_H_ + +namespace webrtc { + +// Interface for temporarily overriding FecController's bitrate allocation. +class FecControllerOverride { + public: + virtual void SetFecAllowed(bool fec_allowed) = 0; + + protected: + virtual ~FecControllerOverride() = default; +}; + +} // namespace webrtc + +#endif // API_FEC_CONTROLLER_OVERRIDE_H_ diff --git a/api/test/mock_video_encoder.h b/api/test/mock_video_encoder.h index adc6859bd3..65de14f98b 100644 --- a/api/test/mock_video_encoder.h +++ b/api/test/mock_video_encoder.h @@ -32,6 +32,8 @@ class MockVideoEncoder : public VideoEncoder { public: MockVideoEncoder(); ~MockVideoEncoder(); + MOCK_METHOD1(SetFecControllerOverride, + void(FecControllerOverride* fec_controller_override)); MOCK_CONST_METHOD2(Version, int32_t(int8_t* version, int32_t length)); MOCK_METHOD3(InitEncode, int32_t(const VideoCodec* codecSettings, diff --git a/api/video/BUILD.gn b/api/video/BUILD.gn index c6e5899715..161e0a0e01 100644 --- a/api/video/BUILD.gn +++ b/api/video/BUILD.gn @@ -223,6 +223,7 @@ rtc_source_set("video_stream_encoder") { ":video_bitrate_allocator", ":video_bitrate_allocator_factory", ":video_frame", + "../:fec_controller_api", "../units:data_rate", # For rtpparameters.h diff --git a/api/video/video_stream_encoder_interface.h b/api/video/video_stream_encoder_interface.h index 4e3c470e36..5ca54e3ad4 100644 --- a/api/video/video_stream_encoder_interface.h +++ b/api/video/video_stream_encoder_interface.h @@ -13,6 +13,7 @@ #include +#include "api/fec_controller_override.h" #include "api/rtp_parameters.h" // For DegradationPreference. #include "api/units/data_rate.h" #include "api/video/video_bitrate_allocator.h" @@ -97,6 +98,13 @@ class VideoStreamEncoderInterface : public rtc::VideoSinkInterface { virtual void SetBitrateAllocationObserver( VideoBitrateAllocationObserver* bitrate_observer) = 0; + // Set a FecControllerOverride, through which the encoder may override + // decisions made by FecController. + // TODO(bugs.webrtc.org/10769): Update downstream projects and then make this + // pure-virtual. + virtual void SetFecControllerOverride( + FecControllerOverride* fec_controller_override) {} + // Creates and configures an encoder with the given |config|. The // |max_data_payload_length| is used to support single NAL unit // packetization for H.264. diff --git a/api/video_codecs/BUILD.gn b/api/video_codecs/BUILD.gn index 66e17e253a..f53ae7bc7c 100644 --- a/api/video_codecs/BUILD.gn +++ b/api/video_codecs/BUILD.gn @@ -36,6 +36,7 @@ rtc_source_set("video_codecs_api") { ] deps = [ + "..:fec_controller_api", "..:scoped_refptr", "../..:webrtc_common", "../../modules/video_coding:codec_globals_headers", @@ -138,6 +139,7 @@ rtc_static_library("rtc_software_fallback_wrappers") { deps = [ ":video_codecs_api", + "..:fec_controller_api", "../../media:rtc_h264_profile_id", "../../media:rtc_media_base", "../../modules/video_coding:video_codec_interface", diff --git a/api/video_codecs/test/BUILD.gn b/api/video_codecs/test/BUILD.gn index 449788d9e4..5e8a0330be 100644 --- a/api/video_codecs/test/BUILD.gn +++ b/api/video_codecs/test/BUILD.gn @@ -21,6 +21,7 @@ if (rtc_include_tests) { "..:builtin_video_encoder_factory", "..:rtc_software_fallback_wrappers", "..:video_codecs_api", + "../..:fec_controller_api", "../..:mock_video_encoder", "../../../api:scoped_refptr", "../../../modules:module_api", diff --git a/api/video_codecs/test/video_encoder_software_fallback_wrapper_unittest.cc b/api/video_codecs/test/video_encoder_software_fallback_wrapper_unittest.cc index 55afb68d99..4c2599ce72 100644 --- a/api/video_codecs/test/video_encoder_software_fallback_wrapper_unittest.cc +++ b/api/video_codecs/test/video_encoder_software_fallback_wrapper_unittest.cc @@ -16,6 +16,7 @@ #include "absl/memory/memory.h" #include "absl/types/optional.h" +#include "api/fec_controller_override.h" #include "api/scoped_refptr.h" #include "api/test/mock_video_encoder.h" #include "api/video/encoded_image.h" @@ -91,6 +92,11 @@ class VideoEncoderSoftwareFallbackWrapperTest : public ::testing::Test { class CountingFakeEncoder : public VideoEncoder { public: + void SetFecControllerOverride( + FecControllerOverride* fec_controller_override) override { + // Ignored. + } + int32_t InitEncode(const VideoCodec* codec_settings, const VideoEncoder::Settings& settings) override { ++init_encode_count_; diff --git a/api/video_codecs/video_encoder.cc b/api/video_codecs/video_encoder.cc index ffdcf8b4a5..d3f16a0053 100644 --- a/api/video_codecs/video_encoder.cc +++ b/api/video_codecs/video_encoder.cc @@ -118,6 +118,9 @@ VideoEncoder::RateControlParameters::RateControlParameters( VideoEncoder::RateControlParameters::~RateControlParameters() = default; +void VideoEncoder::SetFecControllerOverride( + FecControllerOverride* fec_controller_override) {} + int32_t VideoEncoder::InitEncode(const VideoCodec* codec_settings, int32_t number_of_cores, size_t max_payload_size) { diff --git a/api/video_codecs/video_encoder.h b/api/video_codecs/video_encoder.h index 5609e93b64..e7fea873f3 100644 --- a/api/video_codecs/video_encoder.h +++ b/api/video_codecs/video_encoder.h @@ -18,6 +18,7 @@ #include "absl/container/inlined_vector.h" #include "absl/types/optional.h" +#include "api/fec_controller_override.h" #include "api/units/data_rate.h" #include "api/video/encoded_image.h" #include "api/video/video_bitrate_allocation.h" @@ -288,6 +289,12 @@ class RTC_EXPORT VideoEncoder { virtual ~VideoEncoder() {} + // Set a FecControllerOverride, through which the encoder may override + // decisions made by FecController. + // TODO(bugs.webrtc.org/10769): Update downstream, then make pure-virtual. + virtual void SetFecControllerOverride( + FecControllerOverride* fec_controller_override); + // Initialize the encoder with the information from the codecSettings // // Input: diff --git a/api/video_codecs/video_encoder_software_fallback_wrapper.cc b/api/video_codecs/video_encoder_software_fallback_wrapper.cc index 1ccc5a2090..a6eaed1277 100644 --- a/api/video_codecs/video_encoder_software_fallback_wrapper.cc +++ b/api/video_codecs/video_encoder_software_fallback_wrapper.cc @@ -18,6 +18,7 @@ #include "absl/memory/memory.h" #include "absl/types/optional.h" +#include "api/fec_controller_override.h" #include "api/video/video_bitrate_allocation.h" #include "api/video/video_frame.h" #include "api/video_codecs/video_codec.h" @@ -79,6 +80,9 @@ class VideoEncoderSoftwareFallbackWrapper final : public VideoEncoder { std::unique_ptr hw_encoder); ~VideoEncoderSoftwareFallbackWrapper() override; + void SetFecControllerOverride( + FecControllerOverride* fec_controller_override) override; + int32_t InitEncode(const VideoCodec* codec_settings, const VideoEncoder::Settings& settings) override; @@ -88,6 +92,8 @@ class VideoEncoderSoftwareFallbackWrapper final : public VideoEncoder { int32_t Release() override; int32_t Encode(const VideoFrame& frame, const std::vector* frame_types) override; + // TOD(eladalon): Add OnPacketLossRateUpdate, OnRttUpdate and + // OnLossNotification. void SetRates(const RateControlParameters& parameters) override; EncoderInfo GetEncoderInfo() const override; @@ -149,6 +155,7 @@ VideoEncoderSoftwareFallbackWrapper::VideoEncoderSoftwareFallbackWrapper( fallback_encoder_(std::move(sw_encoder)), callback_(nullptr), forced_fallback_possible_(EnableForcedFallback()) { + RTC_DCHECK(fallback_encoder_); if (forced_fallback_possible_) { GetForcedFallbackParamsFromFieldTrialGroup( &forced_fallback_.min_pixels_, &forced_fallback_.max_pixels_, @@ -184,6 +191,15 @@ bool VideoEncoderSoftwareFallbackWrapper::InitFallbackEncoder() { return true; } +void VideoEncoderSoftwareFallbackWrapper::SetFecControllerOverride( + FecControllerOverride* fec_controller_override) { + // It is important that only one of those would ever interact with the + // |fec_controller_override| at a given time. This is the responsibility + // of |this| to maintain. + encoder_->SetFecControllerOverride(fec_controller_override); + fallback_encoder_->SetFecControllerOverride(fec_controller_override); +} + int32_t VideoEncoderSoftwareFallbackWrapper::InitEncode( const VideoCodec* codec_settings, const VideoEncoder::Settings& settings) { diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index eb00c480a5..bf9e94ef63 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -782,6 +782,10 @@ int RtpVideoSender::ProtectionRequest(const FecProtectionParams* delta_params, return 0; } +void RtpVideoSender::SetFecAllowed(bool fec_allowed) { + // TODO(bugs.webrtc.og/10769): Handle this message. +} + void RtpVideoSender::OnPacketFeedbackVector( const std::vector& packet_feedback_vector) { if (fec_controller_->UseLossVectorMask()) { diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h index 48bc864b9b..333bfeaf81 100644 --- a/call/rtp_video_sender.h +++ b/call/rtp_video_sender.h @@ -20,6 +20,7 @@ #include "api/array_view.h" #include "api/call/transport.h" #include "api/fec_controller.h" +#include "api/fec_controller_override.h" #include "api/video_codecs/video_encoder.h" #include "call/rtp_config.h" #include "call/rtp_payload_params.h" @@ -118,6 +119,9 @@ class RtpVideoSender : public RtpVideoSenderInterface, uint32_t* sent_nack_rate_bps, uint32_t* sent_fec_rate_bps) override; + // Implements FecControllerOverride. + void SetFecAllowed(bool fec_allowed) override; + // Implements EncodedImageCallback. // Returns 0 if the packet was routed / sent, -1 otherwise. EncodedImageCallback::Result OnEncodedImage( diff --git a/call/rtp_video_sender_interface.h b/call/rtp_video_sender_interface.h index 3208e94e39..ae9cdaf71c 100644 --- a/call/rtp_video_sender_interface.h +++ b/call/rtp_video_sender_interface.h @@ -16,6 +16,7 @@ #include "absl/types/optional.h" #include "api/array_view.h" +#include "api/fec_controller_override.h" #include "call/rtp_config.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/rtp_sequence_number_map.h" @@ -26,7 +27,8 @@ namespace webrtc { class VideoBitrateAllocation; struct FecProtectionParams; -class RtpVideoSenderInterface : public EncodedImageCallback { +class RtpVideoSenderInterface : public EncodedImageCallback, + public FecControllerOverride { public: virtual void RegisterProcessThread(ProcessThread* module_process_thread) = 0; virtual void DeRegisterProcessThread() = 0; @@ -61,6 +63,9 @@ class RtpVideoSenderInterface : public EncodedImageCallback { virtual std::vector GetSentRtpPacketInfos( uint32_t ssrc, rtc::ArrayView sequence_numbers) const = 0; + + // Implements FecControllerOverride. + void SetFecAllowed(bool fec_allowed) override = 0; }; } // namespace webrtc #endif // CALL_RTP_VIDEO_SENDER_INTERFACE_H_ diff --git a/media/BUILD.gn b/media/BUILD.gn index 3e338fe076..9efdbe640c 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -159,6 +159,7 @@ rtc_static_library("rtc_simulcast_encoder_adapter") { "engine/simulcast_encoder_adapter.h", ] deps = [ + "../api:fec_controller_api", "../api:scoped_refptr", "../api/video:video_codec_constants", "../api/video:video_frame", @@ -421,6 +422,7 @@ if (rtc_include_tests) { ":rtc_media_base", ":rtc_simulcast_encoder_adapter", "../api:call_api", + "../api:fec_controller_api", "../api:libjingle_peerconnection_api", "../api:scoped_refptr", "../api/video:encoded_image", diff --git a/media/engine/encoder_simulcast_proxy.cc b/media/engine/encoder_simulcast_proxy.cc index da769a9c85..7a6638f56f 100644 --- a/media/engine/encoder_simulcast_proxy.cc +++ b/media/engine/encoder_simulcast_proxy.cc @@ -31,6 +31,11 @@ int EncoderSimulcastProxy::Release() { return encoder_->Release(); } +void EncoderSimulcastProxy::SetFecControllerOverride( + FecControllerOverride* fec_controller_override) { + encoder_->SetFecControllerOverride(fec_controller_override); +} + // TODO(eladalon): s/inst/codec_settings/g. int EncoderSimulcastProxy::InitEncode(const VideoCodec* inst, const VideoEncoder::Settings& settings) { diff --git a/media/engine/encoder_simulcast_proxy.h b/media/engine/encoder_simulcast_proxy.h index 7709aa205a..eea8a623df 100644 --- a/media/engine/encoder_simulcast_proxy.h +++ b/media/engine/encoder_simulcast_proxy.h @@ -42,6 +42,8 @@ class RTC_EXPORT EncoderSimulcastProxy : public VideoEncoder { // Implements VideoEncoder. int Release() override; + void SetFecControllerOverride( + FecControllerOverride* fec_controller_override) override; int InitEncode(const VideoCodec* codec_settings, const VideoEncoder::Settings& settings) override; int Encode(const VideoFrame& input_image, diff --git a/media/engine/fake_webrtc_video_engine.cc b/media/engine/fake_webrtc_video_engine.cc index bdc9178305..3a04c7f5bc 100644 --- a/media/engine/fake_webrtc_video_engine.cc +++ b/media/engine/fake_webrtc_video_engine.cc @@ -137,6 +137,11 @@ FakeWebRtcVideoEncoder::~FakeWebRtcVideoEncoder() { } } +void FakeWebRtcVideoEncoder::SetFecControllerOverride( + webrtc::FecControllerOverride* fec_controller_override) { + // Ignored. +} + int32_t FakeWebRtcVideoEncoder::InitEncode( const webrtc::VideoCodec* codecSettings, const VideoEncoder::Settings& settings) { diff --git a/media/engine/fake_webrtc_video_engine.h b/media/engine/fake_webrtc_video_engine.h index 364ec9e769..68a3d463cd 100644 --- a/media/engine/fake_webrtc_video_engine.h +++ b/media/engine/fake_webrtc_video_engine.h @@ -17,6 +17,7 @@ #include #include +#include "api/fec_controller_override.h" #include "api/video/encoded_image.h" #include "api/video/video_bitrate_allocation.h" #include "api/video/video_frame.h" @@ -83,6 +84,8 @@ class FakeWebRtcVideoEncoder : public webrtc::VideoEncoder { explicit FakeWebRtcVideoEncoder(FakeWebRtcVideoEncoderFactory* factory); ~FakeWebRtcVideoEncoder(); + void SetFecControllerOverride( + webrtc::FecControllerOverride* fec_controller_override) override; int32_t InitEncode(const webrtc::VideoCodec* codecSettings, const VideoEncoder::Settings& settings) override; int32_t Encode( diff --git a/media/engine/simulcast_encoder_adapter.cc b/media/engine/simulcast_encoder_adapter.cc index 596c975e42..f3e8427f9e 100644 --- a/media/engine/simulcast_encoder_adapter.cc +++ b/media/engine/simulcast_encoder_adapter.cc @@ -145,6 +145,11 @@ SimulcastEncoderAdapter::~SimulcastEncoderAdapter() { DestroyStoredEncoders(); } +void SimulcastEncoderAdapter::SetFecControllerOverride( + FecControllerOverride* fec_controller_override) { + // Ignored. +} + int SimulcastEncoderAdapter::Release() { RTC_DCHECK_RUN_ON(&encoder_queue_); diff --git a/media/engine/simulcast_encoder_adapter.h b/media/engine/simulcast_encoder_adapter.h index 94762569e9..086968e646 100644 --- a/media/engine/simulcast_encoder_adapter.h +++ b/media/engine/simulcast_encoder_adapter.h @@ -19,6 +19,7 @@ #include #include "absl/types/optional.h" +#include "api/fec_controller_override.h" #include "api/video_codecs/sdp_video_format.h" #include "api/video_codecs/video_encoder.h" #include "modules/video_coding/include/video_codec_interface.h" @@ -42,6 +43,8 @@ class RTC_EXPORT SimulcastEncoderAdapter : public VideoEncoder { virtual ~SimulcastEncoderAdapter(); // Implements VideoEncoder. + void SetFecControllerOverride( + FecControllerOverride* fec_controller_override) override; int Release() override; int InitEncode(const VideoCodec* codec_settings, const VideoEncoder::Settings& settings) override; @@ -49,6 +52,8 @@ class RTC_EXPORT SimulcastEncoderAdapter : public VideoEncoder { const std::vector* frame_types) override; int RegisterEncodeCompleteCallback(EncodedImageCallback* callback) override; void SetRates(const RateControlParameters& parameters) override; + // TOD(eladalon): Add OnPacketLossRateUpdate, OnRttUpdate and + // OnLossNotification. // Eventual handler for the contained encoders' EncodedImageCallbacks, but // called from an internal helper that also knows the correct stream diff --git a/media/engine/simulcast_encoder_adapter_unittest.cc b/media/engine/simulcast_encoder_adapter_unittest.cc index fd5e5ffc50..ee8da309ca 100644 --- a/media/engine/simulcast_encoder_adapter_unittest.cc +++ b/media/engine/simulcast_encoder_adapter_unittest.cc @@ -187,6 +187,9 @@ class MockVideoEncoder : public VideoEncoder { scaling_settings_(VideoEncoder::ScalingSettings::kOff), callback_(nullptr) {} + MOCK_METHOD1(SetFecControllerOverride, + void(FecControllerOverride* fec_controller_override)); + // TODO(nisse): Valid overrides commented out, because the gmock // methods don't use any override declarations, and we want to avoid // warnings from -Winconsistent-missing-override. See diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn index 4ae54db3be..1938a8869c 100644 --- a/modules/video_coding/BUILD.gn +++ b/modules/video_coding/BUILD.gn @@ -337,6 +337,7 @@ rtc_static_library("webrtc_multiplex") { ":video_codec_interface", ":video_coding_utility", "..:module_api", + "../../api:fec_controller_api", "../../api:scoped_refptr", "../../api/video:encoded_image", "../../api/video:video_frame", @@ -371,6 +372,7 @@ rtc_static_library("webrtc_vp8") { ":webrtc_vp8_temporal_layers", "..:module_api", "../..:webrtc_common", + "../../api:fec_controller_api", "../../api:scoped_refptr", "../../api/video:encoded_image", "../../api/video:video_frame", @@ -464,6 +466,7 @@ rtc_static_library("webrtc_vp9") { ":webrtc_vp9_helpers", "..:module_api", "../..:webrtc_common", + "../../api:fec_controller_api", "../../api:scoped_refptr", "../../api/video:video_frame", "../../api/video:video_frame_i010", diff --git a/modules/video_coding/codecs/multiplex/include/multiplex_encoder_adapter.h b/modules/video_coding/codecs/multiplex/include/multiplex_encoder_adapter.h index 9c17ba4c83..5e01d09275 100644 --- a/modules/video_coding/codecs/multiplex/include/multiplex_encoder_adapter.h +++ b/modules/video_coding/codecs/multiplex/include/multiplex_encoder_adapter.h @@ -15,6 +15,7 @@ #include #include +#include "api/fec_controller_override.h" #include "api/video_codecs/sdp_video_format.h" #include "api/video_codecs/video_encoder.h" #include "api/video_codecs/video_encoder_factory.h" @@ -39,6 +40,8 @@ class MultiplexEncoderAdapter : public VideoEncoder { virtual ~MultiplexEncoderAdapter(); // Implements VideoEncoder + void SetFecControllerOverride( + FecControllerOverride* fec_controller_override) override; int InitEncode(const VideoCodec* inst, const VideoEncoder::Settings& settings) override; int Encode(const VideoFrame& input_image, @@ -46,6 +49,8 @@ class MultiplexEncoderAdapter : public VideoEncoder { int RegisterEncodeCompleteCallback(EncodedImageCallback* callback) override; void SetRates(const RateControlParameters& parameters) override; int Release() override; + // TOD(eladalon): Add OnPacketLossRateUpdate, OnRttUpdate and + // OnLossNotification. EncoderInfo GetEncoderInfo() const override; EncodedImageCallback::Result OnEncodedImage( diff --git a/modules/video_coding/codecs/multiplex/multiplex_encoder_adapter.cc b/modules/video_coding/codecs/multiplex/multiplex_encoder_adapter.cc index bfc03f2523..c13a1722ea 100644 --- a/modules/video_coding/codecs/multiplex/multiplex_encoder_adapter.cc +++ b/modules/video_coding/codecs/multiplex/multiplex_encoder_adapter.cc @@ -61,6 +61,11 @@ MultiplexEncoderAdapter::~MultiplexEncoderAdapter() { Release(); } +void MultiplexEncoderAdapter::SetFecControllerOverride( + FecControllerOverride* fec_controller_override) { + // Ignored. +} + int MultiplexEncoderAdapter::InitEncode( const VideoCodec* inst, const VideoEncoder::Settings& settings) { diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc index 005e6f24df..9b984f715c 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc @@ -450,8 +450,13 @@ void LibvpxVp8Encoder::SetStreamState(bool send_stream, int stream_idx) { send_stream_[stream_idx] = send_stream; } +void LibvpxVp8Encoder::SetFecControllerOverride( + FecControllerOverride* fec_controller_override) { + RTC_DCHECK(fec_controller_override); + // TODO(bugs.webrtc.og/10769): Pass on to the frame buffer controller. +} + // TODO(eladalon): s/inst/codec_settings/g. -// TODO(bugs.webrtc.org/10720): Pass |capabilities| to frame buffer controller. int LibvpxVp8Encoder::InitEncode(const VideoCodec* inst, const VideoEncoder::Settings& settings) { if (inst == NULL) { diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h index 90532073d9..3caf3abe02 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h @@ -15,6 +15,7 @@ #include #include +#include "api/fec_controller_override.h" #include "api/video/encoded_image.h" #include "api/video/video_frame.h" #include "api/video_codecs/video_encoder.h" @@ -45,6 +46,9 @@ class LibvpxVp8Encoder : public VideoEncoder { int Release() override; + void SetFecControllerOverride( + FecControllerOverride* fec_controller_override) override; + int InitEncode(const VideoCodec* codec_settings, const VideoEncoder::Settings& settings) override; diff --git a/modules/video_coding/codecs/vp9/vp9_impl.cc b/modules/video_coding/codecs/vp9/vp9_impl.cc index bc1d6187bf..58aac2f88b 100644 --- a/modules/video_coding/codecs/vp9/vp9_impl.cc +++ b/modules/video_coding/codecs/vp9/vp9_impl.cc @@ -251,6 +251,11 @@ VP9EncoderImpl::~VP9EncoderImpl() { Release(); } +void VP9EncoderImpl::SetFecControllerOverride( + FecControllerOverride* fec_controller_override) { + // Ignored. +} + int VP9EncoderImpl::Release() { int ret_val = WEBRTC_VIDEO_CODEC_OK; diff --git a/modules/video_coding/codecs/vp9/vp9_impl.h b/modules/video_coding/codecs/vp9/vp9_impl.h index 7ceb69970b..091858247c 100644 --- a/modules/video_coding/codecs/vp9/vp9_impl.h +++ b/modules/video_coding/codecs/vp9/vp9_impl.h @@ -21,6 +21,7 @@ #include "modules/video_coding/codecs/vp9/include/vp9.h" +#include "api/fec_controller_override.h" #include "api/video_codecs/video_encoder.h" #include "media/base/vp9_profile.h" #include "modules/video_coding/codecs/vp9/vp9_frame_buffer_pool.h" @@ -36,7 +37,10 @@ class VP9EncoderImpl : public VP9Encoder { public: explicit VP9EncoderImpl(const cricket::VideoCodec& codec); - virtual ~VP9EncoderImpl(); + ~VP9EncoderImpl() override; + + void SetFecControllerOverride( + FecControllerOverride* fec_controller_override) override; int Release() override; diff --git a/test/BUILD.gn b/test/BUILD.gn index 233d0efee9..1824f71c59 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -633,6 +633,7 @@ rtc_source_set("fake_video_codecs") { "fake_vp8_encoder.h", ] deps = [ + "../api:fec_controller_api", "../api:scoped_refptr", "../api/task_queue", "../api/video:encoded_image", diff --git a/test/configurable_frame_size_encoder.cc b/test/configurable_frame_size_encoder.cc index 77326077ba..e12298aa93 100644 --- a/test/configurable_frame_size_encoder.cc +++ b/test/configurable_frame_size_encoder.cc @@ -36,6 +36,11 @@ ConfigurableFrameSizeEncoder::ConfigurableFrameSizeEncoder( ConfigurableFrameSizeEncoder::~ConfigurableFrameSizeEncoder() {} +void ConfigurableFrameSizeEncoder::SetFecControllerOverride( + FecControllerOverride* fec_controller_override) { + // Ignored. +} + int32_t ConfigurableFrameSizeEncoder::InitEncode( const VideoCodec* codec_settings, const Settings& settings) { diff --git a/test/configurable_frame_size_encoder.h b/test/configurable_frame_size_encoder.h index 679d3bd9d0..311cb99247 100644 --- a/test/configurable_frame_size_encoder.h +++ b/test/configurable_frame_size_encoder.h @@ -32,6 +32,9 @@ class ConfigurableFrameSizeEncoder : public VideoEncoder { explicit ConfigurableFrameSizeEncoder(size_t max_frame_size); ~ConfigurableFrameSizeEncoder() override; + void SetFecControllerOverride( + FecControllerOverride* fec_controller_override) override; + int32_t InitEncode(const VideoCodec* codec_settings, const Settings& settings) override; diff --git a/test/fake_encoder.cc b/test/fake_encoder.cc index 6bb076ea22..457f4fed88 100644 --- a/test/fake_encoder.cc +++ b/test/fake_encoder.cc @@ -60,6 +60,11 @@ FakeEncoder::FakeEncoder(Clock* clock) } } +void FakeEncoder::SetFecControllerOverride( + FecControllerOverride* fec_controller_override) { + // Ignored. +} + void FakeEncoder::SetMaxBitrate(int max_kbps) { RTC_DCHECK_GE(max_kbps, -1); // max_kbps == -1 disables it. rtc::CritScope cs(&crit_sect_); diff --git a/test/fake_encoder.h b/test/fake_encoder.h index f23ec1ed31..c0dc73913e 100644 --- a/test/fake_encoder.h +++ b/test/fake_encoder.h @@ -16,6 +16,7 @@ #include #include +#include "api/fec_controller_override.h" #include "api/task_queue/task_queue_factory.h" #include "api/video/encoded_image.h" #include "api/video/video_bitrate_allocation.h" @@ -40,6 +41,9 @@ class FakeEncoder : public VideoEncoder { // Sets max bitrate. Not thread-safe, call before registering the encoder. void SetMaxBitrate(int max_kbps); + void SetFecControllerOverride( + FecControllerOverride* fec_controller_override) override; + int32_t InitEncode(const VideoCodec* config, const Settings& settings) override; int32_t Encode(const VideoFrame& input_image, diff --git a/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.cc b/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.cc index 3fa9a5be51..493cdf9ffe 100644 --- a/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.cc +++ b/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.cc @@ -69,6 +69,11 @@ QualityAnalyzingVideoEncoder::QualityAnalyzingVideoEncoder( delegate_callback_(nullptr) {} QualityAnalyzingVideoEncoder::~QualityAnalyzingVideoEncoder() = default; +void QualityAnalyzingVideoEncoder::SetFecControllerOverride( + FecControllerOverride* fec_controller_override) { + // Ignored. +} + int32_t QualityAnalyzingVideoEncoder::InitEncode( const VideoCodec* codec_settings, const Settings& settings) { diff --git a/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.h b/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.h index 6b22ae2369..247be73212 100644 --- a/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.h +++ b/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.h @@ -62,6 +62,8 @@ class QualityAnalyzingVideoEncoder : public VideoEncoder, ~QualityAnalyzingVideoEncoder() override; // Methods of VideoEncoder interface. + void SetFecControllerOverride( + FecControllerOverride* fec_controller_override) override; int32_t InitEncode(const VideoCodec* codec_settings, const Settings& settings) override; int32_t RegisterEncodeCompleteCallback( diff --git a/test/video_encoder_proxy_factory.h b/test/video_encoder_proxy_factory.h index 81183d3d70..d09e6ff694 100644 --- a/test/video_encoder_proxy_factory.h +++ b/test/video_encoder_proxy_factory.h @@ -65,22 +65,32 @@ class VideoEncoderProxyFactory final : public VideoEncoderFactory { explicit EncoderProxy(VideoEncoder* encoder) : encoder_(encoder) {} private: + void SetFecControllerOverride( + FecControllerOverride* fec_controller_override) override { + encoder_->SetFecControllerOverride(fec_controller_override); + } + int32_t Encode(const VideoFrame& input_image, const std::vector* frame_types) override { return encoder_->Encode(input_image, frame_types); } + int32_t InitEncode(const VideoCodec* config, const Settings& settings) override { return encoder_->InitEncode(config, settings); } + int32_t RegisterEncodeCompleteCallback( EncodedImageCallback* callback) override { return encoder_->RegisterEncodeCompleteCallback(callback); } + int32_t Release() override { return encoder_->Release(); } + void SetRates(const RateControlParameters& parameters) override { encoder_->SetRates(parameters); } + VideoEncoder::EncoderInfo GetEncoderInfo() const override { return encoder_->GetEncoderInfo(); } diff --git a/video/test/mock_video_stream_encoder.h b/video/test/mock_video_stream_encoder.h index c27c6e975d..a5d153453d 100644 --- a/video/test/mock_video_stream_encoder.h +++ b/video/test/mock_video_stream_encoder.h @@ -28,6 +28,7 @@ class MockVideoStreamEncoder : public VideoStreamEncoderInterface { MOCK_METHOD1(OnFrame, void(const VideoFrame&)); MOCK_METHOD1(SetBitrateAllocationObserver, void(VideoBitrateAllocationObserver*)); + MOCK_METHOD1(SetFecControllerOverride, void(FecControllerOverride*)); MOCK_METHOD0(Stop, void()); MOCK_METHOD2(MockedConfigureEncoder, void(const VideoEncoderConfig&, size_t)); diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index 172cba1191..c2eac81a68 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -17,6 +17,7 @@ #include #include "absl/memory/memory.h" +#include "api/fec_controller_override.h" #include "api/media_transport_config.h" #include "api/rtc_event_log_output_file.h" #include "api/task_queue/default_task_queue_factory.h" @@ -134,6 +135,11 @@ class QualityTestVideoEncoder : public VideoEncoder, } // Implement VideoEncoder + void SetFecControllerOverride( + FecControllerOverride* fec_controller_override) { + // Ignored. + } + int32_t InitEncode(const VideoCodec* codec_settings, const Settings& settings) override { codec_settings_ = *codec_settings; diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc index 1dc6599b9b..058686905b 100644 --- a/video/video_send_stream_impl.cc +++ b/video/video_send_stream_impl.cc @@ -247,6 +247,7 @@ VideoSendStreamImpl::VideoSendStreamImpl( CreateFrameEncryptionConfig(config_))), weak_ptr_factory_(this), media_transport_(media_transport) { + video_stream_encoder->SetFecControllerOverride(rtp_video_sender_); RTC_DCHECK_RUN_ON(worker_queue_); RTC_LOG(LS_INFO) << "VideoSendStreamInternal: " << config_->ToString(); weak_ptr_ = weak_ptr_factory_.GetWeakPtr(); diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc index 8126835000..e6886aecb2 100644 --- a/video/video_send_stream_impl_unittest.cc +++ b/video/video_send_stream_impl_unittest.cc @@ -73,6 +73,8 @@ class MockRtpVideoSender : public RtpVideoSenderInterface { std::vector( uint32_t ssrc, rtc::ArrayView sequence_numbers)); + + MOCK_METHOD1(SetFecAllowed, void(bool fec_allowed)); }; BitrateAllocationUpdate CreateAllocation(int bitrate_bps) { @@ -101,8 +103,7 @@ class VideoSendStreamImplTest : public ::testing::Test { EXPECT_CALL(transport_controller_, packet_router()) .WillRepeatedly(Return(&packet_router_)); - EXPECT_CALL(transport_controller_, - CreateRtpVideoSender(_, _, _, _, _, _, _, _, _)) + EXPECT_CALL(transport_controller_, CreateRtpVideoSender) .WillRepeatedly(Return(&rtp_video_sender_)); EXPECT_CALL(rtp_video_sender_, SetActive(_)) .WillRepeatedly(::testing::Invoke( diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc index 77a63f6528..47053fa155 100644 --- a/video/video_send_stream_tests.cc +++ b/video/video_send_stream_tests.cc @@ -2307,6 +2307,11 @@ TEST_F(VideoSendStreamTest, EncoderIsProperlyInitializedAndDestroyed) { } private: + void SetFecControllerOverride( + FecControllerOverride* fec_controller_override) override { + // Ignored. + } + int32_t InitEncode(const VideoCodec* codecSettings, const Settings& settings) override { rtc::CritScope lock(&crit_); diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index a2bf3ef392..2f03c533f4 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -487,6 +487,7 @@ VideoStreamEncoder::VideoStreamEncoder( pending_frame_post_time_us_(0), accumulated_update_rect_{0, 0, 0, 0}, bitrate_observer_(nullptr), + fec_controller_override_(nullptr), force_disable_frame_dropper_(false), input_framerate_(kFrameRateAvergingWindowSizeMs, 1000), pending_frame_drops_(0), @@ -537,6 +538,18 @@ void VideoStreamEncoder::SetBitrateAllocationObserver( }); } +void VideoStreamEncoder::SetFecControllerOverride( + FecControllerOverride* fec_controller_override) { + encoder_queue_.PostTask([this, fec_controller_override] { + RTC_DCHECK_RUN_ON(&encoder_queue_); + RTC_DCHECK(!fec_controller_override_); + fec_controller_override_ = fec_controller_override; + if (encoder_) { + encoder_->SetFecControllerOverride(fec_controller_override_); + } + }); +} + void VideoStreamEncoder::SetSource( rtc::VideoSourceInterface* source, const DegradationPreference& degradation_preference) { @@ -729,6 +742,9 @@ void VideoStreamEncoder::ReconfigureEncoder() { // TODO(nisse): What to do if creating the encoder fails? Crash, // or just discard incoming frames? RTC_CHECK(encoder_); + + encoder_->SetFecControllerOverride(fec_controller_override_); + codec_info_ = settings_.encoder_factory->QueryVideoEncoder( encoder_config_.video_format); } diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index 3756e80401..3802b02332 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -74,6 +74,9 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, void SetBitrateAllocationObserver( VideoBitrateAllocationObserver* bitrate_observer) override; + void SetFecControllerOverride( + FecControllerOverride* fec_controller_override) override; + void ConfigureEncoder(VideoEncoderConfig config, size_t max_data_payload_length) override; @@ -317,6 +320,8 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, VideoBitrateAllocationObserver* bitrate_observer_ RTC_GUARDED_BY(&encoder_queue_); + FecControllerOverride* fec_controller_override_ + RTC_GUARDED_BY(&encoder_queue_); absl::optional last_parameters_update_ms_ RTC_GUARDED_BY(&encoder_queue_); absl::optional last_encode_info_ms_ RTC_GUARDED_BY(&encoder_queue_);