diff --git a/webrtc/modules/video_coding/nack_module.cc b/webrtc/modules/video_coding/nack_module.cc index d74f089526..1bd88f0fc1 100644 --- a/webrtc/modules/video_coding/nack_module.cc +++ b/webrtc/modules/video_coding/nack_module.cc @@ -46,7 +46,6 @@ NackModule::NackModule(Clock* clock, nack_sender_(nack_sender), keyframe_request_sender_(keyframe_request_sender), reordering_histogram_(kNumReorderingBuckets, kMaxReorderedPackets), - running_(true), initialized_(false), rtt_ms_(kDefaultRttMs), newest_seq_num_(0), @@ -58,8 +57,6 @@ NackModule::NackModule(Clock* clock, int NackModule::OnReceivedPacket(const VCMPacket& packet) { rtc::CritScope lock(&crit_); - if (!running_) - return -1; uint16_t seq_num = packet.seqNum; // TODO(philipel): When the packet includes information whether it is // retransmitted or not, use that value instead. For @@ -132,21 +129,22 @@ void NackModule::Clear() { keyframe_list_.clear(); } -void NackModule::Stop() { - rtc::CritScope lock(&crit_); - running_ = false; -} - int64_t NackModule::TimeUntilNextProcess() { - rtc::CritScope lock(&crit_); return std::max(next_process_time_ms_ - clock_->TimeInMilliseconds(), 0); } void NackModule::Process() { - rtc::CritScope lock(&crit_); - if (!running_) - return; + if (nack_sender_) { + std::vector nack_batch; + { + rtc::CritScope lock(&crit_); + nack_batch = GetNackBatch(kTimeOnly); + } + + if (!nack_batch.empty()) + nack_sender_->SendNack(nack_batch); + } // Update the next_process_time_ms_ in intervals to achieve // the targeted frequency over time. Also add multiple intervals @@ -160,10 +158,6 @@ void NackModule::Process() { (now_ms - next_process_time_ms_) / kProcessIntervalMs * kProcessIntervalMs; } - - std::vector nack_batch = GetNackBatch(kTimeOnly); - if (!nack_batch.empty() && nack_sender_ != nullptr) - nack_sender_->SendNack(nack_batch); } bool NackModule::RemovePacketsUntilKeyFrame() { diff --git a/webrtc/modules/video_coding/nack_module.h b/webrtc/modules/video_coding/nack_module.h index 58d6cfa985..6cdd0477b2 100644 --- a/webrtc/modules/video_coding/nack_module.h +++ b/webrtc/modules/video_coding/nack_module.h @@ -36,7 +36,6 @@ class NackModule : public Module { void ClearUpTo(uint16_t seq_num); void UpdateRtt(int64_t rtt_ms); void Clear(); - void Stop(); // Module implementation int64_t TimeUntilNextProcess() override; @@ -82,16 +81,20 @@ class NackModule : public Module { NackSender* const nack_sender_; KeyFrameRequestSender* const keyframe_request_sender_; + // TODO(philipel): Some of the variables below are consistently used on a + // known thread (e.g. see |initialized_|). Those probably do not need + // synchronized access. std::map> nack_list_ GUARDED_BY(crit_); std::set> keyframe_list_ GUARDED_BY(crit_); video_coding::Histogram reordering_histogram_ GUARDED_BY(crit_); - bool running_ GUARDED_BY(crit_); bool initialized_ GUARDED_BY(crit_); int64_t rtt_ms_ GUARDED_BY(crit_); uint16_t newest_seq_num_ GUARDED_BY(crit_); - int64_t next_process_time_ms_ GUARDED_BY(crit_); + + // Only touched on the process thread. + int64_t next_process_time_ms_; }; } // namespace webrtc diff --git a/webrtc/video/rtp_stream_receiver.cc b/webrtc/video/rtp_stream_receiver.cc index b8ebc77a5b..e964d10fd8 100644 --- a/webrtc/video/rtp_stream_receiver.cc +++ b/webrtc/video/rtp_stream_receiver.cc @@ -186,11 +186,11 @@ RtpStreamReceiver::RtpStreamReceiver( process_thread_->RegisterModule(rtp_rtcp_.get()); - nack_module_.reset( - new NackModule(clock_, nack_sender, keyframe_request_sender)); - if (config_.rtp.nack.rtp_history_ms == 0) - nack_module_->Stop(); - process_thread_->RegisterModule(nack_module_.get()); + if (config_.rtp.nack.rtp_history_ms != 0) { + nack_module_.reset( + new NackModule(clock_, nack_sender, keyframe_request_sender)); + process_thread_->RegisterModule(nack_module_.get()); + } packet_buffer_ = video_coding::PacketBuffer::Create( clock_, kPacketBufferStartSize, kPacketBufferMaxSixe, this); @@ -198,9 +198,11 @@ RtpStreamReceiver::RtpStreamReceiver( } RtpStreamReceiver::~RtpStreamReceiver() { - process_thread_->DeRegisterModule(rtp_rtcp_.get()); + if (nack_module_) { + process_thread_->DeRegisterModule(nack_module_.get()); + } - process_thread_->DeRegisterModule(nack_module_.get()); + process_thread_->DeRegisterModule(rtp_rtcp_.get()); packet_router_->RemoveRtpModule(rtp_rtcp_.get()); rtp_rtcp_->SetREMBStatus(false); @@ -246,7 +248,8 @@ int32_t RtpStreamReceiver::OnReceivedPayloadData( rtp_header_with_ntp.ntp_time_ms = ntp_estimator_.Estimate(rtp_header->header.timestamp); VCMPacket packet(payload_data, payload_size, rtp_header_with_ntp); - packet.timesNacked = nack_module_->OnReceivedPacket(packet); + packet.timesNacked = + nack_module_ ? nack_module_->OnReceivedPacket(packet) : -1; if (packet.codec == kVideoCodecH264) { // Only when we start to receive packets will we know what payload type @@ -405,7 +408,8 @@ void RtpStreamReceiver::OnCompleteFrame( } void RtpStreamReceiver::OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) { - nack_module_->UpdateRtt(max_rtt_ms); + if (nack_module_) + nack_module_->UpdateRtt(max_rtt_ms); } // TODO(nisse): Drop return value. @@ -534,6 +538,9 @@ bool RtpStreamReceiver::DeliverRtcp(const uint8_t* rtcp_packet, } void RtpStreamReceiver::FrameContinuous(uint16_t picture_id) { + if (!nack_module_) + return; + int seq_num = -1; { rtc::CritScope lock(&last_seq_num_cs_);