Closed Bug 1074262 Opened 6 years ago Closed 6 years ago

[Video] New video playback causes white flashing when play controls are visible

Categories

(Firefox OS Graveyard :: Gaia::Video, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.1+, b2g-v2.0 unaffected, b2g-v2.1 verified, b2g-v2.2 verified)

VERIFIED FIXED
2.1 S6 (10oct)
blocking-b2g 2.1+
Tracking Status
b2g-v2.0 --- unaffected
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: rpribble, Assigned: djf)

References

()

Details

(Keywords: regression, smoketest)

Attachments

(2 files)

Attached file Logcat.txt
Description:
Recording a new video then playing the new video with the player controls showing results in the video disappearing and screen flashing white instead.
   
Repro Steps:
1) Update a Flame device to BuildID: 20140929000203
2) Record a new video
3) Enter the video app
4) Play the video
5) Tap the screen to make the player controls appear
  
Actual:
Video disappears and flashes white when player controls are activated.

Expected: 
Videos play back normally.
  
Environmental Variables:
Device: Flame 2.1
BuildID: 20140929000203
Gaia: 063de64a4ffc606e931ed7b09e93282713c46eca
Gecko: 055d46b81ed1
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
  
Notes:
  
Repro frequency: 100%
Link to failed test case: https://moztrap.mozilla.org/manage/case/2478/
See attached: Video, logcat
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
QA Contact: jmitchell
QA Contact: jmitchell
[Blocking Requested - why for this release]:
blocking-b2g: --- → 2.0?
Blocking Reason: Broken experience (also nom was supposed to be for 2.1 based on the original description of the bug)

QA, A video would help. 

David, 

Could you please investigate?

Thanks
Hema
Assignee: nobody → dflanagan
blocking-b2g: 2.0? → 2.1+
Requesting a window, this blocks smoketests.
Flags: needinfo?(pbylenga)
Please see the attached video in the URL listed above.
QA Contact: pcheng
Rachel: how much memory is your Flame configured for (and does that affect this bug?)
Flags: needinfo?(rpribble)
The logcat indicates that the video is 720x480. These two messages are repeated over and over:

I/cat(248): <3>[  925.428909] mdp3_ppp_verify_scale: y req scale factor beyond capability
I/cat(248): <3>[  925.434599] mdp3_ppp_blit: invalid image!

I don't know what they mean, but will investigate. It makes me wonder, though, whether this bug affects both landscape and portrait mode videos or just one or the other.
One of those messages seems to be coming from this open-source android driver: https://android.googlesource.com/kernel/msm/+/android-msm-hammerhead-3.4-kk-r1/drivers/video/msm/mdss/mdp3_ppp.c

I'd suggest that the first step of looking for a regression window is to see whether it was the upgrade to the KK v180 base image that cause this regression.
Flags: needinfo?(pcheng)
I have not been able to reproduce this yet.

