Speculative fix for division by zero in Vp8EncoderImpl.

Speculative fix for division by zero in Vp8EncoderImpl.

BUG=chromium:597139
R=sprang@webrtc.org

Review-Url: https://codereview.webrtc.org/2743543004 .
Cr-Commit-Position: refs/heads/master@{#17167}
Committed: https://chromium.googlesource.com/external/webrtc/+/144475b3286f4de5059e0333044a72faf5d0171c

[stefan-webrtc, 2017-03-10 12:35:00]

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

[stefan-webrtc, 2017-03-10 12:35:23]

Description was changed from

==========
Speculative fix for division by zero in Encoder.

BUG=chromium:597139
==========

to

==========
Speculative fix for division by zero in Vp8EncoderImpl.

BUG=chromium:597139
==========

[stefan-webrtc, 2017-03-10 12:36:36]

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

[commit-bot: I haz the power, 2017-03-10 12:36:40]

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-10 12:38:57]

lgtm with nit

https://codereview.webrtc.org/2743543004/diff/1/webrtc/modules/video_coding/m...
File webrtc/modules/video_coding/media_optimization.cc (right):

https://codereview.webrtc.org/2743543004/diff/1/webrtc/modules/video_coding/m...
webrtc/modules/video_coding/media_optimization.cc:13: // #include <algorithm>
remove

[stefan-webrtc, 2017-03-10 12:39:25]

Fix nit.

[stefan-webrtc, 2017-03-10 12:39:42]

https://codereview.webrtc.org/2743543004/diff/1/webrtc/modules/video_coding/m...
File webrtc/modules/video_coding/media_optimization.cc (right):

https://codereview.webrtc.org/2743543004/diff/1/webrtc/modules/video_coding/m...
webrtc/modules/video_coding/media_optimization.cc:13: // #include <algorithm>
On 2017/03/10 12:38:57, språng wrote:
> remove

Done.

[tommi, 2017-03-10 12:55:45]

drive by lgtm

https://codereview.webrtc.org/2743543004/diff/20001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/media_optimization.cc (right):

https://codereview.webrtc.org/2743543004/diff/20001/webrtc/modules/video_codi...
webrtc/modules/video_coding/media_optimization.cc:136: return framerate;
should we have a check here?  At least RTC_DCHECK(framerate)?

[stefan-webrtc, 2017-03-10 13:06:55]

https://codereview.webrtc.org/2743543004/diff/20001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/media_optimization.cc (right):

https://codereview.webrtc.org/2743543004/diff/20001/webrtc/modules/video_codi...
webrtc/modules/video_coding/media_optimization.cc:136: return framerate;
On 2017/03/10 12:55:45, tommi (webrtc) wrote:
> should we have a check here?  At least RTC_DCHECK(framerate)?

framerate == 0 is handled outside this class, which I think may be reasonable
since it should be OK to compute a framerate of zero.

The issue I'm fixing here is that incoming_frame_rate_ can overflow when being
assigned to maxFramerate in VideoCodec in common_types.h, since that type was
uint8_t that wasn't too unlikely. With some bad luck that ends up being 0. :)

[stefan-webrtc, 2017-03-10 14:08:30]

Description was changed from

==========
Speculative fix for division by zero in Vp8EncoderImpl.

BUG=chromium:597139
==========

to

==========
Speculative fix for division by zero in Vp8EncoderImpl.

BUG=chromium:597139
R=sprang@webrtc.org

Review-Url: https://codereview.webrtc.org/2743543004 .
Cr-Commit-Position: refs/heads/master@{#17167}
Committed:
https://chromium.googlesource.com/external/webrtc/+/144475b3286f4de5059e03330...
==========

[stefan-webrtc, 2017-03-10 14:08:32]

Committed patchset #2 (id:20001) manually as
144475b3286f4de5059e0333044a72faf5d0171c (presubmit successful).