From b6ef1a736ee94d97cc28f3bd59b826c716a3278f Mon Sep 17 00:00:00 2001 From: Sergey Silkin Date: Mon, 23 Oct 2023 17:11:21 +0200 Subject: [PATCH] Define default max Qp in media/base/media_constants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit kDefaultQpMax=56 was defined in multiple places. Move it to media_constants and split it into two: VPx/AV1 and H26x values. H26x value is set to 51 which is the max bitstream QP value for H264/5. This CL is expected to be a no-op because: 1. VideoCodec::qpMax value has not changed for VP8/9 and AV1. 2. VideoCodec::qpMax is currently not used by OpenH264 wrapper (wiring it up is out-of-scope of this CL). 3. Previous default qpMax=56 exceeded the max value for H26x (=51). External HW H26x encoders likely clamped it and used 51. Bug: webrtc:14852 Change-Id: I1d795e695dac5c78e86ed829b24281e61066f668 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/324282 Reviewed-by: Erik Språng Commit-Queue: Sergey Silkin Reviewed-by: Mirko Bonadei Cr-Commit-Position: refs/heads/main@{#40997} --- media/BUILD.gn | 1 + media/base/media_constants.cc | 5 +++++ media/base/media_constants.h | 2 ++ media/engine/simulcast_encoder_adapter.cc | 8 +------- media/engine/webrtc_video_engine.cc | 15 ++++++++++++++- media/engine/webrtc_video_engine.h | 2 ++ media/engine/webrtc_video_engine_unittest.cc | 17 +++++++++-------- rtc_tools/BUILD.gn | 1 + rtc_tools/video_encoder/video_encoder.cc | 11 ++++++----- test/scenario/video_stream.cc | 5 ++--- video/video_quality_test.cc | 8 +++----- 11 files changed, 46 insertions(+), 29 deletions(-) diff --git a/media/BUILD.gn b/media/BUILD.gn index 28822a1f54..cf7c523201 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -350,6 +350,7 @@ rtc_library("rtc_simulcast_encoder_adapter") { "../api/video_codecs:video_codecs_api", "../call:video_stream_api", "../common_video", + "../media:media_constants", "../modules/video_coding:video_codec_interface", "../modules/video_coding:video_coding_utility", "../rtc_base:checks", diff --git a/media/base/media_constants.cc b/media/base/media_constants.cc index 94ce3c7b21..2af0295a5a 100644 --- a/media/base/media_constants.cc +++ b/media/base/media_constants.cc @@ -127,6 +127,11 @@ const char kH265FmtpTxMode[] = "tx-mode"; const char kVP9ProfileId[] = "profile-id"; const int kDefaultVideoMaxFramerate = 60; +// Max encode quantizer for VP8/9 and AV1 encoders assuming libvpx/libaom API +// range [0, 63] +const int kDefaultVideoMaxQpVpx = 56; +// Max encode quantizer for H264/5 assuming the bitstream range [0, 51]. +const int kDefaultVideoMaxQpH26x = 51; const size_t kConferenceMaxNumSpatialLayers = 3; const size_t kConferenceMaxNumTemporalLayers = 3; diff --git a/media/base/media_constants.h b/media/base/media_constants.h index 3321aac41d..877cc7a296 100644 --- a/media/base/media_constants.h +++ b/media/base/media_constants.h @@ -150,6 +150,8 @@ RTC_EXPORT extern const char kH265FmtpTxMode[]; extern const char kVP9ProfileId[]; extern const int kDefaultVideoMaxFramerate; +extern const int kDefaultVideoMaxQpVpx; +extern const int kDefaultVideoMaxQpH26x; extern const size_t kConferenceMaxNumSpatialLayers; extern const size_t kConferenceMaxNumTemporalLayers; diff --git a/media/engine/simulcast_encoder_adapter.cc b/media/engine/simulcast_encoder_adapter.cc index f0bc6aeda8..4853e68996 100644 --- a/media/engine/simulcast_encoder_adapter.cc +++ b/media/engine/simulcast_encoder_adapter.cc @@ -29,6 +29,7 @@ #include "api/video_codecs/video_encoder.h" #include "api/video_codecs/video_encoder_factory.h" #include "api/video_codecs/video_encoder_software_fallback_wrapper.h" +#include "media/base/media_constants.h" #include "media/base/video_common.h" #include "modules/video_coding/include/video_error_codes.h" #include "modules/video_coding/utility/simulcast_rate_allocator.h" @@ -38,8 +39,6 @@ namespace { -const unsigned int kDefaultMinQp = 2; -const unsigned int kDefaultMaxQp = 56; // Max qp for lowest spatial resolution when doing simulcast. const unsigned int kLowestResMaxQp = 45; @@ -327,11 +326,6 @@ int SimulcastEncoderAdapter::InitEncode( codec_ = *codec_settings; total_streams_count_ = CountAllStreams(*codec_settings); - // TODO(ronghuawu): Remove once this is handled in LibvpxVp8Encoder. - if (codec_.qpMax < kDefaultMinQp) { - codec_.qpMax = kDefaultMaxQp; - } - bool is_legacy_singlecast = codec_.numberOfSimulcastStreams == 0; int lowest_quality_stream_idx = 0; int highest_quality_stream_idx = 0; diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index ca07ef5209..8a9d6ff95c 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -2228,7 +2228,20 @@ WebRtcVideoSendChannel::WebRtcVideoSendStream::CreateVideoEncoderConfig( // Ensure frame dropping is always enabled. encoder_config.frame_drop_enabled = true; - int max_qp = kDefaultQpMax; + int max_qp; + switch (encoder_config.codec_type) { + case webrtc::kVideoCodecH264: + case webrtc::kVideoCodecH265: + max_qp = kDefaultVideoMaxQpH26x; + break; + case webrtc::kVideoCodecVP8: + case webrtc::kVideoCodecVP9: + case webrtc::kVideoCodecAV1: + case webrtc::kVideoCodecGeneric: + case webrtc::kVideoCodecMultiplex: + max_qp = kDefaultVideoMaxQpVpx; + break; + } codec.GetParam(kCodecParamMaxQuantization, &max_qp); encoder_config.max_qp = max_qp; diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index 2d143122a6..e4b1b2765b 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -250,6 +250,8 @@ class WebRtcVideoSendChannel : public MediaChannelUtil, ADAPTREASON_BANDWIDTH = 2, }; + // TODO(webrtc:14852): Update downstream projects to use + // cricket::kDefaultVideoMaxQpVpx/H26x and remove. static constexpr int kDefaultQpMax = 56; // Implements webrtc::EncoderSwitchRequestCallback. diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index d9fcf828e1..e8b7ee4b2d 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -37,6 +37,7 @@ #include "api/video/video_bitrate_allocation.h" #include "api/video_codecs/h264_profile_level_id.h" #include "api/video_codecs/sdp_video_format.h" +#include "api/video_codecs/video_codec.h" #include "api/video_codecs/video_decoder_factory.h" #include "api/video_codecs/video_decoder_factory_template.h" #include "api/video_codecs/video_decoder_factory_template_dav1d_adapter.h" @@ -110,7 +111,6 @@ using ::webrtc::Timestamp; using ::webrtc::test::RtcpPacketParser; namespace { -static const int kDefaultQpMax = 56; static const uint8_t kRedRtxPayloadType = 125; @@ -4866,7 +4866,7 @@ TEST_F(WebRtcVideoChannelFlexfecSendRecvTest, TEST_F(WebRtcVideoChannelTest, SetSendCodecsChangesExistingStreams) { cricket::VideoSenderParameters parameters; cricket::VideoCodec codec = cricket::CreateVideoCodec(100, "VP8"); - codec.SetParam(kCodecParamMaxQuantization, kDefaultQpMax); + codec.SetParam(kCodecParamMaxQuantization, kDefaultVideoMaxQpVpx); parameters.codecs.push_back(codec); ASSERT_TRUE(send_channel_->SetSenderParameters(parameters)); @@ -4878,14 +4878,14 @@ TEST_F(WebRtcVideoChannelTest, SetSendCodecsChangesExistingStreams) { send_channel_->SetVideoSend(last_ssrc_, nullptr, &frame_forwarder)); std::vector streams = stream->GetVideoStreams(); - EXPECT_EQ(kDefaultQpMax, streams[0].max_qp); + EXPECT_EQ(kDefaultVideoMaxQpVpx, streams[0].max_qp); parameters.codecs.clear(); - codec.SetParam(kCodecParamMaxQuantization, kDefaultQpMax + 1); + codec.SetParam(kCodecParamMaxQuantization, kDefaultVideoMaxQpVpx + 1); parameters.codecs.push_back(codec); ASSERT_TRUE(send_channel_->SetSenderParameters(parameters)); streams = fake_call_->GetVideoSendStreams()[0]->GetVideoStreams(); - EXPECT_EQ(kDefaultQpMax + 1, streams[0].max_qp); + EXPECT_EQ(kDefaultVideoMaxQpVpx + 1, streams[0].max_qp); EXPECT_TRUE(send_channel_->SetVideoSend(last_ssrc_, nullptr, nullptr)); } @@ -9772,8 +9772,9 @@ class WebRtcVideoChannelSimulcastTest : public ::testing::Test { if (num_configured_streams > 1 || conference_mode) { expected_streams = GetSimulcastConfig( /*min_layers=*/1, num_configured_streams, capture_width, - capture_height, webrtc::kDefaultBitratePriority, kDefaultQpMax, - screenshare && conference_mode, true, field_trials_); + capture_height, webrtc::kDefaultBitratePriority, + kDefaultVideoMaxQpVpx, screenshare && conference_mode, true, + field_trials_); if (screenshare && conference_mode) { for (const webrtc::VideoStream& stream : expected_streams) { // Never scale screen content. @@ -9789,7 +9790,7 @@ class WebRtcVideoChannelSimulcastTest : public ::testing::Test { stream.min_bitrate_bps = webrtc::kDefaultMinVideoBitrateBps; stream.target_bitrate_bps = stream.max_bitrate_bps = GetMaxDefaultBitrateBps(capture_width, capture_height); - stream.max_qp = kDefaultQpMax; + stream.max_qp = kDefaultVideoMaxQpVpx; expected_streams.push_back(stream); } diff --git a/rtc_tools/BUILD.gn b/rtc_tools/BUILD.gn index 27a9a9d0ae..13d6df4e0f 100644 --- a/rtc_tools/BUILD.gn +++ b/rtc_tools/BUILD.gn @@ -448,6 +448,7 @@ if (!build_with_chromium) { "//api/video:builtin_video_bitrate_allocator_factory", "//api/video_codecs:builtin_video_encoder_factory", "//api/video_codecs:video_codecs_api", + "//media:media_constants", "//modules/video_coding:video_codec_interface", "//modules/video_coding:video_coding_utility", "//modules/video_coding/codecs/av1:av1_svc_config", diff --git a/rtc_tools/video_encoder/video_encoder.cc b/rtc_tools/video_encoder/video_encoder.cc index d2b9b5844e..9436910ed3 100644 --- a/rtc_tools/video_encoder/video_encoder.cc +++ b/rtc_tools/video_encoder/video_encoder.cc @@ -16,6 +16,7 @@ #include "api/test/frame_generator_interface.h" #include "api/video/builtin_video_bitrate_allocator_factory.h" #include "api/video_codecs/builtin_video_encoder_factory.h" +#include "media/base/media_constants.h" #include "modules/video_coding/codecs/av1/av1_svc_config.h" #include "modules/video_coding/include/video_codec_interface.h" #include "modules/video_coding/svc/scalability_mode_util.h" @@ -72,9 +73,6 @@ ABSL_FLAG(bool, verbose, false, "Verbose logs to stderr"); namespace webrtc { namespace { -// See `WebRtcVideoSendChannel::kDefaultQpMax`. -constexpr unsigned int kDefaultQpMax = 56; - [[maybe_unused]] const char* InterLayerPredModeToString( const InterLayerPredMode& inter_layer_pred_mode) { switch (inter_layer_pred_mode) { @@ -257,8 +255,6 @@ class TestVideoEncoderFactoryWrapper final { video_codec.active = true; - video_codec.qpMax = kDefaultQpMax; - // Simulcast is not implemented at this moment. video_codec.numberOfSimulcastStreams = 0; @@ -270,6 +266,7 @@ class TestVideoEncoderFactoryWrapper final { *(video_codec.VP8()) = VideoEncoder::GetDefaultVp8Settings(); video_codec.VP8()->numberOfTemporalLayers = temporal_layers; + video_codec.qpMax = cricket::kDefaultVideoMaxQpVpx; break; case kVideoCodecVP9: @@ -277,6 +274,7 @@ class TestVideoEncoderFactoryWrapper final { video_codec.VP9()->numberOfSpatialLayers = spatial_layers; video_codec.VP9()->numberOfTemporalLayers = temporal_layers; video_codec.VP9()->interLayerPred = inter_layer_pred_mode; + video_codec.qpMax = cricket::kDefaultVideoMaxQpVpx; break; case kVideoCodecH264: @@ -284,6 +282,7 @@ class TestVideoEncoderFactoryWrapper final { *(video_codec.H264()) = VideoEncoder::GetDefaultH264Settings(); video_codec.H264()->numberOfTemporalLayers = temporal_layers; + video_codec.qpMax = cricket::kDefaultVideoMaxQpH26x; break; case kVideoCodecAV1: @@ -294,9 +293,11 @@ class TestVideoEncoderFactoryWrapper final { } else { RTC_LOG(LS_WARNING) << "Failed to configure svc bitrates for av1."; } + video_codec.qpMax = cricket::kDefaultVideoMaxQpVpx; break; case kVideoCodecH265: // TODO(bugs.webrtc.org/13485) + video_codec.qpMax = cricket::kDefaultVideoMaxQpH26x; break; default: RTC_CHECK_NOTREACHED(); diff --git a/test/scenario/video_stream.cc b/test/scenario/video_stream.cc index d376a8fbb3..38e42c88e0 100644 --- a/test/scenario/video_stream.cc +++ b/test/scenario/video_stream.cc @@ -40,7 +40,6 @@ enum : int { // The first valid value is 1. kVideoRotationRtpExtensionId, }; -constexpr int kDefaultMaxQp = cricket::WebRtcVideoSendChannel::kDefaultQpMax; uint8_t CodecTypeToPayloadType(VideoCodecType codec_type) { switch (codec_type) { case VideoCodecType::kVideoCodecGeneric: @@ -234,8 +233,8 @@ VideoEncoderConfig CreateVideoEncoderConfig(VideoStreamConfig config) { VideoStreamConfig::Encoder::ContentType::kScreen; encoder_config.video_stream_factory = rtc::make_ref_counted( - cricket_codec, kDefaultMaxQp, screenshare, screenshare, - encoder_info); + cricket_codec, cricket::kDefaultVideoMaxQpVpx, screenshare, + screenshare, encoder_info); } else { encoder_config.video_stream_factory = rtc::make_ref_counted(); diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index 4086cdf62e..971c129329 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -77,8 +77,6 @@ constexpr int kFramesSentInQuickTest = 1; constexpr uint32_t kThumbnailSendSsrcStart = 0xE0000; constexpr uint32_t kThumbnailRtxSsrcStart = 0xF0000; -constexpr int kDefaultMaxQp = cricket::WebRtcVideoSendChannel::kDefaultQpMax; - const VideoEncoder::Capabilities kCapabilities(false); std::pair GetMinMaxBitratesBps(const VideoCodec& codec, @@ -579,7 +577,7 @@ VideoStream VideoQualityTest::DefaultVideoStream(const Params& params, stream.min_bitrate_bps = params.video[video_idx].min_bitrate_bps; stream.target_bitrate_bps = params.video[video_idx].target_bitrate_bps; stream.max_bitrate_bps = params.video[video_idx].max_bitrate_bps; - stream.max_qp = kDefaultMaxQp; + stream.max_qp = cricket::kDefaultVideoMaxQpVpx; stream.num_temporal_layers = params.video[video_idx].num_temporal_layers; stream.active = true; return stream; @@ -594,7 +592,7 @@ VideoStream VideoQualityTest::DefaultThumbnailStream() { stream.min_bitrate_bps = 7500; stream.target_bitrate_bps = 37500; stream.max_bitrate_bps = 50000; - stream.max_qp = kDefaultMaxQp; + stream.max_qp = cricket::kDefaultVideoMaxQpVpx; return stream; } @@ -627,7 +625,7 @@ void VideoQualityTest::FillScalabilitySettings( encoder_config.simulcast_layers = std::vector(num_streams); encoder_config.video_stream_factory = rtc::make_ref_counted( - params->video[video_idx].codec, kDefaultMaxQp, + params->video[video_idx].codec, cricket::kDefaultVideoMaxQpVpx, params->screenshare[video_idx].enabled, true, encoder_info); params->ss[video_idx].streams = encoder_config.video_stream_factory->CreateEncoderStreams(