Disable encoder scaling on iPhone4S.

Disable encoder scaling on iPhone4S.

Scaling causes us to work the CPU too much, which very quickly degrades quality. This causes us to at least behave better on good networks.

NOTRY=True
BUG=

Committed: https://crrev.com/6ce738da31fc3635295c27bea24605ea666f061e
Cr-Commit-Position: refs/heads/master@{#13630}

[tkchin_webrtc, 2016-08-02 01:00:05]

Description was changed from

==========
Disable scaling on iPhone4S.

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

to

==========
Disable scaling on iPhone4S.

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

[tkchin_webrtc, 2016-08-02 01:00:05]

tkchin@webrtc.org changed reviewers:
+ haysc@webrtc.org, magjed@google.com

[tkchin_webrtc, 2016-08-02 01:18:32]

Description was changed from

==========
Disable scaling on iPhone4S.

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

to

==========
Disable encoder scaling on iPhone4S.

Scaling causes us to work the CPU too much, which very quickly degrades quality.
This causes us to at least behave better on good networks.

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

[Chuck, 2016-08-02 01:27:05]

lgtm

[tkchin_webrtc, 2016-08-02 23:12:44]

tkchin@webrtc.org changed reviewers:
+ mflodman@webrtc.org

[mflodman, 2016-08-03 07:05:25]

mflodman@webrtc.org changed reviewers:
+ stefan@webrtc.org

[mflodman, 2016-08-03 07:05:25]

Adding Stefan as extra reviewer since magjed is OOO this week.

[mflodman, 2016-08-03 10:54:49]

One comment, then LGTM for the functionality.

You should make sure to add more tests around this as well.

https://codereview.webrtc.org/2205763002/diff/1/webrtc/modules/video_coding/c...
File webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_encoder.mm
(right):

https://codereview.webrtc.org/2205763002/diff/1/webrtc/modules/video_coding/c...
webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_encoder.mm:275: if
(res.width == frame->width() && res.height == frame->height())
You can instead check:
if (!enable_scaling_ || (old check))
  return frame;

and remove the change below, one path less in this method.

https://codereview.webrtc.org/2205763002/diff/1/webrtc/sdk/objc/Framework/Cla...
File webrtc/sdk/objc/Framework/Classes/avfoundationvideocapturer.mm (right):

https://codereview.webrtc.org/2205763002/diff/1/webrtc/sdk/objc/Framework/Cla...
webrtc/sdk/objc/Framework/Classes/avfoundationvideocapturer.mm:578:
set_enable_video_adapter(false);
Agree this is godo to have here, but in practice it shouldn't be needed with 
https://codereview.webrtc.org/2206773002/
in place, right? Or is this in case the camera would open with an even lower
resolution?

[tkchin_webrtc, 2016-08-03 18:03:34]

I wish we had tests for this class, but we don't currently. Not immediately
clear how h/w encoder will behave in simulator, and I suspect that it'd be a lot
of work to stub out the actual encoder.

https://codereview.webrtc.org/2205763002/diff/1/webrtc/modules/video_coding/c...
File webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_encoder.mm
(right):

https://codereview.webrtc.org/2205763002/diff/1/webrtc/modules/video_coding/c...
webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_encoder.mm:275: if
(res.width == frame->width() && res.height == frame->height())
On 2016/08/03 10:54:49, mflodman_OOO wrote:
> You can instead check:
> if (!enable_scaling_ || (old check))
>   return frame;
> 
> and remove the change below, one path less in this method.

Done.

https://codereview.webrtc.org/2205763002/diff/1/webrtc/sdk/objc/Framework/Cla...
File webrtc/sdk/objc/Framework/Classes/avfoundationvideocapturer.mm (right):

https://codereview.webrtc.org/2205763002/diff/1/webrtc/sdk/objc/Framework/Cla...
webrtc/sdk/objc/Framework/Classes/avfoundationvideocapturer.mm:578:
set_enable_video_adapter(false);
On 2016/08/03 10:54:49, mflodman_OOO wrote:
> Agree this is godo to have here, but in practice it shouldn't be needed with 
> https://codereview.webrtc.org/2206773002/
> in place, right? Or is this in case the camera would open with an even lower
> resolution?

Technically not needed, but will skip some checks / lock grabbing at the very
least. Camera will not open in lower resolution.

[tkchin_webrtc, 2016-08-03 19:54:36]

Description was changed from

==========
Disable encoder scaling on iPhone4S.

Scaling causes us to work the CPU too much, which very quickly degrades quality.
This causes us to at least behave better on good networks.

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

to

==========
Disable encoder scaling on iPhone4S.

Scaling causes us to work the CPU too much, which very quickly degrades quality.
This causes us to at least behave better on good networks.

NOTRY=True
BUG=
==========

[tkchin_webrtc, 2016-08-03 19:55:50]

The CQ bit was checked by tkchin@webrtc.org

[tkchin_webrtc, 2016-08-03 19:55:50]

The patchset sent to the CQ was uploaded after l-g-t-m from mflodman@webrtc.org,
haysc@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2205763002/#ps100001 (title:
"Revert GYP file.")

[commit-bot: I haz the power, 2016-08-03 19:55:54]

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-08-03 19:57:12]

Description was changed from

==========
Disable encoder scaling on iPhone4S.

Scaling causes us to work the CPU too much, which very quickly degrades quality.
This causes us to at least behave better on good networks.

NOTRY=True
BUG=
==========

to

==========
Disable encoder scaling on iPhone4S.

Scaling causes us to work the CPU too much, which very quickly degrades quality.
This causes us to at least behave better on good networks.

NOTRY=True
BUG=
==========

[commit-bot: I haz the power, 2016-08-03 19:57:13]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-08-03 19:57:23]

Description was changed from

==========
Disable encoder scaling on iPhone4S.

Scaling causes us to work the CPU too much, which very quickly degrades quality.
This causes us to at least behave better on good networks.

NOTRY=True
BUG=
==========

to

==========
Disable encoder scaling on iPhone4S.

Scaling causes us to work the CPU too much, which very quickly degrades quality.
This causes us to at least behave better on good networks.

NOTRY=True
BUG=

Committed: https://crrev.com/6ce738da31fc3635295c27bea24605ea666f061e
Cr-Commit-Position: refs/heads/master@{#13630}
==========

[commit-bot: I haz the power, 2016-08-03 19:57:24]

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