971141 - make sure we don't leak memory when creating video thumbnails during scanning

Status: RESOLVED FIXED

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

Description

[David Flanagan [:djf]]

In bug 963917 I fixed a memory leak in the Gallery app that occured only when scanning new media.  It affects any app that uses mediadb and includes blobs in the returned metadata objects. For Gallery, these blobs were image thumbnails.  It might also affect the music and video apps, so I'm bringing it up here.

Normally, when a media app starts, metadata is read from mediadb, and any blobs in the metadata records refer to files, so they don't take up much memory.  But in the scanning case, mediadb is writing metadata records to the db, but then passing those records to the app.  In this case, any blobs in the records are typically memory-backed blobs rather than file-backed blobs, and they will be expensive if you hold on to them for the lifetime of the app.

For gallery, I fixed this by explicitly looking up the metadata record for each newly scanned file instead of using the record that mediadb passed to me.

This bug is filed to remind us to check whether the Video app has the same memory leak, and to fix it if so.

Comment 1

[Hema Koka [:hema]]

Russ,

Please investigate.

Thanks
Hema

Comment 2

[Russ Nicoletti [:russn]]

Results of my initial research using master:

Device: Hamachi
OS Version 1.4
Platform 30.0a1
Build Id: 20140217040203

I copied 50 of the videos attached to bug 963917 to the device and started video app. Max memory was 116000. 

I copied 100 of the videos attached to bug 963917 to the device and started video app. Max memory was 142000.

Comment 3

[Russ Nicoletti [:russn]]

Attachment: Pull request

I've created a patch similar to the one created for gallery for bug 963917, that reads the file from the db and uses the metadata from the db to create the thumbnail so that we have a file-backed blob in memory instead of a memory-backed blob.

With this patch, the max memory I observed when loading 100 videos for the first time was less than 100 MB (98 or 99 MB).

Comment 4

[John Hu [:johnhu][:johu][:醬糊小弟]]

Comment on attachment 8379159 [details] [review]
Pull request

Russ,

This is a nice patch. I found a small nit at PR. Please update it before landing. Thanks for this patch.

Comment 5

[Russ Nicoletti [:russn]]

Attachment: Updated pull request (addresses review comments)

Comment 6

[Russ Nicoletti [:russn]]

Hi David, shall I land this path in 1.4 or wait until 1.5?

Comment 7

[Russ Nicoletti [:russn]]

Merged to master:

https://github.com/mozilla-b2g/gaia/commit/c0e0f3c719aabfa6bfc7a4991152e3fcfa724bdd