Fix deadlock on register/unregister observer while there is a an going callback.

BUG=2835
R=mallinath@webrtc.org

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

git-svn-id: http://webrtc.googlecode.com/svn/trunk@5421 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
andresp@webrtc.org 2014-01-23 23:09:25 +00:00
parent a8910d2f88
commit 8d375c95b7
8 changed files with 61 additions and 84 deletions

View File

@ -59,21 +59,16 @@ class FakeWebRtcVideoCaptureModule : public webrtc::VideoCaptureModule {
id_ = id;
return 0;
}
virtual int32_t RegisterCaptureDataCallback(
virtual void RegisterCaptureDataCallback(
webrtc::VideoCaptureDataCallback& callback) {
callback_ = &callback;
return 0;
}
virtual int32_t DeRegisterCaptureDataCallback() {
callback_ = NULL;
return 0;
virtual void DeRegisterCaptureDataCallback() { callback_ = NULL; }
virtual void RegisterCaptureCallback(webrtc::VideoCaptureFeedBack& callback) {
// Not implemented.
}
virtual int32_t RegisterCaptureCallback(
webrtc::VideoCaptureFeedBack& callback) {
return -1; // not implemented
}
virtual int32_t DeRegisterCaptureCallback() {
return 0;
virtual void DeRegisterCaptureCallback() {
// Not implemented.
}
virtual int32_t StartCapture(
const webrtc::VideoCaptureCapability& cap) {
@ -98,13 +93,8 @@ class FakeWebRtcVideoCaptureModule : public webrtc::VideoCaptureModule {
settings = cap_;
return 0;
}
virtual int32_t SetCaptureDelay(int32_t delay) {
delay_ = delay;
return 0;
}
virtual int32_t CaptureDelay() {
return delay_;
}
virtual void SetCaptureDelay(int32_t delay) { delay_ = delay; }
virtual int32_t CaptureDelay() { return delay_; }
virtual int32_t SetCaptureRotation(
webrtc::VideoCaptureRotation rotation) {
return -1; // not implemented
@ -113,11 +103,11 @@ class FakeWebRtcVideoCaptureModule : public webrtc::VideoCaptureModule {
const webrtc::VideoCodec& codec) {
return NULL; // not implemented
}
virtual int32_t EnableFrameRateCallback(const bool enable) {
return -1; // not implemented
virtual void EnableFrameRateCallback(const bool enable) {
// not implemented
}
virtual int32_t EnableNoPictureAlarm(const bool enable) {
return -1; // not implemented
virtual void EnableNoPictureAlarm(const bool enable) {
// not implemented
}
virtual int32_t AddRef() {
return 0;

View File

@ -264,8 +264,8 @@ CaptureState WebRtcVideoCapturer::Start(const VideoFormat& capture_format) {
std::string camera_id(GetId());
uint32 start = talk_base::Time();
if (module_->RegisterCaptureDataCallback(*this) != 0 ||
module_->StartCapture(cap) != 0) {
module_->RegisterCaptureDataCallback(*this);
if (module_->StartCapture(cap) != 0) {
LOG(LS_ERROR) << "Camera '" << camera_id << "' failed to start";
return CS_FAILED;
}

View File

@ -105,18 +105,17 @@ class VideoCaptureModule: public RefCountedModule {
};
// Register capture data callback
virtual int32_t RegisterCaptureDataCallback(
virtual void RegisterCaptureDataCallback(
VideoCaptureDataCallback& dataCallback) = 0;
// Remove capture data callback
virtual int32_t DeRegisterCaptureDataCallback() = 0;
virtual void DeRegisterCaptureDataCallback() = 0;
// Register capture callback.
virtual int32_t RegisterCaptureCallback(
VideoCaptureFeedBack& callBack) = 0;
virtual void RegisterCaptureCallback(VideoCaptureFeedBack& callBack) = 0;
// Remove capture callback.
virtual int32_t DeRegisterCaptureCallback() = 0;
virtual void DeRegisterCaptureCallback() = 0;
// Start capture device
virtual int32_t StartCapture(
@ -133,7 +132,7 @@ class VideoCaptureModule: public RefCountedModule {
// Gets the current configuration.
virtual int32_t CaptureSettings(VideoCaptureCapability& settings) = 0;
virtual int32_t SetCaptureDelay(int32_t delayMS) = 0;
virtual void SetCaptureDelay(int32_t delayMS) = 0;
// Returns the current CaptureDelay. Only valid when the camera is running.
virtual int32_t CaptureDelay() = 0;
@ -149,8 +148,8 @@ class VideoCaptureModule: public RefCountedModule {
virtual VideoCaptureEncodeInterface* GetEncodeInterface(
const VideoCodec& codec) = 0;
virtual int32_t EnableFrameRateCallback(const bool enable) = 0;
virtual int32_t EnableNoPictureAlarm(const bool enable) = 0;
virtual void EnableFrameRateCallback(const bool enable) = 0;
virtual void EnableNoPictureAlarm(const bool enable) = 0;
protected:
virtual ~VideoCaptureModule() {};

View File

@ -252,7 +252,7 @@ class VideoCaptureTest : public testing::Test {
EXPECT_FALSE(module->CaptureStarted());
EXPECT_EQ(0, module->RegisterCaptureDataCallback(*callback));
module->RegisterCaptureDataCallback(*callback);
return module;
}
@ -408,11 +408,10 @@ class VideoCaptureExternalTest : public testing::Test {
memset(test_frame_.buffer(webrtc::kVPlane), 127,
((kTestWidth + 1) / 2) * ((kTestHeight + 1) / 2));
EXPECT_EQ(0, capture_module_->RegisterCaptureDataCallback(
capture_callback_));
EXPECT_EQ(0, capture_module_->RegisterCaptureCallback(capture_feedback_));
EXPECT_EQ(0, capture_module_->EnableFrameRateCallback(true));
EXPECT_EQ(0, capture_module_->EnableNoPictureAlarm(true));
capture_module_->RegisterCaptureDataCallback(capture_callback_);
capture_module_->RegisterCaptureCallback(capture_feedback_);
capture_module_->EnableFrameRateCallback(true);
capture_module_->EnableNoPictureAlarm(true);
}
void TearDown() {

View File

@ -187,45 +187,33 @@ VideoCaptureImpl::~VideoCaptureImpl()
delete[] _deviceUniqueId;
}
int32_t VideoCaptureImpl::RegisterCaptureDataCallback(
VideoCaptureDataCallback& dataCallBack)
{
void VideoCaptureImpl::RegisterCaptureDataCallback(
VideoCaptureDataCallback& dataCallBack) {
CriticalSectionScoped cs(&_apiCs);
CriticalSectionScoped cs2(&_callBackCs);
_dataCallBack = &dataCallBack;
return 0;
}
int32_t VideoCaptureImpl::DeRegisterCaptureDataCallback()
{
void VideoCaptureImpl::DeRegisterCaptureDataCallback() {
CriticalSectionScoped cs(&_apiCs);
CriticalSectionScoped cs2(&_callBackCs);
_dataCallBack = NULL;
return 0;
}
int32_t VideoCaptureImpl::RegisterCaptureCallback(VideoCaptureFeedBack& callBack)
{
void VideoCaptureImpl::RegisterCaptureCallback(VideoCaptureFeedBack& callBack) {
CriticalSectionScoped cs(&_apiCs);
CriticalSectionScoped cs2(&_callBackCs);
_captureCallBack = &callBack;
return 0;
}
int32_t VideoCaptureImpl::DeRegisterCaptureCallback()
{
void VideoCaptureImpl::DeRegisterCaptureCallback() {
CriticalSectionScoped cs(&_apiCs);
CriticalSectionScoped cs2(&_callBackCs);
_captureCallBack = NULL;
return 0;
}
int32_t VideoCaptureImpl::SetCaptureDelay(int32_t delayMS)
{
void VideoCaptureImpl::SetCaptureDelay(int32_t delayMS) {
CriticalSectionScoped cs(&_apiCs);
_captureDelay = delayMS;
return 0;
}
int32_t VideoCaptureImpl::CaptureDelay()
{
@ -389,8 +377,7 @@ int32_t VideoCaptureImpl::SetCaptureRotation(VideoCaptureRotation rotation) {
return 0;
}
int32_t VideoCaptureImpl::EnableFrameRateCallback(const bool enable)
{
void VideoCaptureImpl::EnableFrameRateCallback(const bool enable) {
CriticalSectionScoped cs(&_apiCs);
CriticalSectionScoped cs2(&_callBackCs);
_frameRateCallBack = enable;
@ -398,15 +385,12 @@ int32_t VideoCaptureImpl::EnableFrameRateCallback(const bool enable)
{
_lastFrameRateCallbackTime = TickTime::Now();
}
return 0;
}
int32_t VideoCaptureImpl::EnableNoPictureAlarm(const bool enable)
{
void VideoCaptureImpl::EnableNoPictureAlarm(const bool enable) {
CriticalSectionScoped cs(&_apiCs);
CriticalSectionScoped cs2(&_callBackCs);
_noPictureAlarmCallBack = enable;
return 0;
}
void VideoCaptureImpl::UpdateFrameCount()

View File

@ -62,17 +62,18 @@ public:
virtual int32_t ChangeUniqueId(const int32_t id);
//Call backs
virtual int32_t RegisterCaptureDataCallback(VideoCaptureDataCallback& dataCallback);
virtual int32_t DeRegisterCaptureDataCallback();
virtual int32_t RegisterCaptureCallback(VideoCaptureFeedBack& callBack);
virtual int32_t DeRegisterCaptureCallback();
virtual void RegisterCaptureDataCallback(
VideoCaptureDataCallback& dataCallback);
virtual void DeRegisterCaptureDataCallback();
virtual void RegisterCaptureCallback(VideoCaptureFeedBack& callBack);
virtual void DeRegisterCaptureCallback();
virtual int32_t SetCaptureDelay(int32_t delayMS);
virtual void SetCaptureDelay(int32_t delayMS);
virtual int32_t CaptureDelay();
virtual int32_t SetCaptureRotation(VideoCaptureRotation rotation);
virtual int32_t EnableFrameRateCallback(const bool enable);
virtual int32_t EnableNoPictureAlarm(const bool enable);
virtual void EnableFrameRateCallback(const bool enable);
virtual void EnableNoPictureAlarm(const bool enable);
virtual const char* CurrentDeviceName() const;

View File

@ -279,7 +279,8 @@ void ViECapturer::CpuOveruseMeasures(int* capture_jitter_ms,
}
int32_t ViECapturer::SetCaptureDelay(int32_t delay_ms) {
return capture_module_->SetCaptureDelay(delay_ms);
capture_module_->SetCaptureDelay(delay_ms);
return 0;
}
int32_t ViECapturer::SetRotateCapturedFrames(
@ -638,29 +639,31 @@ bool ViECapturer::CaptureCapabilityFixed() {
}
int32_t ViECapturer::RegisterObserver(ViECaptureObserver* observer) {
CriticalSectionScoped cs(observer_cs_.get());
if (observer_) {
WEBRTC_TRACE(kTraceError, kTraceVideo, ViEId(engine_id_, capture_id_),
"%s Observer already registered", __FUNCTION__, capture_id_);
return -1;
}
if (capture_module_->RegisterCaptureCallback(*this) != 0) {
return -1;
{
CriticalSectionScoped cs(observer_cs_.get());
if (observer_) {
WEBRTC_TRACE(kTraceError,
kTraceVideo,
ViEId(engine_id_, capture_id_),
"%s Observer already registered",
__FUNCTION__,
capture_id_);
return -1;
}
observer_ = observer;
}
capture_module_->RegisterCaptureCallback(*this);
capture_module_->EnableFrameRateCallback(true);
capture_module_->EnableNoPictureAlarm(true);
observer_ = observer;
return 0;
}
int32_t ViECapturer::DeRegisterObserver() {
CriticalSectionScoped cs(observer_cs_.get());
if (!observer_) {
return 0;
}
capture_module_->EnableFrameRateCallback(false);
capture_module_->EnableNoPictureAlarm(false);
capture_module_->DeRegisterCaptureCallback();
CriticalSectionScoped cs(observer_cs_.get());
observer_ = NULL;
return 0;
}

View File

@ -20,6 +20,7 @@
#include "webrtc/modules/video_coding/main/interface/video_coding.h"
#include "webrtc/modules/video_processing/main/interface/video_processing.h"
#include "webrtc/system_wrappers/interface/scoped_ptr.h"
#include "webrtc/system_wrappers/interface/thread_annotations.h"
#include "webrtc/typedefs.h"
#include "webrtc/video_engine/include/vie_capture.h"
#include "webrtc/video_engine/vie_defines.h"
@ -185,7 +186,7 @@ class ViECapturer
// Statistics observer.
scoped_ptr<CriticalSectionWrapper> observer_cs_;
ViECaptureObserver* observer_;
ViECaptureObserver* observer_ GUARDED_BY(observer_cs_.get());
CaptureCapability requested_capability_;