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}diff --git a/test/pc/e2e/analyzer/video/default_encoded_image_data_injector.cc b/test/pc/e2e/analyzer/video/default_encoded_image_data_injector.cc
index 68200cc..a7ae253 100644
--- a/test/pc/e2e/analyzer/video/default_encoded_image_data_injector.cc
+++ b/test/pc/e2e/analyzer/video/default_encoded_image_data_injector.cc
@@ -25,11 +25,6 @@
// 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 @@
} // namespace
-DefaultEncodedImageDataInjector::DefaultEncodedImageDataInjector() {
- for (size_t i = 0;
- i < kPreInitCodingEntitiesCount * kBuffersPoolPerCodingEntity; ++i) {
- bufs_pool_.push_back(
- absl::make_unique<std::vector<uint8_t>>(kInitialBufferSize));
- }
-}
+DefaultEncodedImageDataInjector::DefaultEncodedImageDataInjector() = default;
DefaultEncodedImageDataInjector::~DefaultEncodedImageDataInjector() = default;
EncodedImage DefaultEncodedImageDataInjector::InjectData(
uint16_t id,
bool discard,
const EncodedImage& source,
- int coding_entity_id) {
- ExtendIfRequired(coding_entity_id);
-
+ int /*coding_entity_id*/) {
EncodedImage out = source;
- out.Retain();
- 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.Allocate(source.size() + kEncodedImageBufferExpansion);
out.set_size(source.size() + kEncodedImageBufferExpansion);
memcpy(out.data(), source.data(), source.size());
size_t insertion_pos = source.size();
@@ -80,15 +62,9 @@
EncodedImageExtractionResult DefaultEncodedImageDataInjector::ExtractData(
const EncodedImage& source,
- int coding_entity_id) {
- ExtendIfRequired(coding_entity_id);
-
+ int /*coding_entity_id*/) {
EncodedImage out = source;
- std::vector<uint8_t>* buffer = NextBuffer();
- if (buffer->size() < source.capacity() - kEncodedImageBufferExpansion) {
- buffer->resize(source.capacity() - kEncodedImageBufferExpansion);
- }
- out.set_buffer(buffer->data(), buffer->size());
+ out.Allocate(source.size());
size_t source_pos = source.size() - 1;
absl::optional<uint16_t> id = absl::nullopt;
@@ -150,36 +126,5 @@
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
diff --git a/test/pc/e2e/analyzer/video/default_encoded_image_data_injector.h b/test/pc/e2e/analyzer/video/default_encoded_image_data_injector.h
index 0758653..2b6846d 100644
--- a/test/pc/e2e/analyzer/video/default_encoded_image_data_injector.h
+++ b/test/pc/e2e/analyzer/video/default_encoded_image_data_injector.h
@@ -54,21 +54,6 @@
// 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 @@
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
