Use high quantizer value for "big" frames after a sequence of "small" frames.

Use high quantizer value for "big" frames after a sequence of "small" frames.

After a sequence of frames that don't saturate bandwidth libvpx always
chooses lowest allowed quntizer (highest quality). As result these frames
are quite big after being encoded, which results in poor response latency
for those frames. With this change the scheduler detects those frames and
sets min_quantizer to 60 to ensure that they are first encoded with low
quality and quality is topped off later.

This change reduces latency for "big" frames in
ProtocolPerfTest.TotalLatencyWebrtc from 500ms to 200ms in 8Mbps case,
even without proper BW estimation.

Also changed frame duration from 66 to 33 ms, to match actual frame
duration (this doesn't seem to have significant effect on quality).

BUG=645656
Committed: https://crrev.com/52f18b0a5d30a2259b79235dc208b1ebd8400ad0
Cr-Commit-Position: refs/heads/master@{#423738}

[Sergey Ulanov, 2016-09-30 00:52:29]

Patchset #1 (id:1) has been deleted

[Sergey Ulanov, 2016-09-30 00:53:43]

Description was changed from

==========
Use high quantizer value for "big" frames after a sequence of "small" frames.

After a sequence of frames that don't saturate bandwidth libvpx always
chooses lowest allowed quntizer (highest quality). As result these frames
are quite big after being encoded, which results in poor response latency
for those frames. With this change the scheduler detects those frames and
sets min_quantizer to 60 to ensure that they are first encoded with low
quality and quality is topped off later.

This change reduces latency for "big" frames in
ProtocolPerfTest.TotalLatencyWebrtc from 500ms to 200ms (for 8Mbps case).

Also changed frame duration from 66 to 33 ms, to match actual frame
duration. Minimum target bitrate also changed to account for the new frame
duration.

BUG=645656
==========

to

==========
Use high quantizer value for "big" frames after a sequence of "small" frames.

After a sequence of frames that don't saturate bandwidth libvpx always
chooses lowest allowed quntizer (highest quality). As result these frames
are quite big after being encoded, which results in poor response latency
for those frames. With this change the scheduler detects those frames and
sets min_quantizer to 60 to ensure that they are first encoded with low
quality and quality is topped off later.

This change reduces latency for "big" frames in
ProtocolPerfTest.TotalLatencyWebrtc from 500ms to 200ms in 8Mbps case,
even without proper BW estimation.

Also changed frame duration from 66 to 33 ms, to match actual frame
duration.

BUG=645656
==========

[Sergey Ulanov, 2016-09-30 00:54:29]

Description was changed from

==========
Use high quantizer value for "big" frames after a sequence of "small" frames.

After a sequence of frames that don't saturate bandwidth libvpx always
chooses lowest allowed quntizer (highest quality). As result these frames
are quite big after being encoded, which results in poor response latency
for those frames. With this change the scheduler detects those frames and
sets min_quantizer to 60 to ensure that they are first encoded with low
quality and quality is topped off later.

This change reduces latency for "big" frames in
ProtocolPerfTest.TotalLatencyWebrtc from 500ms to 200ms in 8Mbps case,
even without proper BW estimation.

Also changed frame duration from 66 to 33 ms, to match actual frame
duration.

BUG=645656
==========

to

==========
Use high quantizer value for "big" frames after a sequence of "small" frames.

After a sequence of frames that don't saturate bandwidth libvpx always
chooses lowest allowed quntizer (highest quality). As result these frames
are quite big after being encoded, which results in poor response latency
for those frames. With this change the scheduler detects those frames and
sets min_quantizer to 60 to ensure that they are first encoded with low
quality and quality is topped off later.

This change reduces latency for "big" frames in
ProtocolPerfTest.TotalLatencyWebrtc from 500ms to 200ms in 8Mbps case,
even without proper BW estimation.

Also changed frame duration from 66 to 33 ms, to match actual frame
duration (this doesn't seem to have significant effect on quality).

BUG=645656
==========

[Sergey Ulanov, 2016-09-30 00:55:39]

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

[Irfan, 2016-10-03 20:21:49]

https://codereview.chromium.org/2381213002/diff/20001/remoting/protocol/webrt...
File remoting/protocol/webrtc_frame_scheduler_simple.cc (right):

https://codereview.chromium.org/2381213002/diff/20001/remoting/protocol/webrt...
remoting/protocol/webrtc_frame_scheduler_simple.cc:31: const int
kVp8MinimumTargetBitrateBytesPerMegapixel = 300000;
"BytesPerMegapixel" sounds like size. I do not understand how this represents
target bitrate.

May be explain more or reconsider units here.

https://codereview.chromium.org/2381213002/diff/20001/remoting/protocol/webrt...
remoting/protocol/webrtc_frame_scheduler_simple.cc:35: const int
kEstimatedBytesPerMegapixel = 150000;
Clarify what is "target quality"  here ?

https://codereview.chromium.org/2381213002/diff/20001/remoting/protocol/webrt...
remoting/protocol/webrtc_frame_scheduler_simple.cc:50:
encoded_frame_size_(kStatsWindow) {}
may be encoded_frame_size_bytes_ ?

https://codereview.chromium.org/2381213002/diff/20001/remoting/protocol/webrt...
remoting/protocol/webrtc_frame_scheduler_simple.cc:106: if
(encoded_frame_size_.Average() * kTargetFrameRate <
Should we measure the frame rate to get real usage ?

[Sergey Ulanov, 2016-10-06 21:54:35]

Updated this CL with a different logic used to detect "big" frames.
Now instead of using encoded frame size it detects big jumps in updated_region()
size. This works better particularly in case when the input image compresses
well.
PTAL.

https://codereview.chromium.org/2381213002/diff/20001/remoting/protocol/webrt...
File remoting/protocol/webrtc_frame_scheduler_simple.cc (right):

https://codereview.chromium.org/2381213002/diff/20001/remoting/protocol/webrt...
remoting/protocol/webrtc_frame_scheduler_simple.cc:31: const int
kVp8MinimumTargetBitrateBytesPerMegapixel = 300000;
On 2016/10/03 20:21:49, Irfan wrote:
> "BytesPerMegapixel" sounds like size. I do not understand how this represents
> target bitrate.
> 
> May be explain more or reconsider units here.

The correct units would be bytes/(megapixel*second).
Reverted this line back to what it was.

https://codereview.chromium.org/2381213002/diff/20001/remoting/protocol/webrt...
remoting/protocol/webrtc_frame_scheduler_simple.cc:35: const int
kEstimatedBytesPerMegapixel = 150000;
On 2016/10/03 20:21:49, Irfan wrote:
> Clarify what is "target quality"  here ?

Done.

https://codereview.chromium.org/2381213002/diff/20001/remoting/protocol/webrt...
remoting/protocol/webrtc_frame_scheduler_simple.cc:50:
encoded_frame_size_(kStatsWindow) {}
On 2016/10/03 20:21:49, Irfan wrote:
> may be encoded_frame_size_bytes_ ?

Removed it now and replaced with updated_region_area_.

https://codereview.chromium.org/2381213002/diff/20001/remoting/protocol/webrt...
remoting/protocol/webrtc_frame_scheduler_simple.cc:106: if
(encoded_frame_size_.Average() * kTargetFrameRate <
On 2016/10/03 20:21:49, Irfan wrote:
> Should we measure the frame rate to get real usage ?

No. What matters here is the bitrate that libvpx observes, not the actual BW
usages. Duration is set to kTargetFrameInterval, so from libvpx point of view
the stream is always at 30 FPS.

But I updated this logic now to use updated_region area instead of encoded frame
size.

[Irfan, 2016-10-06 22:14:03]

lgtm

[Sergey Ulanov, 2016-10-06 23:12:13]

Also changed to updated_region_area_.Average() to updated_region_area_.Max() to
make big frame detection less aggressive.

[Sergey Ulanov, 2016-10-06 23:12:40]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-10-06 23:12:40]

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

[commit-bot: I haz the power, 2016-10-06 23:13:28]

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-06 23:39:11]

Description was changed from

==========
Use high quantizer value for "big" frames after a sequence of "small" frames.

After a sequence of frames that don't saturate bandwidth libvpx always
chooses lowest allowed quntizer (highest quality). As result these frames
are quite big after being encoded, which results in poor response latency
for those frames. With this change the scheduler detects those frames and
sets min_quantizer to 60 to ensure that they are first encoded with low
quality and quality is topped off later.

This change reduces latency for "big" frames in
ProtocolPerfTest.TotalLatencyWebrtc from 500ms to 200ms in 8Mbps case,
even without proper BW estimation.

Also changed frame duration from 66 to 33 ms, to match actual frame
duration (this doesn't seem to have significant effect on quality).

BUG=645656
==========

to

==========
Use high quantizer value for "big" frames after a sequence of "small" frames.

After a sequence of frames that don't saturate bandwidth libvpx always
chooses lowest allowed quntizer (highest quality). As result these frames
are quite big after being encoded, which results in poor response latency
for those frames. With this change the scheduler detects those frames and
sets min_quantizer to 60 to ensure that they are first encoded with low
quality and quality is topped off later.

This change reduces latency for "big" frames in
ProtocolPerfTest.TotalLatencyWebrtc from 500ms to 200ms in 8Mbps case,
even without proper BW estimation.

Also changed frame duration from 66 to 33 ms, to match actual frame
duration (this doesn't seem to have significant effect on quality).

BUG=645656
==========

[commit-bot: I haz the power, 2016-10-06 23:39:12]

Committed patchset #3 (id:40002)

[commit-bot: I haz the power, 2016-10-06 23:41:05]

Description was changed from

==========
Use high quantizer value for "big" frames after a sequence of "small" frames.

After a sequence of frames that don't saturate bandwidth libvpx always
chooses lowest allowed quntizer (highest quality). As result these frames
are quite big after being encoded, which results in poor response latency
for those frames. With this change the scheduler detects those frames and
sets min_quantizer to 60 to ensure that they are first encoded with low
quality and quality is topped off later.

This change reduces latency for "big" frames in
ProtocolPerfTest.TotalLatencyWebrtc from 500ms to 200ms in 8Mbps case,
even without proper BW estimation.

Also changed frame duration from 66 to 33 ms, to match actual frame
duration (this doesn't seem to have significant effect on quality).

BUG=645656
==========

to

==========
Use high quantizer value for "big" frames after a sequence of "small" frames.

After a sequence of frames that don't saturate bandwidth libvpx always
chooses lowest allowed quntizer (highest quality). As result these frames
are quite big after being encoded, which results in poor response latency
for those frames. With this change the scheduler detects those frames and
sets min_quantizer to 60 to ensure that they are first encoded with low
quality and quality is topped off later.

This change reduces latency for "big" frames in
ProtocolPerfTest.TotalLatencyWebrtc from 500ms to 200ms in 8Mbps case,
even without proper BW estimation.

Also changed frame duration from 66 to 33 ms, to match actual frame
duration (this doesn't seem to have significant effect on quality).

BUG=645656

Committed: https://crrev.com/52f18b0a5d30a2259b79235dc208b1ebd8400ad0
Cr-Commit-Position: refs/heads/master@{#423738}
==========

[commit-bot: I haz the power, 2016-10-06 23:41:06]

Patchset 3 (id:??) landed as
https://crrev.com/52f18b0a5d30a2259b79235dc208b1ebd8400ad0
Cr-Commit-Position: refs/heads/master@{#423738}