Update WebrtcFrameScheduler to avoid concurrent captures.

Update WebrtcFrameScheduler to avoid concurrent captures.

Previously the scheduler would sometime schedule a new frame before the
previous frame is finished encoding. This was causing DCHECK in the
capturer and may lead to other unintended consequences.

BUG=657114
Committed: https://crrev.com/0d63c67ff4554aaa7ba42a11ed33ef3880fe02d3
Cr-Commit-Position: refs/heads/master@{#426102}

[Sergey Ulanov, 2016-10-18 21:46:06]

sergeyu@chromium.org changed reviewers:
+ jamiewalch@chromium.org

[Jamie, 2016-10-18 22:40:26]

lgtm

https://codereview.chromium.org/2431843002/diff/1/remoting/protocol/webrtc_fr...
File remoting/protocol/webrtc_frame_scheduler_simple.cc (right):

https://codereview.chromium.org/2431843002/diff/1/remoting/protocol/webrtc_fr...
remoting/protocol/webrtc_frame_scheduler_simple.cc:183: // no capture callback
set.
Update this comment. As written now, it's just an English transition of the
conditional, which doesn't seem very useful. You could just remove it, or
rephrase it in terms of why frames should not be scheduled under these
circumstances.

https://codereview.chromium.org/2431843002/diff/1/remoting/protocol/webrtc_fr...
File remoting/protocol/webrtc_frame_scheduler_simple.h (right):

https://codereview.chromium.org/2431843002/diff/1/remoting/protocol/webrtc_fr...
remoting/protocol/webrtc_frame_scheduler_simple.h:21: // It schedules each frame
after the previous one is expected to finish sending.
Is this comment still accurate after your change?

[Sergey Ulanov, 2016-10-18 23:11:02]

https://codereview.chromium.org/2431843002/diff/1/remoting/protocol/webrtc_fr...
File remoting/protocol/webrtc_frame_scheduler_simple.cc (right):

https://codereview.chromium.org/2431843002/diff/1/remoting/protocol/webrtc_fr...
remoting/protocol/webrtc_frame_scheduler_simple.cc:183: // no capture callback
set.
On 2016/10/18 22:40:26, Jamie wrote:
> Update this comment. As written now, it's just an English transition of the
> conditional, which doesn't seem very useful. You could just remove it, or
> rephrase it in terms of why frames should not be scheduled under these
> circumstances.

Removed it.

https://codereview.chromium.org/2431843002/diff/1/remoting/protocol/webrtc_fr...
File remoting/protocol/webrtc_frame_scheduler_simple.h (right):

https://codereview.chromium.org/2431843002/diff/1/remoting/protocol/webrtc_fr...
remoting/protocol/webrtc_frame_scheduler_simple.h:21: // It schedules each frame
after the previous one is expected to finish sending.
On 2016/10/18 22:40:26, Jamie wrote:
> Is this comment still accurate after your change?

Updated the comment.

[Sergey Ulanov, 2016-10-18 23:11:06]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-10-18 23:11:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/2431843002/#ps20001
(title: "comments")

[commit-bot: I haz the power, 2016-10-18 23:11:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-19 00:21:16]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-21 13:05:18]

Description was changed from

==========
Update WebrtcFrameScheduler to avoid concurrent captures.

Previously the scheduler would sometime schedule a new frame before the
previous frame is finished encoding. This was causing DCHECK in the
capturer and may lead to other unintended consequences.

BUG=657114
==========

to

==========
Update WebrtcFrameScheduler to avoid concurrent captures.

Previously the scheduler would sometime schedule a new frame before the
previous frame is finished encoding. This was causing DCHECK in the
capturer and may lead to other unintended consequences.

BUG=657114

Committed: https://crrev.com/0d63c67ff4554aaa7ba42a11ed33ef3880fe02d3
Cr-Commit-Position: refs/heads/master@{#426102}
==========

[commit-bot: I haz the power, 2016-10-21 13:05:19]

Patchset 2 (id:??) landed as
https://crrev.com/0d63c67ff4554aaa7ba42a11ed33ef3880fe02d3
Cr-Commit-Position: refs/heads/master@{#426102}