Bug 1107300 (gonk-L-Camera)

Camera Android L Porting

RESOLVED FIXED in 2.2 S4 (23jan)

Status

Firefox OS
Gaia::Camera
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: askeing, Assigned: vliu)

Tracking

unspecified
2.2 S4 (23jan)
All
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(feature-b2g:2.2+)

Details

Attachments

(2 attachments, 5 obsolete attachments)

Comment hidden (empty)
(Assignee)

Updated

3 years ago
Blocks: 1107283
(Reporter)

Comment 1

3 years ago
Porting Camera module to support lollipop release

Comment 2

3 years ago
So, what's the issue now?

Updated

3 years ago
Blocks: 1107670
(Assignee)

Comment 3

3 years ago
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!
Flags: needinfo?(wchi)

Comment 4

3 years ago
Hi Vincent,
Sure, I will check it
Flags: needinfo?(wchi)

Updated

3 years ago
blocking-b2g: --- → 2.2?

Updated

3 years ago
Depends on: 1098970

Comment 5

3 years ago
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.

Updated

3 years ago
No longer blocks: 1107283

Updated

3 years ago
Depends on: 1107283

Updated

3 years ago
feature-b2g: --- → 2.2+
QA Whiteboard: [2.2-feature-qa+]

Comment 6

3 years ago
Created attachment 8536942 [details] [diff] [review]
[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

3 years ago
Create bug 1112017 to track green blinking issue

Comment 8

3 years ago
Wenyuan, you are working on this bug, so I set you as an assignee.
Assignee: nobody → wchi

Comment 9

3 years ago
1112175
Depends on: 1107291, 1112175, 1112017

Comment 10

3 years ago
Created attachment 8538373 [details] [diff] [review]
[wip] integrate with NativeWindowLL (v2)
Attachment #8536942 - Attachment is obsolete: true
Attachment #8538373 - Flags: feedback?(vliu)

Updated

3 years ago
Depends on: 938034
(Assignee)

Comment 11

3 years ago
Since @wchi is on PTO, I will take care of the following thing.
(Assignee)

Updated

3 years ago
Attachment #8538373 - Flags: feedback?(vliu) → feedback-
QA Whiteboard: [2.2-feature-qa+] → [2.2-feature-qa+][COM=Gaia::Camera]
Component: General → Gaia::Camera

Updated

3 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 12

3 years ago
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.
(Assignee)

Comment 13

3 years ago
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
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
Flags: needinfo?(sotaro.ikeda.g)
Flags: needinfo?(mhabicher)
Flags: needinfo?(bwu)
(Assignee)

Comment 14

3 years ago
(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.
(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
Flags: needinfo?(sotaro.ikeda.g)
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.
Flags: needinfo?(bwu)
(Assignee)

Comment 17

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

Comment 18

3 years ago
Created attachment 8539908 [details] [diff] [review]
part1-bug-1107300-nativewindow-camera.patch

Hi Sotaro,

This patch integrates camera NativeWindow for Android L base.
Attachment #8538373 - Attachment is obsolete: true
Attachment #8539650 - Attachment is obsolete: true
Attachment #8539908 - Flags: review?(sotaro.ikeda.g)
(Assignee)

Comment 19

3 years ago
Created attachment 8539909 [details] [diff] [review]
part2-bug-1107300-enable-camera.patch

enabling Camera in configure.in
Attachment #8539909 - Flags: review?(sotaro.ikeda.g)
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.
Attachment #8539908 - Flags: review?(sotaro.ikeda.g) → review+
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.
Attachment #8539909 - Flags: review?(sotaro.ikeda.g) → review+
(Assignee)

Comment 22

3 years ago
Created attachment 8539947 [details] [diff] [review]
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
Attachment #8539908 - Attachment is obsolete: true
Attachment #8539947 - Flags: review?(sotaro.ikeda.g)

Updated

3 years ago
Attachment #8539947 - Flags: review?(sotaro.ikeda.g) → review+
(Assignee)

Comment 23

3 years ago
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
Attachment #8539909 - Flags: review?(mwu)

Updated

3 years ago
Attachment #8539909 - Flags: review?(mwu) → review+
(Reporter)

Updated

3 years ago
Depends on: 1114477

Updated

3 years ago
No longer depends on: 1114477
(Reporter)

Updated

3 years ago
Depends on: 1114477

Updated

3 years ago
Target Milestone: --- → 2.2 S3 (9jan)
Flags: needinfo?(mhabicher)
(Assignee)

Comment 24

3 years ago
Currently the landing of patch in this bug will cause build break without the patch in Bug 1113655.
Depends on: 1113655

Updated

3 years ago
blocking-b2g: 2.2? → ---
(Assignee)

Comment 25

3 years ago
Created attachment 8546517 [details] [diff] [review]
part2-bug-1107300-enable-camera.patch

Reattach the part2 of patch since it has line conflict comparing with the current code base.
Attachment #8539909 - Attachment is obsolete: true

Updated

3 years ago
Target Milestone: 2.2 S3 (9jan) → 2.2 S4 (23jan)
(Assignee)

Updated

3 years ago
Assignee: wchi → vliu
https://hg.mozilla.org/mozilla-central/rev/f0cf1c551727
https://hg.mozilla.org/mozilla-central/rev/ca1f91a96242
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.