RtpFrameObject now takes an EncodedImageBuffer in its ctor.

Bug: webrtc:10979
Change-Id: Ibc8b4a524ca95b5faa8850a41df8f2f0136a2969
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/153666
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Sam Zackrisson <saza@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29251}

modules/video_coding/frame_object.cc

@@ -24,14 +24,16 @@
 namespace webrtc {
 namespace video_coding {
 
-RtpFrameObject::RtpFrameObject(PacketBuffer* packet_buffer,
+RtpFrameObject::RtpFrameObject(
-                               uint16_t first_seq_num,
+    PacketBuffer* packet_buffer,
-                               uint16_t last_seq_num,
+    uint16_t first_seq_num,
-                               size_t frame_size,
+    uint16_t last_seq_num,
-                               int times_nacked,
+    size_t frame_size,
-                               int64_t first_packet_received_time,
+    int times_nacked,
-                               int64_t last_packet_received_time,
+    int64_t first_packet_received_time,
-                               RtpPacketInfos packet_infos)
+    int64_t last_packet_received_time,
+    RtpPacketInfos packet_infos,
+    rtc::scoped_refptr<EncodedImageBuffer> image_buffer)
     : first_seq_num_(first_seq_num),
       last_seq_num_(last_seq_num),
       last_packet_received_time_(last_packet_received_time),
@@ -58,6 +60,7 @@ RtpFrameObject::RtpFrameObject(PacketBuffer* packet_buffer,
   // as of the first packet's.
   SetPlayoutDelay(first_packet->video_header.playout_delay);
 
+  SetEncodedData(std::move(image_buffer));
   _encodedWidth = first_packet->width();
   _encodedHeight = first_packet->height();
 

modules/video_coding/frame_object.h

@@ -23,7 +23,6 @@ class PacketBuffer;
 
 class RtpFrameObject : public EncodedFrame {
  public:
-  // TODO(philipel): Update the ctor to take an EncodedImageBuffer.
   RtpFrameObject(PacketBuffer* packet_buffer,
                  uint16_t first_seq_num,
                  uint16_t last_seq_num,
@@ -31,7 +30,8 @@ class RtpFrameObject : public EncodedFrame {
                  int times_nacked,
                  int64_t first_packet_received_time,
                  int64_t last_packet_received_time,
-                 RtpPacketInfos packet_infos);
+                 RtpPacketInfos packet_infos,
+                 rtc::scoped_refptr<EncodedImageBuffer> image_buffer);
 
   ~RtpFrameObject() override;
   uint16_t first_seq_num() const;

modules/video_coding/packet_buffer.cc

@@ -18,7 +18,6 @@
 
 #include "absl/types/variant.h"
 #include "api/video/encoded_frame.h"
-#include "api/video/encoded_image.h"
 #include "common_video/h264/h264_common.h"
 #include "modules/rtp_rtcp/source/rtp_video_header.h"
 #include "modules/video_coding/codecs/h264/include/h264_globals.h"
@@ -439,10 +438,9 @@ std::vector<std::unique_ptr<RtpFrameObject>> PacketBuffer::FindFrames(
 
       auto frame = std::make_unique<RtpFrameObject>(
           this, start_seq_num, seq_num, frame_size, max_nack_count,
-          min_recv_time, max_recv_time,
+          min_recv_time, max_recv_time, RtpPacketInfos(std::move(packet_infos)),
-          RtpPacketInfos(std::move(packet_infos)));
+          GetEncodedImageBuffer(frame_size, start_seq_num, seq_num));
-      frame->SetEncodedData(EncodedImageBuffer::Create(frame_size));
+
-      GetBitstream(*frame, frame->data());
       found_frames.emplace_back(std::move(frame));
 
       ClearInterval(start_seq_num, seq_num);
@@ -452,43 +450,28 @@ std::vector<std::unique_ptr<RtpFrameObject>> PacketBuffer::FindFrames(
   return found_frames;
 }
 
-// TODO(philipel): Update function to not accept an RtpFrameObject.
+rtc::scoped_refptr<EncodedImageBuffer> PacketBuffer::GetEncodedImageBuffer(
-bool PacketBuffer::GetBitstream(const RtpFrameObject& frame,
+    size_t frame_size,
-                                uint8_t* destination) {
+    uint16_t first_seq_num,
-  rtc::CritScope lock(&crit_);
+    uint16_t last_seq_num) {
+  size_t index = first_seq_num % size_;
+  size_t end = (last_seq_num + 1) % size_;
 
-  size_t index = frame.first_seq_num() % size_;
+  auto buffer = EncodedImageBuffer::Create(frame_size);
-  size_t end = (frame.last_seq_num() + 1) % size_;
+  size_t offset = 0;
-  uint16_t seq_num = frame.first_seq_num();
-  uint32_t timestamp = frame.Timestamp();
-  uint8_t* destination_end = destination + frame.size();
 
   do {
-    // Check both seq_num and timestamp to handle the case when seq_num wraps
+    RTC_DCHECK(sequence_buffer_[index].used);
-    // around too quickly for high packet rates.
-    if (!sequence_buffer_[index].used ||
-        sequence_buffer_[index].seq_num != seq_num ||
-        data_buffer_[index].timestamp != timestamp) {
-      return false;
-    }
 
-    RTC_DCHECK_EQ(data_buffer_[index].seqNum, sequence_buffer_[index].seq_num);
     size_t length = data_buffer_[index].sizeBytes;
-    if (destination + length > destination_end) {
+    RTC_CHECK_LE(offset + length, buffer->size());
-      RTC_LOG(LS_WARNING) << "Frame (" << frame.id.picture_id << ":"
+    memcpy(buffer->data() + offset, data_buffer_[index].dataPtr, length);
-                          << static_cast<int>(frame.id.spatial_layer) << ")"
+    offset += length;
-                          << " bitstream buffer is not large enough.";
-      return false;
-    }
 
-    const uint8_t* source = data_buffer_[index].dataPtr;
-    memcpy(destination, source, length);
-    destination += length;
     index = (index + 1) % size_;
-    ++seq_num;
   } while (index != end);
 
-  return true;
+  return buffer;
 }
 
 VCMPacket* PacketBuffer::GetPacket(uint16_t seq_num) {

modules/video_coding/packet_buffer.h

@@ -17,6 +17,7 @@
 #include <vector>
 
 #include "api/scoped_refptr.h"
+#include "api/video/encoded_image.h"
 #include "modules/include/module_common_types.h"
 #include "modules/video_coding/packet.h"
 #include "rtc_base/critical_section.h"
@@ -113,8 +114,10 @@ class PacketBuffer {
   std::vector<std::unique_ptr<RtpFrameObject>> FindFrames(uint16_t seq_num)
       RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);
 
-  // Copy the bitstream for |frame| to |destination|.
+  rtc::scoped_refptr<EncodedImageBuffer> GetEncodedImageBuffer(
-  bool GetBitstream(const RtpFrameObject& frame, uint8_t* destination);
+      size_t frame_size,
+      uint16_t first_seq_num,
+      uint16_t last_seq_num) RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);
 
   // Get the packet with sequence number |seq_num|.
   // Virtual for testing.

modules/video_coding/rtp_frame_reference_finder_unittest.cc

@@ -84,8 +84,9 @@ class TestRtpFrameReferenceFinder : public ::testing::Test,
     packet.video_header.is_last_packet_in_frame = true;
     ref_packet_buffer_->InsertPacket(&packet);
 
-    std::unique_ptr<RtpFrameObject> frame(new RtpFrameObject(
+    std::unique_ptr<RtpFrameObject> frame(
-        ref_packet_buffer_, seq_num_start, seq_num_end, 0, 0, 0, 0, {}));
+        new RtpFrameObject(ref_packet_buffer_, seq_num_start, seq_num_end, 0, 0,
+                           0, 0, {}, EncodedImageBuffer::Create(/*size=*/0)));
     reference_finder_->ManageFrame(std::move(frame));
   }
 
@@ -118,8 +119,9 @@ class TestRtpFrameReferenceFinder : public ::testing::Test,
       ref_packet_buffer_->InsertPacket(&packet);
     }
 
-    std::unique_ptr<RtpFrameObject> frame(new RtpFrameObject(
+    std::unique_ptr<RtpFrameObject> frame(
-        ref_packet_buffer_, seq_num_start, seq_num_end, 0, 0, 0, 0, {}));
+        new RtpFrameObject(ref_packet_buffer_, seq_num_start, seq_num_end, 0, 0,
+                           0, 0, {}, EncodedImageBuffer::Create(/*size=*/0)));
     reference_finder_->ManageFrame(std::move(frame));
   }
 
@@ -164,8 +166,9 @@ class TestRtpFrameReferenceFinder : public ::testing::Test,
       ref_packet_buffer_->InsertPacket(&packet);
     }
 
-    std::unique_ptr<RtpFrameObject> frame(new RtpFrameObject(
+    std::unique_ptr<RtpFrameObject> frame(
-        ref_packet_buffer_, seq_num_start, seq_num_end, 0, 0, 0, 0, {}));
+        new RtpFrameObject(ref_packet_buffer_, seq_num_start, seq_num_end, 0, 0,
+                           0, 0, {}, EncodedImageBuffer::Create(/*size=*/0)));
     reference_finder_->ManageFrame(std::move(frame));
   }
 
