873301 - Gallery should store image previews and video posters on same volume as image/video.

Status: RESOLVED FIXED

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

Description

[Dave Hylands [:dhylands]]

Now that we can support multiple volumes, I think that it makes sense for picture previews (aka thumbnails) and video posters (aka previews) to be stored on the same volume as the original data.

Comment 1

[Dave Hylands [:dhylands]]

Noming since this should go into the leo devices, which use multiple storage areas.

Comment 2

[Dave Hylands [:dhylands]]

Attachment: Link to https://github.com/mozilla-b2g/gaia/pull/10124

Comment 3

[David Flanagan [:djf]]

Comment on attachment 756340 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/10124

A few nits on github, but r+ once you've considered those and fixed the lint errors.

I notice that the bug title also talks about video poster images.  Those are actually created by the camera.  (In camera/js/filmstrip.js, iirc.)

If you want to fix that in the camera app as part of this bug I'm happy to re-review.

Comment 4

[leo.bugzilla.gecko]

in traiges, it's a blocker for leo.

Comment 5

[Jim Porter (:squib)]

Stealing per IRC.

Comment 6

[David Flanagan [:djf]]

I just remembered that both the Gallery and Video apps can delete videos and both have code to delete the video preview image in addition to the video itself.  That deletion code may need to be updated to use the right path. (Or maybe it already works--I'm not sure.)

Comment 7

[Jim Porter (:squib)]

(In reply to David Flanagan [:djf] from comment #6)
> I just remembered that both the Gallery and Video apps can delete videos and
> both have code to delete the video preview image in addition to the video
> itself.  That deletion code may need to be updated to use the right path.
> (Or maybe it already works--I'm not sure.)

Looking at the code, I'm pretty sure this is already handled. We save the filename we built, and then refer to it again when deleting. Aside from addressing your review comments, is there anything else I should be handling here?

Comment 8

[Jim Porter (:squib)]

Attachment: https://github.com/mozilla-b2g/gaia/pull/10271

Ok, here's an updated PR with the review comments addressed (99% of the credit for this goes to Dave Hylands). As mentioned earlier, I don't think we need to do anything special for deleting previews. Our code should already handle that nicely.

Comment 9

[Dave Hylands [:dhylands]]

Jim, did you look at camera/js/filmstrip.js ?

Comment 10

[David Flanagan [:djf]]

Comment on attachment 759977 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/10271

r=djf for github commit ebff6ad

I did not test, so please make sure you do before landing.  From my inspection of the camera code, it looks like we're already okay there, but be sure you test that, on a Leo device, too with both internal and external storage as the default.

Comment 11

[Jim Porter (:squib)]

(In reply to David Flanagan [:djf] from comment #10)
> I did not test, so please make sure you do before landing.  From my
> inspection of the camera code, it looks like we're already okay there, but
> be sure you test that, on a Leo device, too with both internal and external
> storage as the default.

Well, I'll try to flash my Leo device, but I still don't think I've fully resolved the issues I was having last week. If I can't get it to work, I might need to bug you to run the tests on the device...

Comment 12

[Jim Porter (:squib)]

(In reply to Jim Porter (:squib) from comment #11)
> Well, I'll try to flash my Leo device, but I still don't think I've fully
> resolved the issues I was having last week. If I can't get it to work, I
> might need to bug you to run the tests on the device...

Yeah, it still doesn't work right. I had tried running the flashing tools in Wine, but it doesn't work. I'm not even sure that would fix the issue I'm having anyway...

Comment 13

[David Flanagan [:djf]]

The patch works for me on my Leo device.

Landed to master: https://github.com/mozilla-b2g/gaia/commit/fc0ed1fb2a72327378946c2d6601c63eae31b718

Comment 14

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted fc0ed1fb2a72327378946c2d6601c63eae31b718 to:
v1-train: a9b004a693ec59b5cfd8750b6200eecb861a6583

Comment 15

[John Ford [:jhford] CET/CEST Berlin Time]

1.1hd: a9b004a693ec59b5cfd8750b6200eecb861a6583