admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove |running_| state from NackModule to avoid running a tight loop.

Browse Source 
Also reducing locking in NackModule (and by extension RtpStreamReceiver)

BUG=webrtc:7246

Review-Url: https://codereview.webrtc.org/2720603002
Cr-Commit-Position: refs/heads/master@{#16856}
This commit is contained in:
tommi 2017-02-27 01:59:36 -08:00 committed by Commit bot
parent 92eaec6104
commit f284b7ff5f
3 changed files with 32 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
webrtc/modules/video_coding/nack_module.cc
View File

@ -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<int64_t>(next_process_time_ms_ - clock_->TimeInMilliseconds(),
0);
}
void NackModule::Process() {
rtc::CritScope lock(&crit_);
if (!running_)
return;
if (nack_sender_) {
std::vector<uint16_t> 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<uint16_t> nack_batch = GetNackBatch(kTimeOnly);
if (!nack_batch.empty() && nack_sender_ != nullptr)
nack_sender_->SendNack(nack_batch);
}
bool NackModule::RemovePacketsUntilKeyFrame() {

9
webrtc/modules/video_coding/nack_module.h
View File

@ -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<uint16_t, NackInfo, DescendingSeqNumComp<uint16_t>> nack_list_
GUARDED_BY(crit_);
std::set<uint16_t, DescendingSeqNumComp<uint16_t>> 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

25
webrtc/video/rtp_stream_receiver.cc
View File

@ -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_);