Closed Bug 972779 Opened 6 years ago Closed 6 years ago

[Tarako] Saving picture after editing cause Gallery stop

Categories

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

Other
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.3T+, b2g-v1.3 fixed, b2g-v1.3T verified, b2g-v1.4 fixed)

RESOLVED FIXED
1.4 S3 (14mar)
blocking-b2g 1.3T+
Tracking Status
b2g-v1.3 --- fixed
b2g-v1.3T --- verified
b2g-v1.4 --- fixed

People

(Reporter: angelc04, Assigned: gwagner)

Details

(Whiteboard: [SI-testing-blocker],[systemsfe])

Attachments

(5 files)

STR:
0. Take a picture use camera
1. Launch Gallery
2. View a picture
3. Edit the picture
4. Tap on "Done" after modification
   --> The performance of saving a pic after editing is bad. Sometimes it took over 5s to save pic. Sometimes it may cause Gallery stop.
blocking-b2g: --- → 1.3T?
can we confirm on the latest build? (please provide build ID)
and provide a video? 
thanks
Flags: needinfo?(pcheng)
Attached video STR
Attached is the video. Build number: 20140217032035
Flags: needinfo?(pcheng)
triage: 1.3T+ as this is a broken feature on tarako
blocking-b2g: 1.3T? → 1.3T+
Whiteboard: [SI testing blocker]
Whiteboard: [SI testing blocker] → [SI-testing-blocker]
Hema, can you please help find someone who can investigate this ?
Flags: needinfo?(hkoka)
(In reply to pcheng from comment #0)
> 3. Edit the picture
> 4. Tap on "Done" after modification
>    --> The performance of saving a pic after editing is bad. Sometimes it
> took over 5s to save pic. Sometimes it may cause Gallery stop.

Gallery stop might be caused by killed by lowmemory killer. Can we have a logcat log and dmesg log?
Do we have Tarako devices in MV? If so, can have Punam help investigate.

Please provide info Sotaro asked
Flags: needinfo?(pcheng)
Flags: needinfo?(hkoka)
Flags: needinfo?(bbajaj)
(In reply to Hema Koka [:hema] from comment #6)
> Do we have Tarako devices in MV? If so, can have Punam help investigate.
> 
> Please provide info Sotaro asked

I have one that can be used for that.
Tarako / Android Gallery provide Crop. Rotate function only. no effect editing features. fyi.
Hema, assuming your team will get a tarako device from Fabrice for this. do you mind assigning this bug to someone? thanks
Flags: needinfo?(hkoka)
What size (in megapixels) are the images you are trying to edit?

Does the Tarako build have a custom camera.json file in GAIA_DISTRIBUTION_DIR to set the maximum image size?  If not, you're allowing up to 5mp images which are probably too big for that device.
Flags: needinfo?(pcheng)
Flags: needinfo?(pcheng)
Flags: needinfo?(hkoka)
Flags: needinfo?(bbajaj)
Also, 5s to save an edited image is actually a pretty normal amount of time.
(In reply to David Flanagan [:djf] from comment #10)
> What size (in megapixels) are the images you are trying to edit?
> 
> Does the Tarako build have a custom camera.json file in
> GAIA_DISTRIBUTION_DIR to set the maximum image size?  If not, you're
> allowing up to 5mp images which are probably too big for that device.

The image I used is taken by Tarako. The size is 93KB, resolution is 480*640.

With the latest tarako build 20140310152650, the performance is better. croping/applying filters and saving can be done within 5s.
Flags: needinfo?(pcheng)
(In reply to pcheng from comment #12)
> (In reply to David Flanagan [:djf] from comment #10)
> > What size (in megapixels) are the images you are trying to edit?
> > 
> > Does the Tarako build have a custom camera.json file in
> > GAIA_DISTRIBUTION_DIR to set the maximum image size?  If not, you're
> > allowing up to 5mp images which are probably too big for that device.
> 
> The image I used is taken by Tarako. The size is 93KB, resolution is 480*640.
> 
> With the latest tarako build 20140310152650, the performance is better.
> croping/applying filters and saving can be done within 5s.

James, it seems like from the latest testing, editing picture can be done now. Do you still observe this on your side? and is this still a blocker for you? Thanks
Flags: needinfo?(james.zhang)
Yes, I reduce the camera resolution to 0.5M for test only, and our customer need 2M camera.
It's still blocker issue if I change resolution to 2M.
Flags: needinfo?(james.zhang)
can you please change it back to 2M so the problem can be seen? 
we should reconfirm the problem once the camera is back to 2M. Thanks
blocking-b2g: 1.3T+ → 1.3T?
Flags: needinfo?(james.zhang)
(In reply to Joe Cheng [:jcheng] from comment #15)
> can you please change it back to 2M so the problem can be seen? 
> we should reconfirm the problem once the camera is back to 2M. Thanks

This bug depend on resolution of picture.
Flags: needinfo?(james.zhang)
blocking-b2g: 1.3T? → 1.3T+
So are my assumptions right that the tarako camera produces 2M pics and we are having problems processing 2M pics?
Tried steps in #comment0  on below environment
Device: Tarako
BuildId:20140310141338
Platform 28.0
Os Version 1.3

Picture taken from camera was of resolution 480x640 and size 103 KB. 
I was able to edit pictures in ~ 1-2 secs

Can you please attach the image taken from Tarako that's making gallery crash? Thanks




Can you pl. attach the images taken from tarako that's making gallery crash
Flags: needinfo?(james.zhang)
Please revert device/sprd and kernel change for 0.5M camera.
Then you can reproduce this issue.
The root cause is gallery image editor should support exif data, then kernel can shrink 3M camera rataion reserved memory.

in device/sprd
git revert 055ef046ffa0c9e399b3b7bb28901776bbfc2daf

commit 055ef046ffa0c9e399b3b7bb28901776bbfc2daf
Author: dafeng.xu <dafeng.xu@spreadtrum.com>
Date:   Fri Mar 7 17:20:36 2014 -0500

    Bug #287001 cut the camera runtime memory
    
    [bug number  ]287001
    [root cause  ]the camera resolation is too high
    [changes     ]ajust the camera resolation
    [side effects]none
    [reviewers   ]james.zhang
    
    Change-Id: I007f43534571d8f6e9ed67a49aeb7490af0a2140



in kernel
git revert c72664fa690318ee526d5aa8fa3bcc278c3da480

commit c72664fa690318ee526d5aa8fa3bcc278c3da480
Author: dafeng.xu <dafeng.xu@spreadtrum.com>
Date:   Fri Mar 7 17:26:01 2014 -0500

    Bug #287001 cut the camera runtime memory
    
    [bug number  ]287001
    [root cause  ]the camera resolation is too high
    [changes     ]ajust the camera resolation
    [side effects]none
    [reviewers   ]james.zhang
    
    Change-Id: I552a6a34d86a9b322c9a34ad1693fa39b9f92fcf
Flags: needinfo?(james.zhang)
ImageEditor.prototype.getFullSizeBlob = function(type, done, progress) {
  const TILE_SIZE = 1024;
  var self = this;

The current tile size is 1024*1024*RGBA=4MB. James, can you try to reduce that to 256 and 512 and see whether that helps? If it does we can make this dependent on the device or the image size of some other indicator that memory is tight.

Gregor, we need a crash dump for this to see where exactly we run out of memory.
(also, I bet a smaller tile makes saving the image faster since we have to hit zram less, just a guess)
ni? Gregor so it shows up in his NI queue
Flags: needinfo?(anygregor)
If the TILE size change isn't enough then we should check whether we can use the WEBGL_lose_context extension to eagerly give up the WebGL context. Just setting it to null only releases the OpenGL context after the next GC. The WebGL context is no longer needed once we are done with the ImageProcessing object so we could do something like .destroy() on it.

http://www.khronos.org/registry/webgl/extensions/WEBGL_lose_context/
I think I should change camera resolution back to 2M first.
OK, please sync code in device/sprd and kernel, camera resolution is 2M now.
Hema, do you think someone from your team can look at this as well? there are several gallery related tarako bugs. Thanks
Flags: needinfo?(hkoka)
(In reply to James Zhang from comment #24)
> I think I should change camera resolution back to 2M first.

Yes, please. It really makes us confused the real situation. We should use the configurations for end product's SPEC.
Attaching the log cat  for crash seen on tarako device when trying to edit a 2MB image loaded in gallery app via sdcard. Please note 2MB image was loaded from sdcard and not taken from tarako camera.

I am also attaching log cat of successful edit of a picture - 103KB taken from Tarako camera.

Both the log cat are from tarako device with BuildId:20140310141338
There's not much in this logcat but it's likely we just OOM. Punam, can you attach the picture you were trying to edit?
James, can you test what Andreas said in comment 20?
Flags: needinfo?(james.zhang)
(In reply to Evelyn Hung [:evelyn] from comment #32)
> James, can you test what Andreas said in comment 20?

I'm too busy, assign to yang.zhao.
Yang, please provide this data.
Flags: needinfo?(james.zhang) → needinfo?(yang.zhao)
Joe Cheng -- Punam is helping with the other EXIF Tarako bug. As discussed over email, can you see if John Hu or Gary can help with this one. 

Thanks!
Hema
Flags: needinfo?(hkoka) → needinfo?(jcheng)
Just tested with TILE_SIZE 256 & 512, the saving went much smoother.
Tested with TILE_SIZE 256 & 512,the image could be saved successfully and the saving process went smoother too.
Flags: needinfo?(yang.zhao)
\o/. Thanks Yang.
Flags: needinfo?(jcheng)
Flags: needinfo?(anygregor)
Gregor, can you please patch this for me? It seems 512 is fine and that shouldn't affect 5MP performance too much either. djf can review this.
Assignee: nobody → anygregor
Attached file link
Comment on attachment 8391417 [details] [review]
link

I don't know this part of the code but I guess this is what you asked for.
Attachment #8391417 - Flags: review?(dflanagan)
Comment on attachment 8391417 [details] [review]
link

I love reviewing one line patches!

This looks great. I recall going back and forth on the best tile size.  I suppose it must have been slightly quicker with the bigger tile. But 512x512 seems totally reasonable, and as noted, will result in smoother updating of the progress bar, too.
Attachment #8391417 - Flags: review?(dflanagan) → review+
Whiteboard: [SI-testing-blocker] → [SI-testing-blocker],[systemsfe]
Target Milestone: --- → 1.4 S3 (14mar)
https://github.com/mozilla-b2g/gaia/commit/f3c4a2fc0cfbeccc9c595f06b436707fa2e6ca0a
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Comment on attachment 8391417 [details] [review]
link

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined:
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky):
[String changes made]:
Attachment #8391417 - Flags: approval-gaia-v1.3?(fabrice)
Attachment #8391417 - Flags: approval-gaia-v1.3?(fabrice) → approval-gaia-v1.3+
(In reply to Andreas Gal :gal from comment #37)
> \o/. Thanks Yang.

hi,Andreas
    Now it's ok to save pictures after editing in gallery,but it's not ok if you pick an image and edit it from contacts.Is this the same issue to this one?
Yang, lets file a new bug since this one is done here. Can you please attach steps to reproduce the bug? Thanks!
Verified fixed in todays build

1.3t Environmental Variables:
Device: Tarako 1.3t
BuildID: 20140428014001
Gaia: 8895b180ed636069473703d0e7b73086989601ce
Gecko: 7caf4b5abfce
Version: 28.1
Firmware Version: sp6821
You need to log in before you can comment on or make changes to this bug.