Last Comment Bug 759567 - Add video and music to device storage on gaia
: Add video and music to device storage on gaia
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla15
Assigned To: Doug Turner (:dougt)
:
Mentors:
Depends on:
Blocks: 759826
  Show dependency treegraph
 
Reported: 2012-05-29 15:39 PDT by Doug Turner (:dougt)
Modified: 2012-05-31 06:20 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v.1 (2.29 KB, patch)
2012-05-29 15:50 PDT, Doug Turner (:dougt)
jonas: review+
Details | Diff | Review

Description Doug Turner (:dougt) 2012-05-29 15:39:52 PDT

    
Comment 1 Doug Turner (:dougt) 2012-05-29 15:50:58 PDT
Created attachment 628130 [details] [diff] [review]
patch v.1
Comment 2 David Flanagan [:djf] 2012-05-29 23:21:43 PDT
Note the comments in https://github.com/mozilla-b2g/gaia/issues/1519
and https://github.com/mozilla-b2g/gaia/issues/1520.

It sounds as if Device Storage should not use /data for on-phone storage. And also /sdcard is not actually the removable sd card but is actually a fake on-phone sd card. 

I don't pretend to understand this; I am just the messenger.
Comment 3 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-05-29 23:55:55 PDT
We should treat /sdcard not being present as an unusual condition.  Phones without "real" SD cards will create an /sdcard partition that's carved out of internal memory.

In the common case, there are two reasons why /sdcard wouldn't be present
 1. It's being used by USB mass storage.  (Gecko will know about this because we implement UMS ourselves.)
 2. There's a physical SD card and the user has removed it.

In both cases, it's not entirely clear what MediaStorage should do.  It should probably go into "temporary staging" mode, caching resources to be moved back to the "normal" storage.

We definitely don't want to strand resources in /data while the SD card is temporarily missing.

But I haven't read through the MediaStorage impl yet so not sure how much of this is already handled.
Comment 4 David Flanagan [:djf] 2012-05-30 00:27:30 PDT
Chris, 

As I imagine this, and as Doug has implemented Device Storage, Gaia apps will be able to store media files to either on-phone storage or to an SD card when an SD card exists.

If I'm understanding you, you're suggesting that apps should only store to the SD card, but on phones that do not support sd cards, there will be a fake /sdcard partition so it looks like there is one.

But what about the case where the phone has an SD card slot but the user has not purchased and inserted a card? I think you're saying that the /sdcard partition wouldn't exist in that case. And you've said that we shouldn't use /data.  So where should the camera app, for example, store its files then?

