Somtimes FTU videos don't show up

VERIFIED FIXED in Firefox OS v2.0

Status

Firefox OS
Gaia::First Time Experience
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: sfoster, Assigned: sfoster)

Tracking

unspecified
2.0 S5 (4july)
x86_64
Linux

Firefox Tracking Flags

(blocking-b2g:2.0+, b2g-v2.0 verified, b2g-v2.1 verified)

Details

(Whiteboard: [systemsfe])

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
Occassionally the videos in the FTU tutorial dont appear when you go from step to step. If you hit 'back' they usually show up. 

These are .mp4 files and as we switch step, the URL for the file is assigned to the video element's .src. The video has attributes preload="auto" autoplay hidden loop. When doing the initial implementation I tried calling .play() after assigning to .src, but found it less reliable. We may need to recreate the <video> or employ some other tricks to get the kinks out of this
blocking-b2g: --- → 2.0+
Whiteboard: [systemsfe]
Sam,

Can you take a look at this one?
Flags: needinfo?(sfoster)
(Assignee)

Comment 2

4 years ago
Yes I'll get right on this
Assignee: nobody → sfoster
Flags: needinfo?(sfoster)
Target Milestone: --- → 2.0 S4 (20june)
(Assignee)

Comment 3

4 years ago
I caught this red-handed on my Flame going from step to step 2 in the FTU tutorial, and got the following from logcat: 

E/GeckoConsole(22844): Content JS LOG at app://ftu.gaiamobile.org/js/tutorial.js:47 in loadMedia: loadMedia, args:  [object XrayWrapper [object HTMLVideoElement]] /style/images/tutorial/Notifications.mp4
E/GeckoConsole(22844): [JavaScript Warning: "Media resource app://ftu.gaiamobile.org/style/images/tutorial/Notifications.mp4 could not be decoded." {file: "app://ftu.gaiamobile.org/index.html#browser_privacy" line: 0}]
E/GeckoConsole(22844): Content JS LOG at app://ftu.gaiamobile.org/js/tutorial.js:51 in onMediaLoadOrError: onMediaLoadOrError:  [object XrayWrapper [object Event]]
E/GeckoConsole(22844): [JavaScript Error: "Error: Failed to load media: /style/images/tutorial/Notifications.mp4" {file: "app://ftu.gaiamobile.org/js/tutorial.js" line: 59}]

Going back and then forward again the video loaded and played just fine. I'm not sure why it would not decode the first time - or if this error message is accurate? Hema, can you point me at someone or some resource that might help me diagnose and troubleshoot this? Probably its some rookie mistake I'm making :)
Flags: needinfo?(hkoka)
(Assignee)

Comment 4

4 years ago
David I'm looking for pointers here on this "Media resource could not be decoded" intermittent error. It appears to be the cause of a few bugs filed against the FTU tutorial - which now uses <video> and .mp4. 
My latest pull request(1) takes care in each step to assign src to the <video>, call load() and then play() (see 2) after the first loadeddata event, but we are still seeing an issue where sometimes the video loads, sometimes not. logcat shows the above decoding error. 


1. https://github.com/mozilla-b2g/gaia/pull/20501
2. https://github.com/mozilla-b2g/gaia/pull/20501/files#diff-1f75d3935d5db95cdb2957fec7688864R64
Flags: needinfo?(dflanagan)
Sam,

Try adding the "deprecated-hwvideo" permission to your manifest file. (See gallery, music and video for examples.)  I honestly don't know if that is needed anymore, however.

Make sure that when you switch from one screen to the next that you have unloaded the old video before you set the src for the new video. Our phones typically only have a single hardware decoder for mp4 videos, so you can't play two at a time. To get rid of the old video set src to '' (not null!) or use removeAttribute('src') on the video and then call load(). This will force it to unload. If you are already doing that, then maybe there is a race condition and you need to wait for an event or add a timeout before setting the src on the next video.

If that doesn't fix it, then it is probably not a gaia issues and our Media front-end team won't be able to help. If this turns out to be a gonk issue, then Sotaro is who I would turn to. But if it is gecko, I'd ask for help on IRC in the #media channel. 

Possibly your videos can't be decoded because there is something funky about their encoding. Someone on #media ought to be able to take a look and see if there's something weird in there.  But you could also try transcoding them to .webm or something to see if that makes a difference.

What devices is this occurring on? If this is Flame specific, I'd guess that it is a driver issue. Have you updated to the latest base image?

If I take those videos from the tutoral and push them to my flame, then the video app cannot play them.  If If I push them to my Tarkao, they work fine.  So I suspect this is a driver issue with the Flame.  There have been other video bugs being filed recently, so this might just be a dupe of those. I'm not sure what the bugs are, but search bugzilla for recent gaia bugs on video playback, or ask Marcia if she can tell you what has been filed recently.

The fact that this is intermittent is what seems interesting about it. Do you see any difference in the symptoms when you run FTU after a reboot and when you re-run it from the developer menu?

What I see when I try running on the flame (which I have updated to the latest base image) is that the first video runs the first time, and then no other videos run, even when I go back to the first video.  

