From 79685304182cd81f34c3d2b80527d4e8de92b04c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Wed, 27 Nov 2019 17:26:58 +0100 Subject: [PATCH] Removes caching SimulcastEncoderAdapter::GetEncoderInfo() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are edge cases where the caching of encoder info will cause issues. For instance if a sub-encoder fails en Encode call and falls back to some other implementation, or if the fps targets shift due to SetRates() triggering new layers to be enabled. This CL forces a complete rebuild on every call to GetEncoderInfo(). It also adds new logging of when the info changes, as debugging issues can be very time consuming if we can't tell that happened. Bug: webrtc:11000 Change-Id: I7ec7962a589ccba0e188e60a11f851c9de874fab Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/160960 Commit-Queue: Erik Språng Reviewed-by: Rasmus Brandt Cr-Commit-Position: refs/heads/master@{#29938} --- api/video_codecs/video_encoder.cc | 104 ++++++++++++++++ api/video_codecs/video_encoder.h | 9 ++ media/engine/simulcast_encoder_adapter.cc | 115 +++++++++--------- media/engine/simulcast_encoder_adapter.h | 3 +- .../simulcast_encoder_adapter_unittest.cc | 20 +++ video/video_stream_encoder.cc | 5 + 6 files changed, 198 insertions(+), 58 deletions(-) diff --git a/api/video_codecs/video_encoder.cc b/api/video_codecs/video_encoder.cc index 43f959ba5a..417772f1a4 100644 --- a/api/video_codecs/video_encoder.cc +++ b/api/video_codecs/video_encoder.cc @@ -13,6 +13,7 @@ #include #include "rtc_base/checks.h" +#include "rtc_base/strings/string_builder.h" namespace webrtc { @@ -81,6 +82,14 @@ constexpr VideoEncoder::ScalingSettings::KOff // static constexpr uint8_t VideoEncoder::EncoderInfo::kMaxFramerateFraction; +bool VideoEncoder::ResolutionBitrateLimits::operator==( + const ResolutionBitrateLimits& rhs) const { + return frame_size_pixels == rhs.frame_size_pixels && + min_start_bitrate_bps == rhs.min_start_bitrate_bps && + min_bitrate_bps == rhs.min_bitrate_bps && + max_bitrate_bps == rhs.max_bitrate_bps; +} + VideoEncoder::EncoderInfo::EncoderInfo() : scaling_settings(VideoEncoder::ScalingSettings::kOff), supports_native_handle(false), @@ -97,6 +106,101 @@ VideoEncoder::EncoderInfo::EncoderInfo(const EncoderInfo&) = default; VideoEncoder::EncoderInfo::~EncoderInfo() = default; +std::string VideoEncoder::EncoderInfo::ToString() const { + char string_buf[2048]; + rtc::SimpleStringBuilder oss(string_buf); + + oss << "EncoderInfo { " + << "ScalingSettings { "; + if (scaling_settings.thresholds) { + oss << "Thresholds { " + << "low = " << scaling_settings.thresholds->low + << ", high = " << scaling_settings.thresholds->high << "}, "; + } + oss << "min_pixels_per_frame = " << scaling_settings.min_pixels_per_frame + << " }"; + oss << ", supports_native_handle = " << supports_native_handle + << ", implementation_name = '" << implementation_name << "'" + << ", has_trusted_rate_controller = " << has_trusted_rate_controller + << ", is_hardware_accelerated = " << is_hardware_accelerated + << ", has_internal_source = " << has_internal_source + << ", fps_allocation = ["; + bool first = true; + for (size_t i = 0; i < fps_allocation->size(); ++i) { + if (!first) { + oss << ", "; + } + const absl::InlinedVector& fractions = + fps_allocation[i]; + if (!fractions.empty()) { + first = false; + oss << "[ "; + for (size_t i = 0; i < fractions.size(); ++i) { + if (i > 0) { + oss << ", "; + } + oss << (static_cast(fractions[i]) / kMaxFramerateFraction); + } + oss << "] "; + } + } + oss << "]"; + oss << ", resolution_bitrate_limits = ["; + for (size_t i = 0; i < resolution_bitrate_limits.size(); ++i) { + if (i > 0) { + oss << ", "; + } + ResolutionBitrateLimits l = resolution_bitrate_limits[i]; + oss << "Limits { " + << "frame_size_pixels = " << l.frame_size_pixels + << ", min_start_bitrate_bps = " << l.min_start_bitrate_bps + << ", min_bitrate_bps = " << l.min_bitrate_bps + << ", max_bitrate_bps = " << l.max_bitrate_bps << "} "; + } + oss << "] " + << ", supports_simulcast = " << supports_simulcast << "}"; + return oss.str(); +} + +bool VideoEncoder::EncoderInfo::operator==(const EncoderInfo& rhs) const { + if (scaling_settings.thresholds.has_value() != + rhs.scaling_settings.thresholds.has_value()) { + return false; + } + if (scaling_settings.thresholds.has_value()) { + QpThresholds l = *scaling_settings.thresholds; + QpThresholds r = *rhs.scaling_settings.thresholds; + if (l.low != r.low || l.high != r.high) { + return false; + } + } + if (scaling_settings.min_pixels_per_frame != + rhs.scaling_settings.min_pixels_per_frame) { + return false; + } + + if (supports_native_handle != rhs.supports_native_handle || + implementation_name != rhs.implementation_name || + has_trusted_rate_controller != rhs.has_trusted_rate_controller || + is_hardware_accelerated != rhs.is_hardware_accelerated || + has_internal_source != rhs.has_internal_source) { + return false; + } + + for (size_t i = 0; i < kMaxSpatialLayers; ++i) { + if (fps_allocation[i] != rhs.fps_allocation[i]) { + return false; + } + } + + if (resolution_bitrate_limits != rhs.resolution_bitrate_limits || + supports_simulcast != rhs.supports_simulcast) { + return false; + } + + return true; +} + VideoEncoder::RateControlParameters::RateControlParameters() : bitrate(VideoBitrateAllocation()), framerate_fps(0.0), diff --git a/api/video_codecs/video_encoder.h b/api/video_codecs/video_encoder.h index bd18a22763..14dbf637ee 100644 --- a/api/video_codecs/video_encoder.h +++ b/api/video_codecs/video_encoder.h @@ -141,6 +141,11 @@ class RTC_EXPORT VideoEncoder { int min_bitrate_bps = 0; // Recommended maximum bitrate. int max_bitrate_bps = 0; + + bool operator==(const ResolutionBitrateLimits& rhs) const; + bool operator!=(const ResolutionBitrateLimits& rhs) const { + return !(*this == rhs); + } }; // Struct containing metadata about the encoder implementing this interface. @@ -153,6 +158,10 @@ class RTC_EXPORT VideoEncoder { ~EncoderInfo(); + std::string ToString() const; + bool operator==(const EncoderInfo& rhs) const; + bool operator!=(const EncoderInfo& rhs) const { return !(*this == rhs); } + // Any encoder implementation wishing to use the WebRTC provided // quality scaler must populate this field. ScalingSettings scaling_settings; diff --git a/media/engine/simulcast_encoder_adapter.cc b/media/engine/simulcast_encoder_adapter.cc index 6a585c6c7a..667f0722a1 100644 --- a/media/engine/simulcast_encoder_adapter.cc +++ b/media/engine/simulcast_encoder_adapter.cc @@ -151,7 +151,6 @@ SimulcastEncoderAdapter::SimulcastEncoderAdapter( boost_base_layer_quality_(RateControlSettings::ParseFromFieldTrials() .Vp8BoostBaseLayerQuality()) { RTC_DCHECK(primary_factory); - encoder_info_.implementation_name = "SimulcastEncoderAdapter"; // The adapter is typically created on the worker thread, but operated on // the encoder task queue. @@ -229,10 +228,7 @@ int SimulcastEncoderAdapter::InitEncode( start_bitrates.push_back(stream_bitrate); } - encoder_info_.supports_native_handle = true; - encoder_info_.scaling_settings.thresholds = absl::nullopt; // Create |number_of_streams| of encoder instances and init them. - const auto minmax = std::minmax_element( std::begin(codec_.simulcastStream), std::begin(codec_.simulcastStream) + number_of_streams, @@ -321,7 +317,6 @@ int SimulcastEncoderAdapter::InitEncode( if (!doing_simulcast_using_adapter) { // Without simulcast, just pass through the encoder info from the one // active encoder. - encoder_info_ = encoder->GetEncoderInfo(); encoder->RegisterEncodeCompleteCallback(encoded_complete_callback_); streaminfos_.emplace_back(std::move(encoder), nullptr, stream_codec.width, stream_codec.height, send_stream); @@ -332,59 +327,9 @@ int SimulcastEncoderAdapter::InitEncode( streaminfos_.emplace_back(std::move(encoder), std::move(callback), stream_codec.width, stream_codec.height, send_stream); - - const EncoderInfo encoder_impl_info = - streaminfos_[i].encoder->GetEncoderInfo(); - - if (i == 0) { - // Quality scaling not enabled for simulcast. - encoder_info_.scaling_settings = VideoEncoder::ScalingSettings::kOff; - - // Encoder name indicates names of all sub-encoders. - encoder_info_.implementation_name = "SimulcastEncoderAdapter ("; - encoder_info_.implementation_name += - encoder_impl_info.implementation_name; - - encoder_info_.supports_native_handle = - encoder_impl_info.supports_native_handle; - encoder_info_.has_trusted_rate_controller = - encoder_impl_info.has_trusted_rate_controller; - encoder_info_.is_hardware_accelerated = - encoder_impl_info.is_hardware_accelerated; - encoder_info_.has_internal_source = - encoder_impl_info.has_internal_source; - } else { - encoder_info_.implementation_name += ", "; - encoder_info_.implementation_name += - encoder_impl_info.implementation_name; - - // Native handle supported only if all encoders supports it. - encoder_info_.supports_native_handle &= - encoder_impl_info.supports_native_handle; - - // Trusted rate controller only if all encoders have it. - encoder_info_.has_trusted_rate_controller &= - encoder_impl_info.has_trusted_rate_controller; - - // Uses hardware support if any of the encoders uses it. - // For example, if we are having issues with down-scaling due to - // pipelining delay in HW encoders we need higher encoder usage - // thresholds in CPU adaptation. - encoder_info_.is_hardware_accelerated |= - encoder_impl_info.is_hardware_accelerated; - - // Has internal source only if all encoders have it. - encoder_info_.has_internal_source &= - encoder_impl_info.has_internal_source; - } - encoder_info_.fps_allocation[i] = encoder_impl_info.fps_allocation[0]; } } - if (doing_simulcast_using_adapter) { - encoder_info_.implementation_name += ")"; - } - // To save memory, don't store encoders that we don't use. DestroyStoredEncoders(); @@ -658,7 +603,65 @@ void SimulcastEncoderAdapter::DestroyStoredEncoders() { } VideoEncoder::EncoderInfo SimulcastEncoderAdapter::GetEncoderInfo() const { - return encoder_info_; + if (streaminfos_.size() == 1) { + // Not using simulcast adapting functionality, just pass through. + return streaminfos_[0].encoder->GetEncoderInfo(); + } + + VideoEncoder::EncoderInfo encoder_info; + encoder_info.implementation_name = "SimulcastEncoderAdapter"; + encoder_info.supports_native_handle = true; + encoder_info.scaling_settings.thresholds = absl::nullopt; + if (streaminfos_.empty()) { + return encoder_info; + } + + for (size_t i = 0; i < streaminfos_.size(); ++i) { + VideoEncoder::EncoderInfo encoder_impl_info = + streaminfos_[i].encoder->GetEncoderInfo(); + + if (i == 0) { + // Quality scaling not enabled for simulcast. + encoder_info.scaling_settings = VideoEncoder::ScalingSettings::kOff; + + // Encoder name indicates names of all sub-encoders. + encoder_info.implementation_name += " ("; + encoder_info.implementation_name += encoder_impl_info.implementation_name; + + encoder_info.supports_native_handle = + encoder_impl_info.supports_native_handle; + encoder_info.has_trusted_rate_controller = + encoder_impl_info.has_trusted_rate_controller; + encoder_info.is_hardware_accelerated = + encoder_impl_info.is_hardware_accelerated; + encoder_info.has_internal_source = encoder_impl_info.has_internal_source; + } else { + encoder_info.implementation_name += ", "; + encoder_info.implementation_name += encoder_impl_info.implementation_name; + + // Native handle supported only if all encoders supports it. + encoder_info.supports_native_handle &= + encoder_impl_info.supports_native_handle; + + // Trusted rate controller only if all encoders have it. + encoder_info.has_trusted_rate_controller &= + encoder_impl_info.has_trusted_rate_controller; + + // Uses hardware support if any of the encoders uses it. + // For example, if we are having issues with down-scaling due to + // pipelining delay in HW encoders we need higher encoder usage + // thresholds in CPU adaptation. + encoder_info.is_hardware_accelerated |= + encoder_impl_info.is_hardware_accelerated; + + // Has internal source only if all encoders have it. + encoder_info.has_internal_source &= encoder_impl_info.has_internal_source; + } + encoder_info.fps_allocation[i] = encoder_impl_info.fps_allocation[0]; + } + encoder_info.implementation_name += ")"; + + return encoder_info; } } // namespace webrtc diff --git a/media/engine/simulcast_encoder_adapter.h b/media/engine/simulcast_encoder_adapter.h index 591839c30d..b34519771b 100644 --- a/media/engine/simulcast_encoder_adapter.h +++ b/media/engine/simulcast_encoder_adapter.h @@ -47,7 +47,7 @@ class RTC_EXPORT SimulcastEncoderAdapter : public VideoEncoder { SimulcastEncoderAdapter(VideoEncoderFactory* primary_factory, VideoEncoderFactory* fallback_factory, const SdpVideoFormat& format); - virtual ~SimulcastEncoderAdapter(); + ~SimulcastEncoderAdapter() override; // Implements VideoEncoder. void SetFecControllerOverride( @@ -119,7 +119,6 @@ class RTC_EXPORT SimulcastEncoderAdapter : public VideoEncoder { VideoCodec codec_; std::vector streaminfos_; EncodedImageCallback* encoded_complete_callback_; - EncoderInfo encoder_info_; // Used for checking the single-threaded access of the encoder interface. SequenceChecker encoder_queue_; diff --git a/media/engine/simulcast_encoder_adapter_unittest.cc b/media/engine/simulcast_encoder_adapter_unittest.cc index 48767dc754..9f539e0573 100644 --- a/media/engine/simulcast_encoder_adapter_unittest.cc +++ b/media/engine/simulcast_encoder_adapter_unittest.cc @@ -871,6 +871,26 @@ TEST_F(TestSimulcastEncoderAdapterFake, SupportsImplementationName) { EXPECT_EQ("codec1", adapter_->GetEncoderInfo().implementation_name); } +TEST_F(TestSimulcastEncoderAdapterFake, RuntimeEncoderInfoUpdate) { + SimulcastTestFixtureImpl::DefaultSettings( + &codec_, static_cast(kTestTemporalLayerProfile), + kVideoCodecVP8); + std::vector encoder_names; + encoder_names.push_back("codec1"); + encoder_names.push_back("codec2"); + encoder_names.push_back("codec3"); + helper_->factory()->SetEncoderNames(encoder_names); + EXPECT_EQ(0, adapter_->InitEncode(&codec_, kSettings)); + EXPECT_EQ("SimulcastEncoderAdapter (codec1, codec2, codec3)", + adapter_->GetEncoderInfo().implementation_name); + + // Change name of first encoder to indicate it has done a fallback to another + // implementation. + helper_->factory()->encoders().front()->set_implementation_name("fallback1"); + EXPECT_EQ("SimulcastEncoderAdapter (fallback1, codec2, codec3)", + adapter_->GetEncoderInfo().implementation_name); +} + TEST_F(TestSimulcastEncoderAdapterFake, SupportsNativeHandleForMultipleStreams) { SimulcastTestFixtureImpl::DefaultSettings( diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 458f1ed728..dc3bc16114 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -1440,6 +1440,11 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame, } } + if (encoder_info_ != info) { + RTC_LOG(LS_INFO) << "Encoder settings changed from " + << encoder_info_.ToString() << " to " << info.ToString(); + } + if (bitrate_adjuster_) { for (size_t si = 0; si < kMaxSpatialLayers; ++si) { if (info.fps_allocation[si] != encoder_info_.fps_allocation[si]) {