Closed Bug 1232911 Opened 4 years ago Closed 3 years ago

GIFV files crash Firefox

Categories

(Firefox for Android :: Audio/Video, defect, P1)

43 Branch
defect

Tracking

()

RESOLVED FIXED
Firefox 48
Tracking Status
firefox48 --- fixed
fennec - ---

People

(Reporter: captpackrat, Assigned: esawin)

Details

(Keywords: crash)

Crash Data

Attachments

(3 files, 3 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0
Build ID: 20151208100201

Steps to reproduce:

Open a link to a GIFV file, such as https://imgur.com/YilRiRQ (work safe).


Actual results:

The web page is displayed for a split second, then the Firefox crash report screen appears.


Expected results:

The web page should load and the video should play.
This likely will be device specific. Please let us know the model and the Android version. Getting a crash ID would be a great help as well, https://support.mozilla.org/en-US/kb/mozillacrashreporter
Samsung Galaxy Note 10.1 (GT-N8013) with Android 4.1.2.

There is no details button on the crash report screen in Firefox for Android, and the about:crashes only lists one report from over a month ago, even though I've submitted dozens of reports since.  I think this is the same issue, however, it's been happening for a couple months.  The report is here:  https://crash-stats.mozilla.com/report/index/5abd28ff-c12a-4948-bbe2-62b802151124
Status: UNCONFIRMED → NEW
tracking-fennec: --- → ?
Crash Signature: [@ libstagefright.so@0x7a3e6 ]
Ever confirmed: true
It seems there is an OS update for your device that might fix this issue. Can you give that a try?
tracking-fennec: ? → -
Flags: needinfo?(captpackrat)
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #3)
> It seems there is an OS update for your device that might fix this issue.
> Can you give that a try?

Several of the N80xx models have updates to 4.4.2, but for some reason Samsung decided to screw everyone who bought the N8013 (which appears to be the USA WiFi-only model), it's still stuck at 4.1.2.  http://samsung-updates.com/device/?id=gt-n8013  The last update we got was a security update (probably for Stagefright) in September.  I have installed this update, nothing else is available through OTA or Kies.
Flags: needinfo?(captpackrat)
Keywords: crash
Priority: -- → P1
The best we can do here is block using the Android hardware decoder for WebM on this device.

To do this, block the Android PDM from returning true for VP8 or VP9 in  AndroidDecoderModule::SupportsMimeType(). You can use VPXDecoder::IsVPX(aMimeType) to tell if aMimeType is VP8 or VP9.
Eugen, looks like we need an easy way to block some devices from using MediaCodec for WebM.
Assignee: nobody → esawin
We can just reuse the abandoned patches from bug 1192539.
We need to test for the specific VPX version in ADM in TranslateMimeType and to be future-proof in the new blacklisting mechanics.

Alternatively, we can add dedicated IsVP8 and IsVP9 functions.
Attachment #8732993 - Flags: review?(cpearce)
Allows us to block devices with broken VPX support.
Attachment #8732995 - Flags: review?(snorp)
Use the blocking mechanics in ADM.
Attachment #8732996 - Flags: review?(snorp)
Comment on attachment 8732993 [details] [diff] [review]
0001-Bug-1232911-1.1-Allow-to-test-for-specific-VPX-MIME-.patch

Review of attachment 8732993 [details] [diff] [review]:
-----------------------------------------------------------------

I'll let jya make the call about this.
Attachment #8732993 - Flags: review?(cpearce) → review?(jyavenard)
Comment on attachment 8732993 [details] [diff] [review]
0001-Bug-1232911-1.1-Allow-to-test-for-specific-VPX-MIME-.patch

Review of attachment 8732993 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/platforms/agnostic/VPXDecoder.cpp
@@ +218,2 @@
>  {
> +  return aCodecMask & VPXDecoder::VP8 &&

Please add parenthesis across & and &&, this will cause warnings if compiled with clang

  return ((aCodecMask & VPXDecoder::VP8) &&
         aMimeType.EqualsLiteral("video/webm: codecs=vp8")) ||
         ((aCodecMask & VPXDecoder::VP9) &&
         aMimeType.EqualsLiteral("video/webm: codecs=vp9"));
Attachment #8732993 - Flags: review?(jyavenard) → review+
Attachment #8732996 - Flags: review?(snorp) → review+
Attachment #8732995 - Flags: review?(snorp) → review+
Fixed a typo in the MIME type.
Attachment #8732993 - Attachment is obsolete: true
Attachment #8735956 - Flags: review+
We need to handle the new features' case in GetPrefNameForFeature, however, we cannot provide a pref name for them, otherwise we would try to access prefs off main thread.

With this patch, we would avoid any assertions and OMT prefs access without having to re-implement/change GfxInfo state handling, but I think it's not a nice workaround. I'm open for alternative ideas.
Attachment #8732995 - Attachment is obsolete: true
Attachment #8735960 - Flags: review?(snorp)
Attachment #8735960 - Flags: review?(snorp) → review+
You need to log in before you can comment on or make changes to this bug.