900327 - [Helix] camera recording can't support 720p profile

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Vendcom, defect)

Description

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Attachment: camera-720p.patch

STRs:
1. apply patch of attachment to gaia
2. open camera app
3. switch to recording mode
4. recording

expected:
recorded and save a video file to storage

actually:
can't save video file to storage

note:
1. we have 720p profile in camera.capabilities.recorderProfiles
2. 480p profile works.

Comment 1

[Vincent Liu[:vliu]]

I tried to reproduce the STRs with 480p and 720p and raised some information for my found.

1. I saw strange message when the camera switched to video mode. They pop up both in 480p and 720p cases.

   E/mm-camera(  119): DO NOT DISTURB frameprocESSING IF MODULES are not ENABLED

  This message came out from camera HAL. I need partner's help to figure it out.

2. I found some messages and it might explain why the we couldn't record 720p file.

   E/VENC_OMX(  163): OMX_VENC_MSG_ERROR set_parameter::1325 invalid resolution: supports till (864x480) resolution only
   E/OMXCodec( 3685): Setting Video InPort Definition failed
   E/VENC_OMX(  163): OMX_VENC_MSG_HIGH component_deinit::3314 deinitializing component...
   E/VENC_ENC(  163): VENC_MSG_HIGH venc_unload::1175 Received command VENC_CMD_UNLOAD
   E/VENC_ENC(  163): VENC_MSG_HIGH venci_process_command::4525 Processing command VENC_CMD_UNLOAD
   E/VENC_ENC(  163): VENC_MSG_PROFILE venci_process_command_unload::2665 Encoder time taken to Exit from Stop command: 382494
   E/VENC_OMX(  163): OMX_VENC_MSG_HIGH process_DL_status::4935 got DL status for VENC_CMD_UNLOAD

Comment 2

[Wayne Chang [:wchang]]

Lecky,

We need your help to check on your end why 720p cannot be recorded. 480p is recorded with some error messages.

Comment 3

[DingYu]

Attachment: camera.js

Comment 4

[DingYu]

Hi Wayne:

   In out camera.js, I can not find the proper place to merge the patch, attachment 786145 [details] is our camera.js.

Comment 5

[Vincent Liu[:vliu]]

Attachment: camera.js

