1279348 - Division by 0 in mozilla::VideoInfo::ScaledImageRect

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P1, critical)

Description

[Andrew McCreight [:mccr8]]

This bug was filed from the Socorro interface and is 
report bp-f2f10e9b-f15f-479b-9f01-88b422160608.
=============================================================

This is not a common crash (I only see 6 in crash-stats), but maybe it is easy to fix. I assume that either mImage.width or mImage.height is 0.

Comment 1

[Release mgmt bot [:sylvestre / :calixte / :marco for bugbug]]

Crash volume for signature 'mozilla::VideoInfo::ScaledImageRect':
 - nightly (version 50): 79 crashes from 2016-06-06.
 - aurora  (version 49): 0 crash from 2016-06-07.
 - beta    (version 48): 1 crash from 2016-06-06.
 - release (version 47): 3 crashes from 2016-05-31.
 - esr     (version 45): 0 crash from 2016-04-07.

Crash volume on the last weeks:
             Week N-1   Week N-2   Week N-3   Week N-4   Week N-5   Week N-6   Week N-7
 - nightly         22          6          1          6         19         23          2
 - aurora           0          0          0          0          0          0          0
 - beta             0          1          0          0          0          0          0
 - release          1          0          2          0          0          0          0
 - esr              0          0          0          0          0          0          0

Affected platform: Windows

Comment 2

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Bug 1279348 - Fix guard in scaledImageRect;

[mq]: 1279348_fix_guard.patch

Review commit: https://reviewboard.mozilla.org/r/69152/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69152/

Comment 3

[Jean-Yves Avenard [:jya]]

Comment on attachment 8777622 [details]
Bug 1279348 - Fix guard in scaledImageRect;

https://reviewboard.mozilla.org/r/69152/#review66234

::: dom/media/MediaInfo.h:264
(Diff revision 1)
>    // reports. This is legal in WebM, and we will preserve the ratio of the crop
>    // rectangle as it was reported relative to the picture size reported by the
>    // container.
>    nsIntRect ScaledImageRect(int64_t aWidth, int64_t aHeight) const
>    {
> -    if (aWidth == mImage.width && aHeight == mImage.height) {
> +    if (aWidth == mImage.width || aHeight == mImage.height) {

this will break aspect ratio where only one dimension changed.

Volume is probably low enough to not worry about it but still

Comment 4

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1279348: Avoid division by 0.

Review commit: https://reviewboard.mozilla.org/r/78580/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/78580/

Comment 5

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Comment on attachment 8791022 [details]
Bug 1279348: Avoid division by 0.

https://reviewboard.mozilla.org/r/78580/#review77696

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cdab1a072387
Avoid division by 0. r=kentuckyfriedtakahe

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cdab1a072387

Comment 8

[Frank-Rainer Grahl (:frg)]

Could you uplift this to m-b or at least m-a? 

SeaMonkey suite does crash reliably with webm when benchmarking using this page and/or trying to play some webm videos: https://www.youtube.com/html5 . 

I still wonder why Firefox is not affected but the fix seems to cure it.

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Too late for those releases.

Comment 10

[Ritu Kothari (:ritu)]

Hi Jean-Yves, do you think it's worthwhile uplifting this patch to Beta50?

Comment 11

[Jean-Yves Avenard [:jya]]

Comment on attachment 8791022 [details]
Bug 1279348: Avoid division by 0.

Approval Request Comment
[Feature/regressing bug #]:1243538
[User impact if declined]: Crashes (division by 0)
[Describe test coverage new/current, TreeHerder]: In central. Don't know of a way to reproduce it.
[Risks and why]: Low. Those are likely invalid file to start with. We simply check if one of the dimension is 0 and if so don't attempt to scale the image. 
[String/UUID change made/needed]: None

Comment 12

[Gerry Chang [:gchang]]

Comment on attachment 8791022 [details]
Bug 1279348: Avoid division by 0.

This patch fixes a crash(division by 0). Take it in 51 aurora.

Comment 13

[Gerry Chang [:gchang]]

Per comment# 16, it's already in 51.
Hi :jya,
Do you mean uplift to 50?

Comment 14

[Jean-Yves Avenard [:jya]]

I did mean 50.

Comment 15

[Jean-Yves Avenard [:jya]]

Comment on attachment 8791022 [details]
Bug 1279348: Avoid division by 0.

Approval Request Comment
[Feature/regressing bug #]:1243538
[User impact if declined]: Crashes (division by 0)
[Describe test coverage new/current, TreeHerder]: In central. Don't know of a way to reproduce it.
[Risks and why]: Low. Those are likely invalid file to start with. We simply check if one of the dimension is 0 and if so don't attempt to scale the image. 
[String/UUID change made/needed]: None

Comment 16

[Ritu Kothari (:ritu)]

Comment on attachment 8791022 [details]
Bug 1279348: Avoid division by 0.

Crash fix, Beta50+

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0a03ea21d564

Comment 18

[Frank-Rainer Grahl (:frg)]

Thanks for uplifting this.