Don't reset quality scaler on resolution change.

Don't reset quality scaler on resolution change.

This was causing the QualityScaler to be reconstructed each time
the resolution changes and thus the fast_rampup logic was not working
as intended. We now properly change the checking period to 5 seconds
after a downscale.

BUG=b/36457883

Review-Url: https://codereview.webrtc.org/2766513003
Cr-Commit-Position: refs/heads/master@{#17335}
Committed: https://chromium.googlesource.com/external/webrtc/+/3af6cc0de9ccfa07a3626be652c114ecca1a587f

[kthelgason, 2017-03-21 08:36:12]

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

[commit-bot: I haz the power, 2017-03-21 08:36:16]

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

[kthelgason, 2017-03-21 08:49:49]

kthelgason@webrtc.org changed reviewers:
+ perkj@webrtc.org

[kthelgason, 2017-03-21 08:49:50]

ptal :)

[commit-bot: I haz the power, 2017-03-21 09:00:46]

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

[commit-bot: I haz the power, 2017-03-21 09:00:47]

Dry run: This issue passed the CQ dry run.

[kthelgason, 2017-03-21 09:31:04]

Description was changed from

==========
Don't reset quality scaler on resolution change.

This was causing the QualityScaler to be reconstructed each time
the resolution changes and thus the fast_rampup logic was not working
as intended. We now properly change the checking period to 5 seconds
after a downscale.

BUG=b/36457883
==========

to

==========
Don't reset quality scaler on resolution change.

This was causing the QualityScaler to be reconstructed each time
the resolution changes and thus the fast_rampup logic was not working
as intended. We now properly change the checking period to 5 seconds
after a downscale.

BUG=b/36457883
==========

[perkj_webrtc, 2017-03-21 11:47:52]

Changes here looks good. But I don't know the logic of the quality scaler. Is
that ok with changing resolution between frames? 
You will need an owners approval.

[kthelgason, 2017-03-21 12:23:42]

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

[kthelgason, 2017-03-21 12:23:52]

On 2017/03/21 11:47:52, perkj_webrtc wrote:
> Changes here looks good. But I don't know the logic of the quality scaler. Is
> that ok with changing resolution between frames? 
> You will need an owners approval.

The quality scaler will reset all it's samples after asking for an up/down
scale, so that should be fine.

+sprang@ for owner approval.

[sprang_webrtc, 2017-03-21 12:40:07]

looks good
Any easy way to add a rudimentary unit test as well?

[kthelgason, 2017-03-21 15:04:58]

On 2017/03/21 12:40:07, språng wrote:
> looks good
> Any easy way to add a rudimentary unit test as well?

Not really, those tests would be very flaky as the only externally visible
effect is the interval at which things run. It's on my todo-list to make the
quality scaler accept an external clock source for the tests but until that
happens this is not reliably testable.

[sprang_webrtc, 2017-03-21 15:21:02]

On 2017/03/21 15:04:58, kthelgason wrote:
> On 2017/03/21 12:40:07, språng wrote:
> > looks good
> > Any easy way to add a rudimentary unit test as well?
> 
> Not really, those tests would be very flaky as the only externally visible
> effect is the interval at which things run. It's on my todo-list to make the
> quality scaler accept an external clock source for the tests but until that
> happens this is not reliably testable.

I see.
rtc::FakeClock (and rtc::ScopedFakeClock) will actually work as global
override even for webrtc::Clock, in case you missed that.

[sprang_webrtc, 2017-03-21 15:21:09]

lgtm

[kthelgason, 2017-03-22 07:23:12]

The CQ bit was checked by kthelgason@webrtc.org

[commit-bot: I haz the power, 2017-03-22 07:23:17]

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-22 07:25:29]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1490167392878700, "parent_rev":
"de5954c0a537ace208b65dc5a22cba74df07a323", "commit_rev":
"3af6cc0de9ccfa07a3626be652c114ecca1a587f"}

[commit-bot: I haz the power, 2017-03-22 07:25:31]

Description was changed from

==========
Don't reset quality scaler on resolution change.

This was causing the QualityScaler to be reconstructed each time
the resolution changes and thus the fast_rampup logic was not working
as intended. We now properly change the checking period to 5 seconds
after a downscale.

BUG=b/36457883
==========

to

==========
Don't reset quality scaler on resolution change.

This was causing the QualityScaler to be reconstructed each time
the resolution changes and thus the fast_rampup logic was not working
as intended. We now properly change the checking period to 5 seconds
after a downscale.

BUG=b/36457883

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

[commit-bot: I haz the power, 2017-03-22 07:25:32]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/external/webrtc/+/3af6cc0de9ccfa07a3626be65...