I've got a Flame flashed with the v180 base image and configured with 319mb of memory.
It was flashed with a nightly build  from late last week sometime.   I've updated gaia to master and to 2.1, but can't repro. I have not yet tried reflashing gecko yet, but am on slow public wifi right now so I may not be able to do that
Hi David, this bug is encountered when the Flame is set to 319mb memory and a full flash. I configured the memory to 512mb and no longer encountered the issue. The bug only seems to affect videos while recorded/played back in portrait mode.
Flags: needinfo?(rpribble)
(In reply to David Flanagan [:djf] from comment #7)
> I'd suggest that the first step of looking for a regression window is to see
> whether it was the upgrade to the KK v180 base image that cause this
> regression.

This issue is reproducible in 319 memory on:
KK v180 base image on reported build (full flash)
KK v165 base image on reported build (shallow flash).

This issue is NOT reproducible in 319 memory on:
JB v123 base image on reported build (shallow flash), however I observed that in 319 mem, the device can't record a video without it lagging terribly. The lagging issue is NOT observed in KK.
Flags: needinfo?(pcheng)
Branch checking results:

This issue is reproducible on latest Flame 2.1.

Observed behavior: After following STR, tapping on screen while viewing a video in Video app in portrait mode shows white screen flashing constantly.

Device: Flame (shallow flash)
BuildID: 20140930093624
Gaia: 89a13e9325948488fd4ae777a97f489189a7ed26
Gecko: 9550de0111d4
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

------------

This issue is NOT reproducible on Flame 2.2 master and on Flame 2.0. After following STR, tapping on screen while viewing a video in Video app in portrait mode correctly shows the controls of a video.

Device: Flame (shallow flash)
BuildID: 20140930061521
Gaia: 77ef35f5429bc3dfe9ca192b9aacc3c0bf8857de
Gecko: 2ae57957e4bb
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Device: Flame (shallow flash)
BuildID: 20140930095020
Gaia: 5c2303ec4e367da060aa1b807d541a6549b3d72a
Gecko: f19f4e07010a 
Version: 32.0 (2.0)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
We have landed a fix for Bug 1072013 in 2.2 that needs to be uplifted to 2.1. Since this bug has similar white flashing reported, it might be worth to check if the patch attached to 1072013 helps to fix issue reported in this bug in 2.1. Setting NI flag for piwei to try the patch in 1072013 on  Flame 2.1
Flags: needinfo?(pcheng)
Thanks, Punam. The change that Punam describes is one I've already taken a look at. If we run the video app from gaia master on gecko from 2.1, then we see the very same bug, except that the white flashing is replaced with black flashing!

So uplifting bug 1072013 is not the solution (though it might be part of it, I suppose).

But it does tell us that the flash of white we see is the default background of the app showing through.
I've confirmed that this bug only happens when videos are both recorded and played in portrait orientation or upside down portrait orientation.  For videos recorded in either landscape or upside down landscape the bug never occurs.  For videos recorded in portrait, you can stop the flickering by just rotating into landscape.

It is not the tap to bring up the playback controls that causes the bug, but the presence of the playback controls that cause it.  If you start the video in landscape mode and bring up the controls, then rotating to portrait mode will cause the flickering to start.

Also, this is not dependent on the video being playing. Start the video in landscape, bring up the controls, and then pause it. Rotate to portrait and the screen will go blank, and these lines appear in the console:

I/cat     (  228): <3>[ 1883.115369] mdp3_ppp_verify_scale: y req scale factor beyond capability
I/cat     (  228): <3>[ 1883.121412] mdp3_ppp_blit: invalid image!
I/cat     (  228): <3>[ 1883.483424] mdp3_ppp_verify_scale: y req scale factor beyond capability
I/cat     (  228): <3>[ 1883.489344] mdp3_ppp_blit: invalid image!
I/cat     (  228): <3>[ 1883.518078] mdp3_ppp_verify_scale: y req scale factor beyond capability
I/cat     (  228): <3>[ 1883.525162] mdp3_ppp_blit: invalid image!
I also see this line in the console four times every time I change the orientation of the phone: 

E/GeckoConsole(  208): [JavaScript Warning: "Error in parsing value for 'width'.  Declaration dropped." {file: "app://system.gaiamobile.org/index.html" line: 0 column: 0 source: "undefinedpx"}]

That message goes away if I update to the 2.2 system app (on top of 2.1 gecko) but the bug does not go away, so I'm going to assume that it is unrelated.
Milan,

This is looking to me like a gecko or gonk regression. Is there someone on your team who could investigate?

I'll keep investigating the gaia side of things to see to see if I can figure out anything more about what is going on, but it seems unlikely to me that I'll be able to solve this in gaia.
Flags: needinfo?(milan)
The STR include a step to record a video.  This is not necessary to trigger the bug. If you've got an already-recorded video, then you can see the bug by just viewing it in the video app, without every running the camera app, even after a reboot.
If I change these three lines in apps/camera/js/config/config.js:

-      // {
-      //   key: 'recorderProfiles'
-      // },
+      {
+        key: 'recorderProfiles'
+      },

Then the camera menu allows me to select video resolution. If I change it from the default 480p to 352x288, then I can't reproduce the bug on that video.
(In reply to Punam Dahiya from comment #12)
> We have landed a fix for Bug 1072013 in 2.2 that needs to be uplifted to
> 2.1. Since this bug has similar white flashing reported, it might be worth
> to check if the patch attached to 1072013 helps to fix issue reported in
> this bug in 2.1. Setting NI flag for piwei to try the patch in 1072013 on 
> Flame 2.1

I've applied the patch to 2.1 and can confirm that the findings in comment 13 is true. The issue is NOT fixed after applying the patch. The screen just changes from flashing white to flashing black.
Flags: needinfo?(pcheng)
This bug does not seem to reproduce when playing a video back in Camera or Gallery.  Both of those apps use shared/js/media/video_player.js which is different than what the Video app uses.

The video app (but not camera or gallery) displays the new <gaia-header> web component when the controls are visible. Those have probably changed more between 2.1 and master than the rest of the app has, so I should investigate there.

(I'll note that I did see one weird thing in the gallery app. When displaying and then pausing a video, the paused image would disappear when the controls came up. But this wouldn't happen when the video was playing. And it went away when I connected to wifi. Then it came back when I disconnected. Then went away. Then I couldn't reproduce anymore.  I don't know what was going on. Possibly related, but not closely enough related to investigate any further right now.)
Actually it can't be the gaia-header component, since we know that the bug still reproduces with gaia master on 2.1 gecko. Still could be something web components related, but not the component itself.
Looking into CSS interventions now...

If I remove the opacity property so that the controls are always visible, then then the flickering is always present.

If I leave the opacity, but remove the transition so the controls hide and show instantly, that has no effect.

If I comment out the z-index properties throughout, then the bug does not occur for the first video I watch, but it happens when I watch another large portrait video.
Ah ha! Adding will-change: opacity seems to fix this.
The whole 2.1 Aurora branch is affected with this bug. Therefore the following is a reverse-regression window to find out what FIXED the bug in 2.2 master branch.

mozilla-inbound reversed regression window:

Last Broken Environmental Variables:
Device: Flame
BuildID: 20140915220444
Gaia: e2d70bee03b5380ac327a145e5d694fb2443f018
Gecko: 3b7921328fc1
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

First Working Environmental Variables:
Device: Flame
BuildID: 20140915220644
Gaia: e2d70bee03b5380ac327a145e5d694fb2443f018
Gecko: 66bad3def025
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Gaia is the same. Gecko pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=3b7921328fc1&tochange=66bad3def025

Fixed by Bug 1051636. This confirms David's resolution as well.
Flags: needinfo?(jmitchell)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
But this bug was uplifted to Aurora/2.1, so that's a bit strange.  Perhaps more things are missing from Aurora?  Timothy, Roc is on vacation for a while, and you reviewed patch to bug 1051636, can you shed some light on this?
Flags: needinfo?(milan) → needinfo?(tnikkel)
Attached file workaround for v2.1
Attachment #8497862 - Flags: review+
The attachment is a patch for v2.1 that works around whatever the underlying graphics bug is with a will-change:opacity declaration.

I want to land this ASAP to fix the smoketest. Then follow up with layer tree analysis to make sure we haven't regressed performance with the change.

The patch should apply cleanly on master as well, but I attached the 2.1 version. For simplicity, I'm going to land it on master as well.
Landed on master: https://github.com/mozilla-b2g/gaia/commit/a1a31fa53f9718376048a696689889a7c2eaa915
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Filed bug 1075265 as a followup for the underlying graphics issue. I've set needinfo for Timothy on that one as well.
Filed bug 1075268 as a followup to check that the patch we landed here does not regress peformance by creating a more complex layer tree during video playback.
(In reply to Milan Sreckovic [:milan] from comment #25)
> But this bug was uplifted to Aurora/2.1, so that's a bit strange.  Perhaps
> more things are missing from Aurora?  Timothy, Roc is on vacation for a
> while, and you reviewed patch to bug 1051636, can you shed some light on
> this?

Bug 1073252 was a regression caused by bug 1051636. In bug 1073252 the patch from bug 1051636 got backed out of aurora, but the back out was #ifdef'd to b2g only. So bug 1051636 is not on aurora as far as b2g is concerned. The back out was a temporary measure, the proper fix would take more time.

So if we land bug 1051636 on aurora we will regress bug 1073252. To fix both of these bugs properly we need the proper fix for bug 1073252.
Flags: needinfo?(tnikkel)
(In reply to Timothy Nikkel (:tn) from comment #32)
> So if we land bug 1051636 on aurora we will regress bug 1073252. To fix both
> of these bugs properly we need the proper fix for bug 1073252.

Short of that, we should verify that the work around that landed here works reasonably well.
Lets get this tested on tomorrow's build.   Rachel, during your verification on 2.1, please be aware of the following:

1. this bug is fixed as originally reported
2. check that bug 1072013 should not be reproducing either
3. check the scenario that david mentions in comment 14 and 20 is fixed.

David, timothy, anything else should be verified?

Thanks,
Tony
Flags: needinfo?(rpribble)
Keywords: verifyme
(In reply to Timothy Nikkel (:tn) from comment #32)

> Bug 1073252 was a regression caused by bug 1051636. In bug 1073252 the patch
> from bug 1051636 got backed out of aurora, but the back out was #ifdef'd to
> b2g only. So bug 1051636 is not on aurora as far as b2g is concerned. The
> back out was a temporary measure, the proper fix would take more time.
> 
> So if we land bug 1051636 on aurora we will regress bug 1073252. To fix both
> of these bugs properly we need the proper fix for bug 1073252.

Thanks for explaining this, Timothy. I see in bug 1073252 that Roc thought that a proper fix would be easy and only went the backout route because of time pressure from CAF and release management.

But now we also have this bug.  And I suspect that the white flash in bug 1072013 is related as well.  (We just landed a patch for that, but maybe all it is doing is hiding the white flash by making it black.)

So bug 1051636 is now beginning to manifest in at least two ways in the Video app. I don't like the trend here, and if it pops up in other ways, I don't know if we'll be able to work around it.  I'd suggest that we go back to the real fix that Roc suggested in 1072013 so we can land a proper fix.
Flags: needinfo?(tnikkel)
(In reply to Tony Chung [:tchung] from comment #34)

> David, timothy, anything else should be verified?
> 

Timothy has a suggestion for something else to verify over in https://bugzilla.mozilla.org/show_bug.cgi?id=1075265#c4

He'd like someone to back out the gaia fix from this bug, and back out the back out from bug 1072013 (effectively re-landing 1051636) to see if that also fixes this bug.  Though I suppose this may be what Pi Wei has already done in comment #24. What I'd add, though is a request to see whether in addition to fixing the visual problem that also gets rid of the scary mdp3_ppp messages in the logcat.
Flags: needinfo?(tchung)
(In reply to David Flanagan [:djf] from comment #35)
> So bug 1051636 is now beginning to manifest in at least two ways in the
> Video app. I don't like the trend here, and if it pops up in other ways, I
> don't know if we'll be able to work around it.  I'd suggest that we go back
> to the real fix that Roc suggested in 1072013 so we can land a proper fix.

Yes, I agree, we should do the proper fix mentioned in bug 1073252.
Flags: needinfo?(tnikkel)
Verified fixed on today's Flame v2.1 KK (319mb) and Flame v2.2 KK (319mb).
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(rpribble) → needinfo?(pbylenga)
Keywords: verifyme
Please keep QA posted.  if the patch in 1073252 is fixed and sticks, then we can suggest backing this one out afterwards.
Flags: needinfo?(tchung)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(pbylenga)
Target Milestone: --- → 2.1 S6 (10oct)
You need to log in before you can comment on or make changes to this bug.