Reland "Delete the non-const version of the EncodedImage::data() method."

This is a reland of f2969fa868f4913583e79f74ceced5cc6b7d6b7d

Original change's description:
> Delete the non-const version of the EncodedImage::data() method.
>
> Bug: webrtc:9378
> Change-Id: I84ace3ca6a2eb4d0f7c3d4e62f815d77df581bfa
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/185122
> Reviewed-by: Artem Titov <titovartem@webrtc.org>
> Reviewed-by: Philip Eliasson <philipel@webrtc.org>
> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Commit-Queue: Niels Moller <nisse@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#32197}

Bug: webrtc:9378
Change-Id: I8521ac567749ea547f91cf7549eb48966baffa11
Tbr: ilnik@webrtc.org
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/185807
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Marina Ciocea <marinaciocea@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32209}
This commit is contained in:
Niels Möller
2020-09-23 15:58:12 +02:00
committed by Commit Bot
parent 1e40a0cabd
commit 08ae7cea30
16 changed files with 97 additions and 83 deletions

View File

@ -147,11 +147,6 @@ class RTC_EXPORT EncodedImage {
return encoded_data_;
}
// TODO(nisse): Delete, provide only read-only access to the buffer.
uint8_t* data() {
return buffer_ ? buffer_
: (encoded_data_ ? encoded_data_->data() : nullptr);
}
const uint8_t* data() const {
return buffer_ ? buffer_
: (encoded_data_ ? encoded_data_->data() : nullptr);

View File

@ -110,7 +110,8 @@ static void RtpFragmentize(EncodedImage* encoded_image, SFrameBSInfo* info) {
}
}
// TODO(nisse): Use a cache or buffer pool to avoid allocation?
encoded_image->SetEncodedData(EncodedImageBuffer::Create(required_capacity));
auto buffer = EncodedImageBuffer::Create(required_capacity);
encoded_image->SetEncodedData(buffer);
// Iterate layers and NAL units, note each NAL unit as a fragment and copy
// the data to |encoded_image->_buffer|.
@ -132,8 +133,7 @@ static void RtpFragmentize(EncodedImage* encoded_image, SFrameBSInfo* info) {
layer_len += layerInfo.pNalLengthInByte[nal];
}
// Copy the entire layer's data (including start codes).
memcpy(encoded_image->data() + encoded_image->size(), layerInfo.pBsBuf,
layer_len);
memcpy(buffer->data() + encoded_image->size(), layerInfo.pBsBuf, layer_len);
encoded_image->set_size(encoded_image->size() + layer_len);
}
}

View File

