Closed Bug 997019 Opened 10 years ago Closed 10 years ago

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

Categories

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

Other
Other
defect

Tracking

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

VERIFIED FIXED
1.4 S6 (25apr)
blocking-b2g 1.3T+
Tracking Status
b2g-v1.3T --- verified

People

(Reporter: angelc04, Assigned: Dafeng.Xu)

References

Details

(Keywords: perf, Whiteboard: [c=progress p= s= u=tarako][POVB])

Attachments

(3 files)

Attached file 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
blocking-b2g: --- → 1.3T?
ni? James
is this something on the platform side? Thanks
Flags: needinfo?(james.zhang)
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.
Assignee: nobody → Dafeng.Xu
Flags: needinfo?(james.zhang)
(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.
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [c=progress p= s= u=tarako]
blocking-b2g: 1.3T? → 1.3T+
Dafeng will take it.
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.
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.
Whiteboard: [c=progress p= s= u=tarako] → [c=progress p= s= u=tarako][POVB]
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)
(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.
Thomas, can your guys try this commit in device/sprd git repo.


commit a8eb416207cdc9885df73ab74526c12eca1757a0
Author: ying.xu <ying.xu@spreadtrum.com>
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()
 {
Flags: needinfo?(ttsai)
Thomas, can you try this patch?
Hi Vincent : please try.
Flags: needinfo?(ttsai) → needinfo?(vliu)
And maybe it's sdcard issue. Can you change a sdcard to check?
Maybe there is something wrong in your sdcard, I think.
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.
Flags: needinfo?(vliu)
(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 <ying.xu@spreadtrum.com>
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
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Do you have any metrics on how quick the interaction is now?
Flags: needinfo?(james.zhang)
(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.
Flags: needinfo?(james.zhang)
See Also: → 994077
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
Status: RESOLVED → VERIFIED
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.
Attachment #8411894 - Flags: review?(dflanagan)
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 :-)
Attachment #8411894 - Flags: review?(dflanagan) → review-
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.
Target Milestone: --- → 1.4 S6 (25apr)
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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: