diff --git a/modules/video_coding/packet_buffer.cc b/modules/video_coding/packet_buffer.cc index 0d4c085286..1291f78e3a 100644 --- a/modules/video_coding/packet_buffer.cc +++ b/modules/video_coding/packet_buffer.cc @@ -70,7 +70,6 @@ PacketBuffer::~PacketBuffer() { PacketBuffer::InsertResult PacketBuffer::InsertPacket( std::unique_ptr 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(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(seq_num + 1)); return result; diff --git a/modules/video_coding/packet_buffer.h b/modules/video_coding/packet_buffer.h index eb8d8365a8..c0cc752c3a 100644 --- a/modules/video_coding/packet_buffer.h +++ b/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) - RTC_LOCKS_EXCLUDED(mutex_); - ABSL_MUST_USE_RESULT InsertResult InsertPadding(uint16_t seq_num) - RTC_LOCKS_EXCLUDED(mutex_); - void ClearTo(uint16_t seq_num) RTC_LOCKS_EXCLUDED(mutex_); - void Clear() RTC_LOCKS_EXCLUDED(mutex_); + ABSL_MUST_USE_RESULT InsertResult + InsertPacket(std::unique_ptr packet); + ABSL_MUST_USE_RESULT InsertResult InsertPadding(uint16_t seq_num); + void ClearTo(uint16_t seq_num); + void Clear(); void ForceSpsPpsIdrIsH264Keyframe(); private: - // Clears with |mutex_| taken. - void ClearInternal() RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + void ClearInternal(); // 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 - RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + bool PotentialNewFrame(uint16_t seq_num) const; // Test if all packets of a frame has arrived, and if so, returns packets to // create frames. - std::vector> FindFrames(uint16_t seq_num) - RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + std::vector> FindFrames(uint16_t seq_num); - void UpdateMissingPackets(uint16_t seq_num) - RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_); - - mutable Mutex mutex_; + void UpdateMissingPackets(uint16_t seq_num); // 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> buffer_ RTC_GUARDED_BY(mutex_); + std::vector> buffer_; - absl::optional newest_inserted_seq_num_ RTC_GUARDED_BY(mutex_); - std::set> missing_packets_ - RTC_GUARDED_BY(mutex_); + absl::optional newest_inserted_seq_num_; + std::set> missing_packets_; // Indicates if we should require SPS, PPS, and IDR for a particular // RTP timestamp to treat the corresponding frame as a keyframe. diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index e092fe7374..a5937b027b 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/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) { - packet_buffer_.ClearTo(seq_num); + { + MutexLock lock(&packet_buffer_lock_); + packet_buffer_.ClearTo(seq_num); + } MutexLock lock(&reference_finder_lock_); reference_finder_->ClearTo(seq_num); } diff --git a/video/rtp_video_stream_receiver.h b/video/rtp_video_stream_receiver.h index 7a845194f4..b275fb6e9c 100644 --- a/video/rtp_video_stream_receiver.h +++ b/video/rtp_video_stream_receiver.h @@ -337,7 +337,8 @@ class RtpVideoStreamReceiver : public LossNotificationSender, std::unique_ptr nack_module_; std::unique_ptr 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 frame_id_unwrapper_ RTC_GUARDED_BY(worker_task_checker_); diff --git a/video/rtp_video_stream_receiver2.h b/video/rtp_video_stream_receiver2.h index 207dfe7fb0..4076837ade 100644 --- a/video/rtp_video_stream_receiver2.h +++ b/video/rtp_video_stream_receiver2.h @@ -294,7 +294,8 @@ class RtpVideoStreamReceiver2 : public LossNotificationSender, const std::unique_ptr nack_module_; std::unique_ptr 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 frame_id_unwrapper_ RTC_GUARDED_BY(worker_task_checker_);