Cleanup the VideoAdapter

This cl do a major cleanup of the VideoAdapter and make sure it does care about the VideoSinkWants.max_pixel_count and VideoSinkWants.max_pixel_count_step_up.

Unit tests are updated to test that screen share is not adapted but it does not change the VideoSinkWants in WebRtcVideoEngine2::SendStream due to a switch to screen share. The reason is that it works anyway and sprang is looking into how to do adaptation based on frame rate as well and use the adapter for screen share as well.

BUG=webrtc:5688, webrtc:5426
R=nisse@webrtc.org, pbos@webrtc.org, sprang@google.com

Committed: https://chromium.googlesource.com/external/webrtc/+/766ad3b9899f0d7a61e9de13ac070ca44f9dcc49

[perkj_webrtc, 2016-03-31 15:13:35]

Description was changed from

==========
WIP Fix CPU adapt

BUG=
==========

to

==========
This cl do a major cleanup of the VideoAdapter and make sure it  do care about
the VideoSinkWants.max_pixel_count and VideoSinkWants.max_pixel_count_step_up.

Unit tests are updated to test that screen share is not adapted but it does not
change the VideoSinkWants in WebRtcVideoEngine2::SendStream due to a switch to
screen share. The reason is that it works anyway and sprang is looking into how
to do adaptation based on frame rate as well and use the adapter for screen
share as well. 

BUG=webrtc:5688, webrtc:5426
==========

[perkj_webrtc, 2016-03-31 15:13:35]

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

[perkj_webrtc, 2016-03-31 15:13:59]

Can you please take a first pass?

[nisse-webrtc, 2016-04-01 07:09:35]

Lots of deleted code, nice. What was the point of the CoordinatedVideoAdapter?

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/base/videoad...
File webrtc/media/base/videoadapter.cc (right):

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/base/videoad...
webrtc/media/base/videoadapter.cc:43: float best_distance =
static_cast<float>(INT_MAX);
Should all these values be floats?

For a float, using INT_MAX looks very arbitrary. I think HUGE_VALF (math.h) is
more appropriate.

