VideoStreamEncoder: set at target quality based on codec.
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 <sprang@webrtc.org>
Commit-Queue: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38091}
diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn
index 031db70..c73e011 100644
--- a/modules/video_coding/BUILD.gn
+++ b/modules/video_coding/BUILD.gn
@@ -438,6 +438,7 @@
"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 dc0344c..49ccf2d 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 @@
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 6437587..74477ea 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 @@
// 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 0000000..9321864
--- /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 <stddef.h>
+#include <stdint.h>
+
+#include <string>
+
+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 cbea3b3..e320e43 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 @@
}
}
-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 @@
}
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 @@
RTC_CHECK(videocontenttypehelpers::SetSimulcastId(
&image_copy.content_type_, static_cast<uint8_t>(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 60fd263..c4c461f 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 @@
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 3a5b03d..3baa9c5 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 @@
EXPECT_CALL(factory.GetMockFakeEncoder(), EncodeHook)
.WillRepeatedly(Invoke([](EncodedImage& encoded_image,
rtc::scoped_refptr<EncodedImageBuffer> 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 @@
EXPECT_CALL(factory.GetMockFakeEncoder(), EncodeHook)
.WillRepeatedly(Invoke([](EncodedImage& encoded_image,
rtc::scoped_refptr<EncodedImageBuffer> 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));
