997019 - [tarako][perf] Camera response is slow when taking picture or video

Status: VERIFIED FIXED

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

Description

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

Attachment: Camera.log

Steps to reproduce
------------------------------------------------------------
1. Launch Camera
2. Take a picture
   --> It took over 5s to taken and save a picture. That's from tapping on Capture button to thumbnail appears. Taking video have worse behavior.

Attached adb logcat. Test starts: 04-16 05:38:51.760


Test build
-----------------------------------------------------------------
Gaia      c62bff0bfb8b069c962dfae2640e931cc9ad1bdf
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/7e764399b4fc
BuildID   20140415164003
Version   28.1
ro.build.version.incremental=eng.cltbld.20140415.201139
ro.build.date=Tue Apr 15 20:11:46 EDT 2014

Comment 1

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

ni? James
is this something on the platform side? Thanks

Comment 2

[James Zhang (Spreadtrum)]

Is it caused by ion page cache?
Ion page cache will save 8MB memory if we don't use camera, but it will cause performance issue when we use camera.

Comment 3

[James Zhang (Spreadtrum)]

(In reply to James Zhang from comment #2)
> Is it caused by ion page cache?
> Ion page cache will save 8MB memory if we don't use camera, but it will
> cause performance issue when we use camera.

Try to disable ion page cache when we use camera.

Comment 5

[James Zhang (Spreadtrum)]

Dafeng will take it.

Comment 6

[Dafeng Xu]

I don't think ion page cache affect performance seriously. I just trid the camera app, it took about 1.5s to taken and save a picture. I alse tried to take picture when turn off ion page cache, there is no obvious extension between taken and save a picture.

Comment 7

[thomas tsai]

This can be repro to take several pictures and sometimes it takes very long to save a picture and even stuck. Camera app can be recovered by pressing home and launch camera again.

Comment 8

[Dafeng Xu]

I just read the Camera.log in attachment, and I see some errors in log. But it dose not affect the next taking picture process.  After this error, the camera hal works well. And I continuously take picture about 5 minutes, it dose not bacome slowly. here is the error log:
04-16 04:55:25.550 V/SprdCameraHardware(   89): void android::HAL_camera_device_disable_msg_type(camera_device*, int32_t)
04-16 04:55:25.550 V/SprdCameraHardware(   89): 'mLock:disableMsgType S.
04-16 04:55:25.550 V/SprdCameraHardware(   89): 'mLock:disableMsgType E.
04-16 04:55:25.550 V/SprdCameraHardware(   89): void android::HAL_camera_device_stop_preview(camera_device*)
04-16 04:55:25.550 V/SprdCameraHardware(   89): stopPreview: E
04-16 04:55:25.550 V/SprdCameraHardware(   89): mLock:stopPreview S.
04-16 04:55:25.550 V/SprdCameraHardware(   89): stopPreviewInternal E
04-16 04:55:25.550 E/SprdCameraHardware(   89): Preview not in progress!
04-16 04:55:25.550 V/SprdCameraHardware(   89): stopPreview: X
04-16 04:55:25.550 V/SprdCameraHardware(   89): mLock:stopPreview E.
04-16 04:55:25.550 V/SprdCameraHardware(   89): int android::HAL_camera_device_cancel_picture(camera_device*)
04-16 04:55:25.550 V/SprdCameraHardware(   89): mLock:cancelPicture S.
04-16 04:55:25.550 V/SprdCameraHardware(   89): not taking a picture (state QCS_ERROR)
04-16 04:55:25.550 V/SprdCameraHardware(   89): cancelPicture: X
04-16 04:55:25.550 V/SprdCameraHardware(   89): mLock:cancelPicture E.
04-16 04:55:25.550 V/SprdCameraHardware(   89): void android::HAL_camera_device_release(camera_device*)
04-16 04:55:25.550 V/SprdCameraHardware(   89): release E
04-16 04:55:25.550 V/SprdCameraHardware(   89): mLock:release S .
04-16 04:55:25.550 E/SprdCameraHardware(   89): Serious error: the camera state is QCS_ERROR, not QCS_IDLE or QCS_INIT!
04-16 04:55:25.550 V/SprdCameraHardware(   89): stopping camera.
04-16 04:55:25.600 I/Gecko   (   84): 
04-16 04:55:25.600 I/Gecko   (   84): ###!!! [Parent][MessageChannel] Error: Channel error: cannot send/recv
04-16 04:55:25.600 I/Gecko   (   84): 
04-16 04:55:25.610 V/SprdCameraOEM(   89): #####DCAM: close device.
04-16 04:55:25.610 V/SprdCameraHardware(   89): STATE CAMERA_FUNC_STOP // STATUS 1
04-16 04:55:25.610 V/SprdCameraHardware(   89): start to change_state.
04-16 04:55:25.610 V/SprdCameraHardware(   89): start to lock.
04-16 04:55:25.610 V/SprdCameraHardware(   89): start to mCameraState.
04-16 04:55:25.610 V/SprdCameraHardware(   89): state transition auto --> QCS_INIT
04-16 04:55:25.610 V/SprdCameraHardware(   89): start to getCameraStateStr.
04-16 04:55:25.610 V/SprdCameraHardware(   89): start to signal.
04-16 04:55:25.610 V/SprdCameraHardware(   89): start to unlock.
04-16 04:55:25.610 V/SprdCameraHardware(   89): release X
04-16 04:55:25.610 V/SprdCameraHardware(   89): mLock:release E.
04-16 04:55:25.650 D/Sensors (   84): activate handle=0; drv=0
04-16 04:55:25.660 I/gralloc.sc8810(   89): ION_IOC_FREE success c6694000,phy addr = 7a20000
04-16 04:55:25.660 I/gralloc.sc8810(   89): ION_IOC_FREE success c6694e00,phy addr = 7980000
04-16 04:55:25.660 I/gralloc.sc8810(   89): ION_IOC_FREE success c6694bc0,phy addr = 79a0000
04-16 04:55:25.660 I/gralloc.sc8810(   89): ION_IOC_FREE success c6694ac0,phy addr = 79c0000
04-16 04:55:25.670 W/CameraService(   89): native_window_api_disconnect failed: Broken pipe (-32)

Comment 9

[James Zhang (Spreadtrum)]

(In reply to thomas tsai from comment #7)
> This can be repro to take several pictures and sometimes it takes very long
> to save a picture and even stuck. Camera app can be recovered by pressing
> home and launch camera again.

dafeng, tell thomas how to disable ion page cache when use camera, and check this issue again.

Comment 10

[James Zhang (Spreadtrum)]

Thomas, can your guys try this commit in device/sprd git repo.


commit a8eb416207cdc9885df73ab74526c12eca1757a0
Author: ying.xu <[e-mail]>
Date:   Fri Feb 14 17:15:56 2014 +0800

    Bug#277261 flush ion_pagecache with ioctl in camera
    
    Change-Id: I41c489a5266745287770b808a5cb0f6c7cee41ea

diff --git a/common/libs/libcamera/sc8810/SprdCameraHardwareInterface.cpp b/common/libs/libcamera/sc8810/SprdCameraHardwareInterface.cpp
index 8f70af2..74ddd3d 100644
--- a/common/libs/libcamera/sc8810/SprdCameraHardwareInterface.cpp
+++ b/common/libs/libcamera/sc8810/SprdCameraHardwareInterface.cpp
@@ -36,6 +36,7 @@
 #include "../sc8810/SprdOEMCamera.h"
 #include "../sc8810/SprdCameraHardwareConfig.h"
 //#include "SprdCameraHardwareStub.h"
+#include <linux/ion.h>
 
 #ifdef CONFIG_CAMERA_ISP
 extern "C" {
@@ -217,9 +218,18 @@ gralloc_module_t const* SprdCameraHardware::mGrallocHal;
 #define ROUND_TO_PAGE(x)  (((x)+0xfff)&~0xfff)
 
     // Called with mStateLock held!
+static int ion_cache_fd = 0;
 bool SprdCameraHardware::startCameraIfNecessary()
 {
         if (mCameraState == QCS_INIT) {
+               if (ion_cache_fd == 0) {
+                       ion_cache_fd = open("/dev/ion", O_RDWR);
+                       if (ion_cache_fd < 0)
+                               ALOGE("CameraIfNecessary: fail to open /dev/ion for ion page cache.");
+                       else
+                               ioctl(ion_cache_fd, ION_IOC_DISABLE_CACHE, NULL);
+               }
+
                 ALOGV("waiting for camera_init to initialize.");
                 if(CAMERA_SUCCESS != camera_init(g_camera_id)){
                         mCameraState = QCS_INIT;
@@ -569,7 +579,13 @@ void SprdCameraHardware::release()
         mStateLock.unlock();
         ALOGV("release X");
         ALOGV("mLock:release E.\n");
-    }
+
+       if (ion_cache_fd > 0) {
+               ioctl(ion_cache_fd, ION_IOC_ENABLE_CACHE, NULL);
+               close(ion_cache_fd);
+               ion_cache_fd = 0;
+       }
+}
 
 SprdCameraHardware::~SprdCameraHardware()
 {

Comment 11

[James Zhang (Spreadtrum)]

Attachment: disable_ion.patch

Thomas, can you try this patch?

Comment 12

[thomas tsai]

Hi Vincent : please try.

Comment 13

[James Zhang (Spreadtrum)]

And maybe it's sdcard issue. Can you change a sdcard to check?

Comment 14

[Dafeng Xu]

Maybe there is something wrong in your sdcard, I think.

Comment 15

[Vincent Liu[:vliu]]

Without the patch in Comment 10, Camera app took a little long time (almost 4~5 sec) to store this picture for the first snapshot after launched Camera app. Sometimes the screen stocks amid saving picture. But with the patch, the time consumed by the first snapshot was reduced and the stuck never happens. In my point of view, this patch can improve the performance.

Comment 16

[James Zhang (Spreadtrum)]

(In reply to Vincent Liu[:vliu] from comment #15)
> Without the patch in Comment 10, Camera app took a little long time (almost
> 4~5 sec) to store this picture for the first snapshot after launched Camera
> app. Sometimes the screen stocks amid saving picture. But with the patch,
> the time consumed by the first snapshot was reduced and the stuck never
> happens. In my point of view, this patch can improve the performance.

Fixed on my side.

commit acce8815d3c96e78857e69e5b44ee4392383b124
Author: ying.xu <[e-mail]>
Date:   Fri Feb 14 17:15:56 2014 +0800

    Bug#269156 mozilla Bug 997019 - [tarako][perf] Camera response is slow when taking picture or video
    [self test   ] yes
    
    Change-Id: Id8c2f39aeac179b39a6ef884f04c58205b81c3e0

Comment 17

[:Eli Perelman]

Do you have any metrics on how quick the interaction is now?

Comment 18

[James Zhang (Spreadtrum)]

(In reply to Eli Perelman from comment #17)
> Do you have any metrics on how quick the interaction is now?

See comment 15. It's about 1~2 sec now.
It's our ion page cache performance issue. We disable it when camera works.

Comment 19

[Mike Lien[:mlien]]

verified and fixed with today's daily build
Gaia      3771067de006633df690a590a97b4d28c44ef8b2
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/51a4e4d35e30
BuildID   20140423164005
Version   28.1

Comment 20

[:Eli Perelman]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18649

In addition to the given patch for this bug, I have also added a Sync UI for taking pictures, to give the user a visual indication that there is some processing going on. This is currently implemented as the loading spinner appearing where the Gallery icon is located.

Previously the Gallery icon being disabled meant `display: none` for the entire Gallery container, but with this PR now just the inner span of the actual icon is hidden, and swapped out with the spinner. So the largest risk with this patch is we keep a disabled anchor visible and change the icon, which should be minimal.

This PR is only relevant in prior to v2.0 since the UI has changed, so requesting it for v1.3t as Tarako will be the device that will benefit the user the most for being given visual indication that background processing is taking place.

Comment 21

[David Flanagan [:djf]]

Comment on attachment 8411894 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/18649

r- because the patch has bitrotted.  I think you were working on this before I landed the giant patch in bug 989290.  That patch introduced a spinner in the camera that is displayed when the camera is busy rotating an image before returning it from a pick activity.  (To see it, go to the SMS app, compose a new message, click the paperclip to add an attachment, then select Camera. Once you take a picture and click 'select', you'll see the spinner)

I've put my spinner right in the middle of the screen, and got ui-review for doing that. So I suggest that you modify your patch to leave the gallery button alone and just use the same <progress> element that I already added. 

I agree that this is worth doing. I'm not sure whether we should continue to use this closed bug for it, though on the other hand, we do have the 1.3T+ on this bug :-)

Comment 22

[David Flanagan [:djf]]

Eli: another comment on your patch. I was surprised to see that it is HTML and CSS only.  I don't like that this patch assumes that a spinner should be displayed any time the gallery button is disabled.  I think there are other circumstances when that button might be disabled. If you have a passcode on your lockscreen and invoke the camera from the lockscreen, for example, there will be no gallery button. But we don't want a spinner in that case.  So I think that to fix this correctly you'll have to start up with the spinner running and the hide it as soon as we are actually ready to take pictures.

Comment 23

[Montoya Clemmons (MClemmons)]

Verified following STR from Comment 0, the time from taking picture to seeing it displayed as a thumbnail was 1-2 seconds consistently on 1.3T build. 

Environmental Variables:
Device: Tarako 1.3T
Build ID: 20140609014001
Gaia: adb258b88485ccf1ff2e971de0fda547e4bb7149
Gecko: e8b77a1c7c78
Version: 28.1 (1.3T) 
Firmware Version: SP6821a-Gonk-4.0-5-12
User Agent: Mozilla/5.0 (Mobile; rv:28.1) Gecko/28.1 Firefox/28.1