diff --git a/api/BUILD.gn b/api/BUILD.gn index ec60e6f26c..c170a76f45 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -457,6 +457,7 @@ rtc_source_set("peer_connection_quality_test_fixture_api") { ":video_quality_analyzer_api", "../media:rtc_media_base", "../modules/audio_processing:api", + "../rtc_base:checks", "../rtc_base:rtc_base", "../rtc_base:threading", "audio:audio_mixer_api", diff --git a/api/test/peerconnection_quality_test_fixture.cc b/api/test/peerconnection_quality_test_fixture.cc index b5393c219d..59526f9f52 100644 --- a/api/test/peerconnection_quality_test_fixture.cc +++ b/api/test/peerconnection_quality_test_fixture.cc @@ -12,6 +12,7 @@ #include "absl/types/optional.h" #include "api/array_view.h" +#include "rtc_base/checks.h" namespace webrtc { namespace webrtc_pc_e2e { @@ -21,23 +22,16 @@ using VideoCodecConfig = ::webrtc::webrtc_pc_e2e:: using VideoSubscription = ::webrtc::webrtc_pc_e2e:: PeerConnectionE2EQualityTestFixture::VideoSubscription; -PeerConnectionE2EQualityTestFixture::VideoSubscription::Resolution::Resolution( +PeerConnectionE2EQualityTestFixture::VideoResolution::VideoResolution( size_t width, size_t height, int32_t fps) : width_(width), height_(height), fps_(fps), spec_(Spec::kNone) {} -PeerConnectionE2EQualityTestFixture::VideoSubscription::Resolution::Resolution( - const VideoConfig& video_config) - : width_(video_config.width), - height_(video_config.height), - fps_(video_config.fps), - spec_(Spec::kNone) {} -PeerConnectionE2EQualityTestFixture::VideoSubscription::Resolution::Resolution( - Spec spec) +PeerConnectionE2EQualityTestFixture::VideoResolution::VideoResolution(Spec spec) : width_(0), height_(0), fps_(0), spec_(spec) {} -bool PeerConnectionE2EQualityTestFixture::VideoSubscription::Resolution:: -operator==(const Resolution& other) const { +bool PeerConnectionE2EQualityTestFixture::VideoResolution::operator==( + const VideoResolution& other) const { if (spec_ != Spec::kNone && spec_ == other.spec_) { // If there is some particular spec set, then it doesn't matter what // values we have in other fields. @@ -47,26 +41,25 @@ operator==(const Resolution& other) const { fps_ == other.fps_ && spec_ == other.spec_; } -absl::optional +absl::optional PeerConnectionE2EQualityTestFixture::VideoSubscription::GetMaxResolution( rtc::ArrayView video_configs) { - std::vector resolutions; + std::vector resolutions; for (const auto& video_config : video_configs) { - resolutions.push_back(VideoSubscription::Resolution( - video_config.width, video_config.height, video_config.fps)); + resolutions.push_back(video_config.GetResolution()); } return GetMaxResolution(resolutions); } -absl::optional +absl::optional PeerConnectionE2EQualityTestFixture::VideoSubscription::GetMaxResolution( - rtc::ArrayView resolutions) { + rtc::ArrayView resolutions) { if (resolutions.empty()) { return absl::nullopt; } - VideoSubscription::Resolution max_resolution; - for (const VideoSubscription::Resolution& resolution : resolutions) { + VideoResolution max_resolution; + for (const VideoResolution& resolution : resolutions) { if (max_resolution.width() < resolution.width()) { max_resolution.set_width(resolution.width()); } @@ -80,5 +73,13 @@ PeerConnectionE2EQualityTestFixture::VideoSubscription::GetMaxResolution( return max_resolution; } +PeerConnectionE2EQualityTestFixture::VideoConfig::VideoConfig( + const VideoResolution& resolution) + : width(resolution.width()), + height(resolution.height()), + fps(resolution.fps()) { + RTC_CHECK(resolution.IsRegular()); +} + } // namespace webrtc_pc_e2e } // namespace webrtc diff --git a/api/test/peerconnection_quality_test_fixture.h b/api/test/peerconnection_quality_test_fixture.h index bbeb7ca3e4..fb60998b14 100644 --- a/api/test/peerconnection_quality_test_fixture.h +++ b/api/test/peerconnection_quality_test_fixture.h @@ -179,8 +179,48 @@ class PeerConnectionE2EQualityTestFixture { std::vector encoding_params; }; + class VideoResolution { + public: + // Determines special resolutions, which can't be expressed in terms of + // width, height and fps. + enum class Spec { + // No extra spec set. It describes a regular resolution described by + // width, height and fps. + kNone, + // Describes resolution which contains max value among all sender's + // video streams in each dimension (width, height, fps). + kMaxFromSender + }; + + VideoResolution(size_t width, size_t height, int32_t fps); + explicit VideoResolution(Spec spec = Spec::kNone); + + bool operator==(const VideoResolution& other) const; + bool operator!=(const VideoResolution& other) const { + return !(*this == other); + } + + size_t width() const { return width_; } + void set_width(size_t width) { width_ = width; } + size_t height() const { return height_; } + void set_height(size_t height) { height_ = height; } + int32_t fps() const { return fps_; } + void set_fps(int32_t fps) { fps_ = fps; } + + // Returns if it is a regular resolution or not. The resolution is regular + // if it's spec is `Spec::kNone`. + bool IsRegular() const { return spec_ == Spec::kNone; } + + private: + size_t width_ = 0; + size_t height_ = 0; + int32_t fps_ = 0; + Spec spec_ = Spec::kNone; + }; + // Contains properties of single video stream. struct VideoConfig { + explicit VideoConfig(const VideoResolution& resolution); VideoConfig(size_t width, size_t height, int32_t fps) : width(width), height(height), fps(fps) {} VideoConfig(std::string stream_label, @@ -197,6 +237,10 @@ class PeerConnectionE2EQualityTestFixture { // Video stream height. const size_t height; const int32_t fps; + VideoResolution GetResolution() const { + return VideoResolution(width, height, fps); + } + // Have to be unique among all specified configs for all peers in the call. // Will be auto generated if omitted. absl::optional stream_label; @@ -332,55 +376,24 @@ class PeerConnectionE2EQualityTestFixture { // peer should receive and in which resolution (width x height x fps). class VideoSubscription { public: - class Resolution { - public: - // Determines special resolutions, which can't be expressed in terms of - // width, height and fps. - enum class Spec { - // No extra spec set. It describes a regular resolution described by - // width, height and fps. - kNone, - // Describes resolution which contains max value among all sender's - // video streams in each dimension (width, height, fps). - kMaxFromSender - }; - - Resolution(size_t width, size_t height, int32_t fps); - explicit Resolution(const VideoConfig& video_config); - explicit Resolution(Spec spec = Spec::kNone); - - bool operator==(const Resolution& other) const; - bool operator!=(const Resolution& other) const { - return !(*this == other); - } - - size_t width() const { return width_; } - void set_width(size_t width) { width_ = width; } - size_t height() const { return height_; } - void set_height(size_t height) { height_ = height; } - int32_t fps() const { return fps_; } - void set_fps(int32_t fps) { fps_ = fps; } - - private: - size_t width_ = 0; - size_t height_ = 0; - int32_t fps_ = 0; - Spec spec_ = Spec::kNone; - }; + // Kept for backward compatibility + // TODO(titovartem): remove it when downstream projects will stop use it. + using Resolution = VideoResolution; // Returns the resolution constructed as maximum from all resolution // dimensions: width, height and fps. - static absl::optional GetMaxResolution( + static absl::optional GetMaxResolution( rtc::ArrayView video_configs); - static absl::optional GetMaxResolution( - rtc::ArrayView resolutions); + static absl::optional GetMaxResolution( + rtc::ArrayView resolutions); // Subscribes receiver to all streams sent by the specified peer with // specified resolution. It will override any resolution that was used in // `SubscribeToAll` independently from methods call order. VideoSubscription& SubscribeToPeer( absl::string_view peer_name, - Resolution resolution = Resolution(Resolution::Spec::kMaxFromSender)) { + VideoResolution resolution = + VideoResolution(VideoResolution::Spec::kMaxFromSender)) { peers_resolution_[std::string(peer_name)] = resolution; return *this; } @@ -390,7 +403,8 @@ class PeerConnectionE2EQualityTestFixture { // override resolution passed to this function independently from methods // call order. VideoSubscription& SubscribeToAllPeers( - Resolution resolution = Resolution(Resolution::Spec::kMaxFromSender)) { + VideoResolution resolution = + VideoResolution(VideoResolution::Spec::kMaxFromSender)) { default_resolution_ = resolution; return *this; } @@ -399,7 +413,7 @@ class PeerConnectionE2EQualityTestFixture { // set for this sender, then will return resolution used for all streams. // If subscription doesn't subscribe to all streams, `absl::nullopt` will be // returned. - absl::optional GetResolutionForPeer( + absl::optional GetResolutionForPeer( absl::string_view peer_name) const { auto it = peers_resolution_.find(std::string(peer_name)); if (it == peers_resolution_.end()) { @@ -419,8 +433,8 @@ class PeerConnectionE2EQualityTestFixture { } private: - absl::optional default_resolution_ = absl::nullopt; - std::map peers_resolution_; + absl::optional default_resolution_ = absl::nullopt; + std::map peers_resolution_; }; // This class is used to fully configure one peer inside the call. diff --git a/api/test/peerconnection_quality_test_fixture_unittest.cc b/api/test/peerconnection_quality_test_fixture_unittest.cc index a4d972d48f..c0e739cc23 100644 --- a/api/test/peerconnection_quality_test_fixture_unittest.cc +++ b/api/test/peerconnection_quality_test_fixture_unittest.cc @@ -20,19 +20,19 @@ namespace webrtc { namespace webrtc_pc_e2e { namespace { -using VideoSubscription = ::webrtc::webrtc_pc_e2e:: - PeerConnectionE2EQualityTestFixture::VideoSubscription; +using VideoResolution = ::webrtc::webrtc_pc_e2e:: + PeerConnectionE2EQualityTestFixture::VideoResolution; using VideoConfig = ::webrtc::webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::VideoConfig; +using VideoSubscription = ::webrtc::webrtc_pc_e2e:: + PeerConnectionE2EQualityTestFixture::VideoSubscription; TEST(PclfVideoSubscription, MaxFromSenderSpecEqualIndependentOfOtherFields) { - VideoSubscription::Resolution r1( - VideoSubscription::Resolution::Spec::kMaxFromSender); + VideoResolution r1(VideoResolution::Spec::kMaxFromSender); r1.set_width(1); r1.set_height(2); r1.set_fps(3); - VideoSubscription::Resolution r2( - VideoSubscription::Resolution::Spec::kMaxFromSender); + VideoResolution r2(VideoResolution::Spec::kMaxFromSender); r1.set_width(4); r1.set_height(5); r1.set_fps(6); @@ -40,16 +40,16 @@ TEST(PclfVideoSubscription, MaxFromSenderSpecEqualIndependentOfOtherFields) { } TEST(PclfVideoSubscription, WhenSpecIsNotSetFieldsAreCompared) { - VideoSubscription::Resolution test_resolution(/*width=*/1, /*height=*/2, - /*fps=*/3); - VideoSubscription::Resolution equal_resolution(/*width=*/1, /*height=*/2, - /*fps=*/3); - VideoSubscription::Resolution different_width(/*width=*/10, /*height=*/2, - /*fps=*/3); - VideoSubscription::Resolution different_height(/*width=*/1, /*height=*/20, - /*fps=*/3); - VideoSubscription::Resolution different_fps(/*width=*/1, /*height=*/20, - /*fps=*/30); + VideoResolution test_resolution(/*width=*/1, /*height=*/2, + /*fps=*/3); + VideoResolution equal_resolution(/*width=*/1, /*height=*/2, + /*fps=*/3); + VideoResolution different_width(/*width=*/10, /*height=*/2, + /*fps=*/3); + VideoResolution different_height(/*width=*/1, /*height=*/20, + /*fps=*/3); + VideoResolution different_fps(/*width=*/1, /*height=*/20, + /*fps=*/30); EXPECT_EQ(test_resolution, equal_resolution); EXPECT_NE(test_resolution, different_width); @@ -58,7 +58,7 @@ TEST(PclfVideoSubscription, WhenSpecIsNotSetFieldsAreCompared) { } TEST(PclfVideoSubscription, GetMaxResolutionForEmptyReturnsNullopt) { - absl::optional resolution = + absl::optional resolution = VideoSubscription::GetMaxResolution(std::vector{}); ASSERT_FALSE(resolution.has_value()); } @@ -68,7 +68,7 @@ TEST(PclfVideoSubscription, GetMaxResolutionSelectMaxForEachDimention) { VideoConfig max_height(/*width=*/1, /*height=*/100, /*fps=*/1); VideoConfig max_fps(/*width=*/1, /*height=*/1, /*fps=*/10); - absl::optional resolution = + absl::optional resolution = VideoSubscription::GetMaxResolution( std::vector{max_width, max_height, max_fps}); ASSERT_TRUE(resolution.has_value());