Keep lock after updating encoder parameters.
Additionally adds some thread annotations.
BUG=
R=stefan@webrtc.org
Review URL: https://codereview.webrtc.org/1336023004 .
Cr-Commit-Position: refs/heads/master@{#9941}
diff --git a/webrtc/modules/video_coding/main/source/video_coding_impl.h b/webrtc/modules/video_coding/main/source/video_coding_impl.h
index fd3599f..86a8ca0 100644
--- a/webrtc/modules/video_coding/main/source/video_coding_impl.h
+++ b/webrtc/modules/video_coding/main/source/video_coding_impl.h
@@ -93,7 +93,6 @@
int32_t SetChannelParameters(uint32_t target_bitrate, // bits/s.
uint8_t lossRate,
int64_t rtt);
- int32_t UpdateEncoderParameters();
int32_t RegisterTransportCallback(VCMPacketizationCallback* transport);
int32_t RegisterSendStatisticsCallback(VCMSendStatisticsCallback* sendStats);
@@ -114,17 +113,28 @@
int32_t Process();
private:
- Clock* clock_;
+ struct EncoderParameters {
+ uint32_t target_bitrate;
+ uint8_t loss_rate;
+ int64_t rtt;
+ uint32_t input_frame_rate;
+ bool updated;
+ };
+
+ void SetEncoderParameters(EncoderParameters params)
+ EXCLUSIVE_LOCKS_REQUIRED(send_crit_);
+
+ Clock* const clock_;
rtc::scoped_ptr<CriticalSectionWrapper> process_crit_sect_;
- CriticalSectionWrapper* _sendCritSect;
+ mutable rtc::CriticalSection send_crit_;
VCMGenericEncoder* _encoder;
VCMEncodedFrameCallback _encodedFrameCallback;
std::vector<FrameType> _nextFrameTypes;
media_optimization::MediaOptimization _mediaOpt;
- VCMSendStatisticsCallback* _sendStatsCallback;
- VCMCodecDataBase _codecDataBase;
- bool frame_dropper_enabled_;
+ VCMSendStatisticsCallback* _sendStatsCallback GUARDED_BY(process_crit_sect_);
+ VCMCodecDataBase _codecDataBase GUARDED_BY(send_crit_);
+ bool frame_dropper_enabled_ GUARDED_BY(send_crit_);
VCMProcessTimer _sendStatsTimer;
// Must be accessed on the construction thread of VideoSender.
@@ -135,13 +145,7 @@
VCMProtectionCallback* protection_callback_;
rtc::CriticalSection params_lock_;
- struct EncoderParameters {
- uint32_t target_bitrate;
- uint8_t loss_rate;
- int64_t rtt;
- uint32_t input_frame_rate;
- bool updated;
- } encoder_params_ GUARDED_BY(params_lock_);
+ EncoderParameters encoder_params_ GUARDED_BY(params_lock_);
};
class VideoReceiver {
diff --git a/webrtc/modules/video_coding/main/source/video_sender.cc b/webrtc/modules/video_coding/main/source/video_sender.cc
index 8694f5c..fd5cb1e 100644
--- a/webrtc/modules/video_coding/main/source/video_sender.cc
+++ b/webrtc/modules/video_coding/main/source/video_sender.cc
@@ -30,7 +30,6 @@
VCMQMSettingsCallback* qm_settings_callback)
: clock_(clock),
process_crit_sect_(CriticalSectionWrapper::CreateCriticalSection()),
- _sendCritSect(CriticalSectionWrapper::CreateCriticalSection()),
_encoder(nullptr),
_encodedFrameCallback(post_encode_callback),
_nextFrameTypes(1, kVideoFrameDelta),
@@ -51,9 +50,7 @@
main_thread_.DetachFromThread();
}
-VideoSender::~VideoSender() {
- delete _sendCritSect;
-}
+VideoSender::~VideoSender() {}
int32_t VideoSender::Process() {
int32_t returnValue = VCM_OK;
@@ -88,7 +85,7 @@
uint32_t numberOfCores,
uint32_t maxPayloadSize) {
DCHECK(main_thread_.CalledOnValidThread());
- CriticalSectionScoped cs(_sendCritSect);
+ rtc::CritScope lock(&send_crit_);
if (sendCodec == nullptr) {
return VCM_PARAMETER_ERROR;
}
@@ -141,7 +138,7 @@
}
int32_t VideoSender::SendCodecBlocking(VideoCodec* currentSendCodec) const {
- CriticalSectionScoped cs(_sendCritSect);
+ rtc::CritScope lock(&send_crit_);
if (currentSendCodec == nullptr) {
return VCM_PARAMETER_ERROR;
}
@@ -149,7 +146,7 @@
}
VideoCodecType VideoSender::SendCodecBlocking() const {
- CriticalSectionScoped cs(_sendCritSect);
+ rtc::CritScope lock(&send_crit_);
return _codecDataBase.SendCodec();
}
@@ -160,7 +157,7 @@
bool internalSource /*= false*/) {
DCHECK(main_thread_.CalledOnValidThread());
- CriticalSectionScoped cs(_sendCritSect);
+ rtc::CritScope lock(&send_crit_);
if (externalEncoder == nullptr) {
bool wasSendCodec = false;
@@ -180,7 +177,7 @@
// Get codec config parameters
int32_t VideoSender::CodecConfigParameters(uint8_t* buffer,
int32_t size) const {
- CriticalSectionScoped cs(_sendCritSect);
+ rtc::CritScope lock(&send_crit_);
if (_encoder != nullptr) {
return _encoder->CodecConfigParameters(buffer, size);
}
@@ -238,37 +235,24 @@
return VCM_OK;
}
-int32_t VideoSender::UpdateEncoderParameters() {
- EncoderParameters params;
- {
- rtc::CritScope cs(¶ms_lock_);
- params = encoder_params_;
- encoder_params_.updated = false;
- }
-
+void VideoSender::SetEncoderParameters(EncoderParameters params) {
if (!params.updated || params.target_bitrate == 0)
- return VCM_OK;
-
- CriticalSectionScoped sendCs(_sendCritSect);
- int32_t ret = VCM_UNINITIALIZED;
- static_assert(VCM_UNINITIALIZED < 0, "VCM_UNINITIALIZED must be negative.");
+ return;
if (params.input_frame_rate == 0) {
// No frame rate estimate available, use default.
params.input_frame_rate = current_codec_.maxFramerate;
}
if (_encoder != nullptr) {
- ret = _encoder->SetChannelParameters(params.loss_rate, params.rtt);
- if (ret >= 0) {
- ret = _encoder->SetRates(params.target_bitrate, params.input_frame_rate);
- }
+ _encoder->SetChannelParameters(params.loss_rate, params.rtt);
+ _encoder->SetRates(params.target_bitrate, params.input_frame_rate);
}
- return ret;
+ return;
}
int32_t VideoSender::RegisterTransportCallback(
VCMPacketizationCallback* transport) {
- CriticalSectionScoped cs(_sendCritSect);
+ rtc::CritScope lock(&send_crit_);
_encodedFrameCallback.SetMediaOpt(&_mediaOpt);
_encodedFrameCallback.SetTransportCallback(transport);
return VCM_OK;
@@ -297,7 +281,7 @@
// Enable or disable a video protection method.
void VideoSender::SetVideoProtection(VCMVideoProtection videoProtection) {
- CriticalSectionScoped cs(_sendCritSect);
+ rtc::CritScope lock(&send_crit_);
switch (videoProtection) {
case kProtectionNone:
_mediaOpt.SetProtectionMethod(media_optimization::kNone);
@@ -317,8 +301,14 @@
int32_t VideoSender::AddVideoFrame(const VideoFrame& videoFrame,
const VideoContentMetrics* contentMetrics,
const CodecSpecificInfo* codecSpecificInfo) {
- UpdateEncoderParameters();
- CriticalSectionScoped cs(_sendCritSect);
+ EncoderParameters encoder_params;
+ {
+ rtc::CritScope lock(¶ms_lock_);
+ encoder_params = encoder_params_;
+ encoder_params_.updated = false;
+ }
+ rtc::CritScope lock(&send_crit_);
+ SetEncoderParameters(encoder_params);
if (_encoder == nullptr) {
return VCM_UNINITIALIZED;
}
@@ -362,7 +352,7 @@
}
int32_t VideoSender::IntraFrameRequest(int stream_index) {
- CriticalSectionScoped cs(_sendCritSect);
+ rtc::CritScope lock(&send_crit_);
if (stream_index < 0 ||
static_cast<unsigned int>(stream_index) >= _nextFrameTypes.size()) {
return -1;
@@ -379,7 +369,7 @@
}
int32_t VideoSender::EnableFrameDropper(bool enable) {
- CriticalSectionScoped cs(_sendCritSect);
+ rtc::CritScope lock(&send_crit_);
frame_dropper_enabled_ = enable;
_mediaOpt.EnableFrameDropper(enable);
return VCM_OK;
@@ -400,7 +390,7 @@
}
bool VideoSender::VideoSuspended() const {
- CriticalSectionScoped cs(_sendCritSect);
+ rtc::CritScope lock(&send_crit_);
return _mediaOpt.IsVideoSuspended();
}
} // namespace vcm
