Fix perf regression in screenshare temporal layer bitrate allocation

Fix perf regression in screenshare temporal layer bitrate allocation

A recent cl (https://codereview.webrtc.org/2510583002) introduced an
issue where temporal layers may return incorrect bitrates, given that
they are stateful and that the GetPreferredBitrateBps is called.
The fix is to use a temporary simulcast rate allocator instance, without
temporal layers, and get the preferred bitrate from that.

Additionally, some regression in bitrate allocated stems from overly
often reconfiguring the encoder, which yields suboptimal rate control.
The fix here is to limit encoder updates to when values have actually
changed.

As a bonus, dchecks added by this cl found a bug in the (unused) RealtimeTemporalLayers implementation. Fixed that as well.

BUG=webrtc:6301, chromium:666654
Committed: https://crrev.com/c7805dbd0ea99df91ed4c4a7fccbfeab5bafd6bc
Cr-Commit-Position: refs/heads/master@{#15250}

[sprang_webrtc, 2016-11-25 13:46:55]

The CQ bit was checked by sprang@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-25 13:46:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-11-25 13:50:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-25 13:50:46]

Dry run: Try jobs failed on following builders:
  linux_ubsan on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_ubsan/builds/7184)
  linux_ubsan_vptr on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_ubsan_vptr/builds...)

[sprang_webrtc, 2016-11-25 14:03:16]

Description was changed from

==========
Fix perf regression in screenshare temporal layer bitrate allocation

A recent cl (https://codereview.webrtc.org/2510583002) introduced an
issue where temporal layers may return incorrect bitrates, given that
they are stateful and that the GetPreferredBitrateBps is called.
The fix is to use a temporary simulcast rate allocator instance, without
temporal layers, and get the preferred bitrate from that.

Additionally, some regression in bitrate allocated stems from overly
often reconfiguring the encoder, which yields suboptimal rate control.
The fix here is to limit encoder updates to when values have actually
changed.

BUG=webrtc:6301, chromium:666654
==========

to

==========
Fix perf regression in screenshare temporal layer bitrate allocation

A recent cl (https://codereview.webrtc.org/2510583002) introduced an
issue where temporal layers may return incorrect bitrates, given that
they are stateful and that the GetPreferredBitrateBps is called.
The fix is to use a temporary simulcast rate allocator instance, without
temporal layers, and get the preferred bitrate from that.

Additionally, some regression in bitrate allocated stems from overly
often reconfiguring the encoder, which yields suboptimal rate control.
The fix here is to limit encoder updates to when values have actually
changed.

As a bonus, dchecks added by this cl found a bug in the (unused)
RealtimeTemporalLayers implementation. Fixed that as well.

BUG=webrtc:6301, chromium:666654
==========

[sprang_webrtc, 2016-11-25 14:03:27]

The CQ bit was checked by sprang@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-25 14:03:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[sprang_webrtc, 2016-11-25 14:03:57]

sprang@webrtc.org changed reviewers:
+ danilchap@webrtc.org

[commit-bot: I haz the power, 2016-11-25 14:07:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-25 14:07:33]

Dry run: Try jobs failed on following builders:
  mac_asan on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/19712)
  mac_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_rel/builds/20447)

[sprang_webrtc, 2016-11-25 14:17:42]

The CQ bit was checked by sprang@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-25 14:17:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-11-25 14:23:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-25 14:23:58]

Dry run: Try jobs failed on following builders:
  linux_asan on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_asan/builds/19799)

[danilchap, 2016-11-25 14:40:31]

https://codereview.webrtc.org/2529073003/diff/20001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/utility/simulcast_rate_allocator.cc (right):

https://codereview.webrtc.org/2529073003/diff/20001/webrtc/modules/video_codi...
webrtc/modules/video_coding/utility/simulcast_rate_allocator.cc:137:
RTC_DCHECK_LE(tl_allocation_sum_kbps, target_bitrate_kbps);
may be move this check after the loop, or at least after you increment
tl_allocation_sum_kbps. (right now it doesn't check sum of layers, only  all
layers except last one)

https://codereview.webrtc.org/2529073003/diff/20001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc
(right):

https://codereview.webrtc.org/2529073003/diff/20001/webrtc/modules/video_codi...
webrtc/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc:21:
using ::testing::_;
Put this using inside unnamed namespace.

[sprang_webrtc, 2016-11-25 14:46:37]

https://codereview.webrtc.org/2529073003/diff/20001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/utility/simulcast_rate_allocator.cc (right):

https://codereview.webrtc.org/2529073003/diff/20001/webrtc/modules/video_codi...
webrtc/modules/video_coding/utility/simulcast_rate_allocator.cc:137:
RTC_DCHECK_LE(tl_allocation_sum_kbps, target_bitrate_kbps);
On 2016/11/25 14:40:31, danilchap wrote:
> may be move this check after the loop, or at least after you increment
> tl_allocation_sum_kbps. (right now it doesn't check sum of layers, only  all
> layers except last one)

Done.

https://codereview.webrtc.org/2529073003/diff/20001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc
(right):

