generic-library/webrtc
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Make SendCodec() lock-free.

Browse Source 
Fetching the current codec for sake of gathering stats, is frequently blocked since it's done by acquiring the same lock as is held while encoding frames.  This can mean tens of milliseconds.

To improve this, I'm taking advantage of the fact that the codec information is set on the same thread as is used to query the information.  This means that locking isn't needed for querying this information.  I'm adding checks to make sure debug builds will crash if this isn't followed.

An alternative to this approach could be to add one more lock that is specifically used for the codec information variable.  This would also decouple querying codec information from the encoder itself, but still requires a lock.

This patch depends on making ThreadChecker part of rtc_base_approved:
https://webrtc-codereview.appspot.com/40539004/

BUG=2822
R=mflodman@webrtc.org, pthatcher@webrtc.org

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

Cr-Commit-Position: refs/heads/master@{#8435}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8435 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
tommi@webrtc.org 2015-02-19 17:43:25 +00:00
parent be29b3b4c6
commit e07710cc91
8 changed files with 140 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
talk/media/webrtc/webrtcvideocapturer.cc
View File

@ -34,6 +34,8 @@
#ifdef HAVE_WEBRTC_VIDEO
#include "talk/media/webrtc/webrtcvideoframe.h"
#include "talk/media/webrtc/webrtcvideoframefactory.h"
#include "webrtc/base/bind.h"
#include "webrtc/base/checks.h"
#include "webrtc/base/criticalsection.h"
#include "webrtc/base/logging.h"
#include "webrtc/base/safe_conversions.h"
@ -127,14 +129,16 @@ static bool FormatToCapability(const VideoFormat& format,
WebRtcVideoCapturer::WebRtcVideoCapturer()
: factory_(new WebRtcVcmFactory),
module_(NULL),
captured_frames_(0) {
captured_frames_(0),
start_thread_(nullptr) {
set_frame_factory(new WebRtcVideoFrameFactory());
}
WebRtcVideoCapturer::WebRtcVideoCapturer(WebRtcVcmFactoryInterface* factory)
: factory_(factory),
module_(NULL),
captured_frames_(0) {
captured_frames_(0),
start_thread_(nullptr) {
set_frame_factory(new WebRtcVideoFrameFactory());
}
@ -145,6 +149,7 @@ WebRtcVideoCapturer::~WebRtcVideoCapturer() {
}
bool WebRtcVideoCapturer::Init(const Device& device) {
DCHECK(!start_thread_);
if (module_) {
LOG(LS_ERROR) << "The capturer is already initialized";
return false;
@ -221,6 +226,7 @@ bool WebRtcVideoCapturer::Init(const Device& device) {
}
bool WebRtcVideoCapturer::Init(webrtc::VideoCaptureModule* module) {
DCHECK(!start_thread_);
if (module_) {
LOG(LS_ERROR) << "The capturer is already initialized";
return false;
@ -271,12 +277,15 @@ CaptureState WebRtcVideoCapturer::Start(const VideoFormat& capture_format) {
}
rtc::CritScope cs(&critical_section_stopping_);
// TODO(hellner): weird to return failure when it is in fact actually running.
if (IsRunning()) {
LOG(LS_ERROR) << "The capturer is already running";
return CS_FAILED;
}
DCHECK(!start_thread_);
start_thread_ = rtc::Thread::Current();
SetCaptureFormat(&capture_format);
webrtc::VideoCaptureCapability cap;
@ -290,6 +299,7 @@ CaptureState WebRtcVideoCapturer::Start(const VideoFormat& capture_format) {
module_->RegisterCaptureDataCallback(*this);
if (module_->StartCapture(cap) != 0) {
LOG(LS_ERROR) << "Camera '" << camera_id << "' failed to start";
start_thread_ = nullptr;
return CS_FAILED;
}
@ -310,6 +320,7 @@ CaptureState WebRtcVideoCapturer::Start(const VideoFormat& capture_format) {
void WebRtcVideoCapturer::Stop() {
rtc::CritScope cs(&critical_section_stopping_);
if (IsRunning()) {
DCHECK(start_thread_);
rtc::Thread::Current()->Clear(this);
module_->StopCapture();
module_->DeRegisterCaptureDataCallback();
@ -322,6 +333,7 @@ void WebRtcVideoCapturer::Stop() {
<< drop_ratio << "%";
}
SetCaptureFormat(NULL);
start_thread_ = nullptr;
}
bool WebRtcVideoCapturer::IsRunning() {
@ -363,16 +375,18 @@ void WebRtcVideoCapturer::OnIncomingCapturedFrame(const int32_t id,
<< ". Expected format " << GetCaptureFormat()->ToString();
}
// Signal down stream components on captured frame.
// The CapturedFrame class doesn't support planes. We have to ExtractBuffer
// to one block for it.
size_t length =
webrtc::CalcBufferSize(webrtc::kI420, sample.width(), sample.height());
capture_buffer_.resize(length);
// TODO(ronghuawu): Refactor the WebRtcCapturedFrame to avoid memory copy.
webrtc::ExtractBuffer(sample, length, &capture_buffer_[0]);
WebRtcCapturedFrame frame(sample, &capture_buffer_[0], length);
SignalFrameCaptured(this, &frame);
if (start_thread_->IsCurrent()) {
SignalFrameCapturedOnStartThread(&sample);
} else {
// This currently happens on with at least VideoCaptureModuleV4L2 and
// possibly other implementations of WebRTC's VideoCaptureModule.
// In order to maintain the threading contract with the upper layers and
// consistency with other capturers such as in Chrome, we need to do a
// thread hop.
start_thread_->Invoke<void>(
rtc::Bind(&WebRtcVideoCapturer::SignalFrameCapturedOnStartThread,
this, &sample));
}
}
void WebRtcVideoCapturer::OnCaptureDelayChanged(const int32_t id,
@ -380,6 +394,22 @@ void WebRtcVideoCapturer::OnCaptureDelayChanged(const int32_t id,
LOG(LS_INFO) << "Capture delay changed to " << delay << " ms";
}
void WebRtcVideoCapturer::SignalFrameCapturedOnStartThread(
webrtc::I420VideoFrame* frame) {
DCHECK(start_thread_->IsCurrent());
// Signal down stream components on captured frame.
// The CapturedFrame class doesn't support planes. We have to ExtractBuffer
// to one block for it.
size_t length =
webrtc::CalcBufferSize(webrtc::kI420, frame->width(), frame->height());
capture_buffer_.resize(length);
// TODO(magjed): Refactor the WebRtcCapturedFrame to avoid memory copy or
// take over ownership of the buffer held by |frame| if that's possible.
webrtc::ExtractBuffer(*frame, length, &capture_buffer_[0]);
WebRtcCapturedFrame webrtc_frame(*frame, &capture_buffer_[0], length);
SignalFrameCaptured(this, &webrtc_frame);
}
// WebRtcCapturedFrame
WebRtcCapturedFrame::WebRtcCapturedFrame(const webrtc::I420VideoFrame& sample,
void* buffer,

9
talk/media/webrtc/webrtcvideocapturer.h
View File

@ -85,10 +85,19 @@ class WebRtcVideoCapturer : public VideoCapturer,
virtual void OnCaptureDelayChanged(const int32_t id,
const int32_t delay);
// Used to signal captured frames on the same thread as invoked Start().
// With WebRTC's current VideoCapturer implementations, this will mean a
// thread hop, but in other implementations (e.g. Chrome) it will be called
// directly from OnIncomingCapturedFrame.
// TODO(tommi): Remove this workaround when we've updated the WebRTC capturers
// to follow the same contract.
void SignalFrameCapturedOnStartThread(webrtc::I420VideoFrame* frame);
rtc::scoped_ptr<WebRtcVcmFactoryInterface> factory_;
webrtc::VideoCaptureModule* module_;
int captured_frames_;
std::vector<uint8_t> capture_buffer_;
rtc::Thread* start_thread_; // Set in Start(), unset in Stop();
// Critical section to avoid Stop during an OnIncomingCapturedFrame callback.
rtc::CriticalSection critical_section_stopping_;

1
talk/media/webrtc/webrtcvideoengine.cc
View File

@ -3904,6 +3904,7 @@ int WebRtcVideoMediaChannel::GetRecvChannelId(uint32 ssrc) {
bool WebRtcVideoMediaChannel::SetSendParams(
WebRtcVideoChannelSendInfo* send_channel,
const VideoSendParams& send_params) {
ASSERT(engine()->worker_thread()->IsCurrent());
const int channel_id = send_channel->channel_id();
MaybeRegisterExternalEncoder(send_channel, send_params.codec);

33
webrtc/modules/video_coding/main/interface/video_coding.h
View File

@ -11,6 +11,16 @@
#ifndef WEBRTC_MODULES_INTERFACE_VIDEO_CODING_H_
#define WEBRTC_MODULES_INTERFACE_VIDEO_CODING_H_
#if defined(WEBRTC_WIN)
// This is a workaround on Windows due to the fact that some Windows
// headers define CreateEvent as a macro to either CreateEventW or CreateEventA.
// This can cause problems since we use that name as well and could
// declare them as one thing here whereas in another place a windows header
// may have been included and then implementing CreateEvent() causes compilation
// errors. So for consistency, we include the main windows header here.
#include <windows.h>
#endif
#include "webrtc/common_video/interface/i420_video_frame.h"
#include "webrtc/modules/interface/module.h"
#include "webrtc/modules/interface/module_common_types.h"
@ -112,6 +122,8 @@ public:
// encoding-related settings by calling this function.
// For instance, a send codec has to be registered again.
//
// NOTE: Must be called on the thread that constructed the VCM instance.
//
// Return value : VCM_OK, on success.
// < 0, on error.
virtual int32_t InitializeSender() = 0;
@ -119,6 +131,8 @@ public:
// Registers a codec to be used for encoding. Calling this
// API multiple times overwrites any previously registered codecs.
//
// NOTE: Must be called on the thread that constructed the VCM instance.
//
// Input:
// - sendCodec : Settings for the codec to be registered.
// - numberOfCores : The number of cores the codec is allowed
@ -132,6 +146,17 @@ public:
uint32_t numberOfCores,
uint32_t maxPayloadSize) = 0;
// Get the current send codec in use.
//
// If a codec has not been set yet, the |id| property of the return value
// will be 0 and |name| empty.
//
// NOTE: This method intentionally does not hold locks and minimizes data
// copying. It must be called on the thread where the VCM was constructed.
virtual const VideoCodec& GetSendCodec() const = 0;
// DEPRECATED: Use GetSendCodec() instead.
//
// API to get the current send codec in use.
//
// Input:
@ -139,12 +164,20 @@ public:
//
// Return value : VCM_OK, on success.
// < 0, on error.
//
// NOTE: The returned codec information is not guaranteed to be current when
// the call returns. This method acquires a lock that is aligned with
// video encoding, so it should be assumed to be allowed to block for
// several milliseconds.
virtual int32_t SendCodec(VideoCodec* currentSendCodec) const = 0;
// DEPRECATED: Use GetSendCodec() instead.
//
// API to get the current send codec type
//
// Return value : Codec type, on success.
// kVideoCodecUnknown, on error or if no send codec is set
// NOTE: Same notes apply as for SendCodec() above.
virtual VideoCodecType SendCodec() const = 0;
// Register an external encoder object. This can not be used together with

14
webrtc/modules/video_coding/main/source/video_coding_impl.cc
View File

@ -111,12 +111,18 @@ class VideoCodingModuleImpl : public VideoCodingModule {
return sender_->RegisterSendCodec(sendCodec, numberOfCores, maxPayloadSize);
}
virtual int32_t SendCodec(VideoCodec* currentSendCodec) const OVERRIDE {
return sender_->SendCodec(currentSendCodec);
virtual const VideoCodec& GetSendCodec() const OVERRIDE {
return sender_->GetSendCodec();
}
// DEPRECATED.
virtual int32_t SendCodec(VideoCodec* currentSendCodec) const OVERRIDE {
return sender_->SendCodecBlocking(currentSendCodec);
}
// DEPRECATED.
virtual VideoCodecType SendCodec() const OVERRIDE {
return sender_->SendCodec();
return sender_->SendCodecBlocking();
}
virtual int32_t RegisterExternalEncoder(VideoEncoder* externalEncoder,
@ -351,6 +357,8 @@ class VideoCodingModuleImpl : public VideoCodingModule {
private:
EncodedImageCallbackWrapper post_encode_callback_;
// TODO(tommi): Change sender_ and receiver_ to be non pointers
// (construction is 1 alloc instead of 3).
scoped_ptr<vcm::VideoSender> sender_;
scoped_ptr<vcm::VideoReceiver> receiver_;
scoped_ptr<EventFactory> own_event_factory_;

22
webrtc/modules/video_coding/main/source/video_coding_impl.h
View File

@ -16,6 +16,7 @@
#include <vector>
#include "webrtc/base/thread_annotations.h"
#include "webrtc/base/thread_checker.h"
#include "webrtc/modules/video_coding/main/source/codec_database.h"
#include "webrtc/modules/video_coding/main/source/frame_buffer.h"
#include "webrtc/modules/video_coding/main/source/generic_decoder.h"
@ -62,12 +63,25 @@ class VideoSender {
int32_t InitializeSender();
// Register the send codec to be used.
// This method must be called on the construction thread.
int32_t RegisterSendCodec(const VideoCodec* sendCodec,
uint32_t numberOfCores,
uint32_t maxPayloadSize);
// Non-blocking access to the currently active send codec configuration.
// Must be called from the same thread as the VideoSender instance was
// created on.
const VideoCodec& GetSendCodec() const;
// Get a copy of the currently configured send codec.
// This method acquires a lock to copy the current configuration out,
// so it can block and the returned information is not guaranteed to be
// accurate upon return. Consider using GetSendCodec() instead and make
// decisions on that thread with regards to the current codec.
int32_t SendCodecBlocking(VideoCodec* currentSendCodec) const;
// Same as SendCodecBlocking. Try to use GetSendCodec() instead.
VideoCodecType SendCodecBlocking() const;
int32_t SendCodec(VideoCodec* currentSendCodec) const;
VideoCodecType SendCodec() const;
int32_t RegisterExternalEncoder(VideoEncoder* externalEncoder,
uint8_t payloadType,
bool internalSource);
@ -124,6 +138,10 @@ class VideoSender {
bool frame_dropper_enabled_;
VCMProcessTimer _sendStatsTimer;
// Must be accessed on the construction thread of VideoSender.
VideoCodec current_codec_;
rtc::ThreadChecker main_thread_;
VCMQMSettingsCallback* qm_settings_callback_;
VCMProtectionCallback* protection_callback_;
};

50
webrtc/modules/video_coding/main/source/video_sender.cc
View File

@ -12,6 +12,7 @@
#include <algorithm> // std::max
#include "webrtc/base/checks.h"
#include "webrtc/common_video/libyuv/include/webrtc_libyuv.h"
#include "webrtc/modules/video_coding/codecs/interface/video_codec_interface.h"
#include "webrtc/modules/video_coding/main/source/encoded_frame.h"
@ -72,8 +73,10 @@ VideoSender::VideoSender(Clock* clock,
_codecDataBase(),
frame_dropper_enabled_(true),
_sendStatsTimer(1000, clock_),
current_codec_(),
qm_settings_callback_(NULL),
protection_callback_(NULL) {}
protection_callback_(NULL) {
}
VideoSender::~VideoSender() {
delete _sendCritSect;
@ -86,13 +89,8 @@ int32_t VideoSender::Process() {
_sendStatsTimer.Processed();
CriticalSectionScoped cs(process_crit_sect_.get());
if (_sendStatsCallback != NULL) {
uint32_t bitRate;
uint32_t frameRate;
{
CriticalSectionScoped cs(_sendCritSect);
bitRate = _mediaOpt.SentBitRate();
frameRate = _mediaOpt.SentFrameRate();
}
uint32_t bitRate = _mediaOpt.SentBitRate();
uint32_t frameRate = _mediaOpt.SentFrameRate();
_sendStatsCallback->SendStatistics(bitRate, frameRate);
}
}
@ -102,6 +100,7 @@ int32_t VideoSender::Process() {
// Reset send side to initial state - all components
int32_t VideoSender::InitializeSender() {
DCHECK(main_thread_.CalledOnValidThread());
CriticalSectionScoped cs(_sendCritSect);
_codecDataBase.ResetSender();
_encoder = NULL;
@ -118,6 +117,7 @@ int64_t VideoSender::TimeUntilNextProcess() {
int32_t VideoSender::RegisterSendCodec(const VideoCodec* sendCodec,
uint32_t numberOfCores,
uint32_t maxPayloadSize) {
DCHECK(main_thread_.CalledOnValidThread());
CriticalSectionScoped cs(_sendCritSect);
if (sendCodec == NULL) {
return VCM_PARAMETER_ERROR;
@ -129,6 +129,9 @@ int32_t VideoSender::RegisterSendCodec(const VideoCodec* sendCodec,
// Update encoder regardless of result to make sure that we're not holding on
// to a deleted instance.
_encoder = _codecDataBase.GetEncoder();
// Cache the current codec here so they can be fetched from this thread
// without requiring the _sendCritSect lock.
current_codec_ = *sendCodec;
if (!ret) {
LOG(LS_ERROR) << "Failed to initialize the encoder with payload name "
@ -162,20 +165,21 @@ int32_t VideoSender::RegisterSendCodec(const VideoCodec* sendCodec,
return VCM_OK;
}
// Get current send codec
int32_t VideoSender::SendCodec(VideoCodec* currentSendCodec) const {
CriticalSectionScoped cs(_sendCritSect);
const VideoCodec& VideoSender::GetSendCodec() const {
DCHECK(main_thread_.CalledOnValidThread());
return current_codec_;
}
int32_t VideoSender::SendCodecBlocking(VideoCodec* currentSendCodec) const {
CriticalSectionScoped cs(_sendCritSect);
if (currentSendCodec == NULL) {
return VCM_PARAMETER_ERROR;
}
return _codecDataBase.SendCodec(currentSendCodec) ? 0 : -1;
}
// Get the current send codec type
VideoCodecType VideoSender::SendCodec() const {
VideoCodecType VideoSender::SendCodecBlocking() const {
CriticalSectionScoped cs(_sendCritSect);
return _codecDataBase.SendCodec();
}
@ -214,7 +218,6 @@ int32_t VideoSender::CodecConfigParameters(uint8_t* buffer,
// TODO(andresp): Make const once media_opt is thread-safe and this has a
// pointer to it.
int32_t VideoSender::SentFrameCount(VCMFrameCount* frameCount) {
CriticalSectionScoped cs(_sendCritSect);
*frameCount = _mediaOpt.SentFrameCount();
return VCM_OK;
}
@ -400,8 +403,6 @@ int32_t VideoSender::EnableFrameDropper(bool enable) {
}
int VideoSender::SetSenderNackMode(SenderNackMode mode) {
CriticalSectionScoped cs(_sendCritSect);
switch (mode) {
case VideoCodingModule::kNackNone:
_mediaOpt.EnableProtectionMethod(false, media_optimization::kNack);
@ -411,7 +412,6 @@ int VideoSender::SetSenderNackMode(SenderNackMode mode) {
break;
case VideoCodingModule::kNackSelective:
return VCM_NOT_IMPLEMENTED;
break;
}
return VCM_OK;
}
@ -421,7 +421,6 @@ int VideoSender::SetSenderReferenceSelection(bool enable) {
}
int VideoSender::SetSenderFEC(bool enable) {
CriticalSectionScoped cs(_sendCritSect);
_mediaOpt.EnableProtectionMethod(enable, media_optimization::kFec);
return VCM_OK;
}
@ -439,17 +438,12 @@ void VideoSender::StopDebugRecording() {
}
void VideoSender::SuspendBelowMinBitrate() {
CriticalSectionScoped cs(_sendCritSect);
VideoCodec current_send_codec;
if (SendCodec(&current_send_codec) != 0) {
assert(false); // Must set a send codec before SuspendBelowMinBitrate.
return;
}
DCHECK(main_thread_.CalledOnValidThread());
int threshold_bps;
if (current_send_codec.numberOfSimulcastStreams == 0) {
threshold_bps = current_send_codec.minBitrate * 1000;
if (current_codec_.numberOfSimulcastStreams == 0) {
threshold_bps = current_codec_.minBitrate * 1000;
} else {
threshold_bps = current_send_codec.simulcastStream[0].minBitrate * 1000;
threshold_bps = current_codec_.simulcastStream[0].minBitrate * 1000;
}
// Set the hysteresis window to be at 10% of the threshold, but at least
// 10 kbps.

4
webrtc/video_engine/vie_encoder.cc
View File

@ -414,9 +414,7 @@ int32_t ViEEncoder::SetEncoder(const webrtc::VideoCodec& video_codec) {
}
int32_t ViEEncoder::GetEncoder(VideoCodec* video_codec) {
if (vcm_.SendCodec(video_codec) != 0) {
return -1;
}
*video_codec = vcm_.GetSendCodec();
return 0;
}