I guess it's possible to do this with all integers, keeping the scale factors
multiplied by 16 in an all-integer table. But unclear if that would be an
improvement.

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/base/videoad...
webrtc/media/base/videoadapter.cc:54: if (diff < 0) {
Could you replace continue by break, by ensuring scale factors are listed in the
right order?

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/base/videoad...
webrtc/media/base/videoadapter.cc:76: float best_distance =
static_cast<float>(INT_MAX);
Same here, INT_MAX seems inappropriate.

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/base/videoad...
webrtc/media/base/videoadapter.cc:115: (previous_width != format.width ||
previous_height != format.height);
If this is the only use of the previous_* variables, delete them.

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/base/videoad...
webrtc/media/base/videoadapter.cc:185: const size_t output_width =
static_cast<size_t>(in_width * scale + .5f);
size_t or int?

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/base/videoad...
webrtc/media/base/videoadapter.cc:259: : 1;
Add .0f for float constant?

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/base/videoca...
File webrtc/media/base/videocapturer_unittest.cc (right):

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/base/videoca...
webrtc/media/base/videocapturer_unittest.cc:261: wants.max_pixel_count =
rtc::Optional<int>(1280 * 720 * 3 / 5);
Our magic constants here need commenting. (And more where they appear in the
actual code rather than the test cases. Maybe "same fraction as used ..." is
appropriate here)

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2.h (right):

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.h:175: enum AdaptReasonEnum {
Is this for stats only?

[perkj_webrtc, 2016-04-01 11:56:27]

ptal

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/base/videoad...
File webrtc/media/base/videoadapter.cc (right):

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/base/videoad...
webrtc/media/base/videoadapter.cc:43: float best_distance =
static_cast<float>(INT_MAX);
On 2016/04/01 07:09:35, nisse-webrtc wrote:
> Should all these values be floats?
> 
> For a float, using INT_MAX looks very arbitrary. I think HUGE_VALF (math.h) is
> more appropriate.
> 
> I guess it's possible to do this with all integers, keeping the scale factors
> multiplied by 16 in an all-integer table. But unclear if that would be an
> improvement. 

Done.

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/base/videoad...
webrtc/media/base/videoadapter.cc:54: if (diff < 0) {
On 2016/04/01 07:09:35, nisse-webrtc wrote:
> Could you replace continue by break, by ensuring scale factors are listed in
the
> right order?
Did the opposite to make FindScaleLargerThan be able to break. 
I think the most common case is that we use 1 as scale.

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/base/videoad...
webrtc/media/base/videoadapter.cc:76: float best_distance =
static_cast<float>(INT_MAX);
On 2016/04/01 07:09:35, nisse-webrtc wrote:
> Same here, INT_MAX seems inappropriate.

Done.

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/base/videoad...
webrtc/media/base/videoadapter.cc:115: (previous_width != format.width ||
previous_height != format.height);
On 2016/04/01 07:09:35, nisse-webrtc wrote:
> If this is the only use of the previous_* variables, delete them.

Done.

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/base/videoad...
webrtc/media/base/videoadapter.cc:185: const size_t output_width =
static_cast<size_t>(in_width * scale + .5f);
On 2016/04/01 07:09:35, nisse-webrtc wrote:
> size_t or int?

Done.

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/base/videoad...
webrtc/media/base/videoadapter.cc:259: : 1;
On 2016/04/01 07:09:35, nisse-webrtc wrote:
> Add .0f for float constant?

Done.

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/base/videoca...
File webrtc/media/base/videocapturer_unittest.cc (right):

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/base/videoca...
webrtc/media/base/videocapturer_unittest.cc:261: wants.max_pixel_count =
rtc::Optional<int>(1280 * 720 * 3 / 5);
On 2016/04/01 07:09:35, nisse-webrtc wrote:
> Our magic constants here need commenting. (And more where they appear in the
> actual code rather than the test cases. Maybe "same fraction as used ..." is
> appropriate here)

Done.

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2.h (right):

https://codereview.webrtc.org/1836043004/diff/60001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.h:175: enum AdaptReasonEnum {
On 2016/04/01 07:09:35, nisse-webrtc wrote:
> Is this for stats only?

yes

[nisse-webrtc, 2016-04-01 12:11:07]

https://codereview.webrtc.org/1836043004/diff/80001/webrtc/media/base/videoad...
File webrtc/media/base/videoadapter.cc (right):

https://codereview.webrtc.org/1836043004/diff/80001/webrtc/media/base/videoad...
webrtc/media/base/videoadapter.cc:31: 0.f          //
Still don't understand why 0 needs to be in the table?

[perkj_webrtc, 2016-04-04 06:13:21]

Patchset #6 (id:100001) has been deleted

[perkj_webrtc, 2016-04-04 06:14:05]

ptal

https://codereview.webrtc.org/1836043004/diff/80001/webrtc/media/base/videoad...
File webrtc/media/base/videoadapter.cc (right):

https://codereview.webrtc.org/1836043004/diff/80001/webrtc/media/base/videoad...
webrtc/media/base/videoadapter.cc:31: 0.f          //
On 2016/04/01 12:11:07, nisse-webrtc wrote:
> Still don't understand why 0 needs to be in the table?

removed

[nisse-webrtc, 2016-04-04 06:54:18]

lgtm with some nits addressed.

https://codereview.webrtc.org/1836043004/diff/120001/webrtc/media/base/videoa...
File webrtc/media/base/videoadapter.cc (right):

https://codereview.webrtc.org/1836043004/diff/120001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:82: if (resulting_number_of_pixels) {
resulting_number_if_pixels is never NULL here, right? If so, drop this test.

https://codereview.webrtc.org/1836043004/diff/120001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvideoengine2.h (right):

https://codereview.webrtc.org/1836043004/diff/120001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvideoengine2.h:178: enum AdaptReasonEnum {
Is this intended as bits to be or:ed together? There should be a comment (or two
separate bools instead).

https://codereview.webrtc.org/1836043004/diff/120001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvideoengine2_unittest.cc (right):

https://codereview.webrtc.org/1836043004/diff/120001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvideoengine2_unittest.cc:2912:
WebRtcVideoChannel2::ADAPTREASON_BANDWIDTH,
Prefer | over +, if the intention is to or together flag bits.

[perkj_webrtc, 2016-04-04 07:12:38]

Patchset #7 (id:130001) has been deleted

[perkj_webrtc, 2016-04-04 07:13:25]

perkj@webrtc.org changed reviewers:
+ pbos@webrtc.org, sprang@webrtc.org

[perkj_webrtc, 2016-04-04 07:13:26]

pbos, sprang can you please take a look as well?

https://codereview.webrtc.org/1836043004/diff/120001/webrtc/media/base/videoa...
File webrtc/media/base/videoadapter.cc (right):

https://codereview.webrtc.org/1836043004/diff/120001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:82: if (resulting_number_of_pixels) {
On 2016/04/04 06:54:18, nisse-webrtc wrote:
> resulting_number_if_pixels is never NULL here, right? If so, drop this test.

Done.

https://codereview.webrtc.org/1836043004/diff/120001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvideoengine2.h (right):

https://codereview.webrtc.org/1836043004/diff/120001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvideoengine2.h:178: enum AdaptReasonEnum {
On 2016/04/04 06:54:18, nisse-webrtc wrote:
> Is this intended as bits to be or:ed together? There should be a comment (or
two
> separate bools instead).

Done.

https://codereview.webrtc.org/1836043004/diff/120001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvideoengine2_unittest.cc (right):

https://codereview.webrtc.org/1836043004/diff/120001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvideoengine2_unittest.cc:2912:
WebRtcVideoChannel2::ADAPTREASON_BANDWIDTH,
On 2016/04/04 06:54:18, nisse-webrtc wrote:
> Prefer | over +, if the intention is to or together flag bits.

Done.

[sprang, 2016-04-04 09:02:09]

sprang@google.com changed reviewers:
+ sprang@google.com

[sprang, 2016-04-04 09:02:10]

Did I get it right that this is just a cleanup refactoring, it doesn't change
cpu adaptation behavior?

https://codereview.webrtc.org/1836043004/diff/150001/webrtc/media/base/videoa...
File webrtc/media/base/videoadapter.cc (right):

https://codereview.webrtc.org/1836043004/diff/150001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:37: float best_distance = HUGE_VALF;
Could we use std::numeric_limits<float>::max() instead?
Same for MAX_INT then.

https://codereview.webrtc.org/1836043004/diff/150001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:47: if (diff < best_distance) {
Since you now don't allow negative diff, will this ever be true more than once?

https://codereview.webrtc.org/1836043004/diff/150001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:69: float best_number_of_pixels = 0.0f;
Shouldn't this default to target_num_pixels if nothing matches?

[pbos-webrtc, 2016-04-04 15:59:33]

Description was changed from

==========
This cl do a major cleanup of the VideoAdapter and make sure it  do care about
the VideoSinkWants.max_pixel_count and VideoSinkWants.max_pixel_count_step_up.

Unit tests are updated to test that screen share is not adapted but it does not
change the VideoSinkWants in WebRtcVideoEngine2::SendStream due to a switch to
screen share. The reason is that it works anyway and sprang is looking into how
to do adaptation based on frame rate as well and use the adapter for screen
share as well. 

BUG=webrtc:5688, webrtc:5426
==========

to

==========
This cl do a major cleanup of the VideoAdapter and make sure it does care about
the VideoSinkWants.max_pixel_count and VideoSinkWants.max_pixel_count_step_up.

Unit tests are updated to test that screen share is not adapted but it does not
change the VideoSinkWants in WebRtcVideoEngine2::SendStream due to a switch to
screen share. The reason is that it works anyway and sprang is looking into how
to do adaptation based on frame rate as well and use the adapter for screen
share as well. 

BUG=webrtc:5688, webrtc:5426
==========

[perkj_webrtc, 2016-04-05 07:25:42]

The idea is to clean it up and actually care about the VideoSinkWants.
Previously, it only cared about if the adapter should increase or decrease the
number of output pixels. Not how many steps. 

But the end result should be no behavior change during a call. The adapter
should however reset its state when there are no wants.

https://codereview.webrtc.org/1836043004/diff/150001/webrtc/media/base/videoa...
File webrtc/media/base/videoadapter.cc (right):

https://codereview.webrtc.org/1836043004/diff/150001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:37: float best_distance = HUGE_VALF;
On 2016/04/04 09:02:10, sprang wrote:
> Could we use std::numeric_limits<float>::max() instead?
> Same for MAX_INT then.

Done.

https://codereview.webrtc.org/1836043004/diff/150001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:47: if (diff < best_distance) {
On 2016/04/04 09:02:10, sprang wrote:
> Since you now don't allow negative diff, will this ever be true more than
once?

yes- it tries to find a scale than is just "one step" larger than the the
target.

https://codereview.webrtc.org/1836043004/diff/150001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:69: float best_number_of_pixels = 0.0f;
On 2016/04/04 09:02:10, sprang wrote:
> Shouldn't this default to target_num_pixels if nothing matches?

no - to pixels..

[sprang, 2016-04-05 08:28:35]

On 2016/04/05 07:25:42, perkj_webrtc wrote:
> The idea is to clean it up and actually care about the VideoSinkWants.
> Previously, it only cared about if the adapter should increase or decrease the
> number of output pixels. Not how many steps. 
> 
> But the end result should be no behavior change during a call. The adapter
> should however reset its state when there are no wants.

Alright, gotcha.

lgtm with nit

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoc...
File webrtc/media/base/videocapturer.h (right):

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoc...
webrtc/media/base/videocapturer.h:301: int input_height_
GUARDED_BY(frame_stats_crit_);
Did you revert the change i rev 7, why?
In any case, please initialize input_size_valid_ in the constructor instead.

[perkj_webrtc, 2016-04-05 10:03:03]

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoc...
File webrtc/media/base/videocapturer.h (right):

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoc...
webrtc/media/base/videocapturer.h:301: int input_height_
GUARDED_BY(frame_stats_crit_);
On 2016/04/05 08:28:35, sprang wrote:
> Did you revert the change i rev 7, why?
> In any case, please initialize input_size_valid_ in the constructor instead.

I have only rebased.

[pbos-webrtc, 2016-04-05 10:16:55]

https://codereview.webrtc.org/1836043004/diff/150001/webrtc/media/base/videoc...
File webrtc/media/base/videocapturer_unittest.cc (right):

https://codereview.webrtc.org/1836043004/diff/150001/webrtc/media/base/videoc...
webrtc/media/base/videocapturer_unittest.cc:312: // But resetting the wants
should reset the resolution to what the camera is
was opened with

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoa...
File webrtc/media/base/videoadapter.cc (right):

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:91: VideoAdapter::VideoAdapter() {}
Pref putting INT_MAX or ::max() initialization here (and removing them from the
header)

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:97: input_format_.interval = interval;
Should we do a TODO to actually measure the incoming FPS?

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:135: if (!output_num_pixels_) {
Does this condition still make sense?

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:249: bool increase_resolution =
Can this be called something better? Maybe bool use_max_pixel_count_step_up?

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:259: bool changed = output_num_pixels_ !=
old_num_pixels;
This can't possibly make sense.

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:260: LOG(LS_VERBOSE) << "VAdapt Adapt: "
Should this be LS_INFO and the VAdapt Frame log maybe removed?

Maybe only log if changed?

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoa...
File webrtc/media/base/videoadapter.h (right):

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.h:14: #include <limits.h>  // For INT_MAX
Do we want numerical limits for this instead?

Also consider setting the INT_MAX ones in the constructor to not pull in headers
from the header.

[pbos-webrtc, 2016-04-05 10:17:41]

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvideoengine2.h (right):

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvideoengine2.h:181: enum AdaptReasonEnum {
enum AdaptReason should be enough

[pbos-webrtc, 2016-04-05 10:22:32]

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvideoengine2_unittest.cc (right):

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvideoengine2_unittest.cc:2046:
TEST_F(WebRtcVideoChannel2Test, AdaptsOnOveruseAndSwitchToScreenShare) {
PreviousAdaptationDoesNotApplyToScreenshare?

[perkj_webrtc, 2016-04-05 11:44:45]

ptal

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoa...
File webrtc/media/base/videoadapter.cc (right):

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:91: VideoAdapter::VideoAdapter() {}
On 2016/04/05 10:16:55, pbos-webrtc wrote:
> Pref putting INT_MAX or ::max() initialization here (and removing them from
the
> header)

Done.

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:97: input_format_.interval = interval;
On 2016/04/05 10:16:55, pbos-webrtc wrote:
> Should we do a TODO to actually measure the incoming FPS?

Done.

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:135: if (!output_num_pixels_) {
On 2016/04/05 10:16:55, pbos-webrtc wrote:
> Does this condition still make sense?

max_num_pixels can be 0.

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:249: bool increase_resolution =
On 2016/04/05 10:16:55, pbos-webrtc wrote:
> Can this be called something better? Maybe bool use_max_pixel_count_step_up?

Done.

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:259: bool changed = output_num_pixels_ !=
old_num_pixels;
On 2016/04/05 10:16:55, pbos-webrtc wrote:
> This can't possibly make sense.

Acknowledged.

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:260: LOG(LS_VERBOSE) << "VAdapt Adapt: "
On 2016/04/05 10:16:55, pbos-webrtc wrote:
> Should this be LS_INFO and the VAdapt Frame log maybe removed?
> 
> Maybe only log if changed?

VAdapt Frame log changed to only happen when changed.

Changed this log to LS_INFO. I would like to keep this to log every time so it
can be seen when Adapt is called.

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoa...
File webrtc/media/base/videoadapter.h (right):

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.h:14: #include <limits.h>  // For INT_MAX
On 2016/04/05 10:16:55, pbos-webrtc wrote:
> Do we want numerical limits for this instead?
> 
> Also consider setting the INT_MAX ones in the constructor to not pull in
headers
> from the header.

Done.

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvideoengine2.h (right):

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvideoengine2.h:181: enum AdaptReasonEnum {
On 2016/04/05 10:17:41, pbos-webrtc wrote:
> enum AdaptReason should be enough

Done.

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvideoengine2_unittest.cc (right):

https://codereview.webrtc.org/1836043004/diff/170001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvideoengine2_unittest.cc:2046:
TEST_F(WebRtcVideoChannel2Test, AdaptsOnOveruseAndSwitchToScreenShare) {
On 2016/04/05 10:22:32, pbos-webrtc wrote:
> PreviousAdaptationDoesNotApplyToScreenshare?

Done.

[pbos-webrtc, 2016-04-05 13:17:53]

lgtm

https://codereview.webrtc.org/1836043004/diff/190001/webrtc/media/base/videoa...
File webrtc/media/base/videoadapter.cc (right):

https://codereview.webrtc.org/1836043004/diff/190001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:192: LOG(LS_INFO) << "VAdapt Frame: scaled "
<< frames_scaled_ << " / out "
I think this should log "Frame size changed: " instead of "VAdapt Frame: "

https://codereview.webrtc.org/1836043004/diff/190001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:251: LOG(LS_INFO) << "Adapt: "
Change Adapt: To OnResolutionRequest:

I think that reads more understandably.

https://codereview.webrtc.org/1836043004/diff/190001/webrtc/media/base/videoa...
File webrtc/media/base/videoadapter.h (right):

https://codereview.webrtc.org/1836043004/diff/190001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.h:60: int adaption_changes_;  // Number of
changes in scale factor.
git cl format (comment alignment)

[commit-bot: I haz the power, 2016-04-05 13:24:03]

Description was changed from

==========
This cl do a major cleanup of the VideoAdapter and make sure it does care about
the VideoSinkWants.max_pixel_count and VideoSinkWants.max_pixel_count_step_up.

Unit tests are updated to test that screen share is not adapted but it does not
change the VideoSinkWants in WebRtcVideoEngine2::SendStream due to a switch to
screen share. The reason is that it works anyway and sprang is looking into how
to do adaptation based on frame rate as well and use the adapter for screen
share as well. 

BUG=webrtc:5688, webrtc:5426
==========

to

==========
This cl do a major cleanup of the VideoAdapter and make sure it does care about
the VideoSinkWants.max_pixel_count and VideoSinkWants.max_pixel_count_step_up.

Unit tests are updated to test that screen share is not adapted but it does not
change the VideoSinkWants in WebRtcVideoEngine2::SendStream due to a switch to
screen share. The reason is that it works anyway and sprang is looking into how
to do adaptation based on frame rate as well and use the adapter for screen
share as well.

BUG=webrtc:5688, webrtc:5426
R=nisse@webrtc.org, pbos@webrtc.org, sprang@google.com

Committed: https://crrev.com/766ad3b9899f0d7a61e9de13ac070ca44f9dcc49
Cr-Commit-Position: refs/heads/master@{#12240}
==========

[perkj_webrtc, 2016-04-05 13:24:03]

Description was changed from

==========
This cl do a major cleanup of the VideoAdapter and make sure it does care about
the VideoSinkWants.max_pixel_count and VideoSinkWants.max_pixel_count_step_up.

Unit tests are updated to test that screen share is not adapted but it does not
change the VideoSinkWants in WebRtcVideoEngine2::SendStream due to a switch to
screen share. The reason is that it works anyway and sprang is looking into how
to do adaptation based on frame rate as well and use the adapter for screen
share as well.

BUG=webrtc:5688, webrtc:5426
R=nisse@webrtc.org, pbos@webrtc.org, sprang@google.com

Committed: https://crrev.com/766ad3b9899f0d7a61e9de13ac070ca44f9dcc49
Cr-Commit-Position: refs/heads/master@{#12240}
==========

to

==========
This cl do a major cleanup of the VideoAdapter and make sure it does care about
the VideoSinkWants.max_pixel_count and VideoSinkWants.max_pixel_count_step_up.

Unit tests are updated to test that screen share is not adapted but it does not
change the VideoSinkWants in WebRtcVideoEngine2::SendStream due to a switch to
screen share. The reason is that it works anyway and sprang is looking into how
to do adaptation based on frame rate as well and use the adapter for screen
share as well.

BUG=webrtc:5688, webrtc:5426
R=nisse@webrtc.org, pbos@webrtc.org, sprang@google.com

Committed:
https://chromium.googlesource.com/external/webrtc/+/766ad3b9899f0d7a61e9de13a...
==========

[commit-bot: I haz the power, 2016-04-05 13:24:04]

Patchset 10 (id:??) landed as
https://crrev.com/766ad3b9899f0d7a61e9de13ac070ca44f9dcc49
Cr-Commit-Position: refs/heads/master@{#12240}

[perkj_webrtc, 2016-04-05 13:24:05]

Committed patchset #10 (id:210001) manually as
766ad3b9899f0d7a61e9de13ac070ca44f9dcc49 (presubmit successful).

[perkj_webrtc, 2016-04-05 13:24:15]

https://codereview.webrtc.org/1836043004/diff/190001/webrtc/media/base/videoa...
File webrtc/media/base/videoadapter.cc (right):

https://codereview.webrtc.org/1836043004/diff/190001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:192: LOG(LS_INFO) << "VAdapt Frame: scaled "
<< frames_scaled_ << " / out "
On 2016/04/05 13:17:53, pbos-webrtc wrote:
> I think this should log "Frame size changed: " instead of "VAdapt Frame: "

Done.

https://codereview.webrtc.org/1836043004/diff/190001/webrtc/media/base/videoa...
webrtc/media/base/videoadapter.cc:251: LOG(LS_INFO) << "Adapt: "
On 2016/04/05 13:17:53, pbos-webrtc wrote:
> Change Adapt: To OnResolutionRequest:
> 
> I think that reads more understandably.

Done.