From 8b07305b048fb91840ae7f9a58c6df7115540015 Mon Sep 17 00:00:00 2001 From: eladalon Date: Fri, 25 Aug 2017 00:49:08 -0700 Subject: [PATCH] Eliminate RtpVideoStreamReceiver::receive_cs_ in favor of using a SequencedTaskChecker RtpVideoStreamReceiver::receive_cs_ is not really necessary, since all of the functions where that lock is acquired, are arrived at from functions of BaseChannel which DCHECK being called from BaseChannel::worker_thread_. BUG=webrtc:8037 Review-Url: https://codereview.webrtc.org/2987933003 Cr-Commit-Position: refs/heads/master@{#19508} --- webrtc/video/rtp_video_stream_receiver.cc | 71 ++++++++++------------- webrtc/video/rtp_video_stream_receiver.h | 18 +++--- 2 files changed, 40 insertions(+), 49 deletions(-) diff --git a/webrtc/video/rtp_video_stream_receiver.cc b/webrtc/video/rtp_video_stream_receiver.cc index 6064e69715..01cb4dcc2f 100644 --- a/webrtc/video/rtp_video_stream_receiver.cc +++ b/webrtc/video/rtp_video_stream_receiver.cc @@ -314,37 +314,33 @@ int32_t RtpVideoStreamReceiver::OnInitializeDecoder( // This method handles both regular RTP packets and packets recovered // via FlexFEC. void RtpVideoStreamReceiver::OnRtpPacket(const RtpPacketReceived& packet) { - // TODO(eladalon): https://bugs.chromium.org/p/webrtc/issues/detail?id=8056 - // RTC_DCHECK_RUN_ON(&worker_thread_checker_); + RTC_DCHECK_CALLED_SEQUENTIALLY(&worker_task_checker_); - { - rtc::CritScope lock(&receive_cs_); - if (!receiving_) { - return; - } + if (!receiving_) { + return; + } - if (!packet.recovered()) { - int64_t now_ms = clock_->TimeInMilliseconds(); + if (!packet.recovered()) { + int64_t now_ms = clock_->TimeInMilliseconds(); - // Periodically log the RTP header of incoming packets. - if (now_ms - last_packet_log_ms_ > kPacketLogIntervalMs) { - std::stringstream ss; - ss << "Packet received on SSRC: " << packet.Ssrc() - << " with payload type: " << static_cast(packet.PayloadType()) - << ", timestamp: " << packet.Timestamp() - << ", sequence number: " << packet.SequenceNumber() - << ", arrival time: " << packet.arrival_time_ms(); - int32_t time_offset; - if (packet.GetExtension(&time_offset)) { - ss << ", toffset: " << time_offset; - } - uint32_t send_time; - if (packet.GetExtension(&send_time)) { - ss << ", abs send time: " << send_time; - } - LOG(LS_INFO) << ss.str(); - last_packet_log_ms_ = now_ms; + // Periodically log the RTP header of incoming packets. + if (now_ms - last_packet_log_ms_ > kPacketLogIntervalMs) { + std::stringstream ss; + ss << "Packet received on SSRC: " << packet.Ssrc() + << " with payload type: " << static_cast(packet.PayloadType()) + << ", timestamp: " << packet.Timestamp() + << ", sequence number: " << packet.SequenceNumber() + << ", arrival time: " << packet.arrival_time_ms(); + int32_t time_offset; + if (packet.GetExtension(&time_offset)) { + ss << ", toffset: " << time_offset; } + uint32_t send_time; + if (packet.GetExtension(&send_time)) { + ss << ", abs send time: " << send_time; + } + LOG(LS_INFO) << ss.str(); + last_packet_log_ms_ = now_ms; } } @@ -442,8 +438,7 @@ rtc::Optional RtpVideoStreamReceiver::LastReceivedKeyframePacketMs() } void RtpVideoStreamReceiver::AddSecondarySink(RtpPacketSinkInterface* sink) { - // TODO(eladalon): https://bugs.chromium.org/p/webrtc/issues/detail?id=8056 - // RTC_DCHECK_RUN_ON(&worker_thread_checker_); + RTC_DCHECK_CALLED_SEQUENTIALLY(&worker_task_checker_); RTC_DCHECK(std::find(secondary_sinks_.cbegin(), secondary_sinks_.cend(), sink) == secondary_sinks_.cend()); secondary_sinks_.push_back(sink); @@ -451,8 +446,7 @@ void RtpVideoStreamReceiver::AddSecondarySink(RtpPacketSinkInterface* sink) { void RtpVideoStreamReceiver::RemoveSecondarySink( const RtpPacketSinkInterface* sink) { - // TODO(eladalon): https://bugs.chromium.org/p/webrtc/issues/detail?id=8056 - // RTC_DCHECK_RUN_ON(&worker_thread_checker_); + RTC_DCHECK_CALLED_SEQUENTIALLY(&worker_task_checker_); auto it = std::find(secondary_sinks_.begin(), secondary_sinks_.end(), sink); if (it == secondary_sinks_.end()) { // We might be rolling-back a call whose setup failed mid-way. In such a @@ -486,6 +480,7 @@ void RtpVideoStreamReceiver::ReceivePacket(const uint8_t* packet, void RtpVideoStreamReceiver::ParseAndHandleEncapsulatingHeader( const uint8_t* packet, size_t packet_length, const RTPHeader& header) { + RTC_DCHECK_CALLED_SEQUENTIALLY(&worker_task_checker_); if (rtp_payload_registry_.IsRed(header)) { int8_t ulpfec_pt = rtp_payload_registry_.ulpfec_payload_type(); if (packet[header.headerLength] == ulpfec_pt) { @@ -510,7 +505,6 @@ void RtpVideoStreamReceiver::ParseAndHandleEncapsulatingHeader( return; if (packet_length > sizeof(restored_packet_)) return; - rtc::CritScope lock(&receive_cs_); if (restored_packet_in_use_) { LOG(LS_WARNING) << "Multiple RTX headers detected, dropping packet."; return; @@ -568,11 +562,10 @@ void RtpVideoStreamReceiver::NotifyReceiverOfFecPacket( bool RtpVideoStreamReceiver::DeliverRtcp(const uint8_t* rtcp_packet, size_t rtcp_packet_length) { - { - rtc::CritScope lock(&receive_cs_); - if (!receiving_) { - return false; - } + RTC_DCHECK_CALLED_SEQUENTIALLY(&worker_task_checker_); + + if (!receiving_) { + return false; } rtp_rtcp_->IncomingRtcpPacket(rtcp_packet, rtcp_packet_length); @@ -634,12 +627,12 @@ void RtpVideoStreamReceiver::SignalNetworkState(NetworkState state) { } void RtpVideoStreamReceiver::StartReceive() { - rtc::CritScope lock(&receive_cs_); + RTC_DCHECK_CALLED_SEQUENTIALLY(&worker_task_checker_); receiving_ = true; } void RtpVideoStreamReceiver::StopReceive() { - rtc::CritScope lock(&receive_cs_); + RTC_DCHECK_CALLED_SEQUENTIALLY(&worker_task_checker_); receiving_ = false; } diff --git a/webrtc/video/rtp_video_stream_receiver.h b/webrtc/video/rtp_video_stream_receiver.h index 4d63d76480..15af2e60ee 100644 --- a/webrtc/video/rtp_video_stream_receiver.h +++ b/webrtc/video/rtp_video_stream_receiver.h @@ -32,7 +32,7 @@ #include "webrtc/modules/video_coding/sequence_number_util.h" #include "webrtc/rtc_base/constructormagic.h" #include "webrtc/rtc_base/criticalsection.h" -#include "webrtc/rtc_base/thread_checker.h" +#include "webrtc/rtc_base/sequenced_task_checker.h" #include "webrtc/typedefs.h" namespace webrtc { @@ -183,11 +183,11 @@ class RtpVideoStreamReceiver : public RtpData, const std::unique_ptr rtp_receive_statistics_; std::unique_ptr ulpfec_receiver_; - rtc::CriticalSection receive_cs_; - bool receiving_ GUARDED_BY(receive_cs_); - uint8_t restored_packet_[IP_PACKET_SIZE] GUARDED_BY(receive_cs_); - bool restored_packet_in_use_ GUARDED_BY(receive_cs_); - int64_t last_packet_log_ms_ GUARDED_BY(receive_cs_); + rtc::SequencedTaskChecker worker_task_checker_; + bool receiving_ GUARDED_BY(worker_task_checker_); + uint8_t restored_packet_[IP_PACKET_SIZE] GUARDED_BY(worker_task_checker_); + bool restored_packet_in_use_ GUARDED_BY(worker_task_checker_); + int64_t last_packet_log_ms_ GUARDED_BY(worker_task_checker_); const std::unique_ptr rtp_rtcp_; @@ -210,10 +210,8 @@ class RtpVideoStreamReceiver : public RtpData, bool has_received_frame_; - // TODO(eladalon): https://bugs.chromium.org/p/webrtc/issues/detail?id=8056 - // rtc::ThreadChecker worker_thread_checker_; - std::vector secondary_sinks_; // This needs - // to be GUARDED_BY(worker_thread_checker_). + std::vector secondary_sinks_ + GUARDED_BY(worker_task_checker_); }; } // namespace webrtc