1107300 - (gonk-L-Camera) Camera Android L Porting

Status: RESOLVED FIXED

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

Comment 1

[Askeing Yen[:askeing][:fyen]]

Porting Camera module to support lollipop release

Comment 2

[Kai-Chih Hu [:khu]]

So, what's the issue now?

Comment 3

[Vincent Liu[:vliu]]

Currently the flag of Camera is disabled in L build since it need the patch of 1098970. 

Hi Wenyuan,

Can you check if we can help on this issue based on the WIP of 1098970? Thanks!

Comment 4

[Wenyuan Chi [:wchi]]

Hi Vincent,
Sure, I will check it

Comment 5

[Wenyuan Chi [:wchi]]

Here is the status update.

Camera porting is blocked by #1098970.
There is a linking issue in NativeWindow module.
Right now, we can workaround the linking issue, but there is still a problem on initializing camera.

D/mm-camera-intf(  184): mm_camera_open: dev name = /dev/video1, cam_idx = 1
D/mm-camera-intf(  184): mm_camera_open:failed with I/O error retrying after 20 milli-seconds
D/mm-camera-intf(  184): mm_camera_open:failed with I/O error retrying after 20 milli-seconds


Keep working on this.

Comment 6

[Wenyuan Chi [:wchi]]

Attachment: [wip] integrate with NativeWindowLL

Two extra steps required to use this patch, you have to 
1. manually start qcamerasvr ("adb shell start qcamerasvr")
2. enable sdcard function (modify /system/etc/volume.cfg, change /storage/emulated/legacy to /data)

Comment 7

[Wenyuan Chi [:wchi]]

Create bug 1112017 to track green blinking issue

Comment 8

[Ken Chang[:kenkai]]

Wenyuan, you are working on this bug, so I set you as an assignee.

Comment 9

[Wenyuan Chi [:wchi]]

1112175

Comment 10

[Wenyuan Chi [:wchi]]

Attachment: [wip] integrate with NativeWindowLL (v2)

Comment 11

[Vincent Liu[:vliu]]

Since @wchi is on PTO, I will take care of the following thing.

Comment 12

[Vincent Liu[:vliu]]

Currently the patch can activate camera preview and taking picture. For the camera recording, I tried to set MOZ_OMX_ENCODER=1 but build break happens. For this, I will then try to fix this problem.

Comment 13

[Vincent Liu[:vliu]]

Attachment: 5.log

I got some error messages when I tried to do camera recording. The detailed log print was attached. It seems that these messages cause OMX Component deinit.
Does anyone can help me to figure it out? Thanks.

E/OMXMaster(  186): A component of name 'OMX.qcom.audio.decoder.aac' already exists, ignoring this one.
E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663297) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663297) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663297) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663297) ERROR: 0x8000101a
D/camera  ( 1244): OnNewPreviewFrame: we have 1 preview frame listener(s)
D/camera  ( 1244): OnNewPreviewFrame: got 1920 x 1080 frame
D/GonkNativeWindow( 1244): GonkNativeWindow::returnBuffer
E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663297) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663297) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663297) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663297) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663297) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663297) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663297) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000101a
E/OMXNodeInstance(  186): getParameter(100663297) ERROR: 0x8000101a
....
....
I/OMX-VENC(  186): Component_init : OMX.qcom.video.encoder.mpeg4 : return = 0x0
E/OMX-VENC(  186): get_parameter: OMX_IndexParamVideoProfileLevelQuerySupported nProfileIndex ret NoMore 2
E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000100e
W/ACodec  (  186): do not know color format 0x7fa30c04 = 2141391876
W/ACodec  (  186): do not know color format 0x7f000789 = 2130708361
E/OMXNodeInstance(  186): getParameter(100663297) ERROR: 0x8000100e
D/camera  ( 1244): OnNewPreviewFrame: we have 1 preview frame listener(s)
D/camera  ( 1244): OnNewPreviewFrame: got 1920 x 1080 frame
D/GonkNativeWindow( 1244): GonkNativeWindow::returnBuffer
I/OMX-VENC(  186): Component Deinit

Comment 14

[Vincent Liu[:vliu]]

