Video corruption with Galaxy s3 (SCH i535)

RESOLVED FIXED in Firefox 53

Status

()

Firefox for Android
Audio/Video
P1
normal
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: kbrosnan, Assigned: JamesCheng)

Tracking

(Blocks: 1 bug, {regression})

Trunk
Firefox 55
ARM
Android
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53blocking fixed, firefox54+ fixed, firefox55+ fixed, fennec53+)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

3 months ago
Created attachment 8862934 [details]
Example of video corruption

Seeing a few reports of corrupted video. One user has a North American Galaxy s3 (SCH i535)
(Reporter)

Comment 1

3 months ago
Another report mentions the Galaxy S4.
John, Blake, can you take a look or help find an owner for this bug? Thanks.
tracking-firefox53: ? → +
tracking-firefox54: ? → +
tracking-firefox55: ? → +
Flags: needinfo?(jolin)
Flags: needinfo?(bwu)
Kevin, was Android version mentioned in the reports? Thanks a lot.
Flags: needinfo?(kbrosnan)
Flags: needinfo?(jolin)
Flags: needinfo?(bwu)
(Reporter)

Comment 4

3 months ago
Samsung Galaxy Tab 4 is another affected device.

This only happens with the North American variant of the phones, Qualcom SOC. The international variants use Samsung Exynos SOCs seem not to reproduce. Android versions are 4.2 - 4.4. It may be easier for someone on Snorp's team to get a NA device.
Flags: needinfo?(kbrosnan)

Comment 5

3 months ago
Tagging as release blocking due to it's severity.
tracking-firefox53: + → blocking

Comment 6

3 months ago
Has anyone given a specific URL?

Comment 7

3 months ago
Never mind. No need for a URL. I see it. I'll see if I can track the regression.
(Reporter)

Comment 8

3 months ago
Mike is working on a regression range as he has hardware. If there is a good backout candidate we may take that.
Flags: needinfo?(mozilla)

Comment 9

3 months ago
This was caused by https://bugzil.la/1317239

I have verified it is still happening in current builds.

Mike
Flags: needinfo?(mozilla)
(Reporter)

Comment 10

3 months ago
The full range is https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=47f42f21541b9b98ad7db82edb996b29065debd0&tochange=34fce7c12173bdd6dda54c2ebf6d344252f1ac48

Mike and I agree that 1317239 is the best candidate from that range. There are a couple possibilities

* backout 1317239 - this would remove adaptive streaming from video elements for all devices
* require Android 5 (API 21) for the feature
* blacklist some SOC/GPU combinations
Blocks: 1317239
Flags: needinfo?(jacheng)
Keywords: regressionwindow-wanted

Comment 11

3 months ago
Note that this is holding up 53.0.2 which we need quickly for a partner preload issue, so we'd like to get an answer ASAP.
Looks like Chromium had met similar issues before: https://codereview.chromium.org/1869103002
Comment hidden (mozreview-request)
(Assignee)

Updated

3 months ago
Attachment #8863993 - Flags: review?(jolin)
(Assignee)

Comment 14

3 months ago
Hi Mike, 

Since I don't have Samsung Galaxy S3 or any infected devices, I can't confirm this symptom will be solved by this patch.

I would like to do

1. Let this patch be landed in nightly.

2. Need your help to confirm this issue is solved on Galaxy S3.

3. Request uplifting to beta.

Thanks.
Flags: needinfo?(jacheng)

Comment 15

3 months ago
mozreview-review
Comment on attachment 8863993 [details]
Bug 1360626 - create a blacklist for adaptive playback support.

https://reviewboard.mozilla.org/r/135718/#review138732

::: commit-message-a748a:1
(Diff revision 1)
> +Bug 1360626 - Blacklisted the adaptive playback support.

Nit: 'create a blacklist for adaptive playback support'

::: commit-message-a748a:2
(Diff revision 1)
> +Bug 1360626 - Blacklisted the adaptive playback support.
> +

Describe briefly why blacklisting is needed.

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/HardwareCodecCapabilityUtils.java:67
(Diff revision 1)
>        }
>      }
>      return false;
>    }
>  
> +  // See Bug1360626 and

