995936 - [Tarako][Gallery] Gallery and Email/Contact exit when try to attach pictures from gallery if the pictures haven't been loaded yet

Status: RESOLVED WONTFIX

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

Description

[Bingqing Li]

Attachment: slog-1

Environment:
------------------------------------------------------
Gaia      b2802627a974795ccba989cede0540f20fadc633
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/c8491a42d4e8
BuildID   20140413164001
Version   28.1
ro.build.version.incremental=eng.cltbld.20140413.210357
ro.build.date=Sun Apr 13 21:04:05 EDT 2014


Steps to reproduce:
-----------------------------------------------------
1. Delete all the pictures in the sd card of the testing device
2. Copy some different pictures to the sd card (I copied about 30 pictures for testing), and then unplug the device or disable the usb storage
3. Launch Email / Contact
4. Tap on the gear icon on the top right corner / Tap on 'Add Picture'
5. Select 'Gallery'
   --> It starts to load pictures


Actual results:
-------------------------------------------------------------
Sometimes, after loading for a few seconds, a black screen appears for about 2 seconds, then Gallery and Email / Contact exit, and it goes back to home screen.

Sometimes, after loading for a few seconds, a black screen appears and need to press home key to go back to home screen.

Comment 1

[Bingqing Li]

Attachment: STR-Video

Comment 2

[Bingqing Li]

Attachment: logcat-email

Comment 3

[Bingqing Li]

Additional Info:
------------------------------------------
After coping pictures to sd card, pls do NOT launch gallery to load pictures.

Comment 4

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

looks like we're getting killed:
04-14 10:39:00.430 I/log     (  943): <4>0[ 447.673466] lowmem_shrink select 909 (Communications), adj 2, size 13573, to kill

Comment 5

[Marvin Khoo [:Marvin_Khoo]]

Hi Hema,
may i know if Gallery is not opened, will we generate thumbnail silently when new image is imported / captured?

BR,
Marvin

Comment 6

[Brian Huang [:brianhuang][:brhuang]]

Mike, could you try this scenario to check if Email app is killed? thanks.

1. use Tarako camera to take 30 pictures.
2. launch Email and then attached picture. Picture is chosen from Gallery that just took.

Comment 7

[Mike Lien[:mlien]]

I verify with today's daily build and it's fine on my tarako
Gaia      23488b1a45221c17e6a32fdd4c9d0fdbdcf2d021
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/72055108f470
BuildID   20140414164002
Version   28.1

each picture about 300~400KB and taken from camera

Comment 8

[Joe Cheng [:jcheng] (please needinfo)]

triage: 1.3T+.. to John

Comment 9

[Ting-Yu Chou [:ting] (away)]

Tried to understand what gallery is doing for loading images, here the sample
image is from reference workload:

1.  initThumbnails()
    ...
2.  photodb.scan()
3.  metadataParserWrapper() file=/sdcard/DCIM/100MZLLA/IMG_0152.jpg loaded=true
4.  metadataParser()
5.  getImageSize()
6.  parseJPEGMetadata()
7.  gotImageSize() w=1600 h=1200 preview=[object Object]
8.  parseJPEGMetadata()
9.  previewsuccess() pw=196 ph=147
10. useFullsizeImage()
11. createThumbnailAndPreview() file=/sdcard/DCIM/100MZLLA/IMG_0152.jpg
12. createThumbnailFromElement()
13. gotThumbnail()
14. createAndSavePreview()
15. save() file=/sdcard/.gallery/previews/DCIM/100MZLLA/IMG_0152.jpg pw=426.6666666666667 ph=320

9   if the image contains a preview and big enough, use it to create a thumbnail,
    otherwise use orignal image to create both thumbnail and preview
11  createObjectURL on the passed in file, and set it as offscreenImage's src,
    after the image is loaded, if image is alreay thumbnail size, done
12  draw the offscreenImage on a canvas with thumbnail size, encode the canvas to
    jpeg by toBlob()
13  if preview is not needed or the image is smller than 512*1024 pixels, done
14  draw the offscreenImage on a canvas with preview size, encode the canvas to
    jpeg by toBlob()

From my understand above:

- We can use downsampling url |#-moz-samplesize| instead of full image for the
  offscreenImage src at step 10 for creating preview and thumbnail, and the
  ratio can be set for preview image size.
- If the image is jpeg, the "image is already thumnail size" checking at step 11
  can be done before loading the iamge.
- The canvas of step 12 and 14 can be reused as offscreenImage.

Note seems |#-moz-samplesize| only works for jpeg, I'll double confirm this.

Comment 10

[Ting-Yu Chou [:ting] (away)]

Maybe we can delay creating preview image until it is going to be previewed?

Comment 11

[Hema Koka [:hema]]

(In reply to Marvin Khoo [:Marvin_Khoo] from comment #5)
> Hi Hema,
> may i know if Gallery is not opened, will we generate thumbnail silently
> when new image is imported / captured?
> 
> BR,
> Marvin

No it does not generate silently..the thumbnail generation happens when we start up gallery.

https://github.com/mozilla-b2g/gaia/blob/master/apps/gallery/js/gallery.js#L379

Thanks
Hema

Comment 12

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

(In reply to Ting-Yu Chou [:ting] from comment #9)
> From my understand above:
> 
> - We can use downsampling url |#-moz-samplesize| instead of full image for
> the
>   offscreenImage src at step 10 for creating preview and thumbnail, and the
>   ratio can be set for preview image size.
We already have a bug, bug 949755, for this purpose. I will apply djf's patch and make a memory usage report.

> - If the image is jpeg, the "image is already thumnail size" checking at
> step 11
>   can be done before loading the image.
I didn't get the point of this one. If we don't load the image with offscreenImage, how do we know the image is already in thumbnail size?

> - The canvas of step 12 and 14 can be reused as offscreenImage.
Peter and Jerry had told us about this. It may affect the speed but may not improve a lot in memory, I think. I will also try this one.

> Note seems |#-moz-samplesize| only works for jpeg, I'll double confirm this.
Yes, it seems to be that. So, we still need the draw into canvas when the image is not JPEG.

Comment 13

[Ting-Yu Chou [:ting] (away)]

(In reply to John Hu [:johnhu][:johu][:醬糊小弟] from comment #12)
> > - If the image is jpeg, the "image is already thumnail size" checking at
> > step 11
> >   can be done before loading the image.
> I didn't get the point of this one. If we don't load the image with
> offscreenImage, how do we know the image is already in thumbnail size?

We already know the original image size at step 7.

> 
> > - The canvas of step 12 and 14 can be reused as offscreenImage.
> Peter and Jerry had told us about this. It may affect the speed but may not
> improve a lot in memory, I think. I will also try this one.

Reuse the same instance should help us to avoid memory spike.

Comment 14

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Joe, Hema,

We need to offload John on this bug, and ensure Tarako does not crash on picker refresh even after 2mp limit is set.

Comment 15

[Hema Koka [:hema]]

As discussed in the email thread, David is doing related work as part of bug 989290 and 989026 and to avoid overlap, it probably is good to wait for those big patches. Punam will be back from vacation on Monday and should be able to help David with testing this out too.

Thanks
Hema

Comment 16

[Tim Guan-tin Chien [:timdream] (please needinfo)]

The resize function will be updated to use #-moz-samplefactor or #xywh in bug 989290, so we can re-evaluate if we have exhaust all memory-saving measures available, after that bug is fixed.

I think we should still need to at least re-use the canvas. David, and ideas?

Comment 17

[James Zhang (Spreadtrum)]

Is this bug still exist after bug 998228 fixed?

Comment 18

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Request QA for re-tests (see comment 17)

Comment 19

[David Flanagan [:djf]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #16)
> The resize function will be updated to use #-moz-samplefactor or #xywh in
> bug 989290, so we can re-evaluate if we have exhaust all memory-saving
> measures available, after that bug is fixed.
> 
> I think we should still need to at least re-use the canvas. David, and ideas?

I don't think that reusing a canvas will save us substantial memory.

Using #-moz-samplefactor during the metadata scanning process is a good idea, and John is right that I started that process in bug 949755. I don't remember how far I got, though. That does only help for jpegs, though, so we still need to be careful with other image types.  Maybe the gallery should have two different image size thresholds.  Normally it will scan a 2mp image, but when invoked for a pick activity, it won't scan images larger than 1mp unless they are jpegs?

Comment 20

[Joe Cheng [:jcheng] (please needinfo)]

ni? reporter on comment 18

Comment 21

[Bingqing Li]

I can still reproduce this on build 20140422164001.

I'll try more cases and update results and details later.


Build Info:
--------------------------------------------------------
Gaia      9aa9b04049f0291b124c50e0f9c3ce0e1f547725
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/2e40920b1d13
BuildID   20140422164001
Version   28.1
ro.build.version.incremental=eng.cltbld.20140422.202702
ro.build.date=Tue Apr 22 20:27:12 EDT 2014

Comment 22

[Bingqing Li]

Here are the results:
------------------------------------------------
1. 35 pictures, none of them is larger than 2MP
   ---> Works fine

2. 35 pictures, 7 of them are larger than 2MP
   ---> Can reproduce this issue.

3. 100 pictures, none of them is larger than 2MP
   ---> Sometimes can reproduce this issue, just try to scroll up/down while loading pictures.

4. 100 pictures, 15 of them are larger than 2MP
   ---> Can reproduce this issue

Comment 23

[Peipei Cheng (needinfo if you need my action)]

I think limit the image size may be not a good solution. Because user could have many pictures which also will consume a lot memory to load them into Gallery.

Does anyone know how Gallery load pics? Is it incremental loading? Like we could load a fix number of pics first and then load another fix number of pics on demand.

Comment 24

[Mike Lee [:mlee]]

QA per comment 15, please retest this once bug 989290 and bug 989026 land.

Comment 25

[Keven Kuo [:kkuo]]

Hi! Bingqing,

Since bug 997051 and 998228 landed, you shouldn't be able to attach pictures larger than 2MP.
But in your STR, it doesn't look like that.

Hi! John,

Could you confirm it?

--
Keven


(In reply to Bingqing Li from comment #22)
> Here are the results:
> ------------------------------------------------
> 1. 35 pictures, none of them is larger than 2MP
>    ---> Works fine
> 
> 2. 35 pictures, 7 of them are larger than 2MP
>    ---> Can reproduce this issue.
> 
> 3. 100 pictures, none of them is larger than 2MP
>    ---> Sometimes can reproduce this issue, just try to scroll up/down while
> loading pictures.
> 
> 4. 100 pictures, 15 of them are larger than 2MP
>    ---> Can reproduce this issue

Comment 26

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

(In reply to Keven Kuo [:kkuo] from comment #25)
> Hi! Bingqing,
> 
> Since bug 997051 and 998228 landed, you shouldn't be able to attach pictures
> larger than 2MP.
> But in your STR, it doesn't look like that.
> 
> Hi! John,
> 
> Could you confirm it?

Yes, I can confirm it. Please use latest version to test it. The bug 997051 and bug 998227 limit the picture resolution to 2MP using CONFIG_MAX_IMAGE_PIXEL_SIZE.

(In reply to pcheng from comment #23)
> Does anyone know how Gallery load pics? 

It loads pics when they are in the screen in tarako. If an image is out of screen, gallery app removes this image from the screen immediately.

Comment 27

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

(In reply to John Hu [:johnhu][:johu][:醬糊小弟] from comment #26)
> Yes, I can confirm it. Please use latest version to test it. The bug 997051
> and bug 998227 limit the picture resolution to 2MP using
> CONFIG_MAX_IMAGE_PIXEL_SIZE.

A typo in bug number, it should be bug 997051 and bug 998228

Comment 28

[Bingqing Li]

I tried 10 times on build 20140423164005, and cannot reproduce this issue.


Build Info
-------------------------------------------------------------
Gaia      3771067de006633df690a590a97b4d28c44ef8b2
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/51a4e4d35e30
BuildID   20140423164005
Version   28.1
ro.build.version.incremental=eng.cltbld.20140423.203951
ro.build.date=Wed Apr 23 20:39:59 EDT 2014

Comment 29

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

As per comment 28, change this bug to WONTFIX after the patch of bug 997051 and bug 998228 applied.

Comment 30

[Matthew Vaughan]

Removing qawanted per comment 29.