Refactor DefaultEncodedImageDataInjector to let EncodedImage own the data.

Bug: webrtc:9378
Change-Id: I930935b6d4759dfbdb03a4ca2728a8b637997045
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/128577
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27181}

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

@@ -25,11 +25,6 @@ namespace {
 // This is 2 bytes for uint16_t frame id itself and 4 bytes for original length
 // of the buffer.
 constexpr int kEncodedImageBufferExpansion = 6;
-constexpr size_t kInitialBufferSize = 2 * 1024;
-// Count of coding entities for which buffers pools will be added on
-// construction.
-constexpr int kPreInitCodingEntitiesCount = 2;
-constexpr size_t kBuffersPoolPerCodingEntity = 256;
 
 struct ExtractionInfo {
   size_t length;
@@ -38,29 +33,16 @@ struct ExtractionInfo {
 
 }  // namespace
 
-DefaultEncodedImageDataInjector::DefaultEncodedImageDataInjector() {
+DefaultEncodedImageDataInjector::DefaultEncodedImageDataInjector() = default;
-  for (size_t i = 0;
-       i < kPreInitCodingEntitiesCount * kBuffersPoolPerCodingEntity; ++i) {
-    bufs_pool_.push_back(
-        absl::make_unique<std::vector<uint8_t>>(kInitialBufferSize));
-  }
-}
 DefaultEncodedImageDataInjector::~DefaultEncodedImageDataInjector() = default;
 
 EncodedImage DefaultEncodedImageDataInjector::InjectData(
     uint16_t id,
     bool discard,
     const EncodedImage& source,
-    int coding_entity_id) {
+    int /*coding_entity_id*/) {
-  ExtendIfRequired(coding_entity_id);
-
   EncodedImage out = source;
-  out.Retain();
+  out.Allocate(source.size() + kEncodedImageBufferExpansion);
-  std::vector<uint8_t>* buffer = NextBuffer();
-  if (buffer->size() < source.size() + kEncodedImageBufferExpansion) {
-    buffer->resize(source.size() + kEncodedImageBufferExpansion);
-  }
-  out.set_buffer(buffer->data(), buffer->size());
   out.set_size(source.size() + kEncodedImageBufferExpansion);
   memcpy(out.data(), source.data(), source.size());
   size_t insertion_pos = source.size();
@@ -80,15 +62,9 @@ EncodedImage DefaultEncodedImageDataInjector::InjectData(
 
 EncodedImageExtractionResult DefaultEncodedImageDataInjector::ExtractData(
     const EncodedImage& source,
-    int coding_entity_id) {
+    int /*coding_entity_id*/) {
-  ExtendIfRequired(coding_entity_id);
-
   EncodedImage out = source;
-  std::vector<uint8_t>* buffer = NextBuffer();
+  out.Allocate(source.size());
-  if (buffer->size() < source.capacity() - kEncodedImageBufferExpansion) {
-    buffer->resize(source.capacity() - kEncodedImageBufferExpansion);
-  }
-  out.set_buffer(buffer->data(), buffer->size());
 
   size_t source_pos = source.size() - 1;
   absl::optional<uint16_t> id = absl::nullopt;
@@ -150,36 +126,5 @@ EncodedImageExtractionResult DefaultEncodedImageDataInjector::ExtractData(
   return EncodedImageExtractionResult{id.value(), out, discard};
 }
 
-void DefaultEncodedImageDataInjector::ExtendIfRequired(int coding_entity_id) {
-  rtc::CritScope crit(&lock_);
-  if (coding_entities_.find(coding_entity_id) != coding_entities_.end()) {
-    // This entity is already known for this injector, so buffers are allocated.
-    return;
-  }
-
-  // New coding entity. We need allocate extra buffers for this encoder/decoder
-  // We will put them into front of the queue to use them first.
-  coding_entities_.insert(coding_entity_id);
-  if (coding_entities_.size() <= kPreInitCodingEntitiesCount) {
-    // Buffers for the first kPreInitCodingEntitiesCount coding entities were
-    // allocated during construction.
-    return;
-  }
-  for (size_t i = 0; i < kBuffersPoolPerCodingEntity; ++i) {
-    bufs_pool_.push_front(
-        absl::make_unique<std::vector<uint8_t>>(kInitialBufferSize));
-  }
-}
-
-std::vector<uint8_t>* DefaultEncodedImageDataInjector::NextBuffer() {
-  rtc::CritScope crit(&lock_);
-  // Get buffer from the front of the queue, return it to the caller and
-  // put in the back
-  std::vector<uint8_t>* out = bufs_pool_.front().get();
-  bufs_pool_.push_back(std::move(bufs_pool_.front()));
-  bufs_pool_.pop_front();
-  return out;
-}
-
 }  // namespace test
 }  // namespace webrtc

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

@@ -54,21 +54,6 @@ namespace test {
 //   3. Make a pass from begin to end copying data to the output basing on
 //      previously extracted length
 // Also it will check, that all extracted ids are equals.
-//
-// Because EncodedImage doesn't take ownership of its buffer, injector will keep
-// ownership of the buffers that will be used for EncodedImages with injected
-// data. This is needed because there is no way to inform the injector that
-// a buffer can be disposed. To address this issue injector will use a pool
-// of buffers in round robin manner and will assume, that when it overlaps
-// the buffer can be disposed.
-//
-// Because single injector can be used for different coding entities (encoders
-// or decoders), it will store a |coding_entity_id| in the set for each
-// coding entity seen and if the new one arrives, it will extend its buffers
-// pool, adding 256 more buffers. During initialization injector will
-// preallocate buffers for 2 coding entities, so 512 buffers with initial size
-// 2KB. If in some point of time bigger buffer will be required, it will be also
-// extended.
 class DefaultEncodedImageDataInjector : public EncodedImageDataInjector,
                                         public EncodedImageDataExtractor {
  public:
@@ -79,25 +64,9 @@ class DefaultEncodedImageDataInjector : public EncodedImageDataInjector,
   EncodedImage InjectData(uint16_t id,
                           bool discard,
                           const EncodedImage& source,
-                          int coding_entity_id) override;
+                          int /*coding_entity_id*/) override;
   EncodedImageExtractionResult ExtractData(const EncodedImage& source,
                                            int coding_entity_id) override;
-
- private:
-  void ExtendIfRequired(int coding_entity_id) RTC_LOCKS_EXCLUDED(lock_);
-  std::vector<uint8_t>* NextBuffer() RTC_LOCKS_EXCLUDED(lock_);
-
-  // Because single injector will be used for all encoder and decoders in one
-  // peer and in case of the single process for all encoders and decoders in
-  // another peer, it can be called from different threads. So we need to ensure
-  // that buffers are given consecutively from pools and pool extension won't
-  // be interrupted by getting buffer in other thread.
-  rtc::CriticalSection lock_;
-
-  // Store coding entities for which buffers pool have been already extended.
-  std::set<int> coding_entities_ RTC_GUARDED_BY(lock_);
-  std::deque<std::unique_ptr<std::vector<uint8_t>>> bufs_pool_
-      RTC_GUARDED_BY(lock_);
 };
 
 }  // namespace test