@@ -205,8 +208,9 @@ class TestRtpFrameReferenceFinder : public ::testing::Test,
       ref_packet_buffer_->InsertPacket(&packet);
     }
 
-    std::unique_ptr<RtpFrameObject> frame(new RtpFrameObject(
+    std::unique_ptr<RtpFrameObject> frame(
-        ref_packet_buffer_, seq_num_start, seq_num_end, 0, 0, 0, 0, {}));
+        new RtpFrameObject(ref_packet_buffer_, seq_num_start, seq_num_end, 0, 0,
+                           0, 0, {}, EncodedImageBuffer::Create(/*size=*/0)));
     reference_finder_->ManageFrame(std::move(frame));
   }
 
@@ -235,8 +239,9 @@ class TestRtpFrameReferenceFinder : public ::testing::Test,
       ref_packet_buffer_->InsertPacket(&packet);
     }
 
-    std::unique_ptr<RtpFrameObject> frame(new RtpFrameObject(
+    std::unique_ptr<RtpFrameObject> frame(
-        ref_packet_buffer_, seq_num_start, seq_num_end, 0, 0, 0, 0, {}));
+        new RtpFrameObject(ref_packet_buffer_, seq_num_start, seq_num_end, 0, 0,
+                           0, 0, {}, EncodedImageBuffer::Create(/*size=*/0)));
     reference_finder_->ManageFrame(std::move(frame));
   }
 

