Test that VideoCodecInitializer propagates VP9 resolution alignment.
The GetSvcConfig() call here[1] can result in resolution alignment due
to [2], which gets propagated to the output VideoCodec due to [3].
This CL adds test coverage for this part.
It also removes some comments that are no longer true and updates
VideoCodecInitializerTest's SetUpFor() to make number of simulcast
streams explicit.
[1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/modules/video_coding/video_codec_initializer.cc;l=251;drc=e4a304ed4da869ab6131a06b3e8b7e985f50229d
[2] https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/modules/video_coding/codecs/vp9/svc_config.cc;l=112;drc=31acc7339cf658ce82c7ec490ba38d67170920ed
[3] https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/modules/video_coding/video_codec_initializer.cc;l=285;drc=e4a304ed4da869ab6131a06b3e8b7e985f50229d
Bug: webrtc:14884
Change-Id: Id22e36aebab573f53d15dca688642d32c8c4be7a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296762
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39514}
diff --git a/modules/video_coding/include/video_codec_initializer.h b/modules/video_coding/include/video_codec_initializer.h
index 270c4db..2d10ee4 100644
--- a/modules/video_coding/include/video_codec_initializer.h
+++ b/modules/video_coding/include/video_codec_initializer.h
@@ -19,17 +19,12 @@
namespace webrtc {
-class VideoBitrateAllocator;
class VideoCodec;
class VideoCodecInitializer {
public:
// Takes a VideoEncoderConfig and the VideoStream configuration and
// translates them into the old school VideoCodec type.
- // It also creates a VideoBitrateAllocator instance, suitable for the codec
- // type used. For instance, VP8 will create an allocator than can handle
- // simulcast and temporal layering.
- // GetBitrateAllocator is called implicitly from here, no need to call again.
static bool SetupCodec(const VideoEncoderConfig& config,
const std::vector<VideoStream>& streams,
VideoCodec* codec);
diff --git a/modules/video_coding/video_codec_initializer.cc b/modules/video_coding/video_codec_initializer.cc
index e1885d7..bf59d89 100644
--- a/modules/video_coding/video_codec_initializer.cc
+++ b/modules/video_coding/video_codec_initializer.cc
@@ -18,7 +18,6 @@
#include "absl/types/optional.h"
#include "api/scoped_refptr.h"
#include "api/units/data_rate.h"
-#include "api/video/video_bitrate_allocation.h"
#include "api/video_codecs/video_encoder.h"
#include "modules/video_coding/codecs/av1/av1_svc_config.h"
#include "modules/video_coding/codecs/vp8/vp8_scalability.h"
diff --git a/modules/video_coding/video_codec_initializer_unittest.cc b/modules/video_coding/video_codec_initializer_unittest.cc
index 0e6f2df..04d6354 100644
--- a/modules/video_coding/video_codec_initializer_unittest.cc
+++ b/modules/video_coding/video_codec_initializer_unittest.cc
@@ -58,7 +58,8 @@
protected:
void SetUpFor(VideoCodecType type,
- int num_spatial_streams,
+ absl::optional<int> num_simulcast_streams,
+ absl::optional<int> num_spatial_streams,
int num_temporal_streams,
bool screenshare) {
config_ = VideoEncoderConfig();
@@ -70,14 +71,18 @@
}
if (type == VideoCodecType::kVideoCodecVP8) {
- config_.number_of_streams = num_spatial_streams;
+ ASSERT_TRUE(num_simulcast_streams.has_value());
+ ASSERT_FALSE(num_spatial_streams.has_value());
+ config_.number_of_streams = num_simulcast_streams.value();
VideoCodecVP8 vp8_settings = VideoEncoder::GetDefaultVp8Settings();
vp8_settings.numberOfTemporalLayers = num_temporal_streams;
config_.encoder_specific_settings = rtc::make_ref_counted<
webrtc::VideoEncoderConfig::Vp8EncoderSpecificSettings>(vp8_settings);
} else if (type == VideoCodecType::kVideoCodecVP9) {
+ ASSERT_FALSE(num_simulcast_streams.has_value());
+ ASSERT_TRUE(num_spatial_streams.has_value());
VideoCodecVP9 vp9_settings = VideoEncoder::GetDefaultVp9Settings();
- vp9_settings.numberOfSpatialLayers = num_spatial_streams;
+ vp9_settings.numberOfSpatialLayers = num_spatial_streams.value();
vp9_settings.numberOfTemporalLayers = num_temporal_streams;
config_.encoder_specific_settings = rtc::make_ref_counted<
webrtc::VideoEncoderConfig::Vp9EncoderSpecificSettings>(vp9_settings);
@@ -147,7 +152,7 @@
};
TEST_F(VideoCodecInitializerTest, SingleStreamVp8Screenshare) {
- SetUpFor(VideoCodecType::kVideoCodecVP8, 1, 1, true);
+ SetUpFor(VideoCodecType::kVideoCodecVP8, 1, absl::nullopt, 1, true);
streams_.push_back(DefaultStream());
EXPECT_TRUE(InitializeCodec());
@@ -160,7 +165,7 @@
}
TEST_F(VideoCodecInitializerTest, SingleStreamVp8ScreenshareInactive) {
- SetUpFor(VideoCodecType::kVideoCodecVP8, 1, 1, true);
+ SetUpFor(VideoCodecType::kVideoCodecVP8, 1, absl::nullopt, 1, true);
VideoStream inactive_stream = DefaultStream();
inactive_stream.active = false;
streams_.push_back(inactive_stream);
@@ -175,7 +180,7 @@
}
TEST_F(VideoCodecInitializerTest, TemporalLayeredVp8ScreenshareConference) {
- SetUpFor(VideoCodecType::kVideoCodecVP8, 1, 2, true);
+ SetUpFor(VideoCodecType::kVideoCodecVP8, 1, absl::nullopt, 2, true);
streams_.push_back(DefaultScreenshareStream());
EXPECT_TRUE(InitializeCodec());
bitrate_allocator_->SetLegacyConferenceMode(true);
@@ -192,7 +197,7 @@
}
TEST_F(VideoCodecInitializerTest, TemporalLayeredVp8Screenshare) {
- SetUpFor(VideoCodecType::kVideoCodecVP8, 1, 2, true);
+ SetUpFor(VideoCodecType::kVideoCodecVP8, 1, absl::nullopt, 2, true);
streams_.push_back(DefaultScreenshareStream());
EXPECT_TRUE(InitializeCodec());
@@ -207,7 +212,7 @@
}
TEST_F(VideoCodecInitializerTest, SimulcastVp8Screenshare) {
- SetUpFor(VideoCodecType::kVideoCodecVP8, 2, 1, true);
+ SetUpFor(VideoCodecType::kVideoCodecVP8, 2, absl::nullopt, 1, true);
streams_.push_back(DefaultScreenshareStream());
VideoStream video_stream = DefaultStream();
video_stream.max_framerate = kScreenshareDefaultFramerate;
@@ -231,7 +236,7 @@
// Tests that when a video stream is inactive, then the bitrate allocation will
// be 0 for that stream.
TEST_F(VideoCodecInitializerTest, SimulcastVp8ScreenshareInactive) {
- SetUpFor(VideoCodecType::kVideoCodecVP8, 2, 1, true);
+ SetUpFor(VideoCodecType::kVideoCodecVP8, 2, absl::nullopt, 1, true);
streams_.push_back(DefaultScreenshareStream());
VideoStream inactive_video_stream = DefaultStream();
inactive_video_stream.active = false;
@@ -256,7 +261,7 @@
TEST_F(VideoCodecInitializerTest, HighFpsSimulcastVp8Screenshare) {
// Two simulcast streams, the lower one using legacy settings (two temporal
// streams, 5fps), the higher one using 3 temporal streams and 30fps.
- SetUpFor(VideoCodecType::kVideoCodecVP8, 2, 3, true);
+ SetUpFor(VideoCodecType::kVideoCodecVP8, 2, absl::nullopt, 3, true);
streams_.push_back(DefaultScreenshareStream());
VideoStream video_stream = DefaultStream();
video_stream.num_temporal_layers = 3;
@@ -280,13 +285,13 @@
}
TEST_F(VideoCodecInitializerTest, SingleStreamMultiplexCodec) {
- SetUpFor(VideoCodecType::kVideoCodecMultiplex, 1, 1, true);
+ SetUpFor(VideoCodecType::kVideoCodecMultiplex, absl::nullopt, 1, 1, true);
streams_.push_back(DefaultStream());
EXPECT_TRUE(InitializeCodec());
}
TEST_F(VideoCodecInitializerTest, Vp9SvcDefaultLayering) {
- SetUpFor(VideoCodecType::kVideoCodecVP9, 3, 3, false);
+ SetUpFor(VideoCodecType::kVideoCodecVP9, absl::nullopt, 3, 3, false);
VideoStream stream = DefaultStream();
stream.num_temporal_layers = 3;
streams_.push_back(stream);
@@ -297,7 +302,7 @@
}
TEST_F(VideoCodecInitializerTest, Vp9SvcAdjustedLayering) {
- SetUpFor(VideoCodecType::kVideoCodecVP9, 3, 3, false);
+ SetUpFor(VideoCodecType::kVideoCodecVP9, absl::nullopt, 3, 3, false);
VideoStream stream = DefaultStream();
stream.num_temporal_layers = 3;
// Set resolution which is only enough to produce 2 spatial layers.
@@ -312,7 +317,7 @@
TEST_F(VideoCodecInitializerTest,
Vp9SingleSpatialLayerMaxBitrateIsEqualToCodecMaxBitrate) {
- SetUpFor(VideoCodecType::kVideoCodecVP9, 1, 3, false);
+ SetUpFor(VideoCodecType::kVideoCodecVP9, absl::nullopt, 1, 3, false);
VideoStream stream = DefaultStream();
stream.num_temporal_layers = 3;
streams_.push_back(stream);
@@ -324,7 +329,7 @@
TEST_F(VideoCodecInitializerTest,
Vp9SingleSpatialLayerTargetBitrateIsEqualToCodecMaxBitrate) {
- SetUpFor(VideoCodecType::kVideoCodecVP9, 1, 1, true);
+ SetUpFor(VideoCodecType::kVideoCodecVP9, absl::nullopt, 1, 1, true);
VideoStream stream = DefaultStream();
stream.num_temporal_layers = 1;
streams_.push_back(stream);
@@ -339,7 +344,7 @@
// Request 3 spatial layers for 320x180 input. Actual number of layers will be
// reduced to 1 due to low input resolution but SVC bitrate limits should be
// applied.
- SetUpFor(VideoCodecType::kVideoCodecVP9, 3, 3, false);
+ SetUpFor(VideoCodecType::kVideoCodecVP9, absl::nullopt, 3, 3, false);
VideoStream stream = DefaultStream();
stream.width = 320;
stream.height = 180;
@@ -352,7 +357,7 @@
}
TEST_F(VideoCodecInitializerTest, Vp9DeactivateLayers) {
- SetUpFor(VideoCodecType::kVideoCodecVP9, 3, 1, false);
+ SetUpFor(VideoCodecType::kVideoCodecVP9, absl::nullopt, 3, 1, false);
VideoStream stream = DefaultStream();
streams_.push_back(stream);
@@ -425,6 +430,22 @@
EXPECT_FALSE(codec_out_.spatialLayers[2].active);
}
+TEST_F(VideoCodecInitializerTest, Vp9SvcResolutionAlignment) {
+ SetUpFor(VideoCodecType::kVideoCodecVP9, absl::nullopt, 3, 3, false);
+ VideoStream stream = DefaultStream();
+ stream.width = 1281;
+ stream.height = 721;
+ stream.num_temporal_layers = 3;
+ streams_.push_back(stream);
+
+ EXPECT_TRUE(InitializeCodec());
+ EXPECT_EQ(codec_out_.width, 1280);
+ EXPECT_EQ(codec_out_.height, 720);
+ EXPECT_EQ(codec_out_.numberOfSimulcastStreams, 1);
+ EXPECT_EQ(codec_out_.simulcastStream[0].width, 1280);
+ EXPECT_EQ(codec_out_.simulcastStream[0].height, 720);
+}
+
TEST_F(VideoCodecInitializerTest, Av1SingleSpatialLayerBitratesAreConsistent) {
VideoEncoderConfig config;
config.codec_type = VideoCodecType::kVideoCodecAV1;
