Move kMinPixelsPerFrame constant in VideoStreamEncoder to VideoEncoder::ScalingSettings.

Move kMinPixelsPerFrame constant in VideoStreamEncoder to VideoEncoder::ScalingSettings. 

Make it possible for forced VP8 SW fallback encoder to set min_pixels_per_frame via GetScalingSettings().

Add a min required resolution (in addition to bitrate) before releasing forced SW fallback.

BUG=webrtc:6634
Review-Url: https://codereview.webrtc.org/3000693003
Cr-Commit-Position: refs/heads/master@{#19390}
Committed: https://chromium.googlesource.com/external/webrtc/+/142fcc96d60af65e05596925bea0fc9db2fa9c1d

[åsapersson, 2017-08-16 09:07:57]

Patchset #2 (id:20001) has been deleted

[åsapersson, 2017-08-16 10:09:01]

Patchset #2 (id:40001) has been deleted

[åsapersson, 2017-08-16 10:12:26]

Description was changed from

==========
Forced software encoder fallback, cont.

BUG=webrtc:6634
==========

to

==========
Move kMinPixelsPerFrame constant in VideoStreamEncoder to
VideoEncoder::ScalingSettings. 

Make it possible for forced VP8 SW fallback encoder to set min_pixels_per_frame
via GetScalingSettings().

Add a min required resolution (in addition to bitrate) before releasing forced
SW fallback.

BUG=webrtc:6634
==========

[åsapersson, 2017-08-16 10:12:26]

asapersson@webrtc.org changed reviewers:
+ brandtr@webrtc.org, kthelgason@webrtc.org, mflodman@webrtc.org

[åsapersson, 2017-08-16 12:32:24]

Patchset #5 (id:120001) has been deleted

[åsapersson, 2017-08-16 13:08:25]

Patchset #2 (id:60001) has been deleted

[kthelgason, 2017-08-16 13:42:23]

lgtm with small style comments.

https://codereview.webrtc.org/3000693003/diff/140001/webrtc/api/video_codecs/...
File webrtc/api/video_codecs/video_encoder.cc (right):

https://codereview.webrtc.org/3000693003/diff/140001/webrtc/api/video_codecs/...
webrtc/api/video_codecs/video_encoder.cc:72:
thresholds(rtc::Optional<QpThresholds>(QpThresholds(low, high))),
i prefer to defer to the other constructor when possible, but up to you. I know
it's not widely used in webrtc

https://codereview.webrtc.org/3000693003/diff/140001/webrtc/api/video_codecs/...
webrtc/api/video_codecs/video_encoder.cc:76: : enabled(on),
min_pixels_per_frame(min_pixels) {}
same here.

[brandtr, 2017-08-17 12:06:24]

https://codereview.webrtc.org/3000693003/diff/160001/webrtc/api/video_codecs/...
File webrtc/api/video_codecs/video_encoder.h (right):

https://codereview.webrtc.org/3000693003/diff/160001/webrtc/api/video_codecs/...
webrtc/api/video_codecs/video_encoder.h:91: const int min_pixels_per_frame = 320
* 180;
kMinPixelsPerFrame?

Also, place above the ctors:
https://google.github.io/styleguide/cppguide.html#Declaration_Order

https://codereview.webrtc.org/3000693003/diff/160001/webrtc/media/engine/vide...
File webrtc/media/engine/videoencodersoftwarefallbackwrapper.cc (right):

https://codereview.webrtc.org/3000693003/diff/160001/webrtc/media/engine/vide...
webrtc/media/engine/videoencodersoftwarefallbackwrapper.cc:250: if
(forced_fallback_possible_ && fallback_encoder_)
Unrelated to the forcing of the fallback, should we return the scaling settings
of |fallback_encoder_|, even in the HW failure case?

https://codereview.webrtc.org/3000693003/diff/160001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc (right):

https://codereview.webrtc.org/3000693003/diff/160001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc:462:
TEST_F(TestVp8Impl, ScalingDisabledIfAutomaticResizeOff) {
Nice addition.

[brandtr, 2017-08-17 13:08:32]

lgtm after offline discussion

[åsapersson, 2017-08-17 13:22:36]

https://codereview.webrtc.org/3000693003/diff/160001/webrtc/api/video_codecs/...
File webrtc/api/video_codecs/video_encoder.h (right):

https://codereview.webrtc.org/3000693003/diff/160001/webrtc/api/video_codecs/...
webrtc/api/video_codecs/video_encoder.h:91: const int min_pixels_per_frame = 320
* 180;
On 2017/08/17 12:06:23, brandtr wrote:
> kMinPixelsPerFrame?
> 
> Also, place above the ctors:
> https://google.github.io/styleguide/cppguide.html#Declaration_Order

min_pixels_per_frame can be set in the constructor.

https://codereview.webrtc.org/3000693003/diff/160001/webrtc/media/engine/vide...
File webrtc/media/engine/videoencodersoftwarefallbackwrapper.cc (right):

https://codereview.webrtc.org/3000693003/diff/160001/webrtc/media/engine/vide...
webrtc/media/engine/videoencodersoftwarefallbackwrapper.cc:250: if
(forced_fallback_possible_ && fallback_encoder_)
On 2017/08/17 12:06:24, brandtr wrote:
> Unrelated to the forcing of the fallback, should we return the scaling
settings
> of |fallback_encoder_|, even in the HW failure case?

Since not related to this CL, perhaps make this change separately.

[mflodman, 2017-08-17 13:44:20]

LGTM

[åsapersson, 2017-08-17 13:46:47]

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

[commit-bot: I haz the power, 2017-08-17 13:46:58]

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-08-17 14:56:39]

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

[commit-bot: I haz the power, 2017-08-17 14:56:40]

Dry run: This issue passed the CQ dry run.

[åsapersson, 2017-08-17 15:28:02]

The CQ bit was checked by asapersson@webrtc.org

[åsapersson, 2017-08-17 15:28:03]

The patchset sent to the CQ was uploaded after l-g-t-m from brandtr@webrtc.org,
kthelgason@webrtc.org, mflodman@webrtc.org
Link to the patchset: https://codereview.webrtc.org/3000693003/#ps180001 (title:
"rebase")

