New interface EncodedImageBufferInterface, replacing use of CopyOnWriteBuffer
Bug: webrtc:9378
Change-Id: I62b7adbd9dd539c545b5b1b1520721482a4623c4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/138820
Reviewed-by: Kári Helgason <kthelgason@webrtc.org>
Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28317}
diff --git a/modules/video_coding/codecs/h264/h264_encoder_impl.cc b/modules/video_coding/codecs/h264/h264_encoder_impl.cc
index 1cf9dad..13565bd 100644
--- a/modules/video_coding/codecs/h264/h264_encoder_impl.cc
+++ b/modules/video_coding/codecs/h264/h264_encoder_impl.cc
@@ -115,23 +115,8 @@
required_capacity += layerInfo.pNalLengthInByte[nal];
}
}
- if (encoded_image->capacity() < required_capacity) {
- // Increase buffer size. Allocate enough to hold an unencoded image, this
- // should be more than enough to hold any encoded data of future frames of
- // the same size (avoiding possible future reallocation due to variations in
- // required size).
- size_t new_capacity = CalcBufferSize(VideoType::kI420, frame_buffer.width(),
- frame_buffer.height());
- if (new_capacity < required_capacity) {
- // Encoded data > unencoded data. Allocate required bytes.
- RTC_LOG(LS_WARNING)
- << "Encoding produced more bytes than the original image "
- << "data! Original bytes: " << new_capacity
- << ", encoded bytes: " << required_capacity << ".";
- new_capacity = required_capacity;
- }
- encoded_image->Allocate(new_capacity);
- }
+ // TODO(nisse): Use a cache or buffer pool to avoid allocation?
+ encoded_image->SetEncodedData(EncodedImageBuffer::Create(required_capacity));
// Iterate layers and NAL units, note each NAL unit as a fragment and copy
// the data to |encoded_image->_buffer|.
@@ -300,7 +285,7 @@
const size_t new_capacity =
CalcBufferSize(VideoType::kI420, codec_.simulcastStream[idx].width,
codec_.simulcastStream[idx].height);
- encoded_images_[i].Allocate(new_capacity);
+ encoded_images_[i].SetEncodedData(EncodedImageBuffer::Create(new_capacity));
encoded_images_[i]._completeFrame = true;
encoded_images_[i]._encodedWidth = codec_.simulcastStream[idx].width;
encoded_images_[i]._encodedHeight = codec_.simulcastStream[idx].height;
diff --git a/modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.cc b/modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.cc
index b36ac32..38f16d7 100644
--- a/modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.cc
+++ b/modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.cc
@@ -189,8 +189,8 @@
frame_headers.push_back(frame_header);
}
- combined_image.Allocate(bitstream_offset);
- combined_image.set_size(bitstream_offset);
+ auto buffer = EncodedImageBuffer::Create(bitstream_offset);
+ combined_image.SetEncodedData(buffer);
// header
header_offset = PackHeader(combined_image.data(), header);
@@ -199,8 +199,8 @@
// Frame Header
for (size_t i = 0; i < images.size(); i++) {
- int relative_offset = PackFrameHeader(combined_image.data() + header_offset,
- frame_headers[i]);
+ int relative_offset =
+ PackFrameHeader(buffer->data() + header_offset, frame_headers[i]);
RTC_DCHECK_EQ(relative_offset, kMultiplexImageComponentHeaderSize);
header_offset = frame_headers[i].next_component_header_offset;
@@ -213,16 +213,15 @@
// Augmenting Data
if (multiplex_image.augmenting_data_size != 0) {
- memcpy(combined_image.data() + header.augmenting_data_offset,
+ memcpy(buffer->data() + header.augmenting_data_offset,
multiplex_image.augmenting_data.get(),
multiplex_image.augmenting_data_size);
}
// Bitstreams
for (size_t i = 0; i < images.size(); i++) {
- PackBitstream(combined_image.data() + frame_headers[i].bitstream_offset,
+ PackBitstream(buffer->data() + frame_headers[i].bitstream_offset,
images[i]);
- delete[] images[i].encoded_image.buffer();
}
return combined_image;
@@ -263,11 +262,9 @@
EncodedImage encoded_image = combined_image;
encoded_image.SetTimestamp(combined_image.Timestamp());
encoded_image._frameType = frame_headers[i].frame_type;
- encoded_image.Allocate(frame_headers[i].bitstream_length);
- encoded_image.set_size(frame_headers[i].bitstream_length);
- memcpy(encoded_image.data(),
- combined_image.data() + frame_headers[i].bitstream_offset,
- frame_headers[i].bitstream_length);
+ encoded_image.SetEncodedData(EncodedImageBuffer::Create(
+ combined_image.data() + frame_headers[i].bitstream_offset,
+ frame_headers[i].bitstream_length));
image_component.encoded_image = encoded_image;
diff --git a/modules/video_coding/codecs/test/videoprocessor.cc b/modules/video_coding/codecs/test/videoprocessor.cc
index d0db139..f93d539 100644
--- a/modules/video_coding/codecs/test/videoprocessor.cc
+++ b/modules/video_coding/codecs/test/videoprocessor.cc
@@ -568,7 +568,7 @@
const size_t payload_size_bytes = base_image.size() + encoded_image.size();
EncodedImage copied_image = encoded_image;
- copied_image.Allocate(payload_size_bytes);
+ copied_image.SetEncodedData(EncodedImageBuffer::Create(payload_size_bytes));
if (base_image.size()) {
RTC_CHECK(base_image.data());
memcpy(copied_image.data(), base_image.data(), base_image.size());
diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc
index c3257e4..9324723 100644
--- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc
+++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc
@@ -533,7 +533,8 @@
// allocate memory for encoded image
size_t frame_capacity =
CalcBufferSize(VideoType::kI420, codec_.width, codec_.height);
- encoded_images_[i].Allocate(frame_capacity);
+ encoded_images_[i].SetEncodedData(
+ EncodedImageBuffer::Create(frame_capacity));
encoded_images_[i]._completeFrame = true;
}
// populate encoder configuration with default values
diff --git a/modules/video_coding/codecs/vp9/vp9_impl.cc b/modules/video_coding/codecs/vp9/vp9_impl.cc
index d81c615..bc1d618 100644
--- a/modules/video_coding/codecs/vp9/vp9_impl.cc
+++ b/modules/video_coding/codecs/vp9/vp9_impl.cc
@@ -458,10 +458,6 @@
is_svc_ = (num_spatial_layers_ > 1 || num_temporal_layers_ > 1);
- // Allocate memory for encoded image
- size_t frame_capacity =
- CalcBufferSize(VideoType::kI420, codec_.width, codec_.height);
- encoded_image_.Allocate(frame_capacity);
encoded_image_._completeFrame = true;
// Populate encoder configuration with default values.
if (vpx_codec_enc_config_default(vpx_codec_vp9_cx(), config_, 0)) {
@@ -1417,11 +1413,10 @@
DeliverBufferedFrame(end_of_picture);
}
- if (pkt->data.frame.sz > encoded_image_.capacity()) {
- encoded_image_.Allocate(pkt->data.frame.sz);
- }
- memcpy(encoded_image_.data(), pkt->data.frame.buf, pkt->data.frame.sz);
- encoded_image_.set_size(pkt->data.frame.sz);
+ // TODO(nisse): Introduce some buffer cache or buffer pool, to reduce
+ // allocations and/or copy operations.
+ encoded_image_.SetEncodedData(EncodedImageBuffer::Create(
+ static_cast<const uint8_t*>(pkt->data.frame.buf), pkt->data.frame.sz));
const bool is_key_frame =
(pkt->data.frame.flags & VPX_FRAME_IS_KEY) ? true : false;
diff --git a/modules/video_coding/encoded_frame.cc b/modules/video_coding/encoded_frame.cc
index a53f88d..7a204c1 100644
--- a/modules/video_coding/encoded_frame.cc
+++ b/modules/video_coding/encoded_frame.cc
@@ -162,9 +162,9 @@
void VCMEncodedFrame::VerifyAndAllocate(size_t minimumSize) {
size_t old_capacity = capacity();
if (minimumSize > old_capacity) {
- // TODO(nisse): EncodedImage::Allocate is implemented as
- // std::vector::resize, which means that old contents is kept. Find out if
- // any code depends on that behavior.
+ // TODO(nisse): EncodedImage::Allocate is implemented as a realloc
+ // operation, and is deprecated. Refactor to use EncodedImageBuffer::Realloc
+ // instead.
Allocate(minimumSize);
}
}
diff --git a/modules/video_coding/encoded_frame.h b/modules/video_coding/encoded_frame.h
index eced69b..2ebef31 100644
--- a/modules/video_coding/encoded_frame.h
+++ b/modules/video_coding/encoded_frame.h
@@ -56,6 +56,7 @@
using EncodedImage::data;
using EncodedImage::set_size;
using EncodedImage::SetColorSpace;
+ using EncodedImage::SetEncodedData;
using EncodedImage::SetSpatialIndex;
using EncodedImage::SetSpatialLayerFrameSize;
using EncodedImage::SetTimestamp;
diff --git a/modules/video_coding/frame_object.cc b/modules/video_coding/frame_object.cc
index ccac4bb..fab6066 100644
--- a/modules/video_coding/frame_object.cc
+++ b/modules/video_coding/frame_object.cc
@@ -54,7 +54,8 @@
// as of the first packet's.
SetPlayoutDelay(first_packet->video_header.playout_delay);
- AllocateBitstreamBuffer(frame_size);
+ // TODO(nisse): Change GetBitstream to return the buffer?
+ SetEncodedData(EncodedImageBuffer::Create(frame_size));
bool bitstream_copied = packet_buffer_->GetBitstream(*this, data());
RTC_DCHECK(bitstream_copied);
_encodedWidth = first_packet->width();
@@ -166,13 +167,5 @@
return packet->video_header.frame_marking;
}
-void RtpFrameObject::AllocateBitstreamBuffer(size_t frame_size) {
- if (capacity() < frame_size) {
- Allocate(frame_size);
- }
-
- set_size(frame_size);
-}
-
} // namespace video_coding
} // namespace webrtc
diff --git a/modules/video_coding/frame_object.h b/modules/video_coding/frame_object.h
index c5c8934..1ba99cb 100644
--- a/modules/video_coding/frame_object.h
+++ b/modules/video_coding/frame_object.h
@@ -45,8 +45,6 @@
absl::optional<FrameMarking> GetFrameMarking() const;
private:
- void AllocateBitstreamBuffer(size_t frame_size);
-
rtc::scoped_refptr<PacketBuffer> packet_buffer_;
VideoFrameType frame_type_;
VideoCodecType codec_type_;
diff --git a/modules/video_coding/utility/ivf_file_writer_unittest.cc b/modules/video_coding/utility/ivf_file_writer_unittest.cc
index 365fc4a..12d1d11 100644
--- a/modules/video_coding/utility/ivf_file_writer_unittest.cc
+++ b/modules/video_coding/utility/ivf_file_writer_unittest.cc
@@ -41,8 +41,8 @@
int num_frames,
bool use_capture_tims_ms) {
EncodedImage frame;
- frame.Allocate(sizeof(dummy_payload));
- memcpy(frame.data(), dummy_payload, sizeof(dummy_payload));
+ frame.SetEncodedData(
+ EncodedImageBuffer::Create(dummy_payload, sizeof(dummy_payload)));
frame._encodedWidth = width;
frame._encodedHeight = height;
for (int i = 1; i <= num_frames; ++i) {
diff --git a/modules/video_coding/utility/simulcast_test_fixture_impl.cc b/modules/video_coding/utility/simulcast_test_fixture_impl.cc
index eadb5a2..d63e67f 100644
--- a/modules/video_coding/utility/simulcast_test_fixture_impl.cc
+++ b/modules/video_coding/utility/simulcast_test_fixture_impl.cc
@@ -82,14 +82,16 @@
if (encoded_image.SpatialIndex().value_or(0) == 0) {
if (encoded_image._frameType == VideoFrameType::kVideoFrameKey) {
// TODO(nisse): Why not size() ?
- encoded_key_frame_.Allocate(encoded_image.capacity());
+ encoded_key_frame_.SetEncodedData(
+ EncodedImageBuffer::Create(encoded_image.capacity()));
encoded_key_frame_.set_size(encoded_image.size());
encoded_key_frame_._frameType = VideoFrameType::kVideoFrameKey;
encoded_key_frame_._completeFrame = encoded_image._completeFrame;
memcpy(encoded_key_frame_.data(), encoded_image.data(),
encoded_image.size());
} else {
- encoded_frame_.Allocate(encoded_image.capacity());
+ encoded_frame_.SetEncodedData(
+ EncodedImageBuffer::Create(encoded_image.capacity()));
encoded_frame_.set_size(encoded_image.size());
memcpy(encoded_frame_.data(), encoded_image.data(),
encoded_image.size());
@@ -866,7 +868,8 @@
size_t index = encoded_image.SpatialIndex().value_or(0);
// TODO(nisse): Why not size()
- encoded_frame[index].Allocate(encoded_image.capacity());
+ encoded_frame[index].SetEncodedData(
+ EncodedImageBuffer::Create(encoded_image.capacity()));
encoded_frame[index].set_size(encoded_image.size());
encoded_frame[index]._frameType = encoded_image._frameType;
encoded_frame[index]._completeFrame = encoded_image._completeFrame;