Closed Bug 1191731 Opened 4 years ago Closed 4 years ago

[Aries KK]The pictures and videos don't update in gallery app in time.

Categories

(Firefox OS Graveyard :: Gaia::Camera, defect, P1)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:2.5+, b2g-v2.2 unaffected, b2g-master verified)

VERIFIED FIXED
FxOS-S8 (02Oct)
blocking-b2g 2.5+
Tracking Status
b2g-v2.2 --- unaffected
b2g-master --- verified

People

(Reporter: yelanying, Assigned: aosmond)

References

Details

(Whiteboard: [2.5-aries-test-run-1])

Attachments

(3 files, 2 obsolete files)

Attached video Aries_KK v2.5.3gp (obsolete) —
[1.Description]:
[Aries KK v2.5][Gallery] After you have launched gallery app, launch camera app to take several pictures and videos, then navigate to gallery app. The pictures and videos don't update in gallery app in time.
Time: 16:43
See attachment logcat_1643.txt and Aries_KK v2.5.3gp

[2.Testing Steps]: 
Precondition:
You have launched gallery app.
1.Launch camera app.
2.Take some pictures and vedios.
3.Launch gallery app.

[3.Expected Result]: 
3.The pictures and videos should update in gallery app in time.

[4.Actual Result]: 
3.The pictures and videos don't update in gallery app in time.

[5.Reproduction build]: 
Aries KK 2.5 (Affected)
Build ID               20150804223818
Gaia Revision          c5425d9f1f5184731a59ed4bc99295acbde30390
Gaia Date              2015-08-04 16:09:19
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/f3b757156f69
Gecko Version          42.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150804.220057
Firmware Date          Tue Aug  4 22:01:05 UTC 2015
Bootloader             s1

Flame KK 2.2 (Unaffected)
Build ID               20150805032505
Gaia Revision          f8b119ac30e97df991c97682ac4d4f9ca22e1793
Gaia Date              2015-07-31 13:20:55
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/0c7a85251e10
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150805.065842
Firmware Date          Wed Aug  5 06:58:54 EDT 2015
Firmware Version       v18D v
Bootloader             L1TC000118D0

Flame KK 2.5 (Unaffected)
Build ID               20150805030212
Gaia Revision          c5425d9f1f5184731a59ed4bc99295acbde30390
Gaia Date              2015-08-04 16:09:19
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/f3b757156f69
Gecko Version          42.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150805.063609
Firmware Date          Wed Aug  5 06:36:20 EDT 2015
Firmware Version       v18D v
Bootloader             L1TC000118D0


[6.Reproduction Frequency]: 
Always Recurrence,10/10

[7.TCID]: 
Free test
Attached file logcat_1643.txt (obsolete) —
See Also: → 1177374
[Blocking Requested - why for this release]:

As in Bug 1177374, this happens with downloaded images too it seems. (Also happens only with Aries)
Nominating for 2.5? for bad user experience (being unable to find new images seem to be a major feature breakage)
[Blocking Requested - why for this release]:
Forgot to actually nom this bug, and this still reproduces in 20150818012543.
blocking-b2g: --- → 2.5?
Blocking based on comment 2
blocking-b2g: 2.5? → 2.5+
Duplicate of this bug: 1196495
Dave, can you please investigate this one!
Assignee: nobody → dhylands
Note that to reproduce this bug, you've got to record a video. If you just take still photos, the bug does not appear.

In my tests, if you record a video, then that video and any photos you take after it will not appear in the gallery when you return to the already-running gallery app.

If you kill gallery and restart it, then it finds the videos and the photos.

So this seems to have something to do with the Camera and how it saves video files. I think that changed recently so that gecko was producing the poster image for each video instead of it being created in Gaia in the camera app.  Gallery does not see new videos until it gets a device storage notification for the new poster image.  Andrew: does your code that creates the poster image send the appropriate notifications to device storage so that device storage can notify gallery?

I'm guessing that this is a camera bug and changing the component to Gaia:Camera. If it is actually a Gallery bug and I need to modify some code to detect the poster images, feel free to change it back and let me know.
Component: Gaia::Gallery → Gaia::Camera
Flags: needinfo?(aosmond)
Priority: -- → P1
See also bug 1197733, which sounds related.

