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

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[hanj.kim25]

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}]

Comment 1

[Wayne Chang [:wchang]]

Triage 4/10 - Leo+ as this may seriously impact end user as recorded video cannot be seen in gallery.

Comment 2

[Wayne Chang [:wchang]]

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.

Comment 3

[David Flanagan [:djf]]

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 [e-mail].  Thanks!

Comment 4

[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

Comment 5

[Wayne Chang [:wchang]]

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?

Comment 6

[David Flanagan [:djf]]

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.

Comment 7

[Leo]

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?

Comment 8

[Wayne Chang [:wchang]]

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

Comment 9

[Paul Yang [:pyang] (away)]

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.

Comment 10

[Wayne Chang [:wchang]]

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.

Comment 11

[(inactive after 6/18) Alive Kuo [:alive]]

Change component to gaia first because it's not relavant to system app per comments above.

Comment 12

[Wayne Chang [:wchang]]

Correcting my ni? for djf in comment 10

Comment 13

[David Flanagan [:djf]]

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.

Comment 14

[Ben Tian (inactive)]

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.

Comment 15

[Ben Tian (inactive)]

Attachment: increase byte of additional blob reading from 8 to 16 for large size atoms

David, can you review my patch?

Comment 16

[Ben Tian (inactive)]

forgot to paste patch link in description.
https://github.com/mozilla-b2g/gaia/pull/9331

Comment 17

[David Flanagan [:djf]]

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!

Comment 18

[Ben Tian (inactive)]

master: https://github.com/mozilla-b2g/gaia/commit/4bf6e6fb802aca9d6efd8f45e42852753df19a7a

Comment 19

[James Lal [:lightsofapollo]]

Uplifted 4bf6e6fb802aca9d6efd8f45e42852753df19a7a to:
v1-train: b86136b945fb8c2ad1336dc52a7d39d660d1d8b2

Comment 20

[Leo]

Attachment: 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.

Comment 21

[Leo]

Attachment: screenshot of video file binary header (passed one)

Comment 22

[Leo]

Attachment: screenshot of video file binary header - top (failed one)

Lots of zeros found. Hopefully this gives some hint.

Comment 23

[Leo]

Attachment: screenshot of video file binary header - mid (failed one)

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

Comment 24

[Ben Tian (inactive)]

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

Comment 25

[Ben Tian (inactive)]

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

Comment 26

[Dylan Oliver [:doliver]]

Hi Ben, can you provide current status on this issue?

Comment 27

[Ben Tian (inactive)]

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.

Comment 28

[Ben Tian (inactive)]

Attachment: Listen to video storage event for file writing completion

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

Comment 29

[Dylan Oliver [:doliver]]

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

Comment 30

[Ben Tian (inactive)]

Attachment: link to https://github.com/mozilla-b2g/gaia/pull/9707

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.

Comment 31

[Ben Tian (inactive)]

Attachment: link to https://github.com/mozilla-b2g/gaia/pull/9708

Re-create pull request.

Comment 32

[David Flanagan [:djf]]

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.

Comment 33

[Ben Tian (inactive)]

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.

Comment 34

[David Flanagan [:djf]]

Assuming you've tested the new code, it looks great to me. Go ahead and land it when you're ready.

Comment 35

[Ben Tian (inactive)]

Land in master: https://github.com/mozilla-b2g/gaia/commit/e156df2d648c53349dd4e16c514d34b1a454afc8

Comment 36

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted e156df2d648c53349dd4e16c514d34b1a454afc8 to:
v1-train: 9a524d8ac0c5a9f80edf83a7739aeff696e24fe5

Comment 37

[William Hsu [:whsu]]

Created test case.
- https://moztrap.mozilla.org/manage/case/8471/