Remove PacketBuffers internal mutex.

In RtpVideoStreamReceiver2 it can be protected by the `worker_task_checker_` instead.

Bug: webrtc:12579
Change-Id: I4f7d64f16172139eddc7a3e07d1dbbf338beaf2e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/215224
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33734}

modules/video_coding/packet_buffer.cc

@@ -70,7 +70,6 @@ PacketBuffer::~PacketBuffer() {
 PacketBuffer::InsertResult PacketBuffer::InsertPacket(
     std::unique_ptr<PacketBuffer::Packet> packet) {
   PacketBuffer::InsertResult result;
-  MutexLock lock(&mutex_);
 
   uint16_t seq_num = packet->seq_num;
   size_t index = seq_num % buffer_.size();
@@ -120,7 +119,6 @@ PacketBuffer::InsertResult PacketBuffer::InsertPacket(
 }
 
 void PacketBuffer::ClearTo(uint16_t seq_num) {
-  MutexLock lock(&mutex_);
   // We have already cleared past this sequence number, no need to do anything.
   if (is_cleared_to_first_seq_num_ &&
       AheadOf<uint16_t>(first_seq_num_, seq_num)) {
@@ -157,13 +155,11 @@ void PacketBuffer::ClearTo(uint16_t seq_num) {
 }
 
 void PacketBuffer::Clear() {
-  MutexLock lock(&mutex_);
   ClearInternal();
 }
 
 PacketBuffer::InsertResult PacketBuffer::InsertPadding(uint16_t seq_num) {
   PacketBuffer::InsertResult result;
-  MutexLock lock(&mutex_);
   UpdateMissingPackets(seq_num);
   result.packets = FindFrames(static_cast<uint16_t>(seq_num + 1));
   return result;

modules/video_coding/packet_buffer.h

@@ -23,7 +23,6 @@
 #include "modules/rtp_rtcp/source/rtp_video_header.h"
 #include "rtc_base/copy_on_write_buffer.h"
 #include "rtc_base/numerics/sequence_number_util.h"
-#include "rtc_base/synchronization/mutex.h"
 #include "rtc_base/thread_annotations.h"
 
 namespace webrtc {
@@ -78,55 +77,47 @@ class PacketBuffer {
   PacketBuffer(size_t start_buffer_size, size_t max_buffer_size);
   ~PacketBuffer();
 
-  ABSL_MUST_USE_RESULT InsertResult InsertPacket(std::unique_ptr<Packet> packet)
+  ABSL_MUST_USE_RESULT InsertResult
-      RTC_LOCKS_EXCLUDED(mutex_);
+  InsertPacket(std::unique_ptr<Packet> packet);
-  ABSL_MUST_USE_RESULT InsertResult InsertPadding(uint16_t seq_num)
+  ABSL_MUST_USE_RESULT InsertResult InsertPadding(uint16_t seq_num);
-      RTC_LOCKS_EXCLUDED(mutex_);
+  void ClearTo(uint16_t seq_num);
-  void ClearTo(uint16_t seq_num) RTC_LOCKS_EXCLUDED(mutex_);
+  void Clear();
-  void Clear() RTC_LOCKS_EXCLUDED(mutex_);
 
   void ForceSpsPpsIdrIsH264Keyframe();
 
  private:
-  // Clears with |mutex_| taken.
+  void ClearInternal();
-  void ClearInternal() RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
 
   // Tries to expand the buffer.
-  bool ExpandBufferSize() RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
+  bool ExpandBufferSize();
 
   // Test if all previous packets has arrived for the given sequence number.
-  bool PotentialNewFrame(uint16_t seq_num) const
+  bool PotentialNewFrame(uint16_t seq_num) const;
-      RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
 
   // Test if all packets of a frame has arrived, and if so, returns packets to
   // create frames.
-  std::vector<std::unique_ptr<Packet>> FindFrames(uint16_t seq_num)
+  std::vector<std::unique_ptr<Packet>> FindFrames(uint16_t seq_num);
-      RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
 
-  void UpdateMissingPackets(uint16_t seq_num)
+  void UpdateMissingPackets(uint16_t seq_num);
-      RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
-
-  mutable Mutex mutex_;
 
   // buffer_.size() and max_size_ must always be a power of two.
   const size_t max_size_;
 
   // The fist sequence number currently in the buffer.
-  uint16_t first_seq_num_ RTC_GUARDED_BY(mutex_);
+  uint16_t first_seq_num_;
 
   // If the packet buffer has received its first packet.
-  bool first_packet_received_ RTC_GUARDED_BY(mutex_);
+  bool first_packet_received_;
 
   // If the buffer is cleared to |first_seq_num_|.
-  bool is_cleared_to_first_seq_num_ RTC_GUARDED_BY(mutex_);
+  bool is_cleared_to_first_seq_num_;
 
   // Buffer that holds the the inserted packets and information needed to
   // determine continuity between them.
-  std::vector<std::unique_ptr<Packet>> buffer_ RTC_GUARDED_BY(mutex_);
+  std::vector<std::unique_ptr<Packet>> buffer_;
 
-  absl::optional<uint16_t> newest_inserted_seq_num_ RTC_GUARDED_BY(mutex_);
+  absl::optional<uint16_t> newest_inserted_seq_num_;
-  std::set<uint16_t, DescendingSeqNumComp<uint16_t>> missing_packets_
+  std::set<uint16_t, DescendingSeqNumComp<uint16_t>> missing_packets_;
-      RTC_GUARDED_BY(mutex_);
 
   // Indicates if we should require SPS, PPS, and IDR for a particular
   // RTP timestamp to treat the corresponding frame as a keyframe.

video/rtp_video_stream_receiver.cc

@@ -364,6 +364,7 @@ void RtpVideoStreamReceiver::AddReceiveCodec(
     bool raw_payload) {
   if (codec_params.count(cricket::kH264FmtpSpsPpsIdrInKeyframe) ||
       field_trial::IsEnabled("WebRTC-SpsPpsIdrIsH264Keyframe")) {
+    MutexLock lock(&packet_buffer_lock_);
     packet_buffer_.ForceSpsPpsIdrIsH264Keyframe();
   }
   payload_type_map_.emplace(
@@ -644,7 +645,12 @@ void RtpVideoStreamReceiver::OnReceivedPayloadData(
 
   rtcp_feedback_buffer_.SendBufferedRtcpFeedback();
   frame_counter_.Add(packet->timestamp);
-  OnInsertedPacket(packet_buffer_.InsertPacket(std::move(packet)));
+  video_coding::PacketBuffer::InsertResult insert_result;
+  {
+    MutexLock lock(&packet_buffer_lock_);
+    insert_result = packet_buffer_.InsertPacket(std::move(packet));
+  }
+  OnInsertedPacket(std::move(insert_result));
 }
 
 void RtpVideoStreamReceiver::OnRecoveredPacket(const uint8_t* rtp_packet,
@@ -1016,7 +1022,12 @@ void RtpVideoStreamReceiver::NotifyReceiverOfEmptyPacket(uint16_t seq_num) {
     MutexLock lock(&reference_finder_lock_);
     reference_finder_->PaddingReceived(seq_num);
   }
-  OnInsertedPacket(packet_buffer_.InsertPadding(seq_num));
+  video_coding::PacketBuffer::InsertResult insert_result;
+  {
+    MutexLock lock(&packet_buffer_lock_);
+    insert_result = packet_buffer_.InsertPadding(seq_num);
+  }
+  OnInsertedPacket(std::move(insert_result));
   if (nack_module_) {
     nack_module_->OnReceivedPacket(seq_num, /* is_keyframe = */ false,
                                    /* is _recovered = */ false);
@@ -1098,7 +1109,10 @@ void RtpVideoStreamReceiver::FrameDecoded(int64_t picture_id) {
     }
   }
   if (seq_num != -1) {
+    {
+      MutexLock lock(&packet_buffer_lock_);
       packet_buffer_.ClearTo(seq_num);
+    }
     MutexLock lock(&reference_finder_lock_);
     reference_finder_->ClearTo(seq_num);
   }

video/rtp_video_stream_receiver.h

@@ -337,7 +337,8 @@ class RtpVideoStreamReceiver : public LossNotificationSender,
   std::unique_ptr<DEPRECATED_NackModule> nack_module_;
   std::unique_ptr<LossNotificationController> loss_notification_controller_;
 
-  video_coding::PacketBuffer packet_buffer_;
+  mutable Mutex packet_buffer_lock_;
+  video_coding::PacketBuffer packet_buffer_ RTC_GUARDED_BY(packet_buffer_lock_);
   UniqueTimestampCounter frame_counter_ RTC_GUARDED_BY(worker_task_checker_);
   SeqNumUnwrapper<uint16_t> frame_id_unwrapper_
       RTC_GUARDED_BY(worker_task_checker_);

video/rtp_video_stream_receiver2.h

@@ -294,7 +294,8 @@ class RtpVideoStreamReceiver2 : public LossNotificationSender,
   const std::unique_ptr<NackModule2> nack_module_;
   std::unique_ptr<LossNotificationController> loss_notification_controller_;
 
-  video_coding::PacketBuffer packet_buffer_;
+  video_coding::PacketBuffer packet_buffer_
+      RTC_GUARDED_BY(worker_task_checker_);
   UniqueTimestampCounter frame_counter_ RTC_GUARDED_BY(worker_task_checker_);
   SeqNumUnwrapper<uint16_t> frame_id_unwrapper_
       RTC_GUARDED_BY(worker_task_checker_);