Dave: since you've taken this bug, you might want to add some console.logs to shared/js/mediadb.js in the changeHandler() function near line 777. That should tell you whether the background gallery app is receiving the proper event when a video is recorded.
I put a print in Device Storage where it receives the file notifications, and I also put a print in mediadb's changeHandler (for file events) and I did the following:

- launched gallery
- launched camera
- took 2 photos
- took a video (less than 5 seconds)
- took another photo

switched back to gallery and it only shows the first 2 photos.

I threw a bunch of dump statements in mediadb and this is what I collected:

> mediadb: dispatchEvent
> mediadb: dispatchEvent
> mediadb: dispatchEvent
> mediadb: dispatchEvent
> DSF (parent) file-watcher-notify: created: mStorageType 'pictures' mStorageName 'sdcard' mRootDir '' mPath 'DCIM/100MZLLA/IMG_0001.jpg' mFile->GetPath '/storage/emulated/legacy/DCIM/100MZLLA/IMG_0001.jpg'
> DSF (parent) file-watcher-notify: modified: mStorageType 'pictures' mStorageName 'sdcard' mRootDir '' mPath 'DCIM/100MZLLA/IMG_0001.jpg' mFile->GetPath '/storage/emulated/legacy/DCIM/100MZLLA/IMG_0001.jpg'
> mediadb: changeHandler: path: /sdcard/DCIM/100MZLLA/IMG_0001.jpg reason: modified
> mediadb: Calling insertRecord for /sdcard/DCIM/100MZLLA/IMG_0001.jpg
> mediadb: insertRecord: pushing to queue
> mediadb: next: calling insertFile
> mediadb: insertFile: About to call device storage get on /sdcard/DCIM/100MZLLA/IMG_0001.jpg
> mediadb: get: onsuccess
> mediadb: get: about to parseMetadata for /sdcard/DCIM/100MZLLA/IMG_0001.jpg
> mediadb: gotMetadata
> mediadb: storeRecord for /sdcard/DCIM/100MZLLA/IMG_0001.jpg
> mediadb: queueCreateNotification for /sdcard/DCIM/100MZLLA/IMG_0001.jpg
> mediadb: dispatchEvent
> DSF (parent) file-watcher-notify: created: mStorageType 'pictures' mStorageName 'sdcard' mRootDir '' mPath 'DCIM/100MZLLA/IMG_0002.jpg' mFile->GetPath '/storage/emulated/legacy/DCIM/100MZLLA/IMG_0002.jpg'
> DSF (parent) file-watcher-notify: modified: mStorageType 'pictures' mStorageName 'sdcard' mRootDir '' mPath 'DCIM/100MZLLA/IMG_0002.jpg' mFile->GetPath '/storage/emulated/legacy/DCIM/100MZLLA/IMG_0002.jpg'
> mediadb: changeHandler: path: /sdcard/DCIM/100MZLLA/IMG_0002.jpg reason: modified
> mediadb: Calling insertRecord for /sdcard/DCIM/100MZLLA/IMG_0002.jpg
> mediadb: insertRecord: pushing to queue
> mediadb: next: calling insertFile
> mediadb: insertFile: About to call device storage get on /sdcard/DCIM/100MZLLA/IMG_0002.jpg
> mediadb: get: onsuccess
> mediadb: get: about to parseMetadata for /sdcard/DCIM/100MZLLA/IMG_0002.jpg
> mediadb: gotMetadata
> mediadb: storeRecord for /sdcard/DCIM/100MZLLA/IMG_0002.jpg
> mediadb: queueCreateNotification for /sdcard/DCIM/100MZLLA/IMG_0002.jpg
> mediadb: dispatchEvent
> DSF (parent) file-watcher-notify: created: mStorageType 'videos' mStorageName 'sdcard' mRootDir '' mPath 'DCIM/100MZLLA/.tmp.3gp' mFile->GetPath '/storage/emulated/legacy/DCIM/100MZLLA/.tmp.3gp'
> DSF (parent) file-watcher-notify: modified: mStorageType 'videos' mStorageName 'sdcard' mRootDir '' mPath 'DCIM/100MZLLA/.tmp.3gp' mFile->GetPath '/storage/emulated/legacy/DCIM/100MZLLA/.tmp.3gp'
> DSF (parent) file-watcher-notify: deleted: mStorageType 'videos' mStorageName 'sdcard' mRootDir '' mPath 'DCIM/100MZLLA/.tmp.3gp' mFile->GetPath '/storage/emulated/legacy/DCIM/100MZLLA/.tmp.3gp'
> DSF (parent) file-watcher-notify: created: mStorageType 'pictures' mStorageName 'sdcard' mRootDir '' mPath 'DCIM/100MZLLA/VID_0003.jpg' mFile->GetPath '/storage/emulated/legacy/DCIM/100MZLLA/VID_0003.jpg'
> DSF (parent) file-watcher-notify: modified: mStorageType 'pictures' mStorageName 'sdcard' mRootDir '' mPath 'DCIM/100MZLLA/VID_0003.jpg' mFile->GetPath '/storage/emulated/legacy/DCIM/100MZLLA/VID_0003.jpg'
> mediadb: changeHandler: path: /sdcard/DCIM/100MZLLA/VID_0003.jpg reason: modified
> mediadb: Calling insertRecord for /sdcard/DCIM/100MZLLA/VID_0003.jpg
> mediadb: insertRecord: pushing to queue
> mediadb: next: calling insertFile
> mediadb: insertFile: About to call device storage get on /sdcard/DCIM/100MZLLA/VID_0003.jpg
> mediadb: get: onsuccess
> mediadb: get: about to parseMetadata for /sdcard/DCIM/100MZLLA/VID_0003.jpg
> DSF (child) file-watcher-notify: modified: mStorageType 'videos' mStorageName 'sdcard' mRootDir '' mPath 'DCIM/100MZLLA/VID_0003.3gp' mFile->GetPath '/storage/emulated/legacy/DCIM/100MZLLA/VID_0003.3gp'
> DSF (parent) file-watcher-notify: created: mStorageType 'pictures' mStorageName 'sdcard' mRootDir '' mPath 'DCIM/100MZLLA/IMG_0004.jpg' mFile->GetPath '/storage/emulated/legacy/DCIM/100MZLLA/IMG_0004.jpg'
> DSF (parent) file-watcher-notify: modified: mStorageType 'pictures' mStorageName 'sdcard' mRootDir '' mPath 'DCIM/100MZLLA/IMG_0004.jpg' mFile->GetPath '/storage/emulated/legacy/DCIM/100MZLLA/IMG_0004.jpg'
> mediadb: changeHandler: path: /sdcard/DCIM/100MZLLA/IMG_0004.jpg reason: modified
> mediadb: Calling insertRecord for /sdcard/DCIM/100MZLLA/IMG_0004.jpg
> mediadb: insertRecord: pushing to queue