test/fuzzers/rtp_frame_reference_finder_fuzzer.cc

@@ -126,7 +126,8 @@ void FuzzOneInput(const uint8_t* data, size_t size) {
     last_packet->video_header.is_last_packet_in_frame = true;
 
     auto frame = std::make_unique<video_coding::RtpFrameObject>(
-        pb, first_seq_num, last_seq_num, 0, 0, 0, 0, RtpPacketInfos());
+        pb, first_seq_num, last_seq_num, 0, 0, 0, 0, RtpPacketInfos(),
+        EncodedImageBuffer::Create(/*size=*/0));
     reference_finder.ManageFrame(std::move(frame));
   }
 }

video/buffered_frame_decryptor_unittest.cc

@@ -96,8 +96,9 @@ class BufferedFrameDecryptorTest
     fake_packet_buffer_->InsertPacket(&packet);
 
     return std::unique_ptr<video_coding::RtpFrameObject>(
-        new video_coding::RtpFrameObject(fake_packet_buffer_.get(), seq_num_,
+        new video_coding::RtpFrameObject(
-                                         seq_num_, 0, 0, 0, 0, {}));
+            fake_packet_buffer_.get(), seq_num_, seq_num_, 0, 0, 0, 0, {},
+            EncodedImageBuffer::Create(/*size=*/0)));
   }
 
  protected: