Add owned data buffer to EncodedImage
Bug: webrtc:9378
Change-Id: I6a66b9301cbadf1d6517bf7a96028099970a20a3
Reviewed-on: https://webrtc-review.googlesource.com/c/117964
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26585}diff --git a/api/media_transport_interface.cc b/api/media_transport_interface.cc
index 571e82b..63d7ea1 100644
--- a/api/media_transport_interface.cc
+++ b/api/media_transport_interface.cc
@@ -74,33 +74,10 @@
referenced_frame_ids_(std::move(referenced_frame_ids)) {}
MediaTransportEncodedVideoFrame& MediaTransportEncodedVideoFrame::operator=(
- const MediaTransportEncodedVideoFrame& o) {
- payload_type_ = o.payload_type_;
- encoded_image_ = o.encoded_image_;
- encoded_data_ = o.encoded_data_;
- frame_id_ = o.frame_id_;
- referenced_frame_ids_ = o.referenced_frame_ids_;
- if (!encoded_data_.empty()) {
- // We own the underlying data.
- encoded_image_.set_buffer(encoded_data_.data(), encoded_data_.size());
- }
- return *this;
-}
+ const MediaTransportEncodedVideoFrame&) = default;
MediaTransportEncodedVideoFrame& MediaTransportEncodedVideoFrame::operator=(
- MediaTransportEncodedVideoFrame&& o) {
- payload_type_ = o.payload_type_;
- encoded_image_ = o.encoded_image_;
- encoded_data_ = std::move(o.encoded_data_);
- frame_id_ = o.frame_id_;
- referenced_frame_ids_ = std::move(o.referenced_frame_ids_);
- if (!encoded_data_.empty()) {
- // We take over ownership of the underlying data.
- encoded_image_.set_buffer(encoded_data_.data(), encoded_data_.size());
- o.encoded_image_.set_buffer(nullptr, 0);
- }
- return *this;
-}
+ MediaTransportEncodedVideoFrame&&) = default;
MediaTransportEncodedVideoFrame::MediaTransportEncodedVideoFrame(
const MediaTransportEncodedVideoFrame& o)
@@ -114,14 +91,6 @@
*this = std::move(o);
}
-void MediaTransportEncodedVideoFrame::Retain() {
- if (encoded_image_.data() && encoded_data_.empty()) {
- encoded_data_ = std::vector<uint8_t>(
- encoded_image_.data(), encoded_image_.data() + encoded_image_.size());
- encoded_image_.set_buffer(encoded_data_.data(), encoded_image_.size());
- }
-}
-
SendDataParams::SendDataParams() = default;
SendDataParams::SendDataParams(const SendDataParams&) = default;
diff --git a/api/media_transport_interface.h b/api/media_transport_interface.h
index 1266000..5cd2923 100644
--- a/api/media_transport_interface.h
+++ b/api/media_transport_interface.h
@@ -204,23 +204,20 @@
return referenced_frame_ids_;
}
- // Hack to workaround lack of ownership of the encoded_image_._buffer. If we
+ // Hack to workaround lack of ownership of the EncodedImage buffer. If we
// don't already own the underlying data, make a copy.
- void Retain();
+ void Retain() { encoded_image_.Retain(); }
private:
MediaTransportEncodedVideoFrame();
int payload_type_;
- // The buffer is not owned by the encoded image. On the sender it means that
- // it will need to make a copy using the Retain() method, if it wants to
+ // The buffer is not always owned by the encoded image. On the sender it means
+ // that it will need to make a copy using the Retain() method, if it wants to
// deliver it asynchronously.
webrtc::EncodedImage encoded_image_;
- // If non-empty, this is the data for the encoded image.
- std::vector<uint8_t> encoded_data_;
-
// Frame id uniquely identifies a frame in a stream. It needs to be unique in
// a given time window (i.e. technically unique identifier for the lifetime of
// the connection is not needed, but you need to guarantee that remote side
diff --git a/api/video/encoded_image.cc b/api/video/encoded_image.cc
index b093d1b..f96b9c2 100644
--- a/api/video/encoded_image.cc
+++ b/api/video/encoded_image.cc
@@ -29,10 +29,24 @@
EncodedImage::EncodedImage() : EncodedImage(nullptr, 0, 0) {}
+EncodedImage::EncodedImage(EncodedImage&&) = default;
EncodedImage::EncodedImage(const EncodedImage&) = default;
EncodedImage::EncodedImage(uint8_t* buffer, size_t size, size_t capacity)
- : buffer_(buffer), size_(size), capacity_(capacity) {}
+ : size_(size), buffer_(buffer), capacity_(capacity) {}
+
+EncodedImage::~EncodedImage() = default;
+
+EncodedImage& EncodedImage::operator=(EncodedImage&&) = default;
+EncodedImage& EncodedImage::operator=(const EncodedImage&) = default;
+
+void EncodedImage::Retain() {
+ if (buffer_) {
+ encoded_data_ = std::vector<uint8_t>(size_);
+ memcpy(encoded_data_.data(), buffer_, size_);
+ buffer_ = nullptr;
+ }
+}
void EncodedImage::SetEncodeTime(int64_t encode_start_ms,
int64_t encode_finish_ms) {
diff --git a/api/video/encoded_image.h b/api/video/encoded_image.h
index fb3288f..363d34c 100644
--- a/api/video/encoded_image.h
+++ b/api/video/encoded_image.h
@@ -12,6 +12,7 @@
#define API_VIDEO_ENCODED_IMAGE_H_
#include <stdint.h>
+#include <vector>
#include "absl/types/optional.h"
#include "api/video/color_space.h"
@@ -37,9 +38,17 @@
static size_t GetBufferPaddingBytes(VideoCodecType codec_type);
EncodedImage();
+ EncodedImage(EncodedImage&&);
+ // Discouraged: potentially expensive.
EncodedImage(const EncodedImage&);
EncodedImage(uint8_t* buffer, size_t length, size_t capacity);
+ ~EncodedImage();
+
+ EncodedImage& operator=(EncodedImage&&);
+ // Discouraged: potentially expensive.
+ EncodedImage& operator=(const EncodedImage&);
+
// TODO(nisse): Change style to timestamp(), set_timestamp(), for consistency
// with the VideoFrame class.
// Set frame timestamp (90kHz).
@@ -68,19 +77,38 @@
size_t size() const { return size_; }
void set_size(size_t new_size) {
- RTC_DCHECK_LE(new_size, capacity_);
+ RTC_DCHECK_LE(new_size, capacity());
size_ = new_size;
}
- size_t capacity() const { return capacity_; }
+ size_t capacity() const { return buffer_ ? capacity_ : encoded_data_.size(); }
void set_buffer(uint8_t* buffer, size_t capacity) {
buffer_ = buffer;
capacity_ = capacity;
}
- // TODO(bugs.webrtc.org/9378): When changed to owning the buffer, data() on a
- // const object should return a const uint8_t*.
- uint8_t* data() const { return buffer_; }
+ void Allocate(size_t capacity) {
+ encoded_data_.resize(capacity);
+ buffer_ = nullptr;
+ }
+
+ uint8_t* data() { return buffer_ ? buffer_ : encoded_data_.data(); }
+ const uint8_t* data() const {
+ return buffer_ ? buffer_ : encoded_data_.data();
+ }
+ // TODO(nisse): At some places, code accepts a const ref EncodedImage, but
+ // still writes to it, to clear padding at the end of the encoded data.
+ // Padding is required by ffmpeg; the best way to deal with that is likely to
+ // make this class ensure that buffers always have a few zero padding bytes.
+ uint8_t* mutable_data() const { return const_cast<uint8_t*>(data()); }
+
+ // TODO(bugs.webrtc.org/9378): Delete. Used by code that wants to modify a
+ // buffer corresponding to a const EncodedImage. Requires an un-owned buffer.
+ uint8_t* buffer() const { return buffer_; }
+
+ // Hack to workaround lack of ownership of the encoded data. If we don't
+ // already own the underlying data, make an owned copy.
+ void Retain();
uint32_t _encodedWidth = 0;
uint32_t _encodedHeight = 0;
@@ -111,11 +139,14 @@
} timing_;
private:
- // TODO(bugs.webrtc.org/9378): Fix ownership. Currently not owning the data
- // buffer.
- uint8_t* buffer_;
+ // TODO(bugs.webrtc.org/9378): We're transitioning to always owning the
+ // encoded data.
+ std::vector<uint8_t> encoded_data_;
size_t size_; // Size of encoded frame data.
- size_t capacity_; // Allocated size of _buffer.
+ // Non-null when used with an un-owned buffer.
+ uint8_t* buffer_;
+ // Allocated size of _buffer; relevant only if it's non-null.
+ size_t capacity_;
uint32_t timestamp_rtp_ = 0;
absl::optional<int> spatial_index_;
absl::optional<webrtc::ColorSpace> color_space_;