[DVQA] Add support for removing peer from analyzer instrumentation
Add support for removing peer from EncodedImageDataExtractor and
from VideoQualityAnalyzerInjectionHelper.
Bug: b/231397778
Change-Id: Ic33da18b7a68149ef68e5e4ba0ee7eabfb634973
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/266364
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37285}
diff --git a/test/pc/e2e/analyzer/video/encoded_image_data_injector.h b/test/pc/e2e/analyzer/video/encoded_image_data_injector.h
index 20f7a43..384e901 100644
--- a/test/pc/e2e/analyzer/video/encoded_image_data_injector.h
+++ b/test/pc/e2e/analyzer/video/encoded_image_data_injector.h
@@ -56,6 +56,10 @@
// frames. Will be invoked before that receiver will start receive data.
virtual void AddParticipantInCall() = 0;
+ // Invoked by framework when it is required to remove receiver for frames.
+ // Will be invoked after that receiver will stop receiving data.
+ virtual void RemoveParticipantInCall() = 0;
+
// Returns encoded image id, extracted from payload and also encoded image
// with its original payload. For concatenated spatial layers it should be the
// same id.
diff --git a/test/pc/e2e/analyzer/video/single_process_encoded_image_data_injector.cc b/test/pc/e2e/analyzer/video/single_process_encoded_image_data_injector.cc
index d7ee0f4..ccd2f03 100644
--- a/test/pc/e2e/analyzer/video/single_process_encoded_image_data_injector.cc
+++ b/test/pc/e2e/analyzer/video/single_process_encoded_image_data_injector.cc
@@ -59,6 +59,25 @@
expected_receivers_count_++;
}
+void SingleProcessEncodedImageDataInjector::RemoveParticipantInCall() {
+ MutexLock crit(&lock_);
+ expected_receivers_count_--;
+ // Now we need go over `extraction_cache_` and removed frames which have been
+ // received by `expected_receivers_count_`.
+ for (auto& [frame_id, extraction_infos] : extraction_cache_) {
+ for (auto it = extraction_infos.infos.begin();
+ it != extraction_infos.infos.end();) {
+ // Frame is received if `received_count` equals to
+ // `expected_receivers_count_`.
+ if (it->second.received_count == expected_receivers_count_) {
+ it = extraction_infos.infos.erase(it);
+ } else {
+ ++it;
+ }
+ }
+ }
+}
+
EncodedImageExtractionResult SingleProcessEncodedImageDataInjector::ExtractData(
const EncodedImage& source) {
size_t size = source.size();
diff --git a/test/pc/e2e/analyzer/video/single_process_encoded_image_data_injector.h b/test/pc/e2e/analyzer/video/single_process_encoded_image_data_injector.h
index 81fc433..1082440 100644
--- a/test/pc/e2e/analyzer/video/single_process_encoded_image_data_injector.h
+++ b/test/pc/e2e/analyzer/video/single_process_encoded_image_data_injector.h
@@ -57,6 +57,7 @@
expected_receivers_count_ = expected_receivers_count;
}
void AddParticipantInCall() override;
+ void RemoveParticipantInCall() override;
EncodedImageExtractionResult ExtractData(const EncodedImage& source) override;
private:
diff --git a/test/pc/e2e/analyzer/video/single_process_encoded_image_data_injector_unittest.cc b/test/pc/e2e/analyzer/video/single_process_encoded_image_data_injector_unittest.cc
index cfeab23..f6fa404 100644
--- a/test/pc/e2e/analyzer/video/single_process_encoded_image_data_injector_unittest.cc
+++ b/test/pc/e2e/analyzer/video/single_process_encoded_image_data_injector_unittest.cc
@@ -37,11 +37,18 @@
return image;
}
+EncodedImage DeepCopyEncodedImage(const EncodedImage& source) {
+ EncodedImage copy = source;
+ copy.SetEncodedData(EncodedImageBuffer::Create(source.data(), source.size()));
+ return copy;
+}
+
TEST(SingleProcessEncodedImageDataInjectorTest, InjectExtractDiscardFalse) {
SingleProcessEncodedImageDataInjector injector;
- injector.Start(1);
+ injector.Start(/*expected_receivers_count=*/1);
- EncodedImage source = CreateEncodedImageOfSizeNFilledWithValuesFromX(10, 1);
+ EncodedImage source =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/1);
source.SetTimestamp(123456789);
EncodedImageExtractionResult out =
@@ -57,9 +64,10 @@
TEST(SingleProcessEncodedImageDataInjectorTest, InjectExtractDiscardTrue) {
SingleProcessEncodedImageDataInjector injector;
- injector.Start(1);
+ injector.Start(/*expected_receivers_count=*/1);
- EncodedImage source = CreateEncodedImageOfSizeNFilledWithValuesFromX(10, 1);
+ EncodedImage source =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/1);
source.SetTimestamp(123456789);
EncodedImageExtractionResult out =
@@ -73,9 +81,10 @@
TEST(SingleProcessEncodedImageDataInjectorTest,
InjectWithUnsetSpatialLayerSizes) {
SingleProcessEncodedImageDataInjector injector;
- injector.Start(1);
+ injector.Start(/*expected_receivers_count=*/1);
- EncodedImage source = CreateEncodedImageOfSizeNFilledWithValuesFromX(10, 1);
+ EncodedImage source =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/1);
source.SetTimestamp(123456789);
EncodedImage intermediate = injector.InjectData(512, false, source);
@@ -97,9 +106,10 @@
TEST(SingleProcessEncodedImageDataInjectorTest,
InjectWithZeroSpatialLayerSizes) {
SingleProcessEncodedImageDataInjector injector;
- injector.Start(1);
+ injector.Start(/*expected_receivers_count=*/1);
- EncodedImage source = CreateEncodedImageOfSizeNFilledWithValuesFromX(10, 1);
+ EncodedImage source =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/1);
source.SetTimestamp(123456789);
EncodedImage intermediate = injector.InjectData(512, false, source);
@@ -123,16 +133,19 @@
TEST(SingleProcessEncodedImageDataInjectorTest, Inject3Extract3) {
SingleProcessEncodedImageDataInjector injector;
- injector.Start(1);
+ injector.Start(/*expected_receivers_count=*/1);
// 1st frame
- EncodedImage source1 = CreateEncodedImageOfSizeNFilledWithValuesFromX(10, 1);
+ EncodedImage source1 =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/1);
source1.SetTimestamp(123456710);
// 2nd frame 1st spatial layer
- EncodedImage source2 = CreateEncodedImageOfSizeNFilledWithValuesFromX(10, 11);
+ EncodedImage source2 =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/11);
source2.SetTimestamp(123456720);
// 2nd frame 2nd spatial layer
- EncodedImage source3 = CreateEncodedImageOfSizeNFilledWithValuesFromX(10, 21);
+ EncodedImage source3 =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/21);
source3.SetTimestamp(123456720);
EncodedImage intermediate1 = injector.InjectData(510, false, source1);
@@ -166,13 +179,16 @@
TEST(SingleProcessEncodedImageDataInjectorTest, InjectExtractFromConcatenated) {
SingleProcessEncodedImageDataInjector injector;
- injector.Start(1);
+ injector.Start(/*expected_receivers_count=*/1);
- EncodedImage source1 = CreateEncodedImageOfSizeNFilledWithValuesFromX(10, 1);
+ EncodedImage source1 =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/1);
source1.SetTimestamp(123456710);
- EncodedImage source2 = CreateEncodedImageOfSizeNFilledWithValuesFromX(10, 11);
+ EncodedImage source2 =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/11);
source2.SetTimestamp(123456710);
- EncodedImage source3 = CreateEncodedImageOfSizeNFilledWithValuesFromX(10, 21);
+ EncodedImage source3 =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/21);
source3.SetTimestamp(123456710);
// Inject id into 3 images with same frame id.
@@ -215,13 +231,16 @@
TEST(SingleProcessEncodedImageDataInjector,
InjectExtractFromConcatenatedAllDiscarded) {
SingleProcessEncodedImageDataInjector injector;
- injector.Start(1);
+ injector.Start(/*expected_receivers_count=*/1);
- EncodedImage source1 = CreateEncodedImageOfSizeNFilledWithValuesFromX(10, 1);
+ EncodedImage source1 =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/1);
source1.SetTimestamp(123456710);
- EncodedImage source2 = CreateEncodedImageOfSizeNFilledWithValuesFromX(10, 11);
+ EncodedImage source2 =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/11);
source2.SetTimestamp(123456710);
- EncodedImage source3 = CreateEncodedImageOfSizeNFilledWithValuesFromX(10, 21);
+ EncodedImage source3 =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/21);
source3.SetTimestamp(123456710);
// Inject id into 3 images with same frame id.
@@ -259,9 +278,10 @@
TEST(SingleProcessEncodedImageDataInjectorTest, InjectOnceExtractTwice) {
SingleProcessEncodedImageDataInjector injector;
- injector.Start(2);
+ injector.Start(/*expected_receivers_count=*/2);
- EncodedImage source = CreateEncodedImageOfSizeNFilledWithValuesFromX(10, 1);
+ EncodedImage source =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/1);
source.SetTimestamp(123456789);
EncodedImageExtractionResult out = injector.ExtractData(
@@ -286,9 +306,10 @@
TEST(SingleProcessEncodedImageDataInjectorTest, Add1stReceiverAfterStart) {
SingleProcessEncodedImageDataInjector injector;
- injector.Start(0);
+ injector.Start(/*expected_receivers_count=*/0);
- EncodedImage source = CreateEncodedImageOfSizeNFilledWithValuesFromX(10, 1);
+ EncodedImage source =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/1);
source.SetTimestamp(123456789);
EncodedImage modified_image = injector.InjectData(
/*id=*/512, /*discard=*/false, source);
@@ -307,9 +328,10 @@
TEST(SingleProcessEncodedImageDataInjectorTest, Add3rdReceiverAfterStart) {
SingleProcessEncodedImageDataInjector injector;
- injector.Start(2);
+ injector.Start(/*expected_receivers_count=*/2);
- EncodedImage source = CreateEncodedImageOfSizeNFilledWithValuesFromX(10, 1);
+ EncodedImage source =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/1);
source.SetTimestamp(123456789);
EncodedImage modified_image = injector.InjectData(
/*id=*/512, /*discard=*/false, source);
@@ -328,22 +350,55 @@
}
}
+TEST(SingleProcessEncodedImageDataInjectorTest,
+ RemoveReceiverRemovesOnlyFullyReceivedFrames) {
+ SingleProcessEncodedImageDataInjector injector;
+ injector.Start(/*expected_receivers_count=*/3);
+
+ EncodedImage source1 =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/1);
+ source1.SetTimestamp(10);
+ EncodedImage source2 =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/1);
+ source2.SetTimestamp(20);
+
+ EncodedImage modified_image1 = injector.InjectData(
+ /*id=*/512, /*discard=*/false, source1);
+ EncodedImage modified_image2 = injector.InjectData(
+ /*id=*/513, /*discard=*/false, source2);
+
+ // Out of 3 receivers 1st image received by 2 and 2nd image by 1
+ injector.ExtractData(DeepCopyEncodedImage(modified_image1));
+ injector.ExtractData(DeepCopyEncodedImage(modified_image1));
+ injector.ExtractData(DeepCopyEncodedImage(modified_image2));
+
+ // When we removed one receiver, 2nd image should still be available for
+ // extraction.
+ injector.RemoveParticipantInCall();
+
+ EncodedImageExtractionResult out =
+ injector.ExtractData(DeepCopyEncodedImage(modified_image2));
+
+ EXPECT_EQ(out.id, 513);
+ EXPECT_FALSE(out.discard);
+ EXPECT_EQ(out.image.size(), 10ul);
+ EXPECT_EQ(out.image.SpatialLayerFrameSize(0).value_or(0), 0ul);
+ for (int i = 0; i < 10; ++i) {
+ EXPECT_EQ(out.image.data()[i], i + 1);
+ }
+}
+
// Death tests.
// Disabled on Android because death tests misbehave on Android, see
// base/test/gtest_util.h.
#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
-EncodedImage DeepCopyEncodedImage(const EncodedImage& source) {
- EncodedImage copy = source;
- copy.SetEncodedData(EncodedImageBuffer::Create(source.data(), source.size()));
- return copy;
-}
-
-TEST(SingleProcessEncodedImageDataInjectorTest,
+TEST(SingleProcessEncodedImageDataInjectorTestDeathTest,
InjectOnceExtractMoreThenExpected) {
SingleProcessEncodedImageDataInjector injector;
- injector.Start(2);
+ injector.Start(/*expected_receivers_count=*/2);
- EncodedImage source = CreateEncodedImageOfSizeNFilledWithValuesFromX(10, 1);
+ EncodedImage source =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/1);
source.SetTimestamp(123456789);
EncodedImage modified =
@@ -354,6 +409,35 @@
EXPECT_DEATH(injector.ExtractData(DeepCopyEncodedImage(modified)),
"Unknown sub_id=0 for frame_id=512");
}
+
+TEST(SingleProcessEncodedImageDataInjectorTestDeathTest,
+ RemoveReceiverRemovesOnlyFullyReceivedFramesVerifyFrameIsRemoved) {
+ SingleProcessEncodedImageDataInjector injector;
+ injector.Start(/*expected_receivers_count=*/3);
+
+ EncodedImage source1 =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/1);
+ source1.SetTimestamp(10);
+ EncodedImage source2 =
+ CreateEncodedImageOfSizeNFilledWithValuesFromX(/*n=*/10, /*x=*/1);
+ source2.SetTimestamp(20);
+
+ EncodedImage modified_image1 = injector.InjectData(
+ /*id=*/512, /*discard=*/false, source1);
+ EncodedImage modified_image2 = injector.InjectData(
+ /*id=*/513, /*discard=*/false, source2);
+
+ // Out of 3 receivers 1st image received by 2 and 2nd image by 1
+ injector.ExtractData(DeepCopyEncodedImage(modified_image1));
+ injector.ExtractData(DeepCopyEncodedImage(modified_image1));
+ injector.ExtractData(DeepCopyEncodedImage(modified_image2));
+
+ // When we removed one receiver 1st image should be removed.
+ injector.RemoveParticipantInCall();
+
+ EXPECT_DEATH(injector.ExtractData(DeepCopyEncodedImage(modified_image1)),
+ "Unknown sub_id=0 for frame_id=512");
+}
#endif // RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
} // namespace
diff --git a/test/pc/e2e/analyzer/video/video_frame_tracking_id_injector.h b/test/pc/e2e/analyzer/video/video_frame_tracking_id_injector.h
index d5ad613..ecc3cd3 100644
--- a/test/pc/e2e/analyzer/video/video_frame_tracking_id_injector.h
+++ b/test/pc/e2e/analyzer/video/video_frame_tracking_id_injector.h
@@ -37,6 +37,7 @@
void Start(int) override {}
void AddParticipantInCall() override {}
+ void RemoveParticipantInCall() override {}
};
} // namespace webrtc_pc_e2e
diff --git a/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.cc b/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.cc
index dcdf32b..81caeb3 100644
--- a/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.cc
+++ b/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.cc
@@ -167,6 +167,14 @@
peers_count_++;
}
+void VideoQualityAnalyzerInjectionHelper::UnregisterParticipantInCall(
+ absl::string_view peer_name) {
+ analyzer_->UnregisterParticipantInCall(peer_name);
+ extractor_->RemoveParticipantInCall();
+ MutexLock lock(&mutex_);
+ peers_count_--;
+}
+
void VideoQualityAnalyzerInjectionHelper::OnStatsReports(
absl::string_view pc_label,
const rtc::scoped_refptr<const RTCStatsReport>& report) {
@@ -242,6 +250,7 @@
absl::optional<std::string> output_dump_file_name =
config.output_dump_file_name;
if (output_dump_file_name.has_value() && peers_count_ > 2) {
+ // TODO(titovartem): make this default behavior for any amount of peers.
rtc::StringBuilder builder(*output_dump_file_name);
builder << "." << receiver_stream.peer_name;
output_dump_file_name = builder.str();
diff --git a/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h b/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h
index f130efc..102fa5d 100644
--- a/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h
+++ b/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h
@@ -76,10 +76,15 @@
void Start(std::string test_case_name,
rtc::ArrayView<const std::string> peer_names,
int max_threads_count = 1);
+
// Registers new call participant to the underlying video quality analyzer.
// The method should be called before the participant is actually added.
void RegisterParticipantInCall(absl::string_view peer_name);
+ // Will be called after test removed existing participant in the middle of the
+ // call.
+ void UnregisterParticipantInCall(absl::string_view peer_name);
+
// Forwards `stats_reports` for Peer Connection `pc_label` to
// `analyzer_`.
void OnStatsReports(