Nit: put this method below `checkSupportsAdaptivePlayback()`

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/HardwareCodecCapabilityUtils.java:83
(Diff revision 1)
> +    if (!Build.MANUFACTURER.toLowerCase(Locale.getDefault()).equals("samsung")) {
> +      return false;
> +    }
> +
> +    return Build.MODEL.startsWith("GT-I9300") || // S3 (I9300 / I9300I)
> +           Build.MODEL.startsWith("SCH-I535"); // S3

Galaxy S4 was also mentioned in Kevin's bug comment, perhaps we should list that too?

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/HardwareCodecCapabilityUtils.java:89
(Diff revision 1)
> +  }
> +
>    @WrapForJNI
>    public static boolean checkSupportsAdaptivePlayback(MediaCodec aCodec, String aMimeType) {
>        // isFeatureSupported supported on API level >= 19.
>        if (!(Build.VERSION.SDK_INT >= 19)) {

Nit:
`if (Build.VERSION.SDK_INT < Build.VERSION_CODES.KITKAT || isAdaptivePlaybackBlacklisted(aMimeType))`
Attachment #8863993 - Flags: review?(jolin) → review+
(Assignee)

Comment 16

3 months ago
(In reply to John Lin [:jolin][:jhlin] from comment #15)
> Comment on attachment 8863993 [details]
> Bug 1360626 - Blacklisted the adaptive playback support.
> 
> https://reviewboard.mozilla.org/r/135718/#review138732
> 
> ::: commit-message-a748a:1

> mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/
> HardwareCodecCapabilityUtils.java:83
> (Diff revision 1)
> > +    if (!Build.MANUFACTURER.toLowerCase(Locale.getDefault()).equals("samsung")) {
> > +      return false;
> > +    }
> > +
> > +    return Build.MODEL.startsWith("GT-I9300") || // S3 (I9300 / I9300I)
> > +           Build.MODEL.startsWith("SCH-I535"); // S3
> 
> Galaxy S4 was also mentioned in Kevin's bug comment, perhaps we should list
> that too?

Hi Kevin,

Since the chromium code didn't consider Galaxy S4 for this case,

Could you please double confirm if these symptom really happened on S4 and provide me some detail information about the android version and SOC info?

Since S4 https://en.wikipedia.org/wiki/Samsung_Galaxy_S4#Model_variants used various of SOC model for different countries, it is hard for us to blacklist all the infected devices.

Thanks.
Flags: needinfo?(kbrosnan)
Comment hidden (mozreview-request)
(Assignee)

Updated

3 months ago
Assignee: nobody → jacheng
Comment hidden (mozreview-request)
(Assignee)

Comment 19

3 months ago
Hi Mike and Kevin,

I've pushed try and the apk which includes the patch can be downloaded from

https://queue.taskcluster.net/v1/task/RKGWvzY6StWn0jHxzn9A1A/runs/0/artifacts/public/build/target.apk

Would you please install the app and help to check if the symptom can be solved by this APK on S3?

For S4, I expect the symptom can still be reproduced and also need Kevin's information about the device Model for S4.


Thank you.
Flags: needinfo?(mozilla)
tracking-fennec: ? → 53+
Priority: -- → P1

Comment 20

3 months ago
Yes, the APK you posted fixes the problem on my S3.
Flags: needinfo?(mozilla)

Comment 21

3 months ago
This is a really specific blacklist (and it blacklists all S3).

Is this what Google did?
(Reporter)

Comment 22

3 months ago
These are user reports mostly without model numbers. 

https://support.mozilla.org/en-US/questions/1157981
https://support.mozilla.org/en-US/questions/1158597
https://support.mozilla.org/en-US/questions/1158437 <- claims to run a S4
https://support.mozilla.org/en-US/questions/1158516 <- claims to run a Galaxy Tab 4
https://support.mozilla.org/en-US/questions/1158443
https://www.reddit.com/r/firefox/comments/67oxvz/squiggly_grey_box_video_on_video_sites_like/
Flags: needinfo?(kbrosnan)
It looks like there a couple nits on the patch. James, can you clean that up and land it when you get in? The release folks want to spin a dot release ASAP.
Flags: needinfo?(jacheng)
(Assignee)

Comment 24

3 months ago
Hi snorp, Mike,

I've fixed the nits as John mentioned but my patch only handled Galaxy S3 cases as Google did in [1]

Do you agree that I land it first and create a follow up bug to enlarge the blacklist to cover some sorts of S4 or S4 tab cases?

Since https://en.wikipedia.org/wiki/Samsung_Galaxy_S4#Model_variants has various type of chips that I want to forbidden the failure ones instead of all types of model.


[1]
https://codereview.chromium.org/1869103002/patch/40001/50004
Flags: needinfo?(jacheng) → needinfo?(snorp)
Comment hidden (mozreview-request)

Comment 26

3 months ago
mozreview-review
Comment on attachment 8863993 [details]
Bug 1360626 - create a blacklist for adaptive playback support.

https://reviewboard.mozilla.org/r/135718/#review138750

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/HardwareCodecCapabilityUtils.java:102
(Diff revisions 3 - 4)
>  
>      if (!Build.MANUFACTURER.toLowerCase(Locale.getDefault()).equals("samsung")) {
>        return false;
>      }
>  
> -    return Build.MODEL.startsWith("GT-I9300") || // S3 (I9300 / I9300I)
> +    final String[] blacklist =

Nit: make this `static` class variable.
Comment hidden (mozreview-request)

Comment 28

3 months ago
Is this still a blocker for 53.0.2?

I'm running up against a hard deadline for a partner, so I was hoping 53.0.2 would spin today.

Snorp: Are you ok with his patch? Wondering if we can push.
I'd also like someone to land this so that I can start the 53.0.2 build as early as possible today.
(In reply to James Cheng[:JamesCheng] from comment #24)
> Hi snorp, Mike,
> 
> I've fixed the nits as John mentioned but my patch only handled Galaxy S3
> cases as Google did in [1]
> 
> Do you agree that I land it first and create a follow up bug to enlarge the
> blacklist to cover some sorts of S4 or S4 tab cases?

I think that's fine. We can land this patch even with the small nit to get 53.0.2 out.
Flags: needinfo?(snorp)

Comment 31

3 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/8f8dc26b27eb
Create a blacklist for adaptive playback support. r=jolin
Comment on attachment 8863993 [details]
Bug 1360626 - create a blacklist for adaptive playback support.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1317239
[User impact if declined]: Video corruption on some devices
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: Would be helpful. Simply play a video on an affected device such as a Galaxy S3.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: It only disables a feature that was only recently added on a subset of devices.
[String changes made/needed]: None
Attachment #8863993 - Flags: approval-mozilla-release?
Attachment #8863993 - Flags: approval-mozilla-beta?
Attachment #8863993 - Flags: approval-mozilla-aurora?
Comment on attachment 8863993 [details]
Bug 1360626 - create a blacklist for adaptive playback support.

Great, let's get this on release and beta. 
We don't need it on aurora now because of Project Dawn.
Attachment #8863993 - Flags: approval-mozilla-release?
Attachment #8863993 - Flags: approval-mozilla-release+
Attachment #8863993 - Flags: approval-mozilla-beta?
Attachment #8863993 - Flags: approval-mozilla-beta+
Attachment #8863993 - Flags: approval-mozilla-aurora?
Attachment #8863993 - Flags: approval-mozilla-aurora-

Comment 34

3 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-release/rev/f87a819106bd
status-firefox53: affected → fixed

Comment 35

3 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/6022f5353130
status-firefox54: affected → fixed
(Assignee)

Updated

3 months ago
Blocks: 1362271

Comment 36

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8f8dc26b27eb
Status: NEW → RESOLVED
Last Resolved: 3 months ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55

Comment 37

3 months ago
I have this same issue.  Galaxy S4 running Android 4.4.4
Model SGH-M919

Comment 38

3 months ago
Same here. Samsung Galaxy S4 GT-I9505 Android 4.4.2 / https://queue.taskcluster.net/v1/task/RKGWvzY6StWn0jHxzn9A1A/runs/0/artifacts/public/build/target.apk -no change here.
(Assignee)

Updated

3 months ago
Blocks: 1362918
You need to log in before you can comment on or make changes to this bug.