Discourage use of non-const EncodedImageBufferInterface::data
Update webrtc to work when such non-const accessor is absent,
Remove override of it in WebRTC classes, add a default implementation
so that downstream code can stop overriding it too.
Bug: webrtc:42234570
Change-Id: Iced9fca652f0385742cc4ee9310db78cf55aca71
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/404020
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Magnus Jedvert <magjed@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#45377}
diff --git a/api/video/encoded_image.h b/api/video/encoded_image.h
index 095c4ff..0510682 100644
--- a/api/video/encoded_image.h
+++ b/api/video/encoded_image.h
@@ -43,9 +43,13 @@
using value_type = uint8_t;
virtual const uint8_t* data() const = 0;
- // TODO(bugs.webrtc.org/9378): Make interface essentially read-only, delete
- // this non-const data method.
- virtual uint8_t* data() = 0;
+ // TODO: bugs.webrtc.org/42234570 - Make interface essentially read-only,
+ // delete this non-const data method when an implementation in chromium is
+ // updated not to override it.
+ virtual uint8_t* data() {
+ return const_cast<uint8_t*>(
+ static_cast<const EncodedImageBufferInterface*>(this)->data());
+ }
virtual size_t size() const = 0;
const uint8_t* begin() const { return data(); }
diff --git a/api/video/rtp_video_frame_assembler_unittests.cc b/api/video/rtp_video_frame_assembler_unittests.cc
index 1ac5fe5..5db9d06 100644
--- a/api/video/rtp_video_frame_assembler_unittests.cc
+++ b/api/video/rtp_video_frame_assembler_unittests.cc
@@ -139,8 +139,8 @@
return MakeArrayView(frame->references, frame->num_references);
}
-ArrayView<uint8_t> Payload(const std::unique_ptr<EncodedFrame>& frame) {
- return ArrayView<uint8_t>(*frame->GetEncodedData());
+ArrayView<const uint8_t> Payload(const std::unique_ptr<EncodedFrame>& frame) {
+ return *frame->GetEncodedData();
}
TEST(RtpVideoFrameAssembler, Vp8Packetization) {
diff --git a/api/video/rtp_video_frame_h265_assembler_unittests.cc b/api/video/rtp_video_frame_h265_assembler_unittests.cc
index 5fb8be4..510e41f 100644
--- a/api/video/rtp_video_frame_h265_assembler_unittests.cc
+++ b/api/video/rtp_video_frame_h265_assembler_unittests.cc
@@ -107,8 +107,8 @@
return MakeArrayView(frame->references, frame->num_references);
}
-ArrayView<uint8_t> Payload(const std::unique_ptr<EncodedFrame>& frame) {
- return ArrayView<uint8_t>(*frame->GetEncodedData());
+ArrayView<const uint8_t> Payload(const std::unique_ptr<EncodedFrame>& frame) {
+ return *frame->GetEncodedData();
}
TEST(RtpVideoFrameH265Assembler, H265Packetization) {
diff --git a/sdk/android/src/jni/encoded_image.cc b/sdk/android/src/jni/encoded_image.cc
index e38b445..7e21c04 100644
--- a/sdk/android/src/jni/encoded_image.cc
+++ b/sdk/android/src/jni/encoded_image.cc
@@ -39,19 +39,17 @@
const uint8_t* payload,
size_t size)
: j_encoded_image_(ScopedJavaRefCounted::Retain(env, j_encoded_image)),
- data_(const_cast<uint8_t*>(payload)),
+ data_(payload),
size_(size) {}
const uint8_t* data() const override { return data_; }
- uint8_t* data() override { return data_; }
size_t size() const override { return size_; }
private:
// The Java object owning the buffer.
const ScopedJavaRefCounted j_encoded_image_;
- // TODO(bugs.webrtc.org/9378): Make const, and delete above const_cast.
- uint8_t* const data_;
+ const uint8_t* const data_;
size_t const size_;
};
} // namespace
diff --git a/sdk/objc/api/peerconnection/RTCEncodedImage+Private.mm b/sdk/objc/api/peerconnection/RTCEncodedImage+Private.mm
index 6ff8dd8..def99ea 100644
--- a/sdk/objc/api/peerconnection/RTCEncodedImage+Private.mm
+++ b/sdk/objc/api/peerconnection/RTCEncodedImage+Private.mm
@@ -25,10 +25,6 @@
const uint8_t *data() const override {
return static_cast<const uint8_t *>(data_.bytes);
}
- // TODO(bugs.webrtc.org/9378): delete this non-const data method.
- uint8_t *data() override {
- return const_cast<uint8_t *>(static_cast<const uint8_t *>(data_.bytes));
- }
size_t size() const override { return data_.length; }
protected:
@@ -85,9 +81,15 @@
// long self.buffer references its underlying data.
self.encodedData = encodedImage.GetEncodedData();
// Wrap the buffer in NSData without copying, do not take ownership.
- self.buffer = [NSData dataWithBytesNoCopy:self.encodedData->data()
- length:encodedImage.size()
- freeWhenDone:NO];
+ // `NSData` provides interface that allows to write into the buffer,
+ // `EncodedImageBufferInterface` gives read-only access to the buffer.
+ // As of now write part of ths NSData interface are not used when accessing
+ // `buffer`, however it might be safer to refactor RTCEncodedImage not
+ // to expose buffer as NSData, and then remove this unsafe const_cast.
+ self.buffer = [NSData
+ dataWithBytesNoCopy:const_cast<uint8_t *>(self.encodedData->data())
+ length:encodedImage.size()
+ freeWhenDone:NO];
self.encodedWidth =
webrtc::dchecked_cast<int32_t>(encodedImage._encodedWidth);
self.encodedHeight =
diff --git a/video/frame_encode_metadata_writer.cc b/video/frame_encode_metadata_writer.cc
index 556cc2e..b53f53e 100644
--- a/video/frame_encode_metadata_writer.cc
+++ b/video/frame_encode_metadata_writer.cc
@@ -51,7 +51,6 @@
: buffer_(std::move(buffer)) {}
const uint8_t* data() const override { return buffer_.data(); }
- uint8_t* data() override { return buffer_.data(); }
size_t size() const override { return buffer_.size(); }
private:
