Reland of Add framerate to VideoSinkWants and ability to signal on overuse

Reland of Add framerate to VideoSinkWants and ability to signal on overuse (patchset #1 id:1 of https://codereview.webrtc.org/2764133002/ )

Reason for revert:
Found issue with test case, will add fix to reland cl.

Original issue's description:
> Revert of Add framerate to VideoSinkWants and ability to signal on overuse (patchset #14 id:250001 of https://codereview.webrtc.org/2716643002/ )
> 
> Reason for revert:
> Breaks perf tests:
> https://build.chromium.org/p/client.webrtc.perf/builders/Win7/builds/1679
> https://build.chromium.org/p/client.webrtc.perf/builders/Android32%20Tests%20%28L%20Nexus5%29/builds/2325
> 
> Original issue's description:
> > Add framerate to VideoSinkWants and ability to signal on overuse
> > 
> > In ViEEncoder, try to reduce framerate instead of resolution if the
> > current degradation preference is maintain-resolution rather than
> > balanced.
> > 
> > BUG=webrtc:4172
> > 
> > Review-Url: https://codereview.webrtc.org/2716643002
> > Cr-Commit-Position: refs/heads/master@{#17327}
> > Committed: https://chromium.googlesource.com/external/webrtc/+/72acf2526177bb4dbb5103cd6e165eb4361a5ae6
> 
> TBR=nisse@webrtc.org,magjed@webrtc.org,kthelgason@webrtc.org,ilnik@webrtc.org,stefan@webrtc.org,sprang@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:4172
> 
> Review-Url: https://codereview.webrtc.org/2764133002
> Cr-Commit-Position: refs/heads/master@{#17331}
> Committed: https://chromium.googlesource.com/external/webrtc/+/8b45b11144c968b4173215c76f78c710c9a2ed0b

TBR=nisse@webrtc.org,magjed@webrtc.org,kthelgason@webrtc.org,ilnik@webrtc.org,stefan@webrtc.org,skvlad@webrtc.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=webrtc:4172
Review-Url: https://codereview.webrtc.org/2781433002
Cr-Commit-Position: refs/heads/master@{#17474}
Committed: https://chromium.googlesource.com/external/webrtc/+/3ea3c77e93121b1ab9d5e46641e6764f2cca0d51

[sprang_webrtc, 2017-03-27 07:45:30]

Created Reland of Add framerate to VideoSinkWants and ability to signal on
overuse

[sprang_webrtc, 2017-03-27 07:53:13]

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

[commit-bot: I haz the power, 2017-03-27 07:53:22]

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, 2017-03-27 07:56:52]

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

[commit-bot: I haz the power, 2017-03-27 07:56:53]

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

[sprang_webrtc, 2017-03-27 09:13:51]

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

[commit-bot: I haz the power, 2017-03-27 09:13:59]

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

[sprang_webrtc, 2017-03-27 09:21:33]

ptal
Only change since reverted cl (apart from rebase with screwup) is test case
fixed in webrtc/call/call_perf_tests.cc

[commit-bot: I haz the power, 2017-03-27 09:36:34]

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

[commit-bot: I haz the power, 2017-03-27 09:36:35]

Dry run: This issue passed the CQ dry run.

[nisse-webrtc, 2017-03-27 10:04:17]

I don't quite understand this test, but change looks reasonable. I'm happy if
someone more familiar with the test is happy.

[ilnik, 2017-03-27 10:11:00]

On 2017/03/27 10:04:17, nisse-webrtc wrote:
> I don't quite understand this test, but change looks reasonable. I'm happy if
> someone more familiar with the test is happy.

I am familiar with this test. LGTM with minor adjustment.

[ilnik, 2017-03-27 10:11:10]

https://codereview.webrtc.org/2781433002/diff/330001/webrtc/call/call_perf_te...
File webrtc/call/call_perf_tests.cc (right):

https://codereview.webrtc.org/2781433002/diff/330001/webrtc/call/call_perf_te...
webrtc/call/call_perf_tests.cc:497: switch (test_phase_) {
In some tests there were 2 AdaptDown requests, before utilization were below
85%. You might want to adjust value at line 481 to be 45 instead of 60. Such
high value is a hack against very noisy timers in frame generator, but it is
fixed now.

[sprang_webrtc, 2017-03-27 11:18:38]

https://codereview.webrtc.org/2781433002/diff/330001/webrtc/call/call_perf_te...
File webrtc/call/call_perf_tests.cc (right):

https://codereview.webrtc.org/2781433002/diff/330001/webrtc/call/call_perf_te...
webrtc/call/call_perf_tests.cc:497: switch (test_phase_) {
On 2017/03/27 10:11:10, ilnik wrote:
> In some tests there were 2 AdaptDown requests, before utilization were below
> 85%. You might want to adjust value at line 481 to be 45 instead of 60. Such
> high value is a hack against very noisy timers in frame generator, but it is
> fixed now.

I set it to 150%, as that seems to be a common test usage. ok?

[ilnik, 2017-03-27 11:37:45]

On 2017/03/27 11:18:38, språng wrote:
>
https://codereview.webrtc.org/2781433002/diff/330001/webrtc/call/call_perf_te...
> File webrtc/call/call_perf_tests.cc (right):
> 
>
https://codereview.webrtc.org/2781433002/diff/330001/webrtc/call/call_perf_te...
> webrtc/call/call_perf_tests.cc:497: switch (test_phase_) {
> On 2017/03/27 10:11:10, ilnik wrote:
> > In some tests there were 2 AdaptDown requests, before utilization were below
> > 85%. You might want to adjust value at line 481 to be 45 instead of 60. Such
> > high value is a hack against very noisy timers in frame generator, but it is
> > fixed now.
> 
> I set it to 150%, as that seems to be a common test usage. ok?

That's perfect.

[sprang_webrtc, 2017-03-27 14:27:53]

sprang@webrtc.org changed reviewers:
- kthelgason@webrtc.org, magjed@webrtc.org, nisse@webrtc.org, skvlad@webrtc.org

[sprang_webrtc, 2017-03-27 14:27:53]

Ping?

It's only webrtc/call/call_perf_tests.cc that needs approval.

Moving most owner to cc as this doesn't need re-review for the other parts.

[åsapersson, 2017-03-29 08:59:41]

asapersson@webrtc.org changed reviewers:
+ asapersson@webrtc.org

[åsapersson, 2017-03-29 08:59:41]

https://codereview.webrtc.org/2781433002/diff/350001/webrtc/video/vie_encoder.cc
File webrtc/video/vie_encoder.cc (left):

https://codereview.webrtc.org/2781433002/diff/350001/webrtc/video/vie_encoder...
webrtc/video/vie_encoder.cc:764: current_pixel_count,
AdaptationRequest::Mode::kAdaptUp});
Should last_adaptation_request_ be updated?

[sprang_webrtc, 2017-03-29 09:03:08]

https://codereview.webrtc.org/2781433002/diff/350001/webrtc/video/vie_encoder.cc
File webrtc/video/vie_encoder.cc (left):

https://codereview.webrtc.org/2781433002/diff/350001/webrtc/video/vie_encoder...
webrtc/video/vie_encoder.cc:764: current_pixel_count,
AdaptationRequest::Mode::kAdaptUp});
On 2017/03/29 08:59:41, åsapersson wrote:
> Should last_adaptation_request_ be updated?

Yes? Not sure I understand the question.

[stefan-webrtc, 2017-03-29 15:05:26]

lgtm

[sprang_webrtc, 2017-03-29 16:20:14]

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

[commit-bot: I haz the power, 2017-03-29 16:20:28]

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, 2017-03-29 16:23:03]

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

[commit-bot: I haz the power, 2017-03-29 16:23:04]

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

[sprang_webrtc, 2017-03-29 16:35:39]

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

[commit-bot: I haz the power, 2017-03-29 16:35:50]

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, 2017-03-29 16:50:39]

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

[commit-bot: I haz the power, 2017-03-29 16:50:41]

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

[sprang_webrtc, 2017-03-30 09:14:10]

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

[commit-bot: I haz the power, 2017-03-30 09:14:16]

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

[sprang_webrtc, 2017-03-30 09:15:41]

Thanks Åsa for catching mistake in vie_encoder. Updated.

https://codereview.webrtc.org/2781433002/diff/350001/webrtc/video/vie_encoder.cc
File webrtc/video/vie_encoder.cc (left):

https://codereview.webrtc.org/2781433002/diff/350001/webrtc/video/vie_encoder...
webrtc/video/vie_encoder.cc:764: current_pixel_count,
AdaptationRequest::Mode::kAdaptUp});
On 2017/03/29 09:03:08, språng wrote:
> On 2017/03/29 08:59:41, åsapersson wrote:
> > Should last_adaptation_request_ be updated?
> 
> Yes? Not sure I understand the question.

You're right. 
Looking in the wrong spot...

[commit-bot: I haz the power, 2017-03-30 09:35:10]

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

[commit-bot: I haz the power, 2017-03-30 09:35:13]

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

[sprang_webrtc, 2017-03-30 14:20:21]

The CQ bit was checked by sprang@webrtc.org

[sprang_webrtc, 2017-03-30 14:20:22]

The patchset sent to the CQ was uploaded after l-g-t-m from ilnik@webrtc.org,
stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2781433002/#ps410001 (title:
"Rebase error, initial scaling fix")

[commit-bot: I haz the power, 2017-03-30 14:20:31]

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, 2017-03-30 14:23:48]

CQ is committing da patch.

Bot data: {"patchset_id": 410001, "attempt_start_ts": 1490883621449400,
"parent_rev": "8a256525f13ebf322f3d539639fab79f13d17efa", "commit_rev":
"3ea3c77e93121b1ab9d5e46641e6764f2cca0d51"}

[commit-bot: I haz the power, 2017-03-30 14:23:53]

Description was changed from

==========
Reland of Add framerate to VideoSinkWants and ability to signal on overuse
(patchset #1 id:1 of https://codereview.webrtc.org/2764133002/ )

Reason for revert:
Found issue with test case, will add fix to reland cl.

Original issue's description:
> Revert of Add framerate to VideoSinkWants and ability to signal on overuse
(patchset #14 id:250001 of https://codereview.webrtc.org/2716643002/ )
> 
> Reason for revert:
> Breaks perf tests:
> https://build.chromium.org/p/client.webrtc.perf/builders/Win7/builds/1679
>
https://build.chromium.org/p/client.webrtc.perf/builders/Android32%20Tests%20...
> 
> Original issue's description:
> > Add framerate to VideoSinkWants and ability to signal on overuse
> > 
> > In ViEEncoder, try to reduce framerate instead of resolution if the
> > current degradation preference is maintain-resolution rather than
> > balanced.
> > 
> > BUG=webrtc:4172
> > 
> > Review-Url: https://codereview.webrtc.org/2716643002
> > Cr-Commit-Position: refs/heads/master@{#17327}
> > Committed:
https://chromium.googlesource.com/external/webrtc/+/72acf2526177bb4dbb5103cd6...
> 
>
TBR=nisse@webrtc.org,magjed@webrtc.org,kthelgason@webrtc.org,ilnik@webrtc.org...
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:4172
> 
> Review-Url: https://codereview.webrtc.org/2764133002
> Cr-Commit-Position: refs/heads/master@{#17331}
> Committed:
https://chromium.googlesource.com/external/webrtc/+/8b45b11144c968b4173215c76...

TBR=nisse@webrtc.org,magjed@webrtc.org,kthelgason@webrtc.org,ilnik@webrtc.org...
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=webrtc:4172
==========

to

==========
Reland of Add framerate to VideoSinkWants and ability to signal on overuse
(patchset #1 id:1 of https://codereview.webrtc.org/2764133002/ )

Reason for revert:
Found issue with test case, will add fix to reland cl.

Original issue's description:
> Revert of Add framerate to VideoSinkWants and ability to signal on overuse
(patchset #14 id:250001 of https://codereview.webrtc.org/2716643002/ )
> 
> Reason for revert:
> Breaks perf tests:
> https://build.chromium.org/p/client.webrtc.perf/builders/Win7/builds/1679
>
https://build.chromium.org/p/client.webrtc.perf/builders/Android32%20Tests%20...
> 
> Original issue's description:
> > Add framerate to VideoSinkWants and ability to signal on overuse
> > 
> > In ViEEncoder, try to reduce framerate instead of resolution if the
> > current degradation preference is maintain-resolution rather than
> > balanced.
> > 
> > BUG=webrtc:4172
> > 
> > Review-Url: https://codereview.webrtc.org/2716643002
> > Cr-Commit-Position: refs/heads/master@{#17327}
> > Committed:
https://chromium.googlesource.com/external/webrtc/+/72acf2526177bb4dbb5103cd6...
> 
>
TBR=nisse@webrtc.org,magjed@webrtc.org,kthelgason@webrtc.org,ilnik@webrtc.org...
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:4172
> 
> Review-Url: https://codereview.webrtc.org/2764133002
> Cr-Commit-Position: refs/heads/master@{#17331}
> Committed:
https://chromium.googlesource.com/external/webrtc/+/8b45b11144c968b4173215c76...

TBR=nisse@webrtc.org,magjed@webrtc.org,kthelgason@webrtc.org,ilnik@webrtc.org...
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=webrtc:4172

Review-Url: https://codereview.webrtc.org/2781433002
Cr-Commit-Position: refs/heads/master@{#17474}
Committed:
https://chromium.googlesource.com/external/webrtc/+/3ea3c77e93121b1ab9d5e4664...
==========

[commit-bot: I haz the power, 2017-03-30 14:23:54]

Committed patchset #8 (id:410001) as
https://chromium.googlesource.com/external/webrtc/+/3ea3c77e93121b1ab9d5e4664...

[lliuu, 2017-03-30 17:44:25]

A revert of this CL (patchset #8 id:410001) has been created in
https://codereview.webrtc.org/2783183003/ by lliuu@webrtc.org.

The reason for reverting is: This has resulted in failure of
CallPerfTest.ReceivesCpuOveruseAndUnderuse test on the Win7 build bot
https://build.chromium.org/p/client.webrtc.perf/builders/Win7/builds/1780.