Closed Bug 859171 Opened 7 years ago Closed 7 years ago

[Camera][Video] cannot record more than an hour.

Categories

(Firefox OS Graveyard :: Gaia, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:leo+, b2g18 fixed)

RESOLVED FIXED
1.1 QE1 (5may)
blocking-b2g leo+
Tracking Status
b2g18 --- fixed

People

(Reporter: hanj.kim25, Assigned: ben.tian)

References

Details

(Whiteboard: [TD-9305])

Attachments

(6 files, 2 obsolete files)

1. Title : [Camera][Video] cannot record more than an hour.
2. Precondition : Start recording and wait until the time goes over an hour.
3. Tester's Action : Stop recording after one hour is passed.
4. Detailed Symptom : When video app is launched, the video is displayed as just 'Black' screen.
5. Expected : The Gallery app is displayed and nothing more.
6. Reproducibility: Y
   1)Frequency Rate : 100%


Log File

08-16 21:45:19.109: E/GeckoConsole(149): Content JS LOG at app://system.gaiamobile.org/shared/js/event_helper.js:4 in handle_mozChromeEvent: event_helper :handle_mozChromeEvent: webapps-launch
08-16 21:45:20.149: E/GeckoConsole(502): Content JS LOG at app://video.gaiamobile.org/shared/js/mediadb.js:1045 in anonymous: fullScan :: cursor.onsuccess : file : DCIM/100MZLLA/VID_0046.3gp
08-16 21:45:20.149: E/GeckoConsole(502): Content JS LOG at app://video.gaiamobile.org/shared/js/mediadb.js:921 in ignore: ignore : check mimetypes : filetype : video/3gpp
08-16 21:45:20.159: E/GeckoConsole(502): Content JS LOG at app://video.gaiamobile.org/shared/js/mediadb.js:1045 in anonymous: fullScan :: cursor.onsuccess : file : DCIM/100MZLLA/VID_0047.3gp
08-16 21:45:20.159: E/GeckoConsole(502): Content JS LOG at app://video.gaiamobile.org/shared/js/mediadb.js:921 in ignore: ignore : check mimetypes : filetype : video/3gpp
08-16 21:45:20.169: E/GeckoConsole(502): Content JS LOG at app://video.gaiamobile.org/shared/js/mediadb.js:1045 in anonymous: fullScan :: cursor.onsuccess : file : DCIM/100MZLLA/VID_0048.3gp
08-16 21:45:20.169: E/GeckoConsole(502): Content JS LOG at app://video.gaiamobile.org/shared/js/mediadb.js:921 in ignore: ignore : check mimetypes : filetype : video/3gpp
08-16 21:45:22.709: E/GeckoConsole(502): [JavaScript Error: "RangeError: argument 1 accesses an index that is out of range" {file: "app://video.gaiamobile.org/shared/js/blobview.js" line: 150}]
08-16 21:45:25.239: E/GeckoConsole(577): [JavaScript Warning: "Unknown property '-moz-align-self'.  Declaration dropped." {file: "resource://gre-resources/ua.css" line: 44}]
blocking-b2g: --- → leo?
Triage 4/10 - Leo+ as this may seriously impact end user as recorded video cannot be seen in gallery.
blocking-b2g: leo? → leo+
Hi David, can you help with this issue? According to the reporter offline, the recording was stopped properly within the camera app but the video cannot be seen in gallery.
Flags: needinfo?(dflanagan)
The RangeError in the logcat implies that something is going wrong in metadata parsing for the recorded video. Because we can successfully parse the metadata for shorter videos, that might mean that there is a bug in the metadata parser, or that the video file is corrupted. If we ran out of sdcard space, for example, the recording process might not have created a valid file. (Though the camera is supposed to prevent that...)

hanj.kim: will you share the recorded video file so I can diagnose the problem? It is probably too big to attach directly to this bug, but maybe you could put in on the web and put a link in a comment.  Or email it to dflanagan@mozilla.com.  Thanks!
Flags: needinfo?(dflanagan) → needinfo?(hanj.kim25)
Hi David, 

The video file I have is 925 MB. Uploading a file other than bugzilla.mozilla.org is restricted here and it's too big for an email.

One thing I found is that there are preview images for videos taken. However there is no preview image for the 925 MB video file. Could this be a reason why the 925 MB video is not listed in the gallery?