[commit-bot: I haz the power, 2017-08-17 15:28: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, 2017-08-17 15:58:55]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1502983682847310,
"parent_rev": "b5c319ad8bc86204825ddeb88dcc482348686ff5", "commit_rev":
"142fcc96d60af65e05596925bea0fc9db2fa9c1d"}

[commit-bot: I haz the power, 2017-08-17 15:59:01]

Description was changed from

==========
Move kMinPixelsPerFrame constant in VideoStreamEncoder to
VideoEncoder::ScalingSettings. 

Make it possible for forced VP8 SW fallback encoder to set min_pixels_per_frame
via GetScalingSettings().

Add a min required resolution (in addition to bitrate) before releasing forced
SW fallback.

BUG=webrtc:6634
==========

to

==========
Move kMinPixelsPerFrame constant in VideoStreamEncoder to
VideoEncoder::ScalingSettings. 

Make it possible for forced VP8 SW fallback encoder to set min_pixels_per_frame
via GetScalingSettings().

Add a min required resolution (in addition to bitrate) before releasing forced
SW fallback.

BUG=webrtc:6634

Review-Url: https://codereview.webrtc.org/3000693003
Cr-Commit-Position: refs/heads/master@{#19390}
Committed:
https://chromium.googlesource.com/external/webrtc/+/142fcc96d60af65e05596925b...
==========

[commit-bot: I haz the power, 2017-08-17 15:59:02]

Committed patchset #6 (id:180001) as
https://chromium.googlesource.com/external/webrtc/+/142fcc96d60af65e05596925b...