From 2449d7aa78282a74ba297fc67d01517d344122cc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Niels=20M=C3=B6ller?= <nisse@webrtc.org>
Date: Mon, 30 Sep 2019 10:18:16 +0200
Subject: [PATCH] Refactor legacy FrameBuffer to use
 EncodedImageBuffer::Realloc

Preparation for deleting VCMEncodedFrame::VerifyAndAllocate and
EncodedImage::Allocate.

Bug: webrtc:9378
Change-Id: If7c16061962bbd58c3e7d5720189854e00a3d7bf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/154570
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29339}
---
 api/video/encoded_image.cc                  |  5 +++++
 modules/video_coding/codecs/vp9/vp9_impl.cc |  1 -
 modules/video_coding/frame_buffer.cc        | 12 ++++++++++--
 modules/video_coding/frame_buffer.h         |  3 +++
 4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/api/video/encoded_image.cc b/api/video/encoded_image.cc
index 55970fcd00..72f88d2922 100644
--- a/api/video/encoded_image.cc
+++ b/api/video/encoded_image.cc
@@ -52,6 +52,11 @@ size_t EncodedImageBuffer::size() const {
 }
 
 void EncodedImageBuffer::Realloc(size_t size) {
+  // Calling realloc with size == 0 is equivalent to free, and returns nullptr.
+  // Which is confusing on systems where malloc(0) doesn't return a nullptr.
+  // More specifically, it breaks expectations of
+  // VCMSessionInfo::UpdateDataPointers.
+  RTC_DCHECK(size > 0);
   buffer_ = static_cast<uint8_t*>(realloc(buffer_, size));
   size_ = size;
 }
diff --git a/modules/video_coding/codecs/vp9/vp9_impl.cc b/modules/video_coding/codecs/vp9/vp9_impl.cc
index b379e798c9..bbc1f715b3 100644
--- a/modules/video_coding/codecs/vp9/vp9_impl.cc
+++ b/modules/video_coding/codecs/vp9/vp9_impl.cc
@@ -266,7 +266,6 @@ void VP9EncoderImpl::SetFecControllerOverride(
 int VP9EncoderImpl::Release() {
   int ret_val = WEBRTC_VIDEO_CODEC_OK;
 
-  encoded_image_.Allocate(0);
   if (encoder_ != nullptr) {
     if (inited_) {
       if (vpx_codec_destroy(encoder_)) {
diff --git a/modules/video_coding/frame_buffer.cc b/modules/video_coding/frame_buffer.cc
index c49cde67d1..755acb2940 100644
--- a/modules/video_coding/frame_buffer.cc
+++ b/modules/video_coding/frame_buffer.cc
@@ -101,7 +101,7 @@ VCMFrameBufferEnum VCMFrameBuffer::InsertPacket(const VCMPacket& packet,
   uint32_t requiredSizeBytes =
       size() + packet.sizeBytes +
       (packet.insertStartCode ? kH264StartCodeLengthBytes : 0);
-  if (requiredSizeBytes >= capacity()) {
+  if (requiredSizeBytes > capacity()) {
     const uint8_t* prevBuffer = data();
     const uint32_t increments =
         requiredSizeBytes / kBufferIncStepSizeBytes +
@@ -112,7 +112,15 @@ VCMFrameBufferEnum VCMFrameBuffer::InsertPacket(const VCMPacket& packet,
                            "big.";
       return kSizeError;
     }
-    VerifyAndAllocate(newSize);
+    if (data() == nullptr) {
+      encoded_image_buffer_ = EncodedImageBuffer::Create(newSize);
+      SetEncodedData(encoded_image_buffer_);
+      set_size(0);
+    } else {
+      RTC_CHECK(encoded_image_buffer_ != nullptr);
+      RTC_DCHECK_EQ(encoded_image_buffer_->data(), data());
+      encoded_image_buffer_->Realloc(newSize);
+    }
     _sessionInfo.UpdateDataPointers(prevBuffer, data());
   }
 
diff --git a/modules/video_coding/frame_buffer.h b/modules/video_coding/frame_buffer.h
index d74749c1c5..76df28e588 100644
--- a/modules/video_coding/frame_buffer.h
+++ b/modules/video_coding/frame_buffer.h
@@ -76,6 +76,9 @@ class VCMFrameBuffer : public VCMEncodedFrame {
   void SetState(VCMFrameBufferStateEnum state);  // Set state of frame
 
   VCMFrameBufferStateEnum _state;  // Current state of the frame
+  // Set with SetEncodedData, but keep pointer to the concrete class here, to
+  // enable reallocation and mutation.
+  rtc::scoped_refptr<EncodedImageBuffer> encoded_image_buffer_;
   VCMSessionInfo _sessionInfo;
   uint16_t _nackCount;
   int64_t _latestPacketTimeMs;