One more, the metadata is empty in all videos I have taken (regardless of size/length). I used VLC media player to check the metadata.

Thanks
Flags: needinfo?(hanj.kim25)
Whiteboard: TD-9305
hanj.kim25 - The original reporting mentions 100% reproduce rate, can you confirm this or was this only tested once?  Comment 3 suggests this may be only in cases where the files are corrupted.

ni?djf for comment 4 - suggestions to check anything else?
Flags: needinfo?(hanj.kim25)
Flags: needinfo?(dflanagan)
Setting QA wanted: can someone try recording a super-long video like this and see if you can reproduce the bug. If so, I need access to the video file that results so I can figure out what is going wrong.  

Hanj: the metadata I'm talking about isn't tags like video name, its just the rotation matrix that tells us whether we have to rotate the video. It could be that for really long videos my code that extracts that rotation information is failing.
Flags: needinfo?(dflanagan)
Keywords: qawanted
Wayne: 
It's 100% reproducible, when I record longer than 1 hour. It has been tested many times.

(In reply to Wayne Chang [:wchang] from comment #5)
> hanj.kim25 - The original reporting mentions 100% reproduce rate, can you
> confirm this or was this only tested once?  Comment 3 suggests this may be
> only in cases where the files are corrupted.
> 
> ni?djf for comment 4 - suggestions to check anything else?
I got a video file from reporting partner, its 425mb so I am sharing via drive:

https://docs.google.com/file/d/0BzKlrkRP_-fINjJzdzJhSlBMWG8/edit?usp=sharing
Flags: needinfo?(hanj.kim25)
Wayne:
Just use 4/14 build on leo phone and can be reproduced.
David:
I've pulled the file and please tell the information you need.
Whiteboard: TD-9305 → [TD-9305]
clearing qawanted and ni?djf for comment 8 and comment 9

Hi David, please let us know if there is enough information or anything else needed in addition.
Flags: needinfo?
Keywords: qawanted
Target Milestone: --- → Leo QE1 (5may)
Change component to gaia first because it's not relavant to system app per comments above.
Component: Gaia::System → Gaia
Flags: needinfo?
Correcting my ni? for djf in comment 10
Flags: needinfo?(dflanagan)
Thanks Paul and Wayne.  With the sample file I think I'll be able to diagnose and fix the bug.

I'm assigning this to myself because I think I'm the only one familiar with the relevant code (assuming it is a metadata parsing error).

I won't have time to work on it until next week at least, however, so if anyone wants to steal it, that's fine by me.
Assignee: nobody → dflanagan
Flags: needinfo?(dflanagan)
The cause of this bug is while parsing metadata large size atoms require first 16 bytes read but camera app reads only first 8 bytes, so the reading of bytes 8-15 triggers the range error. Also the time displayed during recording and playback doesn't handle video length > 1 hour cases.

--
More details:
The steps to parse metadata for rotation info are
1) preloading initial 1024 bytes from video blob,
2) traversing atoms by reading first 8 or 16 bytes of each atom,
3) reading additional bytes from blob if needed, first 8 bytes for each atom, and
4) finding the atom with rotation info.

Short videos need no step 3) as step 1) contains rotation info already, and the large size atom is stored after rotation info. However long videos require step 3) and the large size atom is stored before rotation info. Therefore reading bytes 8-15 of the large size atom results in the range error.
--

My patch fixes the range error by increasing first bytes to read from 8 to 16, and displays video time > 1 hr during recording and playback.
Assignee: dflanagan → btian
David, can you review my patch?
Attachment #740219 - Flags: review?(dflanagan)
forgot to paste patch link in description.
https://github.com/mozilla-b2g/gaia/pull/9331
Comment on attachment 740219 [details] [diff] [review]
increase byte of additional blob reading from 8 to 16 for large size atoms