The question of what to do with storage requests while a UMS session is in progress is a longer-term issue (that I don't think has been considered yet). But the immediate question is: What paths should Doug use in his patch?
Comment 5 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-05-30 00:44:23 PDT
(In reply to David Flanagan from comment #4)
> As I imagine this, and as Doug has implemented Device Storage, Gaia apps
> will be able to store media files to either on-phone storage or to an SD
> card when an SD card exists.
> 

Do you mean the API allows choosing, or the Gecko implementation might store to either /data or /sdcard?

> If I'm understanding you, you're suggesting that apps should only store to
> the SD card, but on phones that do not support sd cards, there will be a
> fake /sdcard partition so it looks like there is one.
> 

Almost ... s/should only store to the SD card/should only store to the \/sdcard partition/.  Except when it's not there :).

> But what about the case where the phone has an SD card slot but the user has
> not purchased and inserted a card?

To the best of my knowledge, it's not a common case for phones to ship without an /sdcard partition, however that storage is backed (whether by internal storage or a removable SD card).

> I think you're saying that the /sdcard
> partition wouldn't exist in that case. And you've said that we shouldn't use
> /data.  So where should the camera app, for example, store its files then?
> 

I don't believe we need to design around this case.  Storing to /data and hoping it's temporary is probably our best option.  We would need this code for temporary staging anyway.

> The question of what to do with storage requests while a UMS session is in
> progress is a longer-term issue (that I don't think has been considered
> yet). But the immediate question is: What paths should Doug use in his patch?

UMS has landed, so we need to have the two APIs play well together.  As I recommended above, we should use /sdcard when available and /data as "temporary staging" when /sdcard isn't there.  That work can and should go in a followup bug.

What would not be acceptable is using /data as the backing store when /sdcard is temporarily gone, and then switching back to /sdcard when it returns and "losing" the resources stored in /data while /sdcard was gone.
Comment 6 David Flanagan [:djf] 2012-05-30 01:09:38 PDT
(In reply to Chris Jones [:cjones] [:warhammer] from comment #5)
> (In reply to David Flanagan from comment #4)
> > As I imagine this, and as Doug has implemented Device Storage, Gaia apps
> > will be able to store media files to either on-phone storage or to an SD
> > card when an SD card exists.
> > 
> 
> Do you mean the API allows choosing, or the Gecko implementation might store
> to either /data or /sdcard?
> 

When the Gallery app calls navigator.getDeviceStorage('pictures'), it gets an array of two DeviceStorage objects back. One represents /data/pictures, and one represents /sdcard/DCIM. My plan was to have the gallery display any photos it finds in either location. And to have a setting to control whether the camera saves photos to the sdcard or to internal storage.

This is based on the incorrect assumption that /sdcard only exits if the phone actually has an sdcard, however.

> > If I'm understanding you, you're suggesting that apps should only store to
> > the SD card, but on phones that do not support sd cards, there will be a
> > fake /sdcard partition so it looks like there is one.
> > 
> 
> Almost ... s/should only store to the SD card/should only store to the
> \/sdcard partition/.  Except when it's not there :).
> 
> > But what about the case where the phone has an SD card slot but the user has
> > not purchased and inserted a card?
> 
> To the best of my knowledge, it's not a common case for phones to ship
> without an /sdcard partition, however that storage is backed (whether by
> internal storage or a removable SD card).
> 

My SGS2 has an empty SD card slot. Does it get a fake /sdcard partition? If I take a bunch of photos that and then go out and buy an SD card and stick it in the slot, what happens to all the photos I took? Do they disappear and then reappear when I remove the card? Do they automatically get copied to the card?
Comment 7 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-05-30 01:16:36 PDT
(In reply to David Flanagan from comment #6)
> (In reply to Chris Jones [:cjones] [:warhammer] from comment #5)
> > (In reply to David Flanagan from comment #4)
> > > As I imagine this, and as Doug has implemented Device Storage, Gaia apps
> > > will be able to store media files to either on-phone storage or to an SD
> > > card when an SD card exists.
> > > 
> > 
> > Do you mean the API allows choosing, or the Gecko implementation might store
> > to either /data or /sdcard?
> > 
> 
> When the Gallery app calls navigator.getDeviceStorage('pictures'), it gets
> an array of two DeviceStorage objects back. One represents /data/pictures,
> and one represents /sdcard/DCIM. My plan was to have the gallery display any
> photos it finds in either location. And to have a setting to control whether
> the camera saves photos to the sdcard or to internal storage.
> 
> This is based on the incorrect assumption that /sdcard only exits if the
> phone actually has an sdcard, however.
> 

We should expect that /data will generally be quite small compared to /sdcard, so the storage space in /data is more precious.  That's where apps will be installed.  We should prefer /sdcard by default.  I would kinda prefer that the MediaStorage API hid those details but I didn't follow the API discussion in great detail.

> > > If I'm understanding you, you're suggesting that apps should only store to
> > > the SD card, but on phones that do not support sd cards, there will be a
> > > fake /sdcard partition so it looks like there is one.
> > > 
> > 
> > Almost ... s/should only store to the SD card/should only store to the
> > \/sdcard partition/.  Except when it's not there :).
> > 
> > > But what about the case where the phone has an SD card slot but the user has
> > > not purchased and inserted a card?
> > 
> > To the best of my knowledge, it's not a common case for phones to ship
> > without an /sdcard partition, however that storage is backed (whether by
> > internal storage or a removable SD card).
> > 
> 
> My SGS2 has an empty SD card slot. Does it get a fake /sdcard partition? If
> I take a bunch of photos that and then go out and buy an SD card and stick
> it in the slot, what happens to all the photos I took? Do they disappear and
> then reappear when I remove the card? Do they automatically get copied to
> the card?

/sdcard on the sgs2 points at internal storage.  It's not really "fake", it just doesn't point at a removable SD card.  The removable SD card slot is mounted at /sdcard2 or something like that IIRC.
Comment 9 Mark Finkle (:mfinkle) (use needinfo?) 2012-05-30 05:45:24 PDT
(In reply to Doug Turner (:dougt) from comment #8)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/8f8639307984

No Windows or Android support in this patch. Are new bugs required for that?
Comment 10 Doug Turner (:dougt) 2012-05-30 08:41:11 PDT
mfinkle, yes.  Feel free to file.
Comment 11 David Flanagan [:djf] 2012-05-30 08:49:42 PDT
(In reply to Chris Jones [:cjones] [:warhammer] from comment #7)

> 
> We should expect that /data will generally be quite small compared to
> /sdcard, so the storage space in /data is more precious.  That's where apps
> will be installed.  We should prefer /sdcard by default.  I would kinda
> prefer that the MediaStorage API hid those details but I didn't follow the
> API discussion in great detail.

The fact that the API returns two DeviceStorage objects (for internal storage and removable storage) is based, in part at least, on the requirements here: https://wiki.mozilla.org/Gaia/System/FileManagement  In particular, note the v2 requirement to be able to copy files between internal storage and removable storage.

> > 
> > My SGS2 has an empty SD card slot. Does it get a fake /sdcard partition? If
> > I take a bunch of photos that and then go out and buy an SD card and stick
> > it in the slot, what happens to all the photos I took? Do they disappear and
> > then reappear when I remove the card? Do they automatically get copied to
> > the card?
> 
> /sdcard on the sgs2 points at internal storage.  It's not really "fake", it
> just doesn't point at a removable SD card.  The removable SD card slot is
> mounted at /sdcard2 or something like that IIRC.

Is that specific to the sgs2, or will all B2G phones work that way? It will suck if different devices had different filesystems.  You said earlier that /sdcard will sometimes point to internal storage and sometimes point to removable storage. If so, that would also suck.  But here you seem to be saying that /sdcard is always internal storage, and removable storage always has a different mount point.  

The current device storage implementation uses /data for internal storage and /sdcard for removable storage.  From what you're saying it sounds like that is wrong.  But I still can't understand what the correct paths should be for this.

Doug: I kind of hijacked your bug here...  I'll try to shut up about this now and leave it to you.

And that's a bug, right? Can we not ensure that all B2G devices always use the same mount point for removable storage? Is the Device Storage implementation going to have to be #ifdef-ed on a per-device basis in addition to a per-OS basis?
Comment 12 David Flanagan [:djf] 2012-05-30 08:50:59 PDT
Oops. Ignore the last paragraph of the previous comment, please.
Comment 13 Doug Turner (:dougt) 2012-05-30 08:55:30 PDT
> But I still can't understand what the correct paths should be for this.

I choose these paths based on conversations on irc.  I fully expect that there will be device id checks and different paths returned for different devices.
Comment 14 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-05-30 12:49:20 PDT
Not all phones have removable storage.  /sdcard, whether it points at internal or external storage, will be much larger than /data.  /data is not mountable by USB mass storage.

The product requirements will vary based on actual phone HW, so it's probably better to have those conversations in the context of product targets.
Comment 15 Ed Morley [:emorley] 2012-05-31 06:20:29 PDT
https://hg.mozilla.org/mozilla-central/rev/8f8639307984

Note You need to log in before you can comment on or make changes to this bug.