(In reply to Vincent Liu[:vliu] from comment #13)
> Created attachment 8539650 [details]
> 5.log
> 
> I got some error messages when I tried to do camera recording. The detailed
> log print was attached. It seems that these messages cause OMX Component
> deinit.
> Does anyone can help me to figure it out? Thanks.
> 
> E/OMXMaster(  186): A component of name 'OMX.qcom.audio.decoder.aac' already
> exists, ignoring this one.
> E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000101a
> E/OMXNodeInstance(  186): getParameter(100663297) ERROR: 0x8000101a
> E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000101a
> E/OMXNodeInstance(  186): getParameter(100663297) ERROR: 0x8000101a
> E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000101a

These error codes were indicating OMX_ErrorUnsupportedIndex.

Comment 15

[Sotaro Ikeda [:sotaro]]

(In reply to Vincent Liu[:vliu] from comment #14)
> (In reply to Vincent Liu[:vliu] from comment #13)
> > Created attachment 8539650 [details]
> > 5.log
> > 
> > I got some error messages when I tried to do camera recording. The detailed
> > log print was attached. It seems that these messages cause OMX Component
> > deinit.
> > Does anyone can help me to figure it out? Thanks.
> > 
> > E/OMXMaster(  186): A component of name 'OMX.qcom.audio.decoder.aac' already
> > exists, ignoring this one.
> > E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000101a
> > E/OMXNodeInstance(  186): getParameter(100663297) ERROR: 0x8000101a
> > E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000101a
> > E/OMXNodeInstance(  186): getParameter(100663297) ERROR: 0x8000101a
> > E/OMXNodeInstance(  186): getParameter(100663311) ERROR: 0x8000101a
> 
> These error codes were indicating OMX_ErrorUnsupportedIndex.

They seems video related parameters.
http://androidxref.com/5.0.0_r2/xref/frameworks/native/include/media/openmax/OMX_Index.h#171

Comment 16

[Blake Wu [:bwu][:blakewu]]

I think you could start debugging from OMXCodec.cpp to see which getParameter() function returns the error and check if the set index is valid or not.

Comment 17

[Vincent Liu[:vliu]]

For camera recording, I will file another bug to track it. The bug will keep landing patch for enabling camera preview/taking-picture.

Comment 18

[Vincent Liu[:vliu]]

Attachment: part1-bug-1107300-nativewindow-camera.patch

Hi Sotaro,

This patch integrates camera NativeWindow for Android L base.

Comment 19

[Vincent Liu[:vliu]]

Attachment: part2-bug-1107300-enable-camera.patch

enabling Camera in configure.in

Comment 20

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8539908 [details] [diff] [review]
part1-bug-1107300-nativewindow-camera.patch

Review of attachment 8539908 [details] [diff] [review]:
-----------------------------------------------------------------

review+ if the comment is addressed.

::: dom/camera/GonkCameraControl.cpp
@@ +39,5 @@
>  #include "nsIVolumeService.h"
>  #include "AutoRwLock.h"
>  #include "GonkCameraHwMgr.h"
>  #include "GonkRecorderProfiles.h"
> +//#include "GrallocImages.h"

This seems unnecessary.

Comment 21

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8539909 [details] [diff] [review]
part2-bug-1107300-enable-camera.patch

Review of attachment 8539909 [details] [diff] [review]:
-----------------------------------------------------------------

Seems OK. But change to configure.in need build peer's review. mwu seems correct person for it.

Comment 22

[Vincent Liu[:vliu]]

Attachment: part1-bug-1107300-nativewindow-camera-v2.patch

(In reply to Sotaro Ikeda [:sotaro PTO Dec/24-Jan/6] from comment #20)
> Comment on attachment 8539908 [details] [diff] [review]
> part1-bug-1107300-nativewindow-camera.patch
> 
> Review of attachment 8539908 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> review+ if the comment is addressed.
> 
> ::: dom/camera/GonkCameraControl.cpp
> @@ +39,5 @@
> >  #include "nsIVolumeService.h"
> >  #include "AutoRwLock.h"
> >  #include "GonkCameraHwMgr.h"
> >  #include "GonkRecorderProfiles.h"
> > +//#include "GrallocImages.h"
> 
> This seems unnecessary.

Sorry, the expected patch should include GrallocImages.h. 

diff --git a/dom/camera/GonkCameraControl.cpp b/dom/camera/GonkCameraControl.cpp
index 983aac6..19ad460 100644
--- a/dom/camera/GonkCameraControl.cpp
+++ b/dom/camera/GonkCameraControl.cpp
@@ -40,6 +40,7 @@
 #include "AutoRwLock.h"
 #include "GonkCameraHwMgr.h"
 #include "GonkRecorderProfiles.h"
+#include "GrallocImages.h"
 #include "CameraCommon.h"
 #include "GonkCameraParameters.h"
 #include "DeviceStorageFileDescriptor.h"
diff --git a/dom/camera/GonkCameraHwMgr.cpp b/dom/camera/GonkCameraHwMgr.cpp
index fef9c62..50b74b0 100644

Before L porting, GrallocImages.h was included in GonkNativeWindowXX.cpp/GonkNativeWindowXX.h. In L porting, the patch of bug 1098970 has removed it since the medthods in GrallocImage is used by camera instead of NativeWindow. Put it in GonkCameraControl.cpp seem make sense. The below link is an example.

http://dxr.mozilla.org/mozilla-central/source/dom/camera/GonkCameraControl.cpp#2042

Could you please also give comment if it is reasonable? Thanks

Comment 23

[Vincent Liu[:vliu]]

Comment on attachment 8539909 [details] [diff] [review]
part2-bug-1107300-enable-camera.patch

(In reply to Sotaro Ikeda [:sotaro PTO Dec/24-Jan/6] from comment #21)
> Comment on attachment 8539909 [details] [diff] [review]
> part2-bug-1107300-enable-camera.patch
> 
> Review of attachment 8539909 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Seems OK. But change to configure.in need build peer's review. mwu seems
> correct person for it.

Hi Michael,

Could you please also help me to review it? Thanks

Comment 24

[Vincent Liu[:vliu]]

Currently the landing of patch in this bug will cause build break without the patch in Bug 1113655.

Comment 25

[Vincent Liu[:vliu]]

Attachment: part2-bug-1107300-enable-camera.patch

Reattach the part2 of patch since it has line conflict comparing with the current code base.

Comment 26

[Vincent Liu[:vliu]]

try server result:

[1] : https://treeherder.mozilla.org/#/jobs?repo=try&revision=dcff4aad8af0
[2] : https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e8e5313d3e2

Comment 27

[Vincent Liu[:vliu]]

https://hg.mozilla.org/integration/b2g-inbound/rev/f0cf1c551727
https://hg.mozilla.org/integration/b2g-inbound/rev/ca1f91a96242

Comment 28

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f0cf1c551727
https://hg.mozilla.org/mozilla-central/rev/ca1f91a96242