Thank you for finding and fixing this! (I was guessing that because of the large file size the bug was related to 16 byte atom headers, but I thought that I just didn't handle them at all.)

The video app had the same bugs for displaying times > 1 hour. They got fixed a couple of months ago, but I guess we never noticed that the same bugs existed in the Camera app.  

(Please check to see if there is a bug filed about the time display for long videos in camera, and if so go ahead and close it when you land this one.)

Really nice work here!
Attachment #740219 - Flags: review?(dflanagan) → review+
master: https://github.com/mozilla-b2g/gaia/commit/4bf6e6fb802aca9d6efd8f45e42852753df19a7a
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Uplifted 4bf6e6fb802aca9d6efd8f45e42852753df19a7a to:
v1-train: b86136b945fb8c2ad1336dc52a7d39d660d1d8b2
Attached file Log file
Now it fails with different error message. (refer to the following)

[JavaScript Warning: "Media resource blob:e792dd18-e23a-4fa0-84c8-23fb5f308c52 could not be decoded." {file: "app://camera.gaiamobile.org/index.html" line: 0}]



I still have a feeling that this is related to creating a preview image for the video at the end of recording. A preview image is supposed to be created for each video taken. However, there are no preview image created for those videos with error.
Lots of zeros found. Hopefully this gives some hint.
Lots of zeros found. Hopefully this gives some hint.

Also, again there is no preview image created for this video file. (hopefully this is 2nd hint.)
The decoding error occurs on gaia v1-train but not on gaia master. I'm checking code difference for possible cause.

Link to video file I reproduced on leo.
https://docs.google.com/file/d/0BzKlrkRP_-fINjJzdzJhSlBMWG8/edit
(In reply to Ben Tian [:btian] from comment #24)
> The decoding error occurs on gaia v1-train but not on gaia master. I'm
> checking code difference for possible cause.
CORRECTION: both gaia master and v1-train have the decoding error.

> Link to video file I reproduced on leo.
> https://docs.google.com/file/d/0BzKlrkRP_-fINjJzdzJhSlBMWG8/edit
The video file above doesn't meet decoding error if I throw it directly to thumbnail generation (by hardcoding filename of addVideo() in filmstrip.js. https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/filmstrip.js#L361). I'm investigating where in B2G the decoding error occurs.

Reopen this bug to track.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: FIXED → ---
Hi Ben, can you provide current status on this issue?
Flags: needinfo?(btian)
Still under investigation. Current progress shows in failed cases MediaExtractor finds zero video/audio tracks during metadata parsing, whereas normally two tracks (1 video and 1 audio) should be found. I'm checking detailed metadata parsing flow in stagefright to find the exact cause.
Flags: needinfo?(btian)
The bug cause is that camera parses video metadata before video file writing completes, so camera reads corrupted metadata and hits the decoding error. As longer videos require more time to complete file writing, the fixed delay=1000ms [1] in camera app becomes insufficient.

The attached gaia patch listens to video storage event for writing completion instead of waiting a fixed delay. It depends on video storage event in bug 868891. I'm running more tests to verify the flow change and on longer videos.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/camera.js#L580
Depends on: 868891
(In reply to Ben Tian [:btian] from comment #28)
> It depends on video storage
> event in bug 868891. I'm running more tests to verify the flow change and on
> longer videos.

Thanks Ben -- bug 868891 is now fixed on b2g18.
David, can you review my patch? The patch listens to video storage event for video file writing completion instead of waiting a fixed 1000ms delay.
Attachment #747865 - Attachment is obsolete: true
Attachment #748665 - Flags: review?(dflanagan)
Re-create pull request.
Attachment #748665 - Attachment is obsolete: true
Attachment #748665 - Flags: review?(dflanagan)
Attachment #748668 - Flags: review?(dflanagan)
Comment on attachment 748668 [details] [diff] [review]
link to https://github.com/mozilla-b2g/gaia/pull/9708

Ben,

I didn't realized that this bug had been re-opened. I can't test your fix because I don't have a recent enough gecko build to have the fixed device storage event, but it looks correct, and I'm really happy to have that old broken setTimeout() hack removed!

I'm giving r+ here, but I do have one comment on github that I'd like you to address before landing... It looks to me like there is a potential (though very unlikely) race condition with the way the code is currently structured.
Attachment #748668 - Flags: review?(dflanagan) → review+
David, thanks for your suggestion. I moved the listener into stopRecording() and register a new handler for each video recorded. Please let me know if you have any further suggestion. If not I will land it to gaia master tomorrow.
Assuming you've tested the new code, it looks great to me. Go ahead and land it when you're ready.
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Uplifted e156df2d648c53349dd4e16c514d34b1a454afc8 to:
v1-train: 9a524d8ac0c5a9f80edf83a7739aeff696e24fe5
Flags: in-moztrap?
Created test case.
- https://moztrap.mozilla.org/manage/case/8471/
Flags: in-moztrap? → in-moztrap+
You need to log in before you can comment on or make changes to this bug.