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

Reland 8203 "Reducing locking in OveruseFrameDetect..."

Browse Source 
The issue that was causing the thread checker to report error, turned out to be unrelated.

> Revert 8203 "Reducing locking in OveruseFrameDetector and increa..."
>
> Broke tests in Chrome for some reason:
>
> [ RUN      ] WebRtcAecDumpBrowserTest.CallWithAecDump
> [80131:1287:0129/074432:30561723987517:ERROR:vt_video_decode_accelerator.cc(132)] Failed to create VTDecompressionSession: codecOpenErr (-8973)
> [80129:1287:0129/074432:30562276677373:INFO:CONSOLE(64)] "Looking at video in element remote-view-1", source: http://127.0.0.1:61401/media/webrtc_test_utilities.js (64)
> [80129:1287:0129/074432:30562281435788:INFO:CONSOLE(64)] "Looking at video in element remote-view-2", source: http://127.0.0.1:61401/media/webrtc_test_utilities.js (64)
> [80129:1287:0129/074432:30562315329399:INFO:CONSOLE(800)] "Negotiating call...", source: http://127.0.0.1:61401/media/peerconnection-call.html (800)
> [80133:29187:0129/074432:30562402039578:FATAL:overuse_frame_detector.cc(388)] Check failed: processing_thread_.CalledOnValidThread().
> 0   libbase.dylib                       0x000000010dfd688f base::debug::StackTrace::StackTrace() + 47
> 1   libbase.dylib                       0x000000010dfd68e3 base::debug::StackTrace::StackTrace() + 35
> 2   libbase.dylib                       0x000000010e030076 logging::LogMessage::~LogMessage() + 70
> 3   libbase.dylib                       0x000000010e02f0c3 logging::LogMessage::~LogMessage() + 35
> 4   libcontent.dylib                    0x000000011d8c0cd5 webrtc::OveruseFrameDetector::TimeUntilNextProcess() + 245
> 5   libcontent.dylib                    0x000000011d31ddfd webrtc::ProcessThreadImpl::Process() + 525
> 6   libcontent.dylib                    0x000000011d31d836 webrtc::ProcessThreadImpl::Run(void*) + 38
> 7   libcontent.dylib                    0x000000011d10c390 webrtc::ThreadPosix::Run() + 288
> 8   libcontent.dylib                    0x000000011d10c076 webrtc::StartThread(void*) + 38
> 9   libsystem_pthread.dylib             0x00007fff8e667899 _pthread_body + 138
> 10  libsystem_pthread.dylib             0x00007fff8e66772a _pthread_struct_init + 0
> 11  libsystem_pthread.dylib             0x00007fff8e66bfc9 thread_start + 13
>
>
> > Reducing locking in OveruseFrameDetector and increasing constness.
> >
> > I also added a few TODOs there to see what we can do to reduce the chance of contention.
> > To catch regressions, I've started using the ThreadChecker class on the processing thread but it might also be a good idea to add similar checks for other known threads such as the thread we receive frames on.  I'm sure we can reduce locking even further.
> >
> > BUG=2822
> > R=asapersson@webrtc.org
> >
> > Review URL: https://webrtc-codereview.appspot.com/33129004
>
> TBR=tommi@webrtc.org
>
> Review URL: https://webrtc-codereview.appspot.com/34079004

TBR=tommi@webrtc.org
BUG=

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

