From e1a198b41dceab7818592b32288975489b3a9d12 Mon Sep 17 00:00:00 2001 From: Markus Handell Date: Thu, 15 Sep 2022 08:13:25 +0000 Subject: [PATCH] VideoStreamEncoder: set at target quality based on codec. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Chromium RTCVideoEncoder unfortunately doesn't set if the result is at target quality, and the definition of the threshold is buried in libvpx_vp8_encoder.h. This change * Updates VideoStreamEncoder to postprocess an incoming EncodedImage by interpreting the incoming QP information instead. * Updates the related VideoStreamEncoder test to simulate an encoder producing images around the QP threshold. * Updates the steady state VP8 screencast QP threshold to a central include file. * Moves this and previously existing EncodedImage post-processing to a new method AugmentEncodedImage. Bug: b/245029833 Change-Id: I69ae29ffe501e84f28908f7d9a8cfd066ba82b43 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275380 Reviewed-by: Erik Språng Commit-Queue: Markus Handell Cr-Commit-Position: refs/heads/main@{#38091} --- modules/video_coding/BUILD.gn | 1 + .../codecs/vp8/libvpx_vp8_encoder.cc | 2 -- .../codecs/vp8/libvpx_vp8_encoder.h | 3 +- modules/video_coding/utility/vp8_constants.h | 27 +++++++++++++++ video/video_stream_encoder.cc | 34 +++++++++++++------ video/video_stream_encoder.h | 7 ++++ video/video_stream_encoder_unittest.cc | 12 ++++--- 7 files changed, 68 insertions(+), 18 deletions(-) create mode 100644 modules/video_coding/utility/vp8_constants.h diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn index 031db70564..c73e011fab 100644 --- a/modules/video_coding/BUILD.gn +++ b/modules/video_coding/BUILD.gn @@ -438,6 +438,7 @@ rtc_library("video_coding_utility") { "utility/simulcast_rate_allocator.h", "utility/simulcast_utility.cc", "utility/simulcast_utility.h", + "utility/vp8_constants.h", "utility/vp8_header_parser.cc", "utility/vp8_header_parser.h", "utility/vp9_constants.h", diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc index dc0344c1ed..49ccf2dade 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc @@ -1178,8 +1178,6 @@ int LibvpxVp8Encoder::GetEncodedPartitions(const VideoFrame& input_image, libvpx_->codec_control(&encoders_[encoder_idx], VP8E_GET_LAST_QUANTIZER, &qp_128); encoded_images_[encoder_idx].qp_ = qp_128; - encoded_images_[encoder_idx].SetAtTargetQuality( - qp_128 <= variable_framerate_experiment_.steady_state_qp); encoded_complete_callback_->OnEncodedImage(encoded_images_[encoder_idx], &codec_specific); const size_t steady_state_size = SteadyStateSize( diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h index 643758753d..74477eac7e 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h @@ -25,6 +25,7 @@ #include "modules/video_coding/codecs/vp8/include/vp8.h" #include "modules/video_coding/include/video_codec_interface.h" #include "modules/video_coding/utility/framerate_controller_deprecated.h" +#include "modules/video_coding/utility/vp8_constants.h" #include "rtc_base/experiments/cpu_speed_experiment.h" #include "rtc_base/experiments/encoder_info_settings.h" #include "rtc_base/experiments/rate_control_settings.h" @@ -138,7 +139,7 @@ class LibvpxVp8Encoder : public VideoEncoder { // Framerate is limited to this value in steady state. float framerate_limit = 5.0; // This qp or below is considered a steady state. - int steady_state_qp = 15; + int steady_state_qp = kVp8SteadyStateQpThreshold; // Frames of at least this percentage below ideal for configured bitrate are // considered in a steady state. int steady_state_undershoot_percentage = 30; diff --git a/modules/video_coding/utility/vp8_constants.h b/modules/video_coding/utility/vp8_constants.h new file mode 100644 index 0000000000..9321864dbc --- /dev/null +++ b/modules/video_coding/utility/vp8_constants.h @@ -0,0 +1,27 @@ +/* + * Copyright (c) 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. + */ + +#ifndef MODULES_VIDEO_CODING_UTILITY_VP8_CONSTANTS_H_ +#define MODULES_VIDEO_CODING_UTILITY_VP8_CONSTANTS_H_ + +#include +#include + +#include + +namespace webrtc { + +// QP level below which VP8 variable framerate and zero hertz screencast reduces +// framerate due to diminishing quality enhancement returns. +constexpr int kVp8SteadyStateQpThreshold = 15; + +} // namespace webrtc + +#endif // MODULES_VIDEO_CODING_UTILITY_VP8_CONSTANTS_H_ diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index cbea3b3f06..e320e437cd 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -36,6 +36,7 @@ #include "call/adaptation/video_stream_adapter.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" #include "rtc_base/arraysize.h" #include "rtc_base/checks.h" #include "rtc_base/event.h" @@ -1950,26 +1951,17 @@ void VideoStreamEncoder::OnLossNotification( } } -EncodedImageCallback::Result VideoStreamEncoder::OnEncodedImage( +EncodedImage VideoStreamEncoder::AugmentEncodedImage( const EncodedImage& encoded_image, const CodecSpecificInfo* codec_specific_info) { - TRACE_EVENT_INSTANT1("webrtc", "VCMEncodedFrameCallback::Encoded", - "timestamp", encoded_image.Timestamp()); - - // TODO(bugs.webrtc.org/10520): Signal the simulcast id explicitly. - - const size_t spatial_idx = encoded_image.SpatialIndex().value_or(0); EncodedImage image_copy(encoded_image); - + const size_t spatial_idx = encoded_image.SpatialIndex().value_or(0); frame_encode_metadata_writer_.FillTimingInfo(spatial_idx, &image_copy); - frame_encode_metadata_writer_.UpdateBitstream(codec_specific_info, &image_copy); - VideoCodecType codec_type = codec_specific_info ? codec_specific_info->codecType : VideoCodecType::kVideoCodecGeneric; - if (image_copy.qp_ < 0 && qp_parsing_allowed_) { // Parse encoded frame QP if that was not provided by encoder. image_copy.qp_ = qp_parser_ @@ -1979,6 +1971,8 @@ EncodedImageCallback::Result VideoStreamEncoder::OnEncodedImage( } RTC_LOG(LS_VERBOSE) << __func__ << " spatial_idx " << spatial_idx << " qp " << image_copy.qp_; + image_copy.SetAtTargetQuality(codec_type == kVideoCodecVP8 && + image_copy.qp_ <= kVp8SteadyStateQpThreshold); // Piggyback ALR experiment group id and simulcast id into the content type. const uint8_t experiment_id = @@ -1996,6 +1990,24 @@ EncodedImageCallback::Result VideoStreamEncoder::OnEncodedImage( RTC_CHECK(videocontenttypehelpers::SetSimulcastId( &image_copy.content_type_, static_cast(spatial_idx + 1))); + return image_copy; +} + +EncodedImageCallback::Result VideoStreamEncoder::OnEncodedImage( + const EncodedImage& encoded_image, + const CodecSpecificInfo* codec_specific_info) { + TRACE_EVENT_INSTANT1("webrtc", "VCMEncodedFrameCallback::Encoded", + "timestamp", encoded_image.Timestamp()); + + // TODO(bugs.webrtc.org/10520): Signal the simulcast id explicitly. + + const size_t spatial_idx = encoded_image.SpatialIndex().value_or(0); + const VideoCodecType codec_type = codec_specific_info + ? codec_specific_info->codecType + : VideoCodecType::kVideoCodecGeneric; + EncodedImage image_copy = + AugmentEncodedImage(encoded_image, codec_specific_info); + // Post a task because `send_codec_` requires `encoder_queue_` lock and we // need to update on quality convergence. unsigned int image_width = image_copy._encodedWidth; diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index 60fd263ea4..c4c461f6fb 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -22,6 +22,7 @@ #include "api/sequence_checker.h" #include "api/task_queue/pending_task_safety_flag.h" #include "api/units/data_rate.h" +#include "api/video/encoded_image.h" #include "api/video/video_bitrate_allocator.h" #include "api/video/video_rotation.h" #include "api/video/video_sink_interface.h" @@ -254,6 +255,12 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, void RequestEncoderSwitch() RTC_RUN_ON(&encoder_queue_); + // Augments an EncodedImage received from an encoder with parsable + // information. + EncodedImage AugmentEncodedImage( + const EncodedImage& encoded_image, + const CodecSpecificInfo* codec_specific_info); + const FieldTrialsView& field_trials_; TaskQueueBase* const worker_queue_; diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 3a5b03d1db..3baa9c565d 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -52,6 +52,7 @@ #include "modules/video_coding/codecs/vp9/svc_config.h" #include "modules/video_coding/utility/quality_scaler.h" #include "modules/video_coding/utility/simulcast_rate_allocator.h" +#include "modules/video_coding/utility/vp8_constants.h" #include "rtc_base/event.h" #include "rtc_base/experiments/encoder_info_settings.h" #include "rtc_base/gunit.h" @@ -9338,9 +9339,9 @@ TEST(VideoStreamEncoderFrameCadenceTest, UpdatesQualityConvergence) { EXPECT_CALL(factory.GetMockFakeEncoder(), EncodeHook) .WillRepeatedly(Invoke([](EncodedImage& encoded_image, rtc::scoped_refptr buffer) { - EXPECT_FALSE(encoded_image.IsAtTargetQuality()); + encoded_image.qp_ = kVp8SteadyStateQpThreshold + 1; CodecSpecificInfo codec_specific; - codec_specific.codecType = kVideoCodecGeneric; + codec_specific.codecType = kVideoCodecVP8; return codec_specific; })); EXPECT_CALL(*adapter_ptr, UpdateLayerQualityConvergence(0, false)); @@ -9354,9 +9355,12 @@ TEST(VideoStreamEncoderFrameCadenceTest, UpdatesQualityConvergence) { EXPECT_CALL(factory.GetMockFakeEncoder(), EncodeHook) .WillRepeatedly(Invoke([](EncodedImage& encoded_image, rtc::scoped_refptr buffer) { - encoded_image.SetAtTargetQuality(encoded_image.SpatialIndex() == 0); + // This sets spatial index 0 content to be at target quality, while + // index 1 content is not. + encoded_image.qp_ = kVp8SteadyStateQpThreshold + + (encoded_image.SpatialIndex() == 0 ? 0 : 1); CodecSpecificInfo codec_specific; - codec_specific.codecType = kVideoCodecGeneric; + codec_specific.codecType = kVideoCodecVP8; return codec_specific; })); EXPECT_CALL(*adapter_ptr, UpdateLayerQualityConvergence(0, true));