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

Fix race in Voice Engine's Channel where it accesses RemoteNtpTimeEstimator from both the audio playback thread and the network thread without locking.

Browse Source 
BUG=3681
R=pbos@webrtc.org, xians@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/24379004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@7030 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
stefan@webrtc.org 2014-09-02 18:58:24 +00:00
parent 9f341283f6
commit 8e24d87778
2 changed files with 20 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
webrtc/voice_engine/channel.cc
View File

@ -16,7 +16,6 @@
#include "webrtc/modules/audio_processing/include/audio_processing.h"
#include "webrtc/modules/interface/module_common_types.h"
#include "webrtc/modules/rtp_rtcp/interface/receive_statistics.h"
#include "webrtc/modules/rtp_rtcp/interface/remote_ntp_time_estimator.h"
#include "webrtc/modules/rtp_rtcp/interface/rtp_payload_registry.h"
#include "webrtc/modules/rtp_rtcp/interface/rtp_receiver.h"
#include "webrtc/modules/rtp_rtcp/source/rtp_receiver_strategy.h"
@ -699,15 +698,18 @@ int32_t Channel::GetAudioFrame(int32_t id, AudioFrame& audioFrame)
(unwrap_timestamp - capture_start_rtp_time_stamp_) /
(GetPlayoutFrequency() / 1000);
// Compute ntp time.
audioFrame.ntp_time_ms_ = ntp_estimator_->Estimate(audioFrame.timestamp_);
// |ntp_time_ms_| won't be valid until at least 2 RTCP SRs are received.
if (audioFrame.ntp_time_ms_ > 0) {
// Compute |capture_start_ntp_time_ms_| so that
// |capture_start_ntp_time_ms_| + |elapsed_time_ms_| == |ntp_time_ms_|
{
CriticalSectionScoped lock(ts_stats_lock_.get());
capture_start_ntp_time_ms_ =
audioFrame.ntp_time_ms_ - audioFrame.elapsed_time_ms_;
// Compute ntp time.
audioFrame.ntp_time_ms_ = ntp_estimator_.Estimate(
audioFrame.timestamp_);
// |ntp_time_ms_| won't be valid until at least 2 RTCP SRs are received.
if (audioFrame.ntp_time_ms_ > 0) {
// Compute |capture_start_ntp_time_ms_| so that
// |capture_start_ntp_time_ms_| + |elapsed_time_ms_| == |ntp_time_ms_|
capture_start_ntp_time_ms_ =
audioFrame.ntp_time_ms_ - audioFrame.elapsed_time_ms_;
}
}
}
@ -877,7 +879,7 @@ Channel::Channel(int32_t channelId,
_outputExternalMediaCallbackPtr(NULL),
_timeStamp(0), // This is just an offset, RTP module will add it's own random offset
_sendTelephoneEventPayloadType(106),
ntp_estimator_(new RemoteNtpTimeEstimator(Clock::GetRealTimeClock())),
ntp_estimator_(Clock::GetRealTimeClock()),
jitter_buffer_playout_timestamp_(0),
playout_timestamp_rtp_(0),
playout_timestamp_rtcp_(0),
@ -1949,8 +1951,11 @@ int32_t Channel::ReceivedRTCPPacket(const int8_t* data, int32_t length) {
"Channel::IncomingRTPPacket() RTCP packet is invalid");
}
ntp_estimator_->UpdateRtcpTimestamp(rtp_receiver_->SSRC(),
_rtpRtcpModule.get());
{
CriticalSectionScoped lock(ts_stats_lock_.get());
ntp_estimator_.UpdateRtcpTimestamp(rtp_receiver_->SSRC(),
_rtpRtcpModule.get());
}
return 0;
}

5
webrtc/voice_engine/channel.h
View File

@ -17,6 +17,7 @@
#include "webrtc/modules/audio_conference_mixer/interface/audio_conference_mixer_defines.h"
#include "webrtc/modules/audio_processing/rms_level.h"
#include "webrtc/modules/bitrate_controller/include/bitrate_controller.h"
#include "webrtc/modules/rtp_rtcp/interface/remote_ntp_time_estimator.h"
#include "webrtc/modules/rtp_rtcp/interface/rtp_header_parser.h"
#include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h"
#include "webrtc/modules/utility/interface/file_player.h"
@ -548,7 +549,7 @@ private:
uint32_t _timeStamp;
uint8_t _sendTelephoneEventPayloadType;
scoped_ptr<RemoteNtpTimeEstimator> ntp_estimator_;
RemoteNtpTimeEstimator ntp_estimator_ GUARDED_BY(ts_stats_lock_);
// Timestamp of the audio pulled from NetEq.
uint32_t jitter_buffer_playout_timestamp_;
@ -566,7 +567,7 @@ private:
int64_t capture_start_rtp_time_stamp_;
// The capture ntp time (in local timebase) of the first played out audio
// frame.
int64_t capture_start_ntp_time_ms_;
int64_t capture_start_ntp_time_ms_ GUARDED_BY(ts_stats_lock_);
// uses
Statistics* _engineStatisticsPtr;