1164200 - [B2G][Gaia] Flame: videos recorded by camera app is not shown in gallery app

Status: RESOLVED FIXED

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

Description

[No-Jun Park [:njpark]]

STR:
Open Camera app, record video.
Open Gallery app

Expected:
Video file is visible
Actual:
VIdeo file is not visible. (in Video app, the video file is visible)

Version info:
Build ID               20150512010209
Gaia Revision          6089234ace8b294a8feef064387604bae16254e3
Gaia Date              2015-05-10 13:57:12
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/502e1a5e722f
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150512.043209
Firmware Date          Tue May 12 04:32:19 EDT 2015
Bootloader             L1TC000118D0

Comment 1

[No-Jun Park [:njpark]]

adding regressionwindow wanted flag to find the breakage.  Yesterday's nightly on master seemed to work fine.  ni?ing djf for clues and info.

Comment 2

[David Flanagan [:djf]]

No-Jun,

I can reproduce this with these steps:

1) Launch camera and start recording video
2) Press the home button while the video is recording
3) Launch gallery

Is that what you were doing?  If I wait until I see a thumbnail for the video in the camera app, then I don't see this bug.

In this case, I won't see the video in the gallery app. I tried it twice. Once the video showed up in the video app and once it didn't show up at all.

The video app works for any video file.  The gallery looks for video poster images and relies on the camera app to create those. 

With these STR, I'm guessing that the camera app is being killed with an OOM before it has the chance to create the poster image.  And in the case where the video doesn't even show up in the video app, I'm guessing that the camera app didn't even get to finish the video recording so we ended up with a corrupt .3gp file on the sdcard.

I'm guessing about the OOM here.  You could verify by checking (with adb shell b2g-ps) whether the camera app dies.  It would also be interesting to know if this bug is related to memory: does it reproduce on a 1024mb Flame?  And, given the image memory issues we've been having with gallery, it would be interesting to know whether gallery was running in the background when you recorded the video and how many images did you have on your phone when you tested this?  If the sdcard is empty when you test, does it still reproduce?

Needinfo Andrew because I think he knows more about what the camera app does in cases like this when it is interrupted by a phone call or the Home button

Comment 3

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

I was unable to reproduce this issue on builds from today and the smoke team checked the videos they made which also showed up in the gallery as expected.

No-Jun: Is there possibly some other step you performed to get this issue to occur (assuming that David is incorrect because you mention that it does show up in the video app)?

Environmental Variables:
Device: Flame 3.0
BuildID: 20150512071241
Gaia: 3654ec1d7ce1e0a56a34d5c3b06f6a9b33ff79ad
Gecko: bedce1b405a3
Version: 41.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

Comment 4

[No-Jun Park [:njpark]]

Sorry for not being clear, I thought it was a simple procedure, then I realized that it involves a bit more.

Try following STR:
1. enable usb, and in the phone, delete the DCIM folder, and delete any pictures that are in the phone so the gallery will be empty.
2. Disconnect the phone, disable USB, then start the gallery app.  The gallery app will say use camera to add photos.  Click go to camera button in the gallery app.
3. Change to the video mode, and record video. After recording the video by pressing stop, go to homescreen and launch Gallery app.

Expected:
The video should show up
Actual:
It shows the same message saying 'add photos using the camera'

I can repro it with today's nightly as well, and it looks to me if there are only videos and no photos, the gallery app won't recognize them. sorry for ni?ing you again, but :djf, would this be the case?

And for comment 2, I think it warrants a separate bug, if not already raised before. (I think it is).  when exiting to the homescreen while recording, it does not save, but discards the video recording.  Perhaps aosmond can tell us whether it's the right behavior.

Comment 5

[No-Jun Park [:njpark]]

Note that when STR from comment 4 is executed, the resulting video can be seen in the video app.

Comment 6

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Timing is important on this issue.  You must press homescreen before the preview for the video shows up in the Camera after hitting stop.  I believe that comment 2 is actually the same issue because of this.

Environmental Variables:
Device: Flame 3.0
BuildID: 20150512163044
Gaia: 0d6c04f13fd385bda045f4e539b2a67cb5d84b1d
Gecko: 62d9b117c688
Version: 41.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

Comment 7

[No-Jun Park [:njpark]]

Ahh, you(In reply to No-Jun Park [:njpark] from comment #5)
> Note that when STR from comment 4 is executed, the resulting video can be
> seen in the vid(In reply to Jayme Mercado [:JMercado] from comment #6)
> Timing is important on this issue.  You must press homescreen before the
> preview for the video shows up in the Camera after hitting stop.  I believe
> that comment 2 is actually the same issue because of this.
> 
> Environmental Variables:
> Device: Flame 3.0
> BuildID: 20150512163044
> Gaia: 0d6c04f13fd385bda045f4e539b2a67cb5d84b1d
> Gecko: 62d9b117c688
> Version: 41.0a1 (3.0) 
> Firmware Version: v18D-1
> User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

(In reply to Jayme Mercado [:JMercado] from comment #6)
> Timing is important on this issue.  You must press homescreen before the
> preview for the video shows up in the Camera after hitting stop.  I believe
> that comment 2 is actually the same issue because of this.
> 
Ahh, I just double checked and I think you're right. clearing needinfo for :djf

Comment 8

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Following the updated steps I was able to reproduce this on 2.2, 2.1, and 2.0 Flame builds indicating that it is not a regression.

Environmental Variables:
Device: Flame 2.2
BuildID: 20150513002507
Gaia: e048df68f6f4853b5826a8816e143d95258149de
Gecko: 0e6b4aab2b94
Gonk: ab265fb203390c70b8f2a054f38cf4b2f2dad70a
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Environmental Variables:
Device: Flame 2.1
BuildID: 20150513001201
Gaia: c80865cb0bf73f1b97defbc646083b404feb3ac4
Gecko: 2fd3ef3fc14a
Gonk: ab265fb203390c70b8f2a054f38cf4b2f2dad70a
Version: 34.0 (2.1) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Environmental Variables:
Device: Flame 2.0
BuildID: 20150513000201
Gaia: 84898cadf28b1a1fcd03b726cff658de470282f0
Gecko: 96101b9b9b2b
Gonk: ab265fb203390c70b8f2a054f38cf4b2f2dad70a
Version: 32.0 (2.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 9

[Andrew Osmond [:aosmond] (he/him)]

The video file is saved properly to the DCIM folder but there is no poster JPG generated for the video file; that is done in JS after we stop recording the video but since we exited via the home button, the code never runs. Since the gallery application only searches for photos, it is unable to discover the video files lacking posters.

I'm not sure what ability (if any) the camera app has to run the poster code when the app has been backgrounded? In an ideal world, we could generate the posters in the gallery application for any video files missing one once it has finished enumerating over the photos.

Comment 10

[David Flanagan [:djf]]

It probably would be best to have the gallery app scan for videos. If we ever get rid of the Video app, then we'll have to do that for sure.  For now, the reason we don't do that is that the MediaDB class only supports one media type at a time, so we'd require two MediaDB objects, and, in order to display items in chronological order would have to interleave the results of enumerating both databases.  It is do-able, and I had code to do it at one time.  But it is a lot simpler to just rely on the camera to create the thumbnail.

I'll take this bug so it doesn't get forgotten.

But one more needinfo for Andrew: if you end up modifying the moz camera implementation to add new features (like lossless rotation of jpegs so that we don't have to use EXIF orienation bits on Aries) then please also consider modifying the code so that gecko can automatically create a thumbnail for each recorded video. Digital cameras seem to generate .thm "thumbnail" files (I think they are jpegs) for videos, and I believe that this is a supported part of the DCIF camera filesystem standard. It would be kind of sweet if we could reliably support it ourselves.

Comment 11

[Andrew Osmond [:aosmond] (he/him)]

Bug 1142216 is reporting something very similar. If I implemented the thumbnail generator in Gecko like your ask in comment 10, my guess is that it would solve an entire class of issues...

Perhaps that is the best way. I need some time to investigate how to implement this, since *decoding* is not exactly my forte :). From a first glance, it appears I should be able to create an nsFileChannel object to read the video file, feed that into an HTMLVideoDocument and follow the JS code from there. I could then add a new recorder state change to let you know when the poster was generated.

Comment 12

[Andrew Osmond [:aosmond] (he/him)]

djf: Gecko posters is becoming reality. I'll fix it at my layer (well, with some gaia changes I'll need you and Justin to review as well).

Comment 14

[Andrew Osmond [:aosmond] (he/him)]

Attachment: [gaia] aosmond:bug1164200 > mozilla-b2g:master

Comment 15

[David Flanagan [:djf]]

The shared/js/media parts of the patch look pretty good. I've left a few minor comments on github, and will wait to review until the gecko patch has landed and is testable.

What is going to happen when the gecko patch has landed and the gaia patch for camera has not? Is the fact that a poster is already there going to break the camera code that tries to create the poster? I could see that causing a device storage error.

Comment 16

[Andrew Osmond [:aosmond] (he/him)]

(In reply to David Flanagan [:djf] from comment #15)
> The shared/js/media parts of the patch look pretty good. I've left a few
> minor comments on github, and will wait to review until the gecko patch has
> landed and is testable.
> 
> What is going to happen when the gecko patch has landed and the gaia patch
> for camera has not? Is the fact that a poster is already there going to
> break the camera code that tries to create the poster? I could see that
> causing a device storage error.

Gecko posters are opt in. You have to provide a file path and a device storage object so it knows where to put it. If you don't supply it, everything behaves the same as it does today. This way we can make testing it more accessible to everyone to make sure the quality/performance is acceptable (and if not, change it). But when it comes to UX, I won't claim to have the most discerning eye :). It seems unchanged to me despite the resizing but I'm willing to defer to others on the matter.

Comment 18

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 8623859 [details] [review]
[gaia] aosmond:bug1164200 > mozilla-b2g:master

The latest TBPL b2g-inbound flame-kk-eng build includes the necessary gecko change to test this.

Comment 19

[David Flanagan [:djf]]

Comment on attachment 8623859 [details] [review]
[gaia] aosmond:bug1164200 > mozilla-b2g:master

I left a few minor comments, but really only minor stuff. This looks fine to me, but I have not actually tested it.

I am concerned that gallery may need changes with this patch. Consider this scenario:

1) Launch gallery so it is running in the background
2) Launch camera and start recording a video.
3) Gecko will create a poster image and store it to the device. Gallery will detect the new image and will create a thumbnail for it in the background, I think.
4) Without stopping the recording, use an edge gesture to switch directly to Gallery.
5) Tap in the upper left on the thumbnail for the new video
6) If you do this quickly enough, can you tap on the thumbnail before the video file has been written?  If so, what happens? Black display? Error in logcat? Just no response?

If that does not happen, then I'm happy and please land it.  If it does, then we'll need to make gallery smarter by at least retrying the video after a second or two.

Right now you can kill gallery, delete files, launch gallery and see the thumbnails for files that do not currently exist. If you tap on them before scanning discovers that they aren't there, I don't actually know what happens, but it is the same situation as we could get into here, except that in this case, it is more reproduceable, I think.

If this does turn out to be an issue, Punam might be able to help while I'm on PTO.

Comment 20

[Hema Koka [:hema]]

Andrew,

When you plan to wrap this up? One of the 2.5 bugs depends on this? Please add the sprint target for this. 

Thanks
hema

Comment 21

[GitHub Autolander Bot]

Attachment: [gaia] aosmond:bug1164200 > mozilla-b2g:master

Comment 22

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8623859 [details] [review]
[gaia] aosmond:bug1164200 > mozilla-b2g:master

LGTM, thanks!

Comment 23

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/gaia/commit/0cc109495bf06060e66f993255ab8a23fcfe347b