In the logcat I see this message:

   E/OMX-VDEC-1080P(  295): Error: Insufficient size allocated for extra-data
Flags: needinfo?(dflanagan)
(Assignee)

Comment 6

4 years ago
Thanks for all of that, its a massive help. I've updated my pull request with the change to assign src = '' and load() before assigning the new src. That passes unit tests locally, and improves matters on device but I was still able to get the same 'Media resource could not be decoded' error by going back and next quickly several times on the FTU. I'm guarding against overlapping events and load race-conditions, but it looks to me like even the src='' and load(), then setTimeout isn't enough to truly unload the previous video Is there some event I could listen for to know the decoder is available before loading the new video maybe? 

This is not Flame-specific, though I bricked my Open-C so I've not tested there in a few days.

If you think it will help, I can try adding the deprecated-hwvideo manifest property, but the strange thing here is that it works mostly, just not every time, so I'm not sure thats relevant here. 

I see the same 'Insufficient size allocated for extra-data' message in logcat, but I see that in both the success and failure conditions.
Flags: needinfo?(hkoka) → needinfo?(dflanagan)
(Assignee)

Comment 7

4 years ago
btw I can reproduce the launching the FTU from the developer settings just the same way as after a make reset-gaia
If you're saying that this bug only occurs when you go quickly back and forth between screens, then it does sound like a race condition where sometimes you are not fully unloading the old video before loading the new one. I don't know if a "load" event is fired when you load src='', but if so, maybe you could use that before transitioning backward or forward.

Or, you could try having only a single <video> element that you dynamically insert into the current page. That way loading a new video would implicitly unload the previous one, I'd think.

This is all guesswork, of course.  Hope it helps.
Flags: needinfo?(dflanagan)
(Assignee)

Comment 9

4 years ago
OK, I looked into what gets fired when I assign videoNode.src = ''; and it turns out I get an exception complaining about the invalid resource. So I switched to removeAttribute('src') and so far so good - I've not been able to repro locally. I've updated the pull request and we'll see. My last fallback is as you suggest - to destroy and replace the <video> element, but I'm hoping to avoid that if possible. 

Try run: https://tbpl.mozilla.org/?tree=Gaia-Try&rev=445c462de35f181562ec87a329b58d663f423b20
(Assignee)

Comment 10

4 years ago
The root cause of the intermittent bug here was in how I was assigning a video src to the element: the previous video wasnt always unloaded fully and we got decoding errors. This was compounded by a set of steps that needed to occur during initialization and tutorial-step-setting which was timing-error prone and difficult to debug. 

I ended up refactoring the initialization and its various steps in a sequence of tasks which seems to help. Also all the tutorial unit tests now use a mock XmlHttpRequest. 

The PR is updated and hopefully try will come up green: http://mzl.la/SZSFkG. 

I triggered the last try run 5 times: https://tbpl.mozilla.org/?tree=Gaia-Try&rev=445c462de35f181562ec87a329b58d663f423b20  ...and the unit tests came up green (modulo a couple of unrelated issues). This last PR just removes the console logging, so I'm optimistic we'll be able to close out bug 1023179 as well with these changes.
(Assignee)

Comment 11

4 years ago
Created attachment 8440512 [details] [review]
PR: Fix decoding/video unloading error

The main fix here is the steps to removing the 'src' attribute, load(), assign new src, load(). As the decoding error was getting swallowed and both test and user-facing errors were intermittent, I refactored init to make it both more consistent and predictable and easier to troubleshoot and extend. While it might be possible to come up with a smaller patch, I think these changes are worth making now.
Attachment #8440512 - Flags: review?(borja.bugzilla)
(Assignee)

Comment 12

4 years ago
Just spotted the comments on the PR (can you update the ticket please when you add comments in github? I watch bugmail and my bugzilla dashboard not github notifications.) I'll rebase and address those and put this back in for review.
(Assignee)

Comment 13

4 years ago
PR updated, try: https://tbpl.mozilla.org/?tree=Gaia-Try&rev=d8b1d736b55f0fe9356e4d9df82a78e276716e9c
I'll reset r? flag if Try run comes back green.
(Assignee)

Comment 14

4 years ago
Comment on attachment 8440512 [details] [review]
PR: Fix decoding/video unloading error

Waiting on verdict from tbpl before re-submitting for review. Works well on-device for me though.
Attachment #8440512 - Flags: review?(borja.bugzilla)
(Assignee)

Comment 15

4 years ago
Comment on attachment 8440512 [details] [review]
PR: Fix decoding/video unloading error

Unrelated failures in the try run, I've re-triggered the unit tests but this looks good to go.
Attachment #8440512 - Flags: review?(borja.bugzilla)
(Assignee)

Comment 16

4 years ago
Comment on attachment 8440512 [details] [review]
PR: Fix decoding/video unloading error

There's a collection of bugs that might be fixed by this and its the cause of an intermittent test failure (see bug 1023179) I'd really like to land it before it bitrots futher.
Attachment #8440512 - Flags: review?(fernando.campo)
Comment on attachment 8440512 [details] [review]
PR: Fix decoding/video unloading error