So things seem to be hanging in the metadata parser while trying to parse the video meta data. Basically, none of the callbacks passed into media.metadataParser (which I think Gallery provides) ever get called when metadataParser gets called with VID_0003.jpg and that's where things seem to hang.

So transferring this back to djf since it seems to be a Gallery bug.
Assignee: dhylands → dflanagan
Okay, I can investigate further. I still think that the problem is in the recent camera change where gecko is now creating the thumbnail. I suspect that the thumbnail is being saved before the video file is ready or something. Still waiting on info from Andrew, but I'll investigate and try to confirm my hypothesis before blaming this on the Camera app.
Also note that the gallery app does correctly parse the metadata of the video if you kill it and restart, so there is nothing wrong with the metadata parser. I think that we just need to be sure that the video file is saved before the poster is saved.
Okay, I added logging at apps/gallery/js/MetadataParser.js:563 to print out the size of the video file.

When we get the notification that the poster image is changed, I ask device storage for the video file. In my test, it returned a video file that was 810kb long, and then metadata parsing failed.

When I then killed and restarted gallery, it repeated the metadata parsing process and this time it got the full 6mb video file, and metadata parsing succeeded.

Andrew: the way Galley works is that it only listens for change events on the photos device storage. So the creation of the video poster image is Gallery's signal that there is a new video that it should pick up.  In the past, gecko saved a video file, then the Camera app manually created the poster image for it, then Gallery saw the poster image, and read the video file. Now that you're having gecko create the poster image, the poster is being created before the video is fully written and the metadata parsing is failing.

