[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}

test/pc/e2e/analyzer/video/encoded_image_data_injector.h

@@ -56,6 +56,10 @@ class EncodedImageDataExtractor {
   // 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.

test/pc/e2e/analyzer/video/single_process_encoded_image_data_injector.cc

@@ -59,6 +59,25 @@ void SingleProcessEncodedImageDataInjector::AddParticipantInCall() {
   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();

test/pc/e2e/analyzer/video/single_process_encoded_image_data_injector.h

@@ -57,6 +57,7 @@ class SingleProcessEncodedImageDataInjector
     expected_receivers_count_ = expected_receivers_count;
   }
   void AddParticipantInCall() override;
+  void RemoveParticipantInCall() override;
   EncodedImageExtractionResult ExtractData(const EncodedImage& source) override;
 
  private:

test/pc/e2e/analyzer/video/single_process_encoded_image_data_injector_unittest.cc

@@ -37,11 +37,18 @@ EncodedImage CreateEncodedImageOfSizeNFilledWithValuesFromX(size_t n,
   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, InjectExtractDiscardFalse) {
 
 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, InjectExtractDiscardTrue) {
 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,
 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,
 
 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, Inject3Extract3) {
 
 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(SingleProcessEncodedImageDataInjectorTest, InjectExtractFromConcatenated) {
 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(SingleProcessEncodedImageDataInjector,
 
 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, InjectOnceExtractTwice) {
 
 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, Add1stReceiverAfterStart) {
 
 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, Add3rdReceiverAfterStart) {
   }
 }
 
+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 @@ TEST(SingleProcessEncodedImageDataInjectorTest,
   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

test/pc/e2e/analyzer/video/video_frame_tracking_id_injector.h

@@ -37,6 +37,7 @@ class VideoFrameTrackingIdInjector : public EncodedImageDataPropagator {
 
   void Start(int) override {}
   void AddParticipantInCall() override {}
+  void RemoveParticipantInCall() override {}
 };
 
 }  // namespace webrtc_pc_e2e

test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.cc

@@ -167,6 +167,14 @@ void VideoQualityAnalyzerInjectionHelper::RegisterParticipantInCall(
   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 @@ VideoQualityAnalyzerInjectionHelper::PopulateSinks(
   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();

test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h

@@ -76,10 +76,15 @@ class VideoQualityAnalyzerInjectionHelper : public StatsObserverInterface {
   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(