Cr-Commit-Position: refs/heads/master@{#8287}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8287 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
tommi@webrtc.org 2015-02-08 18:27:46 +00:00
parent 103f3289b5
commit 7a57f8f101
2 changed files with 40 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
webrtc/video_engine/overuse_frame_detector.cc
View File

@ -17,9 +17,9 @@
#include <list> #include <list>
#include <map> #include <map>
#include "webrtc/base/checks.h"
#include "webrtc/base/exp_filter.h" #include "webrtc/base/exp_filter.h"
#include "webrtc/system_wrappers/interface/clock.h" #include "webrtc/system_wrappers/interface/clock.h"
#include "webrtc/system_wrappers/interface/critical_section_wrapper.h"
#include "webrtc/system_wrappers/interface/logging.h" #include "webrtc/system_wrappers/interface/logging.h"
namespace webrtc { namespace webrtc {
@ -318,8 +318,7 @@ class OveruseFrameDetector::CaptureQueueDelay {
}; };
OveruseFrameDetector::OveruseFrameDetector(Clock* clock) OveruseFrameDetector::OveruseFrameDetector(Clock* clock)
: crit_(CriticalSectionWrapper::CreateCriticalSection()), : observer_(NULL),
observer_(NULL),
clock_(clock), clock_(clock),
next_process_time_(clock_->TimeInMilliseconds()), next_process_time_(clock_->TimeInMilliseconds()),
num_process_times_(0), num_process_times_(0),
@ -337,19 +336,20 @@ OveruseFrameDetector::OveruseFrameDetector(Clock* clock)
frame_queue_(new FrameQueue()), frame_queue_(new FrameQueue()),
last_sample_time_ms_(0), last_sample_time_ms_(0),
capture_queue_delay_(new CaptureQueueDelay()) { capture_queue_delay_(new CaptureQueueDelay()) {
processing_thread_.DetachFromThread();
} }
OveruseFrameDetector::~OveruseFrameDetector() { OveruseFrameDetector::~OveruseFrameDetector() {
} }
void OveruseFrameDetector::SetObserver(CpuOveruseObserver* observer) { void OveruseFrameDetector::SetObserver(CpuOveruseObserver* observer) {
CriticalSectionScoped cs(crit_.get()); rtc::CritScope cs(&crit_);
observer_ = observer; observer_ = observer;
} }
void OveruseFrameDetector::SetOptions(const CpuOveruseOptions& options) { void OveruseFrameDetector::SetOptions(const CpuOveruseOptions& options) {
assert(options.min_frame_samples > 0); assert(options.min_frame_samples > 0);
CriticalSectionScoped cs(crit_.get()); rtc::CritScope cs(&crit_);
if (options_.Equals(options)) { if (options_.Equals(options)) {
return; return;
} }
@ -360,23 +360,23 @@ void OveruseFrameDetector::SetOptions(const CpuOveruseOptions& options) {
} }
int OveruseFrameDetector::CaptureQueueDelayMsPerS() const { int OveruseFrameDetector::CaptureQueueDelayMsPerS() const {
CriticalSectionScoped cs(crit_.get()); rtc::CritScope cs(&crit_);
return capture_queue_delay_->delay_ms(); return capture_queue_delay_->delay_ms();
} }
int OveruseFrameDetector::LastProcessingTimeMs() const { int OveruseFrameDetector::LastProcessingTimeMs() const {
CriticalSectionScoped cs(crit_.get()); rtc::CritScope cs(&crit_);
return frame_queue_->last_processing_time_ms(); return frame_queue_->last_processing_time_ms();
} }
int OveruseFrameDetector::FramesInQueue() const { int OveruseFrameDetector::FramesInQueue() const {
CriticalSectionScoped cs(crit_.get()); rtc::CritScope cs(&crit_);
return frame_queue_->NumFrames(); return frame_queue_->NumFrames();
} }
void OveruseFrameDetector::GetCpuOveruseMetrics( void OveruseFrameDetector::GetCpuOveruseMetrics(
CpuOveruseMetrics* metrics) const { CpuOveruseMetrics* metrics) const {
CriticalSectionScoped cs(crit_.get()); rtc::CritScope cs(&crit_);
metrics->capture_jitter_ms = static_cast<int>(capture_deltas_.StdDev() + 0.5); metrics->capture_jitter_ms = static_cast<int>(capture_deltas_.StdDev() + 0.5);
metrics->avg_encode_time_ms = encode_time_->Value(); metrics->avg_encode_time_ms = encode_time_->Value();
metrics->encode_rsd = 0; metrics->encode_rsd = 0;
@ -385,7 +385,7 @@ void OveruseFrameDetector::GetCpuOveruseMetrics(
} }
int64_t OveruseFrameDetector::TimeUntilNextProcess() { int64_t OveruseFrameDetector::TimeUntilNextProcess() {
CriticalSectionScoped cs(crit_.get()); DCHECK(processing_thread_.CalledOnValidThread());
return next_process_time_ - clock_->TimeInMilliseconds(); return next_process_time_ - clock_->TimeInMilliseconds();
} }
@ -416,7 +416,7 @@ void OveruseFrameDetector::ResetAll(int num_pixels) {
void OveruseFrameDetector::FrameCaptured(int width, void OveruseFrameDetector::FrameCaptured(int width,
int height, int height,
int64_t capture_time_ms) { int64_t capture_time_ms) {
CriticalSectionScoped cs(crit_.get()); rtc::CritScope cs(&crit_);
int64_t now = clock_->TimeInMilliseconds(); int64_t now = clock_->TimeInMilliseconds();
if (FrameSizeChanged(width * height) || FrameTimeoutDetected(now)) { if (FrameSizeChanged(width * height) || FrameTimeoutDetected(now)) {
@ -437,12 +437,12 @@ void OveruseFrameDetector::FrameCaptured(int width,
} }
void OveruseFrameDetector::FrameProcessingStarted() { void OveruseFrameDetector::FrameProcessingStarted() {
CriticalSectionScoped cs(crit_.get()); rtc::CritScope cs(&crit_);
capture_queue_delay_->FrameProcessingStarted(clock_->TimeInMilliseconds()); capture_queue_delay_->FrameProcessingStarted(clock_->TimeInMilliseconds());
} }
void OveruseFrameDetector::FrameEncoded(int encode_time_ms) { void OveruseFrameDetector::FrameEncoded(int encode_time_ms) {
CriticalSectionScoped cs(crit_.get()); rtc::CritScope cs(&crit_);
int64_t now = clock_->TimeInMilliseconds(); int64_t now = clock_->TimeInMilliseconds();
if (last_encode_sample_ms_ != 0) { if (last_encode_sample_ms_ != 0) {
int64_t diff_ms = now - last_encode_sample_ms_; int64_t diff_ms = now - last_encode_sample_ms_;
@ -456,7 +456,7 @@ void OveruseFrameDetector::FrameEncoded(int encode_time_ms) {
} }
void OveruseFrameDetector::FrameSent(int64_t capture_time_ms) { void OveruseFrameDetector::FrameSent(int64_t capture_time_ms) {
CriticalSectionScoped cs(crit_.get()); rtc::CritScope cs(&crit_);
if (!options_.enable_extended_processing_usage) { if (!options_.enable_extended_processing_usage) {
return; return;
} }
@ -477,7 +477,7 @@ void OveruseFrameDetector::AddProcessingTime(int elapsed_ms) {
} }
int32_t OveruseFrameDetector::Process() { int32_t OveruseFrameDetector::Process() {
CriticalSectionScoped cs(crit_.get()); DCHECK(processing_thread_.CalledOnValidThread());
int64_t now = clock_->TimeInMilliseconds(); int64_t now = clock_->TimeInMilliseconds();
@ -487,6 +487,8 @@ int32_t OveruseFrameDetector::Process() {
int64_t diff_ms = now - next_process_time_ + kProcessIntervalMs; int64_t diff_ms = now - next_process_time_ + kProcessIntervalMs;
next_process_time_ = now + kProcessIntervalMs; next_process_time_ = now + kProcessIntervalMs;
rtc::CritScope cs(&crit_);
++num_process_times_; ++num_process_times_;
capture_queue_delay_->CalculateDelayChange(diff_ms); capture_queue_delay_->CalculateDelayChange(diff_ms);

31
webrtc/video_engine/overuse_frame_detector.h
View File

@ -12,8 +12,10 @@
#define WEBRTC_VIDEO_ENGINE_OVERUSE_FRAME_DETECTOR_H_ #define WEBRTC_VIDEO_ENGINE_OVERUSE_FRAME_DETECTOR_H_
#include "webrtc/base/constructormagic.h" #include "webrtc/base/constructormagic.h"
#include "webrtc/base/criticalsection.h"
#include "webrtc/base/exp_filter.h" #include "webrtc/base/exp_filter.h"
#include "webrtc/base/thread_annotations.h" #include "webrtc/base/thread_annotations.h"
#include "webrtc/base/thread_checker.h"
#include "webrtc/modules/interface/module.h" #include "webrtc/modules/interface/module.h"
#include "webrtc/system_wrappers/interface/scoped_ptr.h" #include "webrtc/system_wrappers/interface/scoped_ptr.h"
#include "webrtc/video_engine/include/vie_base.h" #include "webrtc/video_engine/include/vie_base.h"
@ -22,7 +24,6 @@ namespace webrtc {
class Clock; class Clock;
class CpuOveruseObserver; class CpuOveruseObserver;
class CriticalSectionWrapper;
// TODO(pbos): Move this somewhere appropriate. // TODO(pbos): Move this somewhere appropriate.
class Statistics { class Statistics {
@ -108,8 +109,14 @@ class OveruseFrameDetector : public Module {
class CaptureQueueDelay; class CaptureQueueDelay;
class FrameQueue; class FrameQueue;
// TODO(asapersson): This method is only used on one thread, so it shouldn't
// need a guard.
void AddProcessingTime(int elapsed_ms) EXCLUSIVE_LOCKS_REQUIRED(crit_); void AddProcessingTime(int elapsed_ms) EXCLUSIVE_LOCKS_REQUIRED(crit_);
// TODO(asapersson): This method is always called on the processing thread.
// If locking is required, consider doing that locking inside the
// implementation and reduce scope as much as possible. We should also
// see if we can avoid calling out to other methods while holding the lock.
bool IsOverusing() EXCLUSIVE_LOCKS_REQUIRED(crit_); bool IsOverusing() EXCLUSIVE_LOCKS_REQUIRED(crit_);
bool IsUnderusing(int64_t time_now) EXCLUSIVE_LOCKS_REQUIRED(crit_); bool IsUnderusing(int64_t time_now) EXCLUSIVE_LOCKS_REQUIRED(crit_);
@ -118,8 +125,11 @@ class OveruseFrameDetector : public Module {
void ResetAll(int num_pixels) EXCLUSIVE_LOCKS_REQUIRED(crit_); void ResetAll(int num_pixels) EXCLUSIVE_LOCKS_REQUIRED(crit_);
// Protecting all members. // Protecting all members except const and those that are only accessed on the
scoped_ptr<CriticalSectionWrapper> crit_; // processing thread.
// TODO(asapersson): See if we can reduce locking. As is, video frame
// processing contends with reading stats and the processing thread.
mutable rtc::CriticalSection crit_;
// Observer getting overuse reports. // Observer getting overuse reports.
CpuOveruseObserver* observer_ GUARDED_BY(crit_); CpuOveruseObserver* observer_ GUARDED_BY(crit_);
@ -127,12 +137,13 @@ class OveruseFrameDetector : public Module {
CpuOveruseOptions options_ GUARDED_BY(crit_); CpuOveruseOptions options_ GUARDED_BY(crit_);
Clock* const clock_; Clock* const clock_;
int64_t next_process_time_; int64_t next_process_time_; // Only accessed on the processing thread.
int64_t num_process_times_ GUARDED_BY(crit_); int64_t num_process_times_ GUARDED_BY(crit_);
Statistics capture_deltas_ GUARDED_BY(crit_); Statistics capture_deltas_ GUARDED_BY(crit_);
int64_t last_capture_time_ GUARDED_BY(crit_); int64_t last_capture_time_ GUARDED_BY(crit_);
// These six members are only accessed on the processing thread.
int64_t last_overuse_time_; int64_t last_overuse_time_;
int checks_above_threshold_; int checks_above_threshold_;
int num_overuse_detections_; int num_overuse_detections_;
@ -146,13 +157,17 @@ class OveruseFrameDetector : public Module {
int64_t last_encode_sample_ms_; // Only accessed by one thread. int64_t last_encode_sample_ms_; // Only accessed by one thread.
scoped_ptr<EncodeTimeAvg> encode_time_ GUARDED_BY(crit_); // TODO(asapersson): Can these be regular members (avoid separate heap
scoped_ptr<SendProcessingUsage> usage_ GUARDED_BY(crit_); // allocs)?
scoped_ptr<FrameQueue> frame_queue_ GUARDED_BY(crit_); const scoped_ptr<EncodeTimeAvg> encode_time_ GUARDED_BY(crit_);
const scoped_ptr<SendProcessingUsage> usage_ GUARDED_BY(crit_);
const scoped_ptr<FrameQueue> frame_queue_ GUARDED_BY(crit_);
int64_t last_sample_time_ms_; // Only accessed by one thread. int64_t last_sample_time_ms_; // Only accessed by one thread.
scoped_ptr<CaptureQueueDelay> capture_queue_delay_ GUARDED_BY(crit_); const scoped_ptr<CaptureQueueDelay> capture_queue_delay_ GUARDED_BY(crit_);
rtc::ThreadChecker processing_thread_;
DISALLOW_COPY_AND_ASSIGN(OveruseFrameDetector); DISALLOW_COPY_AND_ASSIGN(OveruseFrameDetector);
}; };