I could probably fix this in the Gallery app by listening listening for change events on a device storage object for videos, but I suspect it might be easier for you to simply delay writing the poster image, or maybe to just delay sending out the device storage notification until the video file is fully written. I suppose the ideal thing would be to have the notifications for the two files to go out at the same time.
But there still seems to be a problem in that the metadataParser encounters an error and never calls the metadataError function.

So running across a truncated video file could still cause things to "hang".
It's possible that the video parser issue may also be the cause of bug 1198169
I'll change the order of creation back to what it was. My original fear was creating it too late because when you exit the camera app with the homescreen, the poster wasn't getting created. It seems if we don't depend on the 2d canvas to create the poster ourselves, (after some testing today) we can still save an existing blob to disk via device storage.
Assignee: dflanagan → aosmond
Status: NEW → ASSIGNED
Flags: needinfo?(aosmond)
See Also: → 1164200
Comment on attachment 8661323 [details] [diff] [review]
[gecko] update gecko poster API, v1

gecko try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e7acac654047
Attachment #8661323 - Flags: review?(dhylands)
Attachment #8661324 - Flags: review?(jdarcangelo)
Thanks Andrew!

Dave: you're right that there is a metadata parser issue here, still.  I'm not too concerned because Gallery only looks at files in DCIM/, and if we trust the camera not to produce corrupt videos, it is unlikely that the user will screw that up.

But I went ahead and took bug 1198169. If that does not turn out to be the same as the metadata parsing issue here, I can try to investigate that in a separate (non-blocking) bug.
Comment on attachment 8661323 [details] [diff] [review]
[gecko] update gecko poster API, v1

Review of attachment 8661323 [details] [diff] [review]:
-----------------------------------------------------------------

I think you may need to get a DOM peer to review the webidl changes (I seem to recall that there is a mercurial hook which checks the reviewer).
Attachment #8661323 - Flags: review?(dhylands) → review+
Comment on attachment 8661323 [details] [diff] [review]
[gecko] update gecko poster API, v1

bz: Can you review the WebIDL? This is a breaking change but hasn't made its way into any releases yet as the initial checkin was with bug 1175656.
Attachment #8661323 - Flags: review?(bzbarsky)
Comment on attachment 8661323 [details] [diff] [review]
[gecko] update gecko poster API, v1

r=me
Attachment #8661323 - Flags: review?(bzbarsky) → review+
Summary: [Aries KK][Gallery] The pictures and videos don't update in gallery app in time. → [Aries KK]The pictures and videos don't update in gallery app in time.
Comment on attachment 8661324 [details] [review]
[gaia] aosmond:bug1191731 > mozilla-b2g:master

Left a few minor comments/nits on the PR.
Attachment #8661324 - Flags: review?(jdarcangelo) → review+
Blocks: 1197733
Attachment #8644247 - Attachment is obsolete: true
Attachment #8644245 - Attachment is obsolete: true
Note that the gecko and the gaia changes are mutually dependent on each other, so they should be landed around the same time. In the interim period while builds are mixed/matched, recording videos will be semi-broken (no poster generated, but the video is recorded).
Keywords: checkin-needed
https://github.com/mozilla-b2g/gaia/commit/899860aa2f58cb2597405cd3f8572a938e7ca384
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S8 (02Oct)
According to the STR of Comment 0 & Comment 9, this bug has been verified as pass on latest Flame KK v2.5 and Aries KK v2.5.

Actual results: The pictures and videos should update in gallery app in time.
See attachment: verified_Aries KK v2.5.3gp
Reproduce rate: 0/6


Device: Flame KK v2.5(Pass)
Build ID               20150922150204
Gaia Revision          68361828ae88dffd04b250121b5f2472a63f4bf0
Gaia Date              2015-09-22 03:46:57
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/a1ccea59e254a88f7bb44b0ad8a58b77b7eca339
Gecko Version          44.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150922.183124
Firmware Date          Tue Sep 22 18:31:40 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Aries KK v2.5(Pass)
Build ID               20150923004630
Gaia Revision          864500d40633bbf0e9a83c92a03cea46bb901906
Gaia Date              2015-09-22 17:27:46
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/05a7ee49d40a4cfabf2479337fd5e1312e178b6d
Gecko Version          44.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150923.000647
Firmware Date          Wed Sep 23 00:06:55 UTC 2015
Bootloader             s1
QA Whiteboard: [MGSEI-Triage+]
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.