1360626 - Video corruption with Galaxy s3 (SCH i535)

Status: RESOLVED FIXED

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

Description

[Kevin Brosnan [Ex-Mozilla]]

Attachment: Example of video corruption

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

Comment 1

[Kevin Brosnan [Ex-Mozilla]]

Another report mentions the Galaxy S4.

Comment 2

[Liz Henry (:lizzard) (relman/hg->git project)]

John, Blake, can you take a look or help find an owner for this bug? Thanks.

Comment 3

[John Lin [:jhlin][:jolin]]

Kevin, was Android version mentioned in the reports? Thanks a lot.

Comment 4

[Kevin Brosnan [Ex-Mozilla]]

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.

Comment 5

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tagging as release blocking due to it's severity.

Comment 6

[Mike Kaply [:mkaply]]

Has anyone given a specific URL?

Comment 7

[Mike Kaply [:mkaply]]

Never mind. No need for a URL. I see it. I'll see if I can track the regression.

Comment 8

[Kevin Brosnan [Ex-Mozilla]]

Mike is working on a regression range as he has hardware. If there is a good backout candidate we may take that.

Comment 9

[Mike Kaply [:mkaply]]

This was caused by https://bugzil.la/1317239

I have verified it is still happening in current builds.

Mike

Comment 10

[Kevin Brosnan [Ex-Mozilla]]

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

Comment 11

[Mike Kaply [:mkaply]]

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.

Comment 12

[John Lin [:jhlin][:jolin]]

Looks like Chromium had met similar issues before: https://codereview.chromium.org/1869103002

Comment 13

[James Cheng[:JamesCheng]]

Attachment: Bug 1360626 - create a blacklist for adaptive playback support.

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

Comment 14

[James Cheng[:JamesCheng]]

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.

Comment 15

[John Lin [:jhlin][:jolin]]

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))`

Comment 16

[James Cheng[:JamesCheng]]

(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.

Comment 17

[James Cheng[:JamesCheng]]

Comment on attachment 8863993 [details]
Bug 1360626 - create a blacklist for adaptive playback support.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135718/diff/1-2/

Comment 18

[James Cheng[:JamesCheng]]

Comment on attachment 8863993 [details]
Bug 1360626 - create a blacklist for adaptive playback support.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135718/diff/2-3/

Comment 19

[James Cheng[:JamesCheng]]

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.

Comment 20

[Mike Kaply [:mkaply]]

Yes, the APK you posted fixes the problem on my S3.

Comment 21

[Mike Kaply [:mkaply]]

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

Is this what Google did?

Comment 22

[Kevin Brosnan [Ex-Mozilla]]

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/

Comment 23

[James Willcox (:snorp) ([e-mail]) (he/him)]

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.

Comment 24

[James Cheng[:JamesCheng]]

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

Comment 25

[James Cheng[:JamesCheng]]

Comment on attachment 8863993 [details]
Bug 1360626 - create a blacklist for adaptive playback support.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135718/diff/3-4/

Comment 26

[John Lin [:jhlin][:jolin]]

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 27

[James Cheng[:JamesCheng]]

Comment on attachment 8863993 [details]
Bug 1360626 - create a blacklist for adaptive playback support.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135718/diff/4-5/

Comment 28

[Mike Kaply [:mkaply]]

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.

Comment 29

[Liz Henry (:lizzard) (relman/hg->git project)]

I'd also like someone to land this so that I can start the 53.0.2 build as early as possible today.

Comment 30

[James Willcox (:snorp) ([e-mail]) (he/him)]

(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.

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8f8dc26b27eb
Create a blacklist for adaptive playback support. r=jolin

Comment 32

[James Willcox (:snorp) ([e-mail]) (he/him)]

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

Comment 33

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 34

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/f87a819106bd

Comment 35

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6022f5353130

Comment 36

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8f8dc26b27eb

Comment 37

[clgillum]

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

Comment 38

[Jens]

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.

Comment 39

[kickstand03]

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.

Comment 40

[James Cheng[:JamesCheng]]

Hi kickstand03,

I will handle it in bug 1390022.

Thanks for your report.

Comment 41

[JackP.]

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

Comment 42

[James Cheng[:JamesCheng]]

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

Comment 43

[AjvarXX]

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

[AjvarXX]

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

Comment 45

[John Lin [:jhlin][:jolin]]

(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.

Comment 46

[AjvarXX]

(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.

Comment 47

[John Lin [:jhlin][:jolin]]

(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. :)