I left a few comments on bugzilla. Most of them just code nits and style stuff, but also some questions about functions from the patch.

Sorry if the answers might be obvious, but I'm not familiar with the previous patches (or the bug tbh), and I'm maybe missing some points.

If it's better for you (faster landing), we can discuss it in the morning on IRC, but leave the answers anyway on github, as I will be a connection-beggar tomorrow.
Flags: needinfo?(sfoster)
Target Milestone: 2.0 S4 (20june) → 2.0 S5 (4july)
(Assignee)

Comment 18

4 years ago
Thanks for the comments fcampo. I think I've addressed them all and updated the PR. gaia-try results will be here: https://tbpl.mozilla.org/?tree=Gaia-Try&rev=03041814eef6b2001b990100be95d6676bdc57e7
Flags: needinfo?(sfoster)
Comment on attachment 8440512 [details] [review]
PR: Fix decoding/video unloading error

Just saw another style nit, optional as it doesn't affect functionality, so up to you if you want to change it.

Apart from that, code looks great (a little unclear on the _loadmedia dependent functions, but good to go), tested on hamachi (only one available right now) and loaded the videos 10 out of 10 times, so just waiting for gaia-try results to merge (or maybe we should try first on a flame, which is the one giving some trouble right?)
Attachment #8440512 - Flags: review?(fernando.campo) → review+
(Assignee)

Updated

4 years ago
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
status-b2g-v2.0: --- → affected
status-b2g-v2.1: --- → fixed
(Assignee)

Comment 21

4 years ago
Comment on attachment 8440512 [details] [review]
PR: Fix decoding/video unloading error

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Video in FTU was new feature introduced by bug 990035
[User impact] if declined: videos may not load or load sporadically when user is stepping through FTU tutorial
[Testing completed]: Manual testing on Flame by author and reviewer, unit tests passing on Gaia-Try
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: None
Attachment #8440512 - Flags: review?(borja.bugzilla) → approval-gaia-v2.0?

Updated

4 years ago
Attachment #8440512 - Flags: approval-gaia-v2.0? → approval-gaia-v2.0+
(Assignee)

Comment 22

4 years ago
Reminder for myself to uplift once this is baked on trunk for a day or 2 and the intermittent stays fixed
Flags: needinfo?(sfoster)
Reverted in https://github.com/mozilla-b2g/gaia/commit/127e16c424bc24b2b42e66aab382063d4b4b9cd5 for Gaia-ui test failures on TBPL.
Ever since this landed, Gu has been consistently failing with https://tbpl.mozilla.org/php/getParsedLog.php?id=42468602&tree=B2g-Inbound
(Assignee)

Comment 24

4 years ago
Created attachment 8446158 [details] [review]
Revised PR to fix FTU tutorial video loading/unloading

Carrying fcampo's r+, the new PR just adjusts the initialization logic to continue initializing if images/video fail to load (as its too late to do anything about it and we don't want to block the user from completing FTU). In practice this is only a problem for the emulator which doesn't like loading .mp4 files, and caused permanent test failure there. 
I've added a unit test to cover this case.
Attachment #8440512 - Attachment is obsolete: true
Flags: needinfo?(sfoster)
(Assignee)

Comment 25

4 years ago
Gaia-Try results: https://tbpl.mozilla.org/?tree=Gaia-Try&rev=9c15674fbaf3b81dc86d521d3801260f093f3415
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 26

4 years ago
Re-landed with the above changes the after green on Gaia-Try

https://github.com/mozilla-b2g/gaia/commit/f2ae5afc000c385d7b53082cb02240a492b8c69f
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Duplicate of this bug: 1023179
(Assignee)

Comment 29

4 years ago
Comment on attachment 8446158 [details] [review]
Revised PR to fix FTU tutorial video loading/unloading

Re-requesting uplift. And earlier version of this patch was backed out due to UI test failure

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Video steps for FTU tutorial were added in bug 990035
[User impact] if declined: FTU video tutorial content may sometimes not load
[Testing completed]: On Gaia-Try/Travis and on-device by author and reviewer (fcampo)
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: None
Attachment #8446158 - Flags: approval-gaia-v2.0?
Comment on attachment 8446158 [details] [review]
Revised PR to fix FTU tutorial video loading/unloading

clearing the approval as this has already landed in 2.0
Attachment #8446158 - Flags: approval-gaia-v2.0?
Created attachment 8531928 [details]
Verify_Video_Flame.MP4

This issue has been verified successfully on Flame 2.0 & 2.1. FTU tutorial videos can be loaded.

See attachment: Verify_Video_Flame.MP4
Reproducing rate: 0/5

Flame v2.0 version:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/29222e215db8
Build-ID        20141203000201
Version         32.0

Flame v2.1 version:
Gaia-Rev        dbaf3e31c9ba9c3436e074381744f2971e15c7bf
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/ebce587d2194
Build-ID        20141203001205
Version         34.0
status-b2g-v2.0: fixed → verified
status-b2g-v2.1: fixed → verified
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.