DeviceStorage does not fire a change event when a video is recorded.

RESOLVED FIXED in Firefox 18

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: djf, Assigned: mikeh)

Tracking

Trunk
mozilla20
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(blocking-basecamp:+, firefox18 fixed, firefox19 fixed, firefox20 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
When the camera app records a video, DeviceStorage never fires a change event for the creation or modification of the video file.  If the Gallery app is running at the same time as the Camera app, it will not display the new video until the user quits and restarts it.

This isn't actually a Gaia bug, but I'm filing it here because I don't know the right place to put it. It is a Gecko bug, but is it a DeviceStorage bug? Or is it a Camera hardware bug?  I'm nominating it as blocking, however, because the camera and gallery will not work correctly together unless it is fixed.

Note that when the Camera app takes a new still photo, it explicitly stores it with a call to DeviceStorage.addNamed(), and the Gallery app gets the event it needs to know that there is a new photo.  But for video recording, the Camera app just passes a DeviceStorage object and a filename to the camera API, and Gecko (or more likely something much lower in the stack) actually writes the file that contains the video.  The file is being created at some low level, but DeviceStorage is not noticing.

Early on in the development of DeviceStorage there was an intern working on code that would detect changes to the filesystem even when they came from other processes (such as on the desktop). This turned out to be too hard, and we decided that it would be okay to only issues change events for file creations and deletions that were done through DeviceStorage. That change was made with the understanding, however, that all parts of FirefoxOS that dealt with user-facing files would use DeviceStorage under the hood.  I suspect that what is wrong here is that the camera does not do this.

I also suspect that the video file is being written at a device driver level, and it really can't be modified to use DeviceStorage. Instead, I suspect that the camera code at the level of Gecko is going to have to somehow ping DeviceStorage when the file is written and say "hey, here's a new file you should know about" so that DeviceStorage can send out the modified event that the Gallery depends on.

Updated

6 years ago
Component: Gaia → DOM: Device Interfaces
Product: Boot2Gecko → Core
Version: unspecified → Trunk

Updated

6 years ago
blocking-basecamp: --- → ?
(Reporter)

Comment 1

6 years ago
Wow, that was fast Jason! Adding some more people to the cc list.
(Reporter)

Comment 2

6 years ago
Possibly related to #815079?
I would be really surprised to find out that the device driver is performing any file I/O. While it is possible, it's almost never done (there used to be a kernel based HTML server that was plagued with problems).

File I/O is almost always performed from userspace.

Comment 4

6 years ago
Correct - if the camera isn't writing its files through devicestorage, devicestorage will not post any change notifications.
(In reply to Doug Turner (:dougt) from comment #4)
>
> Correct - if the camera isn't writing its files through devicestorage,
> devicestorage will not post any change notifications.

I can confirm that video recording does not write data through device storage; it uses the POSIX file primitives to record video.  The call to open() is in Gecko, so we can play with that, BUT the calls to write() and close() take place in library code that (I suspect) we can't change.
Assignee: nobody → doug.turner
blocking-basecamp: ? → +

Comment 6

6 years ago
Mike,
okay - here is what we can do....

your code, when done with the write, broadcast a nsIObserver message like:
   http://mxr.mozilla.org/mozilla-central/source/dom/devicestorage/nsDeviceStorage.cpp#238

if you did this, device storage would think that it made that change and send a change event.

If you point me at where this happens, i can write the patch.  Alternatively, I could review such a patch.
Flags: needinfo?(mhabicher)
(In reply to Doug Turner (:dougt) from comment #6)
> Mike,
> okay - here is what we can do....
> 
> your code, when done with the write, broadcast a nsIObserver message like:
>   
> http://mxr.mozilla.org/mozilla-central/source/dom/devicestorage/
> nsDeviceStorage.cpp#238
> 
> if you did this, device storage would think that it made that change and
> send a change event.
> 
> If you point me at where this happens, i can write the patch. 
> Alternatively, I could review such a patch.

Hi Doug, the relevent function is here:
https://mxr.mozilla.org/mozilla-central/source/dom/camera/GonkCameraControl.cpp#868

I'll take care of it--it looks like I'll need to build a DeviceStorageFile object to pass into NotifyObservers.  Currently the Gonk camera layer gets a base path (e.g. "/sdcard") and a subpath-and-filename (e.g. "DCIM/100FFXOS/VID_0001.3gp"); can those be passed to the DeviceStrorageFile constructor as-is? e.g.

    nsRefPtr<DeviceStorageFile> mDeviceStorageFile;
        ...
    nsCOMPtr<nsIFile> filename = aStartRecording->mFolder;
    filename->AppendRelativePath(aStartRecording->mFilename);
    mDeviceStorageFile = new DeviceStorageFile(NS_LITERAL_STRING(DEVICESTORAGE_VIDEOS), filename);
       ...
    obs->NotifyObservers(mDeviceStorageFile, "file-watcher-update", data.get());
Assignee: doug.turner → mhabicher
Status: NEW → ASSIGNED
Flags: needinfo?(mhabicher)
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
Created attachment 688345 [details] [diff] [review]
Notify DeviceStorage when a new video is finished recording

Tested with:
function onChange(e) {
  dump("we saw '" + e.path + "' " + e.reason + "\n");
}

var storage = navigator.getDeviceStorage("sdcard");
storage.addEventListener("change", onChange);

Output:
I/GeckoDump(  832): we saw 'DCIM/104MZLLA/VID_0001.3gp' video/3gpp
Attachment #688345 - Flags: review?(doug.turner)
(In reply to Mike Habicher [:mikeh] from comment #8)
> Created attachment 688345 [details] [diff] [review]
> Notify DeviceStorage when a new video is finished recording
> 
> Tested with:
> function onChange(e) {
>   dump("we saw '" + e.path + "' " + e.reason + "\n");
> }
> 
> var storage = navigator.getDeviceStorage("sdcard");
> storage.addEventListener("change", onChange);
> 
> Output:
> I/GeckoDump(  832): we saw 'DCIM/104MZLLA/VID_0001.3gp' video/3gpp

Sorry, "sdcard" should be "videos".

Updated

6 years ago
Attachment #688345 - Flags: review?(doug.turner) → review+
Keywords: checkin-needed
Whiteboard: needs-checkin-aurora
Whiteboard: needs-checkin-aurora
https://hg.mozilla.org/mozilla-central/rev/aadac7d01161
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Whiteboard: needs-checkin-aurora, needs-checkin-beta
https://hg.mozilla.org/releases/mozilla-aurora/rev/f724e7d93865
https://hg.mozilla.org/releases/mozilla-beta/rev/fd58f34f4fb7
status-firefox18: --- → fixed
status-firefox19: --- → fixed
status-firefox20: --- → fixed
Whiteboard: needs-checkin-aurora, needs-checkin-beta
You need to log in before you can comment on or make changes to this bug.