https://codereview.webrtc.org/2529073003/diff/20001/webrtc/modules/video_codi...
webrtc/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc:21:
using ::testing::_;
On 2016/11/25 14:40:31, danilchap wrote:
> Put this using inside unnamed namespace.

Done.

[sprang_webrtc, 2016-11-25 14:47:04]

The CQ bit was checked by sprang@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-25 14:47:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-11-25 14:51:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-25 14:51:25]

Dry run: Try jobs failed on following builders:
  linux_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_rel/builds/20559)
  linux_ubsan on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_ubsan/builds/7192)

[sprang_webrtc, 2016-11-25 15:04:18]

The CQ bit was checked by sprang@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-25 15:04:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[sprang_webrtc, 2016-11-25 15:04:37]

...and another failing test fixed.

[commit-bot: I haz the power, 2016-11-25 15:17:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-25 15:17:19]

Dry run: This issue passed the CQ dry run.

[danilchap, 2016-11-25 16:07:15]

lgtm

[sprang_webrtc, 2016-11-25 16:08:00]

The CQ bit was checked by sprang@webrtc.org

[commit-bot: I haz the power, 2016-11-25 16:08:08]

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

[commit-bot: I haz the power, 2016-11-25 16:09:43]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1480090080417750,
"parent_rev": "a578b76f404b0995a6f59b8a458db4017fb55a71", "commit_rev":
"b8c96187f1e9f2c726f1ecc19abe1a8d3ec655e8"}

[commit-bot: I haz the power, 2016-11-25 16:09:46]

Description was changed from

==========
Fix perf regression in screenshare temporal layer bitrate allocation

A recent cl (https://codereview.webrtc.org/2510583002) introduced an
issue where temporal layers may return incorrect bitrates, given that
they are stateful and that the GetPreferredBitrateBps is called.
The fix is to use a temporary simulcast rate allocator instance, without
temporal layers, and get the preferred bitrate from that.

Additionally, some regression in bitrate allocated stems from overly
often reconfiguring the encoder, which yields suboptimal rate control.
The fix here is to limit encoder updates to when values have actually
changed.

As a bonus, dchecks added by this cl found a bug in the (unused)
RealtimeTemporalLayers implementation. Fixed that as well.

BUG=webrtc:6301, chromium:666654
==========

to

==========
Fix perf regression in screenshare temporal layer bitrate allocation

A recent cl (https://codereview.webrtc.org/2510583002) introduced an
issue where temporal layers may return incorrect bitrates, given that
they are stateful and that the GetPreferredBitrateBps is called.
The fix is to use a temporary simulcast rate allocator instance, without
temporal layers, and get the preferred bitrate from that.

Additionally, some regression in bitrate allocated stems from overly
often reconfiguring the encoder, which yields suboptimal rate control.
The fix here is to limit encoder updates to when values have actually
changed.

As a bonus, dchecks added by this cl found a bug in the (unused)
RealtimeTemporalLayers implementation. Fixed that as well.

BUG=webrtc:6301, chromium:666654
==========

[commit-bot: I haz the power, 2016-11-25 16:09:47]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-11-25 16:09:58]

Description was changed from

==========
Fix perf regression in screenshare temporal layer bitrate allocation

A recent cl (https://codereview.webrtc.org/2510583002) introduced an
issue where temporal layers may return incorrect bitrates, given that
they are stateful and that the GetPreferredBitrateBps is called.
The fix is to use a temporary simulcast rate allocator instance, without
temporal layers, and get the preferred bitrate from that.

Additionally, some regression in bitrate allocated stems from overly
often reconfiguring the encoder, which yields suboptimal rate control.
The fix here is to limit encoder updates to when values have actually
changed.

As a bonus, dchecks added by this cl found a bug in the (unused)
RealtimeTemporalLayers implementation. Fixed that as well.

BUG=webrtc:6301, chromium:666654
==========

to

==========
Fix perf regression in screenshare temporal layer bitrate allocation

A recent cl (https://codereview.webrtc.org/2510583002) introduced an
issue where temporal layers may return incorrect bitrates, given that
they are stateful and that the GetPreferredBitrateBps is called.
The fix is to use a temporary simulcast rate allocator instance, without
temporal layers, and get the preferred bitrate from that.

Additionally, some regression in bitrate allocated stems from overly
often reconfiguring the encoder, which yields suboptimal rate control.
The fix here is to limit encoder updates to when values have actually
changed.

As a bonus, dchecks added by this cl found a bug in the (unused)
RealtimeTemporalLayers implementation. Fixed that as well.

BUG=webrtc:6301, chromium:666654

Committed: https://crrev.com/c7805dbd0ea99df91ed4c4a7fccbfeab5bafd6bc
Cr-Commit-Position: refs/heads/master@{#15250}
==========

[commit-bot: I haz the power, 2016-11-25 16:09:59]

Patchset 6 (id:??) landed as
https://crrev.com/c7805dbd0ea99df91ed4c4a7fccbfeab5bafd6bc
Cr-Commit-Position: refs/heads/master@{#15250}