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
Sam, Can you take a look at this one?
Yes I'll get right on this
Assignee: nobody → sfoster
Target Milestone: --- → 2.0 S4 (20june)
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
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
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)
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.
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
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.
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.
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.
PR updated, try: https://tbpl.mozilla.org/?tree=Gaia-Try&rev=d8b1d736b55f0fe9356e4d9df82a78e276716e9c I'll reset r? flag if Try run comes back green.
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.
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.
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.
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
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+
gaia-try was green: https://tbpl.mozilla.org/?tree=Gaia-Try&rev=5f53ad7001ea3be48f0aec2492db5c6dc5ad0bbe Landed with suggested changes: https://github.com/mozilla-b2g/gaia/commit/82cccd171556baa330a2ae26aaf994987aee19c2
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
status-b2g-v2.0: --- → affected
status-b2g-v2.1: --- → fixed
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?
Attachment #8440512 - Flags: approval-gaia-v2.0? → approval-gaia-v2.0+
Reminder for myself to uplift once this is baked on trunk for a day or 2 and the intermittent stays fixed
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
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
Gaia-Try results: https://tbpl.mozilla.org/?tree=Gaia-Try&rev=9c15674fbaf3b81dc86d521d3801260f093f3415
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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 ago → 4 years ago
Resolution: --- → FIXED
status-b2g-v2.0: affected → fixed
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
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
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
You need to log in before you can comment on or make changes to this bug.