Closed Bug 1114477 Opened 5 years ago Closed 5 years ago

[Gonk-L-Camera]:Integrate Camera recording.

Categories

(Firefox OS Graveyard :: GonkIntegration, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(feature-b2g:2.2+, b2g-v2.2 fixed)

RESOLVED FIXED
2.2 S3 (9jan)
feature-b2g 2.2+
Tracking Status
b2g-v2.2 --- fixed

People

(Reporter: vliu, Assigned: jhlin)

References

Details

Attachments

(5 files)

This is a porting issue based on bug 1107300 Comment 17.
Attached file 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

These error codes were indicating OMX_ErrorUnsupportedIndex.
Attached file gdb-bt.log
gdb backtrack for the first error code which OMXCodec was trying to QueryCodec.
Hi Blake,

Would you please have some idea for the log I attached? Thanks.
Flags: needinfo?(bwu)
Comment on attachment 8539970 [details]
gdb-bt.log

John should be better than me to answer this question. :)
Flags: needinfo?(bwu) → needinfo?(jolin)
According to the backtrace in gdb-bt.log, it's querying audio codec (mime type "audio/mpeg"), but the index to query is for  video codec only (IndexParamVideoProfileLevelQuerySupported).
FWICT, QueryCodec() returns OK (with empty CodecCapabilities) and MediaCodecList just pass that to MediaCodecInfo. It seems those are not actual errors in this case.
Flags: needinfo?(jolin)
Attached file camera-rec-7.log
Hi John,

I tried to enable the log flag for OMXCodec/ACodec and make sure the library has beed updated by date. The file was attached. Could you please also have a check for me? Thanks
Blocks: gonk-L
It's the porting issue. Marked feature-b2g : 2.2+.

Hi Vincent, we will need ETA for better understanding the risk to the next sprint. Thanks.
No longer blocks: gonk-L-Camera
feature-b2g: --- → 2.2+
Flags: needinfo?(vliu)
Flags: needinfo?(jolin)
Target Milestone: --- → 2.2 S3 (9jan)
Tried adding some code to dump data in the AVC/H.264 buffers from encoder and found it playable in VLC. I guess the problem is not the encoder.
Flags: needinfo?(jolin)
A hacky patch to work around the 'stagefright log not showing up' problem (It's because of failed socket() call at [1], but I don't know why that happens). Anyway this should make debugging much easier.

[1] https://android.googlesource.com/platform/system/core/+/master/liblog/logd_write.c#113
Hi John, assign to you since you're working on it.
Assignee: nobody → jolin
(In reply to Steven Yang [:styang] from comment #10)
> Hi John, assign to you since you're working on it.

Actually Vincent & Becker are working very hard on this one, but I don't mind taking the credit. :P
(In reply to John Lin [:jolin][:jhlin] from comment #9)
> Created attachment 8541687 [details] [diff] [review]
> ** NOT FOR CHECKING IN** patch to make stagefright log appear.
> 
> A hacky patch to work around the 'stagefright log not showing up' problem
> (It's because of failed socket() call at [1], but I don't know why that
> happens). Anyway this should make debugging much easier.
> 
> [1]
> https://android.googlesource.com/platform/system/core/+/master/liblog/
> logd_write.c#113
Maybe we should create a bug for this logging problem.
When stoping the recording, video track would block at pthread_join().

https://github.com/mozilla-b2g/platform_frameworks_av/blob/b2g-5.0.0_r6/media/libstagefright/MPEG4Writer.cpp#L1791

The root casue would be blocking in the below wait function.

https://github.com/mozilla-b2g/platform_frameworks_av/blob/b2g-5.0.0_r6/media/libstagefright/OMXCodec.cpp#L3999

Here has a considered solution. 

https://android.googlesource.com/platform/frameworks/av/+/bf37f33/media/libstagefright/MPEG4Writer.cpp

The main idea is switching the calling order for |pthread_join(mThread, &dummy)| and |mSource->stop()|. When mDone turns to true, making sure the thread is done from exiting pthread_join and then calling mSource->stop() to stop the data feeding. Flame-kk also run on this order.
Flags: needinfo?(vliu)
Status: NEW → ASSIGNED
(In reply to Vincent Liu[:vliu] from comment #13)
> When stoping the recording, video track would block at pthread_join().
> 
> https://github.com/mozilla-b2g/platform_frameworks_av/blob/b2g-5.0.0_r6/
> media/libstagefright/MPEG4Writer.cpp#L1791
> 
> The root casue would be blocking in the below wait function.
> 
> https://github.com/mozilla-b2g/platform_frameworks_av/blob/b2g-5.0.0_r6/
> media/libstagefright/OMXCodec.cpp#L3999
> 
> Here has a considered solution. 
> 
> https://android.googlesource.com/platform/frameworks/av/+/bf37f33/media/
> libstagefright/MPEG4Writer.cpp
> 
> The main idea is switching the calling order for |pthread_join(mThread,
> &dummy)| and |mSource->stop()|. When mDone turns to true, making sure the
> thread is done from exiting pthread_join and then calling mSource->stop() to
> stop the data feeding. Flame-kk also run on this order.

 The proposed fix essentially undo what Google did in [1]. From the commit message it seems to me that the change is part of rewriting Android recorder implementation to use MediaCodec (and deprecate OMXCodec). I'll test if the fix breaks Android recording functionality. If not then this could be a low risk solution for us.

 [1] https://android.googlesource.com/platform/frameworks/av/+/72cecca17d735db6532c45f0a7e10c47ee6f065a
Change component to gonk integration, where the fix will be more likely to land.
Component: Gaia::Camera → GonkIntegration
Hi Mike, could you please help review this PR? I tested it on AOSP as mentioned in comment 14 and it seems no problem. Thanks a lot & happy new year.
Attachment #8543814 - Flags: review?(mhabicher)
Comment on attachment 8543814 [details] [review]
Don't stop source until track thread is stopped.

LGTM.
Attachment #8543814 - Flags: review?(mhabicher) → review+
Keywords: checkin-needed
b2g-5.0.0_r6: https://github.com/mozilla-b2g/platform_frameworks_av/commit/8642b85281d90c0d804c382a884ebd1c113b9570
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Blocks: 1120780
Please also land the patch to v2.2 branch, thanks.
You need to log in before you can comment on or make changes to this bug.