Reland "Avoid critsect for protection- and qm setting callbacks in

VideoSender."

The original Cl is uploaded as patch set 1, the fix in ps#2 and I'll rebase in ps#3.

BUG=4534
R=pbos@webrtc.org

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

Cr-Commit-Position: refs/heads/master@{#9000}
This commit is contained in:
mflodman
2015-04-14 21:28:08 +02:00
parent 73ba7a690f
commit fcf54bdabb
18 changed files with 383 additions and 458 deletions

View File

@ -245,7 +245,7 @@ uint32_t MediaOptimization::SetTargetRates(
// Update protection settings, when applicable.
float sent_video_rate_kbps = 0.0f;
if (selected_method) {
if (loss_prot_logic_->SelectedType() != kNone) {
// Update protection method with content metrics.
selected_method->UpdateContentMetrics(content_->ShortTermAvgData());

View File

@ -74,11 +74,13 @@ class VideoCodingModuleImpl : public VideoCodingModule {
VideoCodingModuleImpl(Clock* clock,
EventFactory* event_factory,
bool owns_event_factory,
VideoEncoderRateObserver* encoder_rate_observer)
VideoEncoderRateObserver* encoder_rate_observer,
VCMQMSettingsCallback* qm_settings_callback)
: VideoCodingModule(),
sender_(new vcm::VideoSender(clock,
&post_encode_callback_,
encoder_rate_observer)),
encoder_rate_observer,
qm_settings_callback)),
receiver_(new vcm::VideoReceiver(clock, event_factory)),
own_event_factory_(owns_event_factory ? event_factory : NULL) {}
@ -161,11 +163,6 @@ class VideoCodingModuleImpl : public VideoCodingModule {
return sender_->RegisterSendStatisticsCallback(sendStats);
}
int32_t RegisterVideoQMCallback(
VCMQMSettingsCallback* videoQMSettings) override {
return sender_->RegisterVideoQMCallback(videoQMSettings);
}
int32_t RegisterProtectionCallback(
VCMProtectionCallback* protection) override {
return sender_->RegisterProtectionCallback(protection);
@ -359,10 +356,11 @@ int32_t VideoCodingModule::Codec(VideoCodecType codecType, VideoCodec* codec) {
}
VideoCodingModule* VideoCodingModule::Create(
VideoEncoderRateObserver* encoder_rate_observer) {
return new VideoCodingModuleImpl(Clock::GetRealTimeClock(),
new EventFactoryImpl, true,
encoder_rate_observer);
Clock* clock,
VideoEncoderRateObserver* encoder_rate_observer,
VCMQMSettingsCallback* qm_settings_callback) {
return new VideoCodingModuleImpl(clock, new EventFactoryImpl, true,
encoder_rate_observer, qm_settings_callback);
}
VideoCodingModule* VideoCodingModule::Create(
@ -370,7 +368,8 @@ VideoCodingModule* VideoCodingModule::Create(
EventFactory* event_factory) {
assert(clock);
assert(event_factory);
return new VideoCodingModuleImpl(clock, event_factory, false, nullptr);
return new VideoCodingModuleImpl(clock, event_factory, false, nullptr,
nullptr);
}
void VideoCodingModule::Destroy(VideoCodingModule* module) {

View File

@ -58,7 +58,8 @@ class VideoSender {
VideoSender(Clock* clock,
EncodedImageCallback* post_encode_callback,
VideoEncoderRateObserver* encoder_rate_observer);
VideoEncoderRateObserver* encoder_rate_observer,
VCMQMSettingsCallback* qm_settings_callback);
~VideoSender();
@ -99,7 +100,6 @@ class VideoSender {
int32_t RegisterTransportCallback(VCMPacketizationCallback* transport);
int32_t RegisterSendStatisticsCallback(VCMSendStatisticsCallback* sendStats);
int32_t RegisterVideoQMCallback(VCMQMSettingsCallback* videoQMSettings);
int32_t RegisterProtectionCallback(VCMProtectionCallback* protection);
void SetVideoProtection(bool enable, VCMVideoProtection videoProtection);
@ -139,7 +139,7 @@ class VideoSender {
VideoCodec current_codec_;
rtc::ThreadChecker main_thread_;
VCMQMSettingsCallback* qm_settings_callback_;
VCMQMSettingsCallback* const qm_settings_callback_;
VCMProtectionCallback* protection_callback_;
};

View File

@ -26,7 +26,7 @@ namespace vcm {
class DebugRecorder {
public:
DebugRecorder()
: cs_(CriticalSectionWrapper::CreateCriticalSection()), file_(NULL) {}
: cs_(CriticalSectionWrapper::CreateCriticalSection()), file_(nullptr) {}
~DebugRecorder() { Stop(); }
@ -44,7 +44,7 @@ class DebugRecorder {
CriticalSectionScoped cs(cs_.get());
if (file_) {
fclose(file_);
file_ = NULL;
file_ = nullptr;
}
}
@ -61,7 +61,8 @@ class DebugRecorder {
VideoSender::VideoSender(Clock* clock,
EncodedImageCallback* post_encode_callback,
VideoEncoderRateObserver* encoder_rate_observer)
VideoEncoderRateObserver* encoder_rate_observer,
VCMQMSettingsCallback* qm_settings_callback)
: clock_(clock),
recorder_(new DebugRecorder()),
process_crit_sect_(CriticalSectionWrapper::CreateCriticalSection()),
@ -70,16 +71,17 @@ VideoSender::VideoSender(Clock* clock,
_encodedFrameCallback(post_encode_callback),
_nextFrameTypes(1, kVideoFrameDelta),
_mediaOpt(clock_),
_sendStatsCallback(NULL),
_sendStatsCallback(nullptr),
_codecDataBase(encoder_rate_observer),
frame_dropper_enabled_(true),
_sendStatsTimer(1000, clock_),
current_codec_(),
qm_settings_callback_(NULL),
protection_callback_(NULL) {
qm_settings_callback_(qm_settings_callback),
protection_callback_(nullptr) {
// Allow VideoSender to be created on one thread but used on another, post
// construction. This is currently how this class is being used by at least
// one external project (diffractor).
_mediaOpt.EnableQM(qm_settings_callback_ != nullptr);
main_thread_.DetachFromThread();
}
@ -93,7 +95,7 @@ int32_t VideoSender::Process() {
if (_sendStatsTimer.TimeUntilProcess() == 0) {
_sendStatsTimer.Processed();
CriticalSectionScoped cs(process_crit_sect_.get());
if (_sendStatsCallback != NULL) {
if (_sendStatsCallback != nullptr) {
uint32_t bitRate = _mediaOpt.SentBitRate();
uint32_t frameRate = _mediaOpt.SentFrameRate();
_sendStatsCallback->SendStatistics(bitRate, frameRate);
@ -108,8 +110,8 @@ int32_t VideoSender::InitializeSender() {
DCHECK(main_thread_.CalledOnValidThread());
CriticalSectionScoped cs(_sendCritSect);
_codecDataBase.ResetSender();
_encoder = NULL;
_encodedFrameCallback.SetTransportCallback(NULL);
_encoder = nullptr;
_encodedFrameCallback.SetTransportCallback(nullptr);
_mediaOpt.Reset(); // Resetting frame dropper
return VCM_OK;
}
@ -124,7 +126,7 @@ int32_t VideoSender::RegisterSendCodec(const VideoCodec* sendCodec,
uint32_t maxPayloadSize) {
DCHECK(main_thread_.CalledOnValidThread());
CriticalSectionScoped cs(_sendCritSect);
if (sendCodec == NULL) {
if (sendCodec == nullptr) {
return VCM_PARAMETER_ERROR;
}
@ -177,7 +179,7 @@ const VideoCodec& VideoSender::GetSendCodec() const {
int32_t VideoSender::SendCodecBlocking(VideoCodec* currentSendCodec) const {
CriticalSectionScoped cs(_sendCritSect);
if (currentSendCodec == NULL) {
if (currentSendCodec == nullptr) {
return VCM_PARAMETER_ERROR;
}
return _codecDataBase.SendCodec(currentSendCodec) ? 0 : -1;
@ -197,13 +199,13 @@ int32_t VideoSender::RegisterExternalEncoder(VideoEncoder* externalEncoder,
CriticalSectionScoped cs(_sendCritSect);
if (externalEncoder == NULL) {
if (externalEncoder == nullptr) {
bool wasSendCodec = false;
const bool ret =
_codecDataBase.DeregisterExternalEncoder(payloadType, &wasSendCodec);
if (wasSendCodec) {
// Make sure the VCM doesn't use the de-registered codec
_encoder = NULL;
_encoder = nullptr;
}
return ret ? 0 : -1;
}
@ -216,7 +218,7 @@ int32_t VideoSender::RegisterExternalEncoder(VideoEncoder* externalEncoder,
int32_t VideoSender::CodecConfigParameters(uint8_t* buffer,
int32_t size) const {
CriticalSectionScoped cs(_sendCritSect);
if (_encoder != NULL) {
if (_encoder != nullptr) {
return _encoder->CodecConfigParameters(buffer, size);
}
return VCM_UNINITIALIZED;
@ -266,7 +268,6 @@ int32_t VideoSender::SetChannelParameters(uint32_t target_bitrate,
// here? This effectively means that the network thread will be blocked for
// as much as frame encoding period.
CriticalSectionScoped sendCs(_sendCritSect);
uint32_t target_rate = _mediaOpt.SetTargetRates(target_bitrate,
lossRate,
rtt,
@ -274,10 +275,11 @@ int32_t VideoSender::SetChannelParameters(uint32_t target_bitrate,
qm_settings_callback_);
uint32_t input_frame_rate = _mediaOpt.InputFrameRate();
CriticalSectionScoped sendCs(_sendCritSect);
int32_t ret = VCM_UNINITIALIZED;
static_assert(VCM_UNINITIALIZED < 0, "VCM_UNINITIALIZED must be negative.");
if (_encoder != NULL) {
if (_encoder != nullptr) {
ret = _encoder->SetChannelParameters(lossRate, rtt);
if (ret >= 0) {
ret = _encoder->SetRates(target_rate, input_frame_rate);
@ -304,27 +306,13 @@ int32_t VideoSender::RegisterSendStatisticsCallback(
return VCM_OK;
}
// Register a video quality settings callback which will be called when frame
// rate/dimensions need to be updated for video quality optimization
int32_t VideoSender::RegisterVideoQMCallback(
VCMQMSettingsCallback* qm_settings_callback) {
CriticalSectionScoped cs(_sendCritSect);
DCHECK(qm_settings_callback_ == qm_settings_callback ||
!qm_settings_callback_ ||
!qm_settings_callback) << "Overwriting the previous callback?";
qm_settings_callback_ = qm_settings_callback;
_mediaOpt.EnableQM(qm_settings_callback_ != NULL);
return VCM_OK;
}
// Register a video protection callback which will be called to deliver the
// requested FEC rate and NACK status (on/off).
// Note: this callback is assumed to only be registered once and before it is
// used in this class.
int32_t VideoSender::RegisterProtectionCallback(
VCMProtectionCallback* protection_callback) {
CriticalSectionScoped cs(_sendCritSect);
DCHECK(protection_callback_ == protection_callback ||
!protection_callback_ ||
!protection_callback) << "Overwriting the previous callback?";
DCHECK(protection_callback == nullptr || protection_callback_ == nullptr);
protection_callback_ = protection_callback;
return VCM_OK;
}
@ -359,7 +347,7 @@ int32_t VideoSender::AddVideoFrame(const I420VideoFrame& videoFrame,
const VideoContentMetrics* contentMetrics,
const CodecSpecificInfo* codecSpecificInfo) {
CriticalSectionScoped cs(_sendCritSect);
if (_encoder == NULL) {
if (_encoder == nullptr) {
return VCM_UNINITIALIZED;
}
// TODO(holmer): Add support for dropping frames per stream. Currently we
@ -398,7 +386,7 @@ int32_t VideoSender::IntraFrameRequest(int stream_index) {
return -1;
}
_nextFrameTypes[stream_index] = kVideoFrameKey;
if (_encoder != NULL && _encoder->InternalSource()) {
if (_encoder != nullptr && _encoder->InternalSource()) {
// Try to request the frame if we have an external encoder with
// internal source since AddVideoFrame never will be called.
if (_encoder->RequestFrame(_nextFrameTypes) == WEBRTC_VIDEO_CODEC_OK) {

View File

@ -177,7 +177,8 @@ class TestVideoSender : public ::testing::Test {
TestVideoSender() : clock_(1000), packetization_callback_(&clock_) {}
void SetUp() override {
sender_.reset(new VideoSender(&clock_, &post_encode_callback_, nullptr));
sender_.reset(
new VideoSender(&clock_, &post_encode_callback_, nullptr, nullptr));
EXPECT_EQ(0, sender_->InitializeSender());
EXPECT_EQ(0, sender_->RegisterTransportCallback(&packetization_callback_));
}