Closed Bug 973616 Opened 7 years ago Closed 7 years ago

[Camera] Videos should be recorded to final location

Categories

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

x86
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wilsonpage, Assigned: wilsonpage)

References

Details

Attachments

(1 file)

Currently videos are being recorded to a temporary location, and then moved to a final location when complete.

This could potentially be risky if a device is low on space. When moving the file, a copy is created, meaning for a short period of time the device will require twice the size of the current video, and this space may not be available.
Assignee: nobody → wilsonpage
Attached file pull-request (master)
Attachment #8377204 - Flags: review?(dmarcos)
Attachment #8377204 - Flags: review?(dmarcos) → review+
I'm on the fence about landing this bug to master. Doesn't feel like it is critical.

Why in the world are we using a temporary filename in the first place, and why have you abandoned the DCF filename convention VID_dddd.3gp?  To be honest, I'm more concerned that you've broken the filename convention than that the file has to be copied.  Though I suppose both need to be fixed.

Not landing this on master or on the branch right now.
If the issue that was making you use temporary filenames was the startup time hit of finding out what the next number in the DCIM directory is, you now have a better option with your settings code. You can store the directory number and the item number in the cookie, I think.
Attachment #8377204 - Flags: review+ → review?(dflanagan)
djf: Apologies, it was not my intention to bypass the DCIM naming logic, this was due to a typo on line 94 in js/controllers/camera.js:

`this.camera.getVideoFilepath = this.storage.createVideoFilepath;`

Should have been:

`this.camera.createVideoFilepath = this.storage.createVideoFilepath;`

The idea behind this is that lib/camera.js is a standalone camera interface that *could* be used in another FirefoxOS application. We allow the application to override `camera.createVideoFilepath` if they wish to record their videos to an alternative location. We override this to use our DCIM filepath.

It is critical that this lands in master, as currently files are being moved from tmp storage location to final storage location. It has come to light that as the 'move' is in fact a copy, this means we may max out user storage while the file is duplicated, before the tmp file is deleted. This was an oversight in initial camera refactor.

Hope this all makes sense.
Comment on attachment 8377204 [details] [review]
pull-request (master)

A few nits on github that I'd like you to fix.

Also, you've broken some unit tests.

And, please write a test that at least shows that a valid DCF filename (i.e. not a temp file) is being passed to the mozCamera API.  (Or explain why it is not possible to write such a test.)

With those things done, feel free to land on master and then rebase the new camera-new-features branch. (Or ask me if you'd like me to do that.)
Attachment #8377204 - Flags: review?(dflanagan) → review+
Blocks: 976628
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.