Adjust AnalyzingVideoSink to work with empty requested resolution
- avoid trying to log requested resolution when it is nullopt
- avoid scaling when required resolution happens to be empty. Frame may still arrive in such scenario either because of bugs test tries to catch, or simly because of asynchronous nature of the system under test.
Bug: b/227581196
Change-Id: If1f210c7e372285be38b3f30482827afcb80ede0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/371920
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Jeremy Leconte <jleconte@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43590}
diff --git a/test/pc/e2e/analyzer/video/analyzing_video_sink.cc b/test/pc/e2e/analyzer/video/analyzing_video_sink.cc
index 20e3966..56a2bd3 100644
--- a/test/pc/e2e/analyzer/video/analyzing_video_sink.cc
+++ b/test/pc/e2e/analyzer/video/analyzing_video_sink.cc
@@ -9,6 +9,7 @@
*/
#include "test/pc/e2e/analyzer/video/analyzing_video_sink.h"
+#include <cstddef>
#include <memory>
#include <optional>
#include <set>
@@ -60,13 +61,14 @@
for (auto it = stream_sinks_.cbegin(); it != stream_sinks_.cend();) {
std::optional<VideoResolution> new_requested_resolution =
subscription_.GetResolutionForPeer(it->second.sender_peer_name);
- if (!new_requested_resolution.has_value() ||
- (*new_requested_resolution != it->second.resolution)) {
+ if (new_requested_resolution != it->second.resolution) {
RTC_LOG(LS_INFO) << peer_name_ << ": Subscribed resolution for stream "
<< it->first << " from " << it->second.sender_peer_name
<< " was updated from "
<< it->second.resolution.ToString() << " to "
- << new_requested_resolution->ToString()
+ << (new_requested_resolution.has_value()
+ ? new_requested_resolution->ToString()
+ : "none")
<< ". Repopulating all video sinks and recreating "
<< "requested video writers";
writers_to_close.insert(it->second.video_frame_writer);
@@ -140,8 +142,10 @@
const VideoFrame& frame,
const VideoResolution& required_resolution) {
Timestamp processing_started = clock_->CurrentTime();
- if (required_resolution.width() == static_cast<size_t>(frame.width()) &&
- required_resolution.height() == static_cast<size_t>(frame.height())) {
+ if ((required_resolution.width() == static_cast<size_t>(frame.width()) &&
+ required_resolution.height() == static_cast<size_t>(frame.height())) ||
+ !required_resolution.IsRegular() ||
+ (required_resolution.width() == 0 || required_resolution.height() == 0)) {
if (report_infra_stats_) {
stats_.scaling_tims_ms.AddSample(
(clock_->CurrentTime() - processing_started).ms<double>());
diff --git a/test/pc/e2e/analyzer/video/analyzing_video_sink_test.cc b/test/pc/e2e/analyzer/video/analyzing_video_sink_test.cc
index b2f8ef8..23c3bae 100644
--- a/test/pc/e2e/analyzer/video/analyzing_video_sink_test.cc
+++ b/test/pc/e2e/analyzer/video/analyzing_video_sink_test.cc
@@ -321,6 +321,74 @@
ExpectOutputFilesCount(2);
}
+TEST_F(AnalyzingVideoSinkTest, KeepsCountingFrameWhenUnsucsribed) {
+ VideoSubscription subscription_before;
+ subscription_before.SubscribeToPeer(
+ "alice", VideoResolution(/*width=*/1280, /*height=*/720, /*fps=*/30));
+
+ VideoConfig video_config("alice_video", /*width=*/1280, /*height=*/720,
+ /*fps=*/30);
+
+ ExampleVideoQualityAnalyzer analyzer;
+ std::unique_ptr<test::FrameGeneratorInterface> frame_generator =
+ CreateFrameGenerator(/*width=*/1280, /*height=*/720);
+ VideoFrame frame_before = CreateFrame(*frame_generator);
+ frame_before.set_id(
+ analyzer.OnFrameCaptured("alice", "alice_video", frame_before));
+ VideoFrame frame_after = CreateFrame(*frame_generator);
+ frame_after.set_id(
+ analyzer.OnFrameCaptured("alice", "alice_video", frame_after));
+
+ {
+ AnalyzingVideoSinksHelper helper;
+ helper.AddConfig("alice", video_config);
+ AnalyzingVideoSink sink("bob", Clock::GetRealTimeClock(), analyzer, helper,
+ subscription_before, /*report_infra_stats=*/false);
+ sink.OnFrame(frame_before);
+
+ sink.UpdateSubscription(VideoSubscription());
+ sink.OnFrame(frame_after);
+ }
+
+ EXPECT_THAT(analyzer.frames_rendered(), Eq(2));
+}
+
+TEST_F(AnalyzingVideoSinkTest,
+ KeepsCountingFrameWhenUnsucsribedUsingEmptyResolution) {
+ VideoSubscription subscription_before;
+ subscription_before.SubscribeToPeer(
+ "alice", VideoResolution(/*width=*/1280, /*height=*/720, /*fps=*/30));
+ VideoSubscription subscription_after;
+ subscription_after.SubscribeToPeer(
+ "alice", VideoResolution(/*width=*/0, /*height=*/0, /*fps=*/0));
+
+ VideoConfig video_config("alice_video", /*width=*/1280, /*height=*/720,
+ /*fps=*/30);
+
+ ExampleVideoQualityAnalyzer analyzer;
+ std::unique_ptr<test::FrameGeneratorInterface> frame_generator =
+ CreateFrameGenerator(/*width=*/1280, /*height=*/720);
+ VideoFrame frame_before = CreateFrame(*frame_generator);
+ frame_before.set_id(
+ analyzer.OnFrameCaptured("alice", "alice_video", frame_before));
+ VideoFrame frame_after = CreateFrame(*frame_generator);
+ frame_after.set_id(
+ analyzer.OnFrameCaptured("alice", "alice_video", frame_after));
+
+ {
+ AnalyzingVideoSinksHelper helper;
+ helper.AddConfig("alice", video_config);
+ AnalyzingVideoSink sink("bob", Clock::GetRealTimeClock(), analyzer, helper,
+ subscription_before, /*report_infra_stats=*/false);
+ sink.OnFrame(frame_before);
+
+ sink.UpdateSubscription(subscription_after);
+ sink.OnFrame(frame_after);
+ }
+
+ EXPECT_THAT(analyzer.frames_rendered(), Eq(2));
+}
+
TEST_F(AnalyzingVideoSinkTest,
VideoFramesAreDumpedCorrectlyWhenSubscriptionChangedOnTheSameOne) {
VideoSubscription subscription_before;