@ -571,16 +571,16 @@ const webrtc::EncodedImage* VideoProcessor::BuildAndStoreSuperframe(
}
const size_t payload_size_bytes = base_image.size() + encoded_image.size();
EncodedImage copied_image = encoded_image;
copied_image.SetEncodedData(EncodedImageBuffer::Create(payload_size_bytes));
auto buffer = EncodedImageBuffer::Create(payload_size_bytes);
if (base_image.size()) {
RTC_CHECK(base_image.data());
memcpy(copied_image.data(), base_image.data(), base_image.size());
memcpy(buffer->data(), base_image.data(), base_image.size());
}
memcpy(copied_image.data() + base_image.size(), encoded_image.data(),
memcpy(buffer->data() + base_image.size(), encoded_image.data(),
encoded_image.size());
copied_image.set_size(payload_size_bytes);
EncodedImage copied_image = encoded_image;
copied_image.SetEncodedData(buffer);
// Replace previous EncodedImage for this spatial layer.
merged_encoded_frames_.at(spatial_idx) = std::move(copied_image);

View File

@ -134,7 +134,9 @@ VCMFrameBufferEnum VCMFrameBuffer::InsertPacket(const VCMPacket& packet,
if (packet.sizeBytes > 0)
CopyCodecSpecific(&packet.video_header);
int retVal = _sessionInfo.InsertPacket(packet, data(), frame_data);
int retVal = _sessionInfo.InsertPacket(
packet, encoded_image_buffer_ ? encoded_image_buffer_->data() : nullptr,
frame_data);
if (retVal == -1) {
return kSizeError;
} else if (retVal == -2) {

View File

@ -38,7 +38,8 @@ RtpFrameObject::RtpFrameObject(
const absl::optional<webrtc::ColorSpace>& color_space,
RtpPacketInfos packet_infos,
rtc::scoped_refptr<EncodedImageBuffer> image_buffer)
: first_seq_num_(first_seq_num),
: image_buffer_(image_buffer),
first_seq_num_(first_seq_num),
last_seq_num_(last_seq_num),
last_packet_received_time_(last_packet_received_time),
times_nacked_(times_nacked) {
@ -60,7 +61,7 @@ RtpFrameObject::RtpFrameObject(
// as of the first packet's.
SetPlayoutDelay(rtp_video_header_.playout_delay);
SetEncodedData(std::move(image_buffer));
SetEncodedData(image_buffer_);
_encodedWidth = rtp_video_header_.width;
_encodedHeight = rtp_video_header_.height;

View File

@ -48,7 +48,11 @@ class RtpFrameObject : public EncodedFrame {
bool delayed_by_retransmission() const override;
const RTPVideoHeader& GetRtpVideoHeader() const;
uint8_t* mutable_data() { return image_buffer_->data(); }
private:
// Reference for mutable access.
rtc::scoped_refptr<EncodedImageBuffer> image_buffer_;
RTPVideoHeader rtp_video_header_;
VideoCodecType codec_type_;
uint16_t first_seq_num_;

View File

@ -81,20 +81,13 @@ class SimulcastTestFixtureImpl::TestEncodedImageCallback
// Only store the base layer.
if (encoded_image.SpatialIndex().value_or(0) == 0) {
if (encoded_image._frameType == VideoFrameType::kVideoFrameKey) {
// TODO(nisse): Why not size() ?
encoded_key_frame_.SetEncodedData(
EncodedImageBuffer::Create(encoded_image.size()));
encoded_key_frame_.set_size(encoded_image.size());
encoded_key_frame_.SetEncodedData(EncodedImageBuffer::Create(
encoded_image.data(), 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_.SetEncodedData(
EncodedImageBuffer::Create(encoded_image.size()));
encoded_frame_.set_size(encoded_image.size());
memcpy(encoded_frame_.data(), encoded_image.data(),
encoded_image.size());
encoded_frame_.SetEncodedData(EncodedImageBuffer::Create(
encoded_image.data(), encoded_image.size()));
}
}
if (is_vp8) {
@ -873,13 +866,10 @@ void SimulcastTestFixtureImpl::TestDecodeWidthHeightSet() {
EXPECT_EQ(encoded_image._frameType, VideoFrameType::kVideoFrameKey);
size_t index = encoded_image.SpatialIndex().value_or(0);
encoded_frame[index].SetEncodedData(
EncodedImageBuffer::Create(encoded_image.size()));
encoded_frame[index].set_size(encoded_image.size());
encoded_frame[index].SetEncodedData(EncodedImageBuffer::Create(
encoded_image.data(), encoded_image.size()));
encoded_frame[index]._frameType = encoded_image._frameType;
encoded_frame[index]._completeFrame = encoded_image._completeFrame;
memcpy(encoded_frame[index].data(), encoded_image.data(),
encoded_image.size());
return EncodedImageCallback::Result(
EncodedImageCallback::Result::OK, 0);
}));

View File

@ -128,14 +128,15 @@ int32_t FakeEncoder::Encode(const VideoFrame& input_image,
continue;
}
EncodedImage encoded;
encoded.SetEncodedData(
EncodedImageBuffer::Create(frame_info.layers[i].size));
auto buffer = EncodedImageBuffer::Create(frame_info.layers[i].size);
// Fill the buffer with arbitrary data. Write someting to make Asan happy.
memset(encoded.data(), 9, frame_info.layers[i].size);
memset(buffer->data(), 9, frame_info.layers[i].size);
// Write a counter to the image to make each frame unique.
WriteCounter(encoded.data() + frame_info.layers[i].size - 4, counter);
WriteCounter(buffer->data() + frame_info.layers[i].size - 4, counter);
EncodedImage encoded;
encoded.SetEncodedData(buffer);
encoded.SetTimestamp(input_image.timestamp());
encoded._frameType = frame_info.keyframe ? VideoFrameType::kVideoFrameKey
: VideoFrameType::kVideoFrameDelta;
@ -144,7 +145,7 @@ int32_t FakeEncoder::Encode(const VideoFrame& input_image,
if (qp)
encoded.qp_ = *qp;
encoded.SetSpatialIndex(i);
CodecSpecificInfo codec_specific = EncodeHook(encoded);
CodecSpecificInfo codec_specific = EncodeHook(encoded, buffer);
if (callback->OnEncodedImage(encoded, &codec_specific).error !=
EncodedImageCallback::Result::OK) {
@ -154,7 +155,9 @@ int32_t FakeEncoder::Encode(const VideoFrame& input_image,
return 0;
}
CodecSpecificInfo FakeEncoder::EncodeHook(EncodedImage& encoded_image) {
CodecSpecificInfo FakeEncoder::EncodeHook(
EncodedImage& encoded_image,
rtc::scoped_refptr<EncodedImageBuffer> buffer) {
CodecSpecificInfo codec_specific;
codec_specific.codecType = kVideoCodecGeneric;
return codec_specific;
@ -284,7 +287,9 @@ int FakeEncoder::GetConfiguredInputFramerate() const {
FakeH264Encoder::FakeH264Encoder(Clock* clock)
: FakeEncoder(clock), idr_counter_(0) {}
CodecSpecificInfo FakeH264Encoder::EncodeHook(EncodedImage& encoded_image) {
CodecSpecificInfo FakeH264Encoder::EncodeHook(
EncodedImage& encoded_image,
rtc::scoped_refptr<EncodedImageBuffer> buffer) {
static constexpr std::array<uint8_t, 3> kStartCode = {0, 0, 1};
const size_t kSpsSize = 8;
const size_t kPpsSize = 11;
@ -296,7 +301,7 @@ CodecSpecificInfo FakeH264Encoder::EncodeHook(EncodedImage& encoded_image) {
++idr_counter_;
}
for (size_t i = 0; i < encoded_image.size(); ++i) {
encoded_image.data()[i] = static_cast<uint8_t>(i);
buffer->data()[i] = static_cast<uint8_t>(i);
}
if (current_idr_counter % kIdrFrequency == 0 &&
@ -304,7 +309,7 @@ CodecSpecificInfo FakeH264Encoder::EncodeHook(EncodedImage& encoded_image) {
const size_t kSpsNalHeader = 0x67;
const size_t kPpsNalHeader = 0x68;
const size_t kIdrNalHeader = 0x65;
uint8_t* data = encoded_image.data();
uint8_t* data = buffer->data();
memcpy(data, kStartCode.data(), kStartCode.size());
data += kStartCode.size();
data[0] = kSpsNalHeader;
@ -319,9 +324,9 @@ CodecSpecificInfo FakeH264Encoder::EncodeHook(EncodedImage& encoded_image) {
data += kStartCode.size();
data[0] = kIdrNalHeader;
} else {
memcpy(encoded_image.data(), kStartCode.data(), kStartCode.size());
memcpy(buffer->data(), kStartCode.data(), kStartCode.size());
const size_t kNalHeader = 0x41;
encoded_image.data()[kStartCode.size()] = kNalHeader;
buffer->data()[kStartCode.size()] = kNalHeader;
}
CodecSpecificInfo codec_specific;

View File

@ -83,8 +83,11 @@ class FakeEncoder : public VideoEncoder {
int framerate) RTC_LOCKS_EXCLUDED(mutex_);
// Called before the frame is passed to callback_->OnEncodedImage, to let
// subclasses fill out CodecSpecificInfo, possibly modify |encoded_image|.
virtual CodecSpecificInfo EncodeHook(EncodedImage& encoded_image);
// subclasses fill out CodecSpecificInfo, possibly modify |encoded_image| or
// |buffer|.
virtual CodecSpecificInfo EncodeHook(
EncodedImage& encoded_image,
rtc::scoped_refptr<EncodedImageBuffer> buffer);
void SetRatesLocked(const RateControlParameters& parameters)
RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
@ -113,7 +116,9 @@ class FakeH264Encoder : public FakeEncoder {
virtual ~FakeH264Encoder() = default;
private:
CodecSpecificInfo EncodeHook(EncodedImage& encoded_image) override;
CodecSpecificInfo EncodeHook(
EncodedImage& encoded_image,
rtc::scoped_refptr<EncodedImageBuffer> buffer) override;
int idr_counter_ RTC_GUARDED_BY(local_mutex_);
Mutex local_mutex_;

View File

@ -90,7 +90,9 @@ CodecSpecificInfo FakeVp8Encoder::PopulateCodecSpecific(
return codec_specific;
}
CodecSpecificInfo FakeVp8Encoder::EncodeHook(EncodedImage& encoded_image) {
CodecSpecificInfo FakeVp8Encoder::EncodeHook(
EncodedImage& encoded_image,
rtc::scoped_refptr<EncodedImageBuffer> buffer) {
RTC_DCHECK_RUN_ON(&sequence_checker_);
uint8_t stream_idx = encoded_image.SpatialIndex().value_or(0);
frame_buffer_controller_->NextFrameConfig(stream_idx,
@ -101,7 +103,7 @@ CodecSpecificInfo FakeVp8Encoder::EncodeHook(EncodedImage& encoded_image) {
// Write width and height to the payload the same way as the real encoder
// does.
WriteFakeVp8(encoded_image.data(), encoded_image._encodedWidth,
WriteFakeVp8(buffer->data(), encoded_image._encodedWidth,
encoded_image._encodedHeight,
encoded_image._frameType == VideoFrameType::kVideoFrameKey);
return codec_specific;

View File

@ -49,7 +49,9 @@ class FakeVp8Encoder : public FakeEncoder {
int stream_idx,
uint32_t timestamp);
CodecSpecificInfo EncodeHook(EncodedImage& encoded_image) override;
CodecSpecificInfo EncodeHook(
EncodedImage& encoded_image,
rtc::scoped_refptr<EncodedImageBuffer> buffer) override;
SequenceChecker sequence_checker_;

View File

@ -41,30 +41,34 @@ EncodedImage DefaultEncodedImageDataInjector::InjectData(
bool discard,
const EncodedImage& source,
int /*coding_entity_id*/) {
EncodedImage out = source;
out.SetEncodedData(
EncodedImageBuffer::Create(source.size() + kEncodedImageBufferExpansion));
memcpy(out.data(), source.data(), source.size());
auto buffer =
EncodedImageBuffer::Create(source.size() + kEncodedImageBufferExpansion);
memcpy(buffer->data(), source.data(), source.size());
size_t insertion_pos = source.size();
out.data()[insertion_pos] = id & 0x00ff;
out.data()[insertion_pos + 1] = (id & 0xff00) >> 8;
out.data()[insertion_pos + 2] = source.size() & 0x000000ff;
out.data()[insertion_pos + 3] = (source.size() & 0x0000ff00) >> 8;
out.data()[insertion_pos + 4] = (source.size() & 0x00ff0000) >> 16;
out.data()[insertion_pos + 5] = (source.size() & 0xff000000) >> 24;
buffer->data()[insertion_pos] = id & 0x00ff;
buffer->data()[insertion_pos + 1] = (id & 0xff00) >> 8;
buffer->data()[insertion_pos + 2] = source.size() & 0x000000ff;
buffer->data()[insertion_pos + 3] = (source.size() & 0x0000ff00) >> 8;
buffer->data()[insertion_pos + 4] = (source.size() & 0x00ff0000) >> 16;
buffer->data()[insertion_pos + 5] = (source.size() & 0xff000000) >> 24;
// We will store discard flag in the high bit of high byte of the size.
RTC_CHECK_LT(source.size(), 1U << 31) << "High bit is already in use";
out.data()[insertion_pos + 5] =
out.data()[insertion_pos + 5] | ((discard ? 1 : 0) << 7);
buffer->data()[insertion_pos + 5] =
buffer->data()[insertion_pos + 5] | ((discard ? 1 : 0) << 7);
EncodedImage out = source;
out.SetEncodedData(buffer);
return out;
}
EncodedImageExtractionResult DefaultEncodedImageDataInjector::ExtractData(
const EncodedImage& source,
int /*coding_entity_id*/) {
auto buffer = EncodedImageBuffer::Create(source.size());
EncodedImage out = source;
out.SetEncodedData(EncodedImageBuffer::Create(source.size()));
out.SetEncodedData(buffer);
size_t source_pos = source.size() - 1;
absl::optional<uint16_t> id = absl::nullopt;
@ -115,7 +119,7 @@ EncodedImageExtractionResult DefaultEncodedImageDataInjector::ExtractData(
if (!info.discard) {
// Copy next encoded image payload from concatenated buffer only if it is
// not discarded.
memcpy(&out.data()[out_pos], &source.data()[source_pos], info.length);
memcpy(&buffer->data()[out_pos], &source.data()[source_pos], info.length);
out_pos += info.length;
}
source_pos += info.length + kEncodedImageBufferExpansion;

View File

@ -45,22 +45,23 @@ EncodedImage SingleProcessEncodedImageDataInjector::InjectData(
ev.infos[info.sub_id] = info;
}
auto buffer = EncodedImageBuffer::Create(source.data(), source.size());
buffer->data()[insertion_pos] = id & 0x00ff;
buffer->data()[insertion_pos + 1] = (id & 0xff00) >> 8;
buffer->data()[insertion_pos + 2] = info.sub_id;
EncodedImage out = source;
out.data()[insertion_pos] = id & 0x00ff;
out.data()[insertion_pos + 1] = (id & 0xff00) >> 8;
out.data()[insertion_pos + 2] = info.sub_id;
out.SetEncodedData(buffer);
return out;
}
EncodedImageExtractionResult SingleProcessEncodedImageDataInjector::ExtractData(
const EncodedImage& source,
int coding_entity_id) {
size_t size = source.size();
auto buffer = EncodedImageBuffer::Create(source.data(), source.size());
EncodedImage out = source;
// Both |source| and |out| image will share the same buffer for payload or
// out will have a copy for it, so we can operate on the |out| buffer only.
uint8_t* buffer = out.data();
size_t size = out.size();
out.SetEncodedData(buffer);
std::vector<size_t> frame_sizes;
std::vector<size_t> frame_sl_index;
@ -84,9 +85,10 @@ EncodedImageExtractionResult SingleProcessEncodedImageDataInjector::ExtractData(
size_t insertion_pos =
prev_frames_size + frame_size - ExtractionInfo::kUsedBufferSize;
// Extract frame id from first 2 bytes starting from insertion pos.
uint16_t next_id = buffer[insertion_pos] + (buffer[insertion_pos + 1] << 8);
uint16_t next_id = buffer->data()[insertion_pos] +
(buffer->data()[insertion_pos + 1] << 8);
// Extract frame sub id from second 3 byte starting from insertion pos.
uint8_t sub_id = buffer[insertion_pos + 2];
uint8_t sub_id = buffer->data()[insertion_pos + 2];
RTC_CHECK(!id || *id == next_id)
<< "Different frames encoded into single encoded image: " << *id
<< " vs " << next_id;
@ -135,14 +137,16 @@ EncodedImageExtractionResult SingleProcessEncodedImageDataInjector::ExtractData(
if (info.discard) {
// If this encoded image is marked to be discarded - erase it's payload
// from the buffer.
memmove(&buffer[pos], &buffer[pos + frame_size], size - pos - frame_size);
memmove(&buffer->data()[pos], &buffer->data()[pos + frame_size],
size - pos - frame_size);
RTC_CHECK_LT(frame_index, frame_sl_index.size())
<< "codec doesn't support discard option or the image, that was "
"supposed to be discarded, is lost";
out.SetSpatialLayerFrameSize(frame_sl_index[frame_index], 0);
size -= frame_size;
} else {
memcpy(&buffer[pos + frame_size - ExtractionInfo::kUsedBufferSize],
memcpy(
&buffer->data()[pos + frame_size - ExtractionInfo::kUsedBufferSize],
info.origin_data, ExtractionInfo::kUsedBufferSize);
pos += frame_size;
}

View File

@ -238,7 +238,6 @@ TEST(SingleProcessEncodedImageDataInjector,
EncodedImage concatenated;
concatenated.SetEncodedData(EncodedImageBuffer::Create(
concatenated_buffer.data(), concatenated_length));
memcpy(concatenated.data(), concatenated_buffer.data(), concatenated_length);
concatenated.SetSpatialIndex(2);
concatenated.SetSpatialLayerFrameSize(0, intermediate1.size());
concatenated.SetSpatialLayerFrameSize(1, intermediate2.size());
@ -293,8 +292,7 @@ TEST(SingleProcessEncodedImageDataInjector, InjectOnceExtractTwice) {
#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.size()));
memcpy(copy.data(), source.data(), source.size());
copy.SetEncodedData(EncodedImageBuffer::Create(source.data(), source.size()));
return copy;
}

View File

@ -73,7 +73,7 @@ BufferedFrameDecryptor::FrameDecision BufferedFrameDecryptor::DecryptFrame(
frame->size());
RTC_CHECK_LE(max_plaintext_byte_size, frame->size());
// Place the decrypted frame inline into the existing frame.
rtc::ArrayView<uint8_t> inline_decrypted_bitstream(frame->data(),
rtc::ArrayView<uint8_t> inline_decrypted_bitstream(frame->mutable_data(),
max_plaintext_byte_size);
// Enable authenticating the header if the field trial isn't disabled.

View File

@ -996,7 +996,9 @@ class VideoStreamEncoderTest : public ::testing::Test {
return result;
}
CodecSpecificInfo EncodeHook(EncodedImage& encoded_image) override {
CodecSpecificInfo EncodeHook(
EncodedImage& encoded_image,
rtc::scoped_refptr<EncodedImageBuffer> buffer) override {
CodecSpecificInfo codec_specific;
{
MutexLock lock(&mutex_);