Video corruption with Galaxy s3 (SCH i535)

RESOLVED FIXED in Firefox 53

Status

()

defect
P1
normal
RESOLVED FIXED
2 years ago
5 months ago

People

(Reporter: kbrosnan, Assigned: JamesCheng)

Tracking

(Blocks 1 bug, {regression})

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

Firefox Tracking Flags

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

Details

Attachments

(2 attachments)

Reporter

Description

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

Comment 1

2 years ago
Another report mentions the Galaxy S4.
John, Blake, can you take a look or help find an owner for this bug? Thanks.
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

2 years 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)
Tagging as release blocking due to it's severity.
Has anyone given a specific URL?
Never mind. No need for a URL. I see it. I'll see if I can track the regression.
Reporter

Comment 8

2 years 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)
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

2 years 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)
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
Assignee

Updated

2 years ago
Attachment #8863993 - Flags: review?(jolin)
Assignee

Comment 14

2 years 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

2 years 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

2 years 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

2 years ago
Assignee: nobody → jacheng
Comment hidden (mozreview-request)
Assignee

Comment 19

2 years 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
Yes, the APK you posted fixes the problem on my S3.
Flags: needinfo?(mozilla)
This is a really specific blacklist (and it blacklists all S3).

Is this what Google did?
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

2 years 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

2 years 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)
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

2 years 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-
Assignee

Updated

2 years ago
Blocks: 1362271

Comment 36

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8f8dc26b27eb
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55

Comment 37

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

Comment 38

2 years 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

2 years ago
Blocks: 1362918

Comment 39

2 years ago
Hello, 
Has there been a fix to this? I have the same issue with my Galaxy S4 L720 (sprints version) running Android 4.4.2 with the latest update of Firefox 55.0. How can I get this fixed. It's only on my S4 not on my galaxy tab 3.
Assignee

Comment 40

2 years ago
Hi kickstand03,

I will handle it in bug 1390022.

Thanks for your report.

Comment 41

2 years ago
It appears that this video corruption issue also affects the S4, model SCH-R970, Android 4.4.
Please add it to your blocklist. Thanks.

Firefox was run on the following to give these results:
R52 … Works
R53.0.2 … Corrupt
R54.0.1 … Corrupt
R55.0.2 … Corrupt
R56.0 … Corrupt
Assignee

Comment 42

2 years ago
Hi JackP,
I will handle it in bug 1412736, thanks for your report.

Comment 43

5 months ago

I was waiting for a bugfix of youtube playback on my L720 (S4) and T999L/T999 (S3) for a year using other mobile browsers for youtube playback until recently I found that nobody even planned to fix it considering it was already "fixed".
Why to google "Qualcomm Snapdragon based Samsung Galaxy S4" and "Qualcomm Snapdragon based Samsung Galaxy S3" if you can just wait for betatesting users to report about it, right? WRONG!
Because I know at least 3 people who dropped Firefox long time ago due to this bug and never went back even after switching to new phones.

Now imagine how many owners of SAMSUNG-SGH-T999, SGH-T999L, SGH-L720 did the same? 99% of them, I'd say.

Comment 44

5 months ago

Oh, yeah, SGH-L720T too... but who cares as S4 is so old now, no need to hassle blacklisting already, right?

(In reply to AjvarXX from comment #43)

I was waiting for a bugfix of youtube playback on my L720 (S4) and T999L/T999 (S3) for a year using other mobile browsers for youtube playback until recently I found that nobody even planned to fix it considering it was already "fixed".
Why to google "Qualcomm Snapdragon based Samsung Galaxy S4" and "Qualcomm Snapdragon based Samsung Galaxy S3" if you can just wait for betatesting users to report about it, right? WRONG!
Because I know at least 3 people who dropped Firefox long time ago due to this bug and never went back even after switching to new phones.

Now imagine how many owners of SAMSUNG-SGH-T999, SGH-T999L, SGH-L720 did the same? 99% of them, I'd say.

I'm very sorry to hear that you've been bothered by this bug for such a long time and appretiate the info about these models.

With our limited resource, it is very difficult to find and test every model for devices released by many carriers in different regions such as S4 and S3. And in this particular case (blacklisting), we felt reluctant to add models unless they are verified to be affected. That's why we rely heavily on users' reports and helps to find/investigate/fix/verify bugs like this one.

Bug 1518673 was filed and I will add T999* models to the blacklist there. As for L720(T), they should have been fixed by bug 1390022. If the symptom persists on your phone, then either the model string doesn't match, or there is another issue that we need to investigate. We will be grateful if you are willing to offer your help to verify that.

Thanks again for your feedback.

Flags: needinfo?(AjvarXX)

Comment 46

5 months ago

(In reply to John Lin [:jhlin][:jolin] from comment #45)

(In reply to AjvarXX from comment #43)

I'm very sorry to hear that you've been bothered by this bug for such a long time and appretiate the info about these models.

With our limited resource, it is very difficult to find and test every model for devices released by many carriers in different regions such as S4 and S3. And in this particular case (blacklisting), we felt reluctant to add models unless they are verified to be affected. That's why we rely heavily on users' reports and helps to find/investigate/fix/verify bugs like this one.

Bug 1518673 was filed and I will add T999* models to the blacklist there. As for L720(T), they should have been fixed by bug 1390022. If the symptom persists on your phone, then either the model string doesn't match, or there is another issue that we need to investigate. We will be grateful if you are willing to offer your help to verify that.

Thanks again for your feedback.

Well, thanks for kind words.
As my S4 is broken for some time already I trust that it's problem is solved as I can't test now anyway. I installed newest apk on S3 (bug was still there) and decided that same still goes for L720 too. Replied in new bugreport about S3.

offtopic: maybe general informational pop up after update would be useful for such things like "if you have problem with youtube on S3 models after v53 update please inform us by email" etc.

Flags: needinfo?(AjvarXX)

(In reply to AjvarXX from comment #46)

Well, thanks for kind words.
As my S4 is broken for some time already I trust that it's problem is solved as I can't test now anyway. I installed newest apk on S3 (bug was still there) and decided that same still goes for L720 too. Replied in new bugreport about S3.

offtopic: maybe general informational pop up after update would be useful for such things like "if you have problem with youtube on S3 models after v53 update please inform us by email" etc.

Thanks for staying with us and the suggestion. :)

You need to log in before you can comment on or make changes to this bug.