Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1312)

Unified Diff: remoting/host/capture_scheduler.cc

Issue 850983002: Implement video frame acknowledgements in the chromoting protocol. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « remoting/host/capture_scheduler.h ('k') | remoting/host/capture_scheduler_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: remoting/host/capture_scheduler.cc
diff --git a/remoting/host/capture_scheduler.cc b/remoting/host/capture_scheduler.cc
index ccd141ddcdc85f8c5cb8c1916fe41f12a17a4d06..56a2d28d715f616ca0ef4a88c8bdfc1cfac93bae 100644
--- a/remoting/host/capture_scheduler.cc
+++ b/remoting/host/capture_scheduler.cc
@@ -10,6 +10,7 @@
#include "base/sys_info.h"
#include "base/time/default_tick_clock.h"
#include "base/time/time.h"
+#include "remoting/proto/video.pb.h"
namespace {
@@ -25,7 +26,22 @@ const int64 kDefaultMinimumIntervalMs = 33;
// available while 1 means using 100% of all CPUs available.
const double kRecordingCpuConsumption = 0.5;
-// Maximum number of frames that can be processed simultaneously.
+// Maximum number of captured frames in the encoding queue. Currently capturer
+// implementations do not allow to keep more than 2 DesktopFrame objects.
+static const int kMaxFramesInEncodingQueue = 2;
+
+// Maximum number of unacknowledged frames. Ignored if the client doesn't
+// support ACKs. This value was chosen experimentally, using synthetic
+// performance tests (see ProtocolPerfTest), to maximize frame rate, while
+// keeping round-trip latency low.
+static const int kMaxUnacknowledgedFrames = 4;
+
+// Maximum number of frames that can be processed (captured, encoded or sent)
+// simultaneously. It's used only in the case when the client doesn't support
+// ACKs.
+//
+// TODO(sergeyu): Remove this once all current versions support ACKs.
+// crbug.com/460963 .
static const int kMaxPendingFrames = 2;
} // namespace
@@ -35,6 +51,7 @@ namespace remoting {
// We assume that the number of available cores is constant.
CaptureScheduler::CaptureScheduler(const base::Closure& capture_closure)
: capture_closure_(capture_closure),
+ acks_supported_(false),
tick_clock_(new base::DefaultTickClock()),
capture_timer_(new base::Timer(false, false)),
minimum_interval_(
@@ -42,9 +59,12 @@ CaptureScheduler::CaptureScheduler(const base::Closure& capture_closure)
num_of_processors_(base::SysInfo::NumberOfProcessors()),
capture_time_(kStatisticsWindow),
encode_time_(kStatisticsWindow),
- pending_frames_(0),
+ num_encoding_frames_(0),
+ num_sending_frames_(0),
+ num_unacknowledged_frames_(0),
capture_pending_(false),
- is_paused_(false) {
+ is_paused_(false),
+ next_frame_id_(0) {
DCHECK(num_of_processors_);
}
@@ -52,13 +72,13 @@ CaptureScheduler::~CaptureScheduler() {
}
void CaptureScheduler::Start() {
- DCHECK(CalledOnValidThread());
+ DCHECK(thread_checker_.CalledOnValidThread());
ScheduleNextCapture();
}
void CaptureScheduler::Pause(bool pause) {
- DCHECK(CalledOnValidThread());
+ DCHECK(thread_checker_.CalledOnValidThread());
if (is_paused_ != pause) {
is_paused_ = pause;
@@ -72,29 +92,54 @@ void CaptureScheduler::Pause(bool pause) {
}
void CaptureScheduler::OnCaptureCompleted() {
- DCHECK(CalledOnValidThread());
+ DCHECK(thread_checker_.CalledOnValidThread());
capture_pending_ = false;
capture_time_.Record(
(tick_clock_->NowTicks() - last_capture_started_time_).InMilliseconds());
+ ++num_encoding_frames_;
+
+ ScheduleNextCapture();
+}
+
+void CaptureScheduler::OnFrameEncoded(VideoPacket* packet) {
+ DCHECK(thread_checker_.CalledOnValidThread());
+
+ // Set packet_id for the outgoing packet.
+ packet->set_frame_id(next_frame_id_);
+ ++next_frame_id_;
+
+ // Update internal stats.
+ encode_time_.Record(packet->encode_time_ms());
+
+ --num_encoding_frames_;
+ ++num_sending_frames_;
+ ++num_unacknowledged_frames_;
+
ScheduleNextCapture();
}
void CaptureScheduler::OnFrameSent() {
- DCHECK(CalledOnValidThread());
+ DCHECK(thread_checker_.CalledOnValidThread());
- // Decrement the pending capture count.
- pending_frames_--;
- DCHECK_GE(pending_frames_, 0);
+ --num_sending_frames_;
+ DCHECK_GE(num_sending_frames_, 0);
ScheduleNextCapture();
}
-void CaptureScheduler::OnFrameEncoded(base::TimeDelta encode_time) {
- DCHECK(CalledOnValidThread());
+void CaptureScheduler::ProcessVideoAck(scoped_ptr<VideoAck> video_ack) {
+ DCHECK(thread_checker_.CalledOnValidThread());
+
+ // Host always sets |frame_id| field to indicated that it expects ACK from the
+ // client. It's assumed that the client doesn't support ACKs until the first
+ // ACK message is received.
+ acks_supported_ = true;
+
+ --num_unacknowledged_frames_;
+ DCHECK_GE(num_unacknowledged_frames_, 0);
- encode_time_.Record(encode_time.InMilliseconds());
ScheduleNextCapture();
}
@@ -102,18 +147,35 @@ void CaptureScheduler::SetTickClockForTest(
scoped_ptr<base::TickClock> tick_clock) {
tick_clock_ = tick_clock.Pass();
}
+
void CaptureScheduler::SetTimerForTest(scoped_ptr<base::Timer> timer) {
capture_timer_ = timer.Pass();
}
+
void CaptureScheduler::SetNumOfProcessorsForTest(int num_of_processors) {
num_of_processors_ = num_of_processors;
}
void CaptureScheduler::ScheduleNextCapture() {
- DCHECK(CalledOnValidThread());
+ DCHECK(thread_checker_.CalledOnValidThread());
- if (is_paused_ || pending_frames_ >= kMaxPendingFrames || capture_pending_)
+ if (is_paused_ || capture_pending_ ||
+ num_encoding_frames_ >= kMaxFramesInEncodingQueue) {
return;
+ }
+
+ if (acks_supported_) {
+ if (num_encoding_frames_ + num_unacknowledged_frames_ >=
+ kMaxUnacknowledgedFrames) {
+ return;
+ }
+ } else {
+ // TODO(sergeyu): Remove this once all current versions support ACKs.
+ // crbug.com/460963 .
+ if (num_encoding_frames_ + num_sending_frames_ >= kMaxPendingFrames) {
+ return;
+ }
+ }
// Delay by an amount chosen such that if capture and encode times
// continue to follow the averages, then we'll consume the target
@@ -134,13 +196,10 @@ void CaptureScheduler::ScheduleNextCapture() {
}
void CaptureScheduler::CaptureNextFrame() {
- DCHECK(CalledOnValidThread());
+ DCHECK(thread_checker_.CalledOnValidThread());
DCHECK(!is_paused_);
DCHECK(!capture_pending_);
- pending_frames_++;
- DCHECK_LE(pending_frames_, kMaxPendingFrames);
-
capture_pending_ = true;
last_capture_started_time_ = tick_clock_->NowTicks();
capture_closure_.Run();
« no previous file with comments | « remoting/host/capture_scheduler.h ('k') | remoting/host/capture_scheduler_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698