(In reply to DingYu from comment #4)
> Hi Wayne:
> 
>    In out camera.js, I can not find the proper place to merge the patch,
> attachment 786145 [details] is our camera.js.

You can try the attachment and add the patch Yuren Ju offered.

Comment 6

[DingYu]

Vincent,Wayne:
    I do the test of 720p, it works fine. I check the video file, its size is 1280x720. 
    There is a problem, if I take a video recording and go back to the preview viewfinder, there is no thumbnail of the video.

Yuren:
    Do you have camera-vga.patch?

Comment 7

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

no, I don't have, but you can reference camera-720p.patch and make it by yourself.

Comment 8

[DingYu]

Yuren, I do a test reference camera-720p, but It can't work. 
In camera.js, I add the following code in Line 781;
    
    var profiles = camera.capabilities.recorderProfiles;
    for (var key in profiles) {
       console.log(key + ": " + profiles[key])
    }

According to the log,the key are low,high,qcif,cif,480p and 720p, the vga is not include.

Comment 9

[DingYu]

(In reply to Yuren Ju [:yurenju] from comment #7)
> no, I don't have, but you can reference camera-720p.patch and make it by
> yourself.

Yuren, I change GonkRecorderProfiles.def as follows:
   - //DEF_GONK_RECORDER_PROFILE(CAMCORDER_QUALITY_VGA,   "vga")
   + DEF_GONK_RECORDER_PROFILE(CAMCORDER_QUALITY_VGA,   "vga")

and reference camera-720p.patch to vga, it works fine.

Is there any side effect?

Comment 10

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Vicent may answer your question since I'm a gaia developer.

needinfo? vliu.

Comment 11

[Vincent Liu[:vliu]]

Attachment: video-resolution.pdf

Hi DinYu,

May I discuss with you for what I found when you talked in Comment 6? When I tried to record again with 720p, it actually wrote into a 3gp file and is stored into /sdcard/DCIM/. But there is a problem here that the thumbnail didn't show on the screen. I think there is a problem need to be checked.
Another problem is the recorded 720p 3gp file didn't reach to the expected resolution. I tried to play this file in the player and checked the resolution, it only reach to 515 X 916. Please see the attached file to know the detail. For this, we still need your support.
For Comment 9, I will look into it. If I found anything, I will update on Comment.

Comment 12

[Vincent Liu[:vliu]]

Hi Mike,
From the partner's comment 9, he mentioned about vga support in GonkRecorderProfiles.def. I tried to enabled it in Helix and confirmed this device has this capability. But according to Bug 804359, you mentioned about runtime-detected platform-specific profiles. May I know the detail about this? 
Another question is if I hard coding to enable vga and landed into master, what side effect will happens? Thanks.

diff --git a/dom/camera/GonkRecorderProfiles.def b/dom/camera/GonkRecorderProfiles.def
index 91b342d..2e135c2 100644
--- a/dom/camera/GonkRecorderProfiles.def
+++ b/dom/camera/GonkRecorderProfiles.def
@@ -25,6 +25,7 @@ DEF_GONK_RECORDER_PROFILE(CAMCORDER_QUALITY_CIF,   "cif")
 DEF_GONK_RECORDER_PROFILE(CAMCORDER_QUALITY_480P,  "480p")
 DEF_GONK_RECORDER_PROFILE(CAMCORDER_QUALITY_720P,  "720p")
 DEF_GONK_RECORDER_PROFILE(CAMCORDER_QUALITY_1080P, "1080p")
+DEF_GONK_RECORDER_PROFILE(CAMCORDER_QUALITY_VGA,   "vga")
 /**

Comment 13

[DingYu]

Hi Vincent:
   In Comment 11, I found that the 720p 3gp file can not be identified and played in my device, but if I pull the file to computer and ues windows media player to play it, it played fine.
   which player you used?

Comment 14

[Vincent Liu[:vliu]]

(In reply to DingYu from comment #13)
> Hi Vincent:
>    In Comment 11, I found that the 720p 3gp file can not be identified and
> played in my device, but if I pull the file to computer and ues windows
> media player to play it, it played fine.
>    which player you used?

I also play 3gp file in my computer. The player is QuickTime. This player can indicate media file information. Please make sure you check the real resolution after you played it.

Comment 15

[DingYu]

Hi Vincent:
   I think it may be the QuickTime that case this problem. 
   1. The first time I load the 720p 3gp file, current size is 720*1280
   2. If I set View-->Half Size，current size is 360*640
   3. If I set View-->Normal size ，Current size is 720*1280
   4. If I set View-->Double size,current size is 1140*2560
   5. If I set View-->Proper size,current size is 498*886
So the current size is related with the player's setting, It is irrelevant with the actual size of the 720p 3gp file

Comment 16

[DingYu]

Attachment: video resolution.rar

Comment 17

[Vincent Liu[:vliu]]

(In reply to DingYu from comment #15)
> Hi Vincent:
>    I think it may be the QuickTime that case this problem. 
>    1. The first time I load the 720p 3gp file, current size is 720*1280
>    2. If I set View-->Half Size，current size is 360*640
>    3. If I set View-->Normal size ，Current size is 720*1280
>    4. If I set View-->Double size,current size is 1140*2560
>    5. If I set View-->Proper size,current size is 498*886
> So the current size is related with the player's setting, It is irrelevant
> with the actual size of the 720p 3gp file

Yes, you are right. The tool configuration seems the root cause for what I saw.  One more question. For the error messages I saw in Comment 1, did you ever see it? I still see these error messages. Can you please figure out these errors? Thanks.

Comment 18

[Mike Habicher [:mikeh] (high bugzilla latency)]

(In reply to Vincent Liu[:vliu] from comment #12)
>
> Hi Mike,
> From the partner's comment 9, he mentioned about vga support in
> GonkRecorderProfiles.def. I tried to enabled it in Helix and confirmed this
> device has this capability. But according to Bug 804359, you mentioned about
> runtime-detected platform-specific profiles. May I know the detail about
> this? 

Hi Vincent,

There are a couple of things that need to be in place to support a given video profile:
1. defined in media_profiles.xml
2. defined in GonkRecorderProfiles.def (mostly just to give the profile enum value a name)
3. supported by the underlying camera (i.e. returned by CameraParameters::KEY_SUPPORTED_VIDEO_SIZES)

Bug 804359 is about getting the intersection of these three (ideally ((1) ∩ (3)) ∩ (2) == ((1) ∩ (3)), as (2) is just used to assign names to the profiles based on resolution) and reporting that to JS in capabilities.recorderProfiles.

But for v1 we decided to only support cif (and now qcif), so we put off implementing bug 804359 since all camera drivers seemed to support those resolutions, and the easiest solution was to hard-code them.

It sounds like this has changed, so we should probably re-nom bug 804359 for leo/koi.

> Another question is if I hard coding to enable vga and landed into master,
> what side effect will happens? Thanks.

As long as the hardware supports it, we should be fine. The problem we ran into on unagi with the higher resolutions modes was that, because of our CPU/DSP memory partitioning, the DSP was running out of memory encoding the larger frames, and would fail intermittently (usually after taking some photos in camera mode).

Comment 19

[Mike Habicher [:mikeh] (high bugzilla latency)]

The take-away from my last paragraph in comment 18 should be that just because a higher resoltuion, e.g. 480p or higher, works on leo, doesn't mean it will work on other platforms. Either more platform testing will be needed, or we'll need some way to limit the profiles exposed by platform (which it sounds like bug 896425 tries to do--though I'd rather have everything covered in the platform than have JS lift this).

Comment 20

[Vincent Liu[:vliu]]

(In reply to Mike Habicher [:mikeh] from comment #18)
> (In reply to Vincent Liu[:vliu] from comment #12)
> >

> There are a couple of things that need to be in place to support a given
> video profile:
> 1. defined in media_profiles.xml
> 2. defined in GonkRecorderProfiles.def (mostly just to give the profile enum
> value a name)
> 3. supported by the underlying camera (i.e. returned by
> CameraParameters::KEY_SUPPORTED_VIDEO_SIZES)

Thanks for your Comment. I will take a detailed look for the missing part.

> Bug 804359 is about getting the intersection of these three (ideally ((1) ∩
> (3)) ∩ (2) == ((1) ∩ (3)), as (2) is just used to assign names to the
> profiles based on resolution) and reporting that to JS in
> capabilities.recorderProfiles.
> 
> But for v1 we decided to only support cif (and now qcif), so we put off
> implementing bug 804359 since all camera drivers seemed to support those
> resolutions, and the easiest solution was to hard-code them.
> 
> It sounds like this has changed, so we should probably re-nom bug 804359 for
> leo/koi.

The idea in bug 804359 seems good for the future. It's more a flexiable way to fulfill different camera capabilities.

> > Another question is if I hard coding to enable vga and landed into master,
> > what side effect will happens? Thanks.
> 
> As long as the hardware supports it, we should be fine. The problem we ran
> into on unagi with the higher resolutions modes was that, because of our
> CPU/DSP memory partitioning, the DSP was running out of memory encoding the
> larger frames, and would fail intermittently (usually after taking some
> photos in camera mode).

If we blocked by unagi's capability, we can consider only land to hd branch or just add modification in partner's side. Actually for 720p case, I found there still had issues in OpenMAX layer and still looked into it. For vga, I still need to take time to investigate it.

Comment 21

[Vincent Liu[:vliu]]

Checked the criteria Mike offered, Helix can support 720p video profile. The thing is the recorded video file can't list in Gallery. By tracking it, I found OmxDecoder::SetVideoFormat() return false by reading zero mVideoWidth/mVideoHeight and unrecognized componentName. 

http://mxr.mozilla.org/mozilla-central/source/content/media/omx/OmxDecoder.cpp#577

Also, some messages showed in Comment 1 tried to tell me there exists limitation for OMX_VENC. There might have two parts to discuss.

1. The capability of camera HW. I think it should support 720p from listing supported camera video size.
2. The capability of OMX. I am not quite sure the capability of OMX also depends on Camera HW. Does anyone can give me suggestion? Thanks.

Comment 22

[Mike Habicher [:mikeh] (high bugzilla latency)]

Edwin, OmxDecoder.cpp looks like your baby--any idea what might be going on here?

Comment 23

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

I don't currently have a helix build. I'm spinning one up now, but I can get started on this a lot quicker if I can get a logcat from this.

Vincent?

Comment 24

[Vincent Liu[:vliu]]

Attachment: logs-camera.log

(In reply to Edwin Flores [:eflores] [:edwin] from comment #23)
> I don't currently have a helix build. I'm spinning one up now, but I can get
> started on this a lot quicker if I can get a logcat from this.
> 
> Vincent?

The logcat file is attached. I added a log point in the following patch to show the issue.

+#define LOGOMX(args...)  __android_log_print(ANDROID_LOG_INFO, "OmxDecoder" , ## args)
+
 VideoGraphicBuffer::VideoGraphicBuffer(const android::wp<android::OmxDecoder> aOmxDecoder,
                                        android::MediaBuffer *aBuffer,
                                        SurfaceDescriptor *aDescriptor)
@@ -437,6 +439,7 @@ bool OmxDecoder::SetVideoFormat() {
       !mVideoSource->getFormat()->findInt32(kKeyHeight, &mVideoHeight) ||
       !mVideoSource->getFormat()->findCString(kKeyDecoderComponent, &componentName) ||
       !mVideoSource->getFormat()->findInt32(kKeyColorFormat, &mVideoColorFormat) ) {
+    LOGOMX("%s:%d : mVideoWidth=%d, mVideoHeight=%d, componentName=%s, mVideoColorFormat=%d",
     return false;
   }

Please give me any suggestion if you found anything. Thanks.

Comment 25

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

I'm not making much progress here; in large part because I can't seem to make a gonk build without breaking the phone completely.

Sotaro might be able to shed a bit more light on this; he knows libstagefright a lot better.

Comment 26

[Sotaro Ikeda [:sotaro]]

From following log, it seems to fail decoding somewhere around omx by some reason.
--------------------------------------------
E/GeckoConsole(  545): [JavaScript Warning: "Media resource blob:0f13e6af-42da-4e8b-a8db-4ebe1d7b7cb1 could not be decoded." {file: "app://camera.gaiamobile.org/index.html" line: 0}]

Comment 27

[Sotaro Ikeda [:sotaro]]

Vincent, did you add correct log out? Added log of componentName and color format seems strange. From other part of the log, it should be the following.

- 'OMX.qcom.video.decoder.avc'
- format.eColorFormat 0x7fa30c01

Comment 28

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Logcat playing 720p

Comment 29

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Logcat playing CIF

Comment 30

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

I've attached a couple logcats for comparison: one playing successfully (CIF resolution); the other one failing (720p).

Looks like it's constructing the right decoder, but throwing it out before OMXCodec::Create returns.

Comment 31

[Sotaro Ikeda [:sotaro]]

attachment 790592 [details] do not have an information about why decode fails. You can enable OMXCodec's log out by comment out of "//#define LOG_NDEBUG 0".

http://androidxref.com/4.0.4/xref/frameworks/base/media/libstagefright/OMXCodec.cpp#17

Comment 32

[Sotaro Ikeda [:sotaro]]

Following log seems that 'OMX.qcom.video.decoder.avc' allocation via IOMX interface succeeded once but failed to allocate and configure OMXCodec. 

----------------------------------------
E/OMXCodec(  545): Attempting to allocate OMX node 'OMX.TI.DUCATI1.VIDEO.DECODER'
E/OMXCodec(  545): Attempting to allocate OMX node 'OMX.qcom.audio.decoder.aac'
E/OMXCodec(  545): Attempting to allocate OMX node 'OMX.Nvidia.h264.decode'
E/OMXCodec(  545): Attempting to allocate OMX node 'OMX.qcom.7x30.video.decoder.avc'
E/OMXCodec(  545): Successfully allocated OMX node 'OMX.qcom.audio.decoder.aac'
E/OMXCodec(  545): Attempting to allocate OMX node 'OMX.qcom.video.decoder.avc'
E/OMXCodec(  545): Successfully allocated OMX node 'OMX.qcom.video.decoder.avc'
E/OMXCodec(  545): [OMX.qcom.video.decoder.avc] Video O/P format.eColorFormat 0x7fa30c01
W/omx_vdec(  164): ======================================================================
W/omx_vdec(  164):                    Open Max Statistics                                
W/omx_vdec(  164): ======================================================================
W/omx_vdec(  164): empty this buffer rate = NaN
W/omx_vdec(  164): empty this buffer total time = 0
W/omx_vdec(  164): empty this buffer count = 0
W/omx_vdec(  164): ======================================================================
E/OMXCodec(  545): Attempting to allocate OMX node 'OMX.ittiam.video.decoder.avc'
E/OMXCodec(  545): Attempting to allocate OMX node 'OMX.TI.Video.Decoder'
E/OMXCodec(  545): Attempting to allocate OMX node 'OMX.SEC.AVC.

Comment 33

[Sotaro Ikeda [:sotaro]]

comment #32 seems like a code around following.

https://www.codeaurora.org/cgit/external/gigabyte/platform/frameworks/base/tree/media/libstagefright/OMXCodec.cpp?h=caf/b2g/ics_strawberry#n813

Comment 34

[Sotaro Ikeda [:sotaro]]

About decoding failure, there seems the following possibilities.
- fail to parse mp4 in MPEG4Extractor
- hw video codec is not configured to decode 720p
- do not have enough pmem_adsp/pmem

Comment 35

[Vincent Liu[:vliu]]

(In reply to Sotaro Ikeda [:sotaro] from comment #27)
> Vincent, did you add correct log out? Added log of componentName and color
> format seems strange. From other part of the log, it should be the following.
> 
> - 'OMX.qcom.video.decoder.avc'
> - format.eColorFormat 0x7fa30c01

I changed the log out position to observe 480p and cif case, and got the expected log out.

diff --git a/content/media/omx/OmxDecoder.cpp b/content/media/omx/OmxDecoder.cpp
index 9a6fdaa..b3501cf 100644
--- a/content/media/omx/OmxDecoder.cpp
+++ b/content/media/omx/OmxDecoder.cpp
@@ -37,6 +37,8 @@ using namespace MPAPI;
 namespace mozilla {
 namespace layers {
 
+#define LOGOMX(args...)  __android_log_print(ANDROID_LOG_INFO, "OmxDecoder" , ## args)
+
 VideoGraphicBuffer::VideoGraphicBuffer(const android::wp<android::OmxDecoder> aOmxDecoder,
                                        android::MediaBuffer *aBuffer,
                                        SurfaceDescriptor *aDescriptor)
@@ -440,6 +442,8 @@ bool OmxDecoder::SetVideoFormat() {
     return false;
   }
 
+  LOGOMX("%s:%d : mVideoWidth=%d, mVideoHeight=%d, componentName=%s, mVideoColorFormat=%d", _
+

480p case
I/OmxDecoder( 6647): bool android::OmxDecoder::SetVideoFormat():445 : mVideoWidth=720, mVideoHeight=480, componentName=OMX.qcom.video.decoder.avc, mVideoColorFormat=2141391873

cif
I/OmxDecoder( 7135): bool android::OmxDecoder::SetVideoFormat():445 : mVideoWidth=352, mVideoHeight=288, componentName=OMX.qcom.video.decoder.avc, mVideoColorFormat=2141391873

The comment 34 you mentioned is a good point to track for next move. One more question. Is it possible to check the encoded blob itself? The blob contains the header for extractor parsing and put in fixed position. Do you know how to check it? Thanks.

Comment 36

[Vincent Liu[:vliu]]

Narrowed down the issue into following.

https://www.codeaurora.org/cgit/external/gigabyte/platform/frameworks/base/tree/media/libstagefright/OMXCodec.cpp?h=caf/b2g/ics_strawberry#n2132

the returned value err=-1010 by decimal. I tried to dump def.nBufferSize and got 327680. Does this value have any connection with the resolution of 720p (1280 X 720)? Is it possible the fail happens because the resolution of 720p much larger than def.nBufferSize? Please correct me if I got anything wrong. Thanks.

Comment 37

[Sotaro Ikeda [:sotaro]]

(In reply to Vincent Liu[:vliu] from comment #35)
> The comment 34 you mentioned is a good point to track for next move. One
> more question. Is it possible to check the encoded blob itself? The blob
> contains the header for extractor parsing and put in fixed position. Do you
> know how to check it? Thanks.

Soory, I never checked the blob itself. If blob does not provide the correct data, MPEG4Extractor almost always causes a crash at CHECK() macro. When I investigate about data in the mp4 file from mp4 box point of view, I always add logout to MPEG4Extractor.

http://androidxref.com/4.0.4/xref/frameworks/base/media/libstagefright/MPEG4Extractor.cpp

Comment 38

[Sotaro Ikeda [:sotaro]]

(In reply to Vincent Liu[:vliu] from comment #36)
> the returned value err=-1010 by decimal.

It seems ERROR_UNSUPPORTED.

http://androidxref.com/4.0.4/xref/frameworks/base/include/media/stagefright/MediaErrors.h#37

The error is converted from OMX OMX_ERRORTYPE at StatusFromOMXError()

http://androidxref.com/4.0.4/xref/frameworks/base/media/libstagefright/omx/OMXNodeInstance.cpp#115

> I tried to dump def.nBufferSize and
> got 327680. Does this value have any connection with the resolution of 720p
> (1280 X 720)?

It seems already end to unsupported so. So I am not sure, the above info have some meaning.

Comment 39

[Sotaro Ikeda [:sotaro]]

Moz Build do not build around "qcom-opensource/omx/mm-video". Though omx_vdec::set_parameter() seems to be called from mOMX->setParameter().

https://www.codeaurora.org/cgit/quic/qrd-android/platform/vendor/qcom-opensource/omx/mm-video/tree/vidc/vdec/src/omx_vdec.cpp?h=ics_master_qrd_cs2#n2918

Comment 40

[Bruce Sun [:brsun]]

Hi Michael,

We have return value err=-1010 from mOMX->setParameter(mNode, OMX_IndexParamPortDefinition, &def, sizeof(def)) on "OMX.qcom.video.decoder.avc" component.

Fields of used def is:
 - def.nSize: 4
 - def.nVersion: 1.1.0.0
 - def.nPortIndex: 0
 - def.eDir: 0x00000000
 - def.nBufferCountActual: 2
 - def.nBufferCountMin: 2
 - def.nBufferSize: 327680
 - def.bEnabled: 1
 - def.bPopulated: 0
 - def.eDomain: 0x00000001
 - def.format.video.cMIMEType: (null)
 - def.format.video.pNativeRender: 0x20
 - def.format.video.nFrameWidth: 1280
 - def.format.video.nFrameHeight: 720
 - def.format.video.nStride: 176
 - def.format.video.nSliceHeight: 144
 - def.format.video.nBitrate: 16
 - def.format.video.xFramerate: 25
 - def.format.video.bFlagErrorConcealment: 1098866756
 - def.format.video.eCompressionFormat: 7
 - def.format.video.eColorFormat: 0
 - def.format.video.pNativeWindow: 0x400d5037
 - def.bBuffersContiguous: 1098867040
 - def.nBufferAlignment: 1122537232

Do we fill some improper values for configuring 1280*720 resolution?

Comment 41

[Michael Vines [:m1] [:evilmachines]]

For product support please request partners use the SR system.  Thanks!

Comment 42

[Wayne Chang [:wchang]]

I am unblocking this from HD and marking POVB assuming 720p support is a new requirement for v1.1.

Lecky, please investigate this via SR as :m1 suggests if you plan to launch the device with 720p support.

Comment 43

[Wilson Page [:wilsonpage]]

lecky: We're experiencing similar issues (bug 987102), except we're unable to record 480p or 720p. Do you have any updates/suggestions?

Comment 44

[BMO Automation]

Firefox OS is not being worked on