Closed Bug 990761 Opened 6 years ago Closed 6 years ago

[Camera][Madai][QRD Only] Camera preview freezes after taking 1st picture when HDR is ON

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
blocker

Tracking

(blocking-b2g:1.4+, firefox29 wontfix, firefox30 fixed, firefox31 fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)

RESOLVED FIXED
blocking-b2g 1.4+
Tracking Status
firefox29 --- wontfix
firefox30 --- fixed
firefox31 --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: hkoka, Assigned: mikeh)

References

()

Details

(Whiteboard: [m+])

Attachments

(1 file, 1 obsolete file)

Issue: Camera preview freezes after taking 1st picture when HDR is ON.

Device: QRD (Qualcomm)

Build: Gecko rev: 38bc15c969fb1b5d435fa943f488bb4bd8918397

Gaia rev: eee964fc7e8dc519218dee6b72595ad0f7dd8231
Youngjun, please have someone from your team take this up and see if you can reproduce on nexus4.
No longer depends on: 990753
Flags: needinfo?(jjoons79)
Dear Ashish,

Please check it whether it's reproduced on Nexus4.
Assignee: nobody → singhashish1887
Flags: needinfo?(jjoons79)
Hi Hema,

This bug is not reproducible on Nexus-4 Devices.
We need your help to reproduce this issue on Nexus Device .
Flags: needinfo?(hkoka)
I can also confirm that this does not reproduce on a nexus-4, running:
- gecko: b2g-inbound:275feb8db98a
- gaia: camera-new-features:bb7e44e428640f15e1c07b0ab89c712e797fb9ea
Blocks: 983405
Flags: needinfo?(hkoka)
Summary: [Camera][Madai][QRD] Camera preview freezes after taking 1st picture when HDR is ON → [Camera][Madai][QRD Only] Camera preview freezes after taking 1st picture when HDR is ON
Assignee: singhashish1887 → nobody
blocking-b2g: --- → 1.4+
Mike - need your help reproducing this on the QRD that you have (new build info from QC is here: https://bugzilla.mozilla.org/show_bug.cgi?id=961246)
Flags: needinfo?(mhabicher)
I can certainly reproduce this one:
- gecko: b2g_autogen_ephemeral_branch:8e91fa5a127e5cbfa92a99896f4501ede717d347
- gaia: master:43255a9fac46da49937eb145aeeee0389d62ef07

I don't know what 'b2g-autogen_ephemeral_branch" is, but that seems to be what the QRD build script leaves $B2G/gecko in.
Flags: needinfo?(mhabicher)
Attached file logcat of issue reproducing (obsolete) —
In this logcat, there seems to be a single call to takePicture():

04-10 13:05:32.370   199  1507 E QCamera2HWI: [KPI Perf] static int qcamera::QCamera2HardwareInterface::take_picture(camera_device*): E PROFILE_TAKE_PICTURE
04-10 13:05:32.370   199  1572 D QCamera2HWI: int qcamera::QCamera2HardwareInterface::takePicture(): E
   ...
04-10 13:05:32.680   199  1572 D QCamera2HWI: int qcamera::QCamera2HardwareInterface::takePicture(): X
04-10 13:05:32.680   199  1507 D QCamera2HWI: [KPI Perf] static int qcamera::QCamera2HardwareInterface::take_picture(camera_device*): X

BUT there are _two_ callbacks invoked, 100ms apart:

04-10 13:05:35.890  1114  1651 I Gecko   : Got picture, type 'image/jpeg', 1121299 bytes
04-10 13:05:35.890  1114  1651 I Gecko   : nsGonkCameraControl::OnTakePictureComplete() done

and:

04-10 13:05:35.960  1114  1122 I Gecko   : Got picture, type 'image/jpeg', 645673 bytes
04-10 13:05:35.960  1114  1122 I Gecko   : nsGonkCameraControl::OnTakePictureComplete() done
The logcat also shows, after the first callback invocation:

04-10 13:05:35.890  1114  1651 I Gecko   : Got picture, type 'image/jpeg', 1121299 bytes
04-10 13:05:35.890  1114  1651 I Gecko   : nsGonkCameraControl::OnTakePictureComplete() done
04-10 13:05:35.890  1114  1548 I Gecko   : Starting preview (this=0xb2ce6640)
04-10 13:05:35.890  1114  1548 I Gecko   : virtual int android::GonkCameraHardware::StartPreview():326 : this=0xb2a034c0
04-10 13:05:35.890   199  1572 E QCameraStateMachine: Cannot set preview window when preview is running
04-10 13:05:35.890   199   199 E QCamera2HWI: [KPI Perf] static int qcamera::QCamera2HardwareInterface::start_preview(camera_device*): E PROFILE_START_PREVIEW
04-10 13:05:35.890   199  1572 E QCameraStateMachine: int32_t qcamera::QCameraStateMachine::procEvtPicTakingState(qcamera::qcamera_sm_evt_enum_t, void*): cannot handle evt(9) in state(4)
04-10 13:05:35.890   199   199 D QCamera2HWI: [KPI Perf] static int qcamera::QCamera2HardwareInterface::start_preview(camera_device*): X
04-10 13:05:35.890  1114  1548 I Gecko   : Failed to start camera preview
04-10 13:05:35.890  1114  1548 I Gecko   : Camera control API failed at 7 with 0x80004005
04-10 13:05:35.890  1114  1114 I Gecko   : DOM OnError context=7, error='FAILURE'

Except the preview was stopped earlier:

04-10 13:05:22.990   199  1572 D QCamera2HWI: int qcamera::QCamera2HardwareInterface::stopPreview(): E
   ...
04-10 13:05:23.080   199  1572 D QCamera2HWI: int qcamera::QCamera2HardwareInterface::stopPreview(): X
Bhargav, Inder: is the snippet in comment 8 a bug in your CameraStateMachine?
Flags: needinfo?(ikumar)
Flags: needinfo?(bhargavg1)
Also, in comment 7, I assume we're expecting two returned photos? I'm guessing the first one is just a normal photo, and the second one is the HDR version?
Actually, looking through the code, I can see that the logcat error message "Cannot set preview window when preview is running" appears in three places, and is descriptively inaccurate for two of them, including procEvtPicTakingState().
So, the high-level question is: since we currently expect to be able to restart the preview after getting a picture, but HDR returns multiple pictures and while the process is taking place, we can't restart the preview, is there a way to explicitly determine how many pictures will be returned? Or can we just assume that with HDR ON, we will always get 2?
Mike -- let me grok the code and get back.

> I don't know what 'b2g-autogen_ephemeral_branch" is

Our build scripts has a way to apply any local patches we may have for gecko/gaia to try some changes and creates this temporary branch.
Flags: needinfo?(bhargavg1)
I'll 'take' this so that someone is working on it, but am waiting for info from CAF.
Assignee: nobody → mhabicher
(In reply to Mike Habicher [:mikeh] from comment #10)
> Also, in comment 7, I assume we're expecting two returned photos? I'm
> guessing the first one is just a normal photo, and the second one is the HDR
> version?

How many pictures are returned when HDR is ON is decided by key, "KEY_QC_HDR_NEED_1X" if set to TRUE would be 2 else 1
MikeH -- please let us know if you need anything else.
Flags: needinfo?(ikumar)
Bhargav, is the second photo the HDR one? Always? If so, we'll probably just discard the first one.
Flags: needinfo?(bhargavg1)
mikeh -- are you planning to hardcode the logic to always discard the 2nd image? I don't think that would be a good considering this param can be altered. Maybe you should check the param value first?
(In reply to Inder from comment #18)
>
> mikeh -- are you planning to hardcode the logic to always discard the 2nd
> image? I don't think that would be a good considering this param can be
> altered. Maybe you should check the param value first?

Hey Inder, I was planning to check CameraParameters::KEY_QC_HDR_NEED_1X, and if it's "true" (or whatever the 'true' value is), then I was just going to discard the first image result and only accept/return the second one.

Do you have any plans to extend this to cover more than one photo?
(In reply to Mike Habicher [:mikeh] from comment #19)
> (In reply to Inder from comment #18)
> >
> > mikeh -- are you planning to hardcode the logic to always discard the 2nd
> > image? I don't think that would be a good considering this param can be
> > altered. Maybe you should check the param value first?
> 
> Hey Inder, I was planning to check CameraParameters::KEY_QC_HDR_NEED_1X, and
> if it's "true" (or whatever the 'true' value is), then I was just going to
> discard the first image result and only accept/return the second one.
> 
> Do you have any plans to extend this to cover more than one photo?

Or may be set it to false and only image returned would be HDR
Flags: needinfo?(bhargavg1)
(In reply to bhargavg1 from comment #20)
> (In reply to Mike Habicher [:mikeh] from comment #19)
> > (In reply to Inder from comment #18)
> > >
> > > mikeh -- are you planning to hardcode the logic to always discard the 2nd
> > > image? I don't think that would be a good considering this param can be
> > > altered. Maybe you should check the param value first?
> > 
> > Hey Inder, I was planning to check CameraParameters::KEY_QC_HDR_NEED_1X, and
> > if it's "true" (or whatever the 'true' value is), then I was just going to
> > discard the first image result and only accept/return the second one.
> > 
> > Do you have any plans to extend this to cover more than one photo?
> 
> Or may be set it to false and only image returned would be HDR

Just to clarify, set CameraParameters::KEY_QC_HDR_NEED_1X to false and only one image that of HDR would be returned
(In reply to bhargavg1 from comment #21)
> 
> Just to clarify, set CameraParameters::KEY_QC_HDR_NEED_1X to false and only
> one image that of HDR would be returned

Thanks, that's much clearer.
Attachment #8404884 - Attachment is obsolete: true
Attachment #8407901 - Flags: review?(dhylands)
Comment on attachment 8407901 [details] [diff] [review]
Disable normal photos in HDR scene mode, v1

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

::: dom/camera/GonkCameraParameters.cpp
@@ +690,5 @@
>  }
>  
> +// Handle bools
> +nsresult
> +GonkCameraParameters::SetTranslated(uint32_t aKey, const bool& aValue)

Passing a const bool ref seems odd to me (unless you're using this as part of a template somewhere).

Why not just pass a bool ?
Attachment #8407901 - Flags: review?(dhylands) → review+
(In reply to Dave Hylands [:dhylands] (away - back Tue April 15) from comment #25)
>
> Passing a const bool ref seems odd to me (unless you're using this as part
> of a template somewhere).
> 
> Why not just pass a bool ?

Because the functions this method is called from and the functions it calls are all templates. :)
https://hg.mozilla.org/mozilla-central/rev/4953b30842cd
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Duplicate of this bug: 985079
Test case needs to be updated.

https://moztrap.mozilla.org/manage/case/8471/
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Flags: in-moztrap?(rmead)
Test case in moztrap:

https://moztrap.mozilla.org/manage/case/13295/
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Flags: in-moztrap?(rmead)
Flags: in-moztrap+
Duplicate of this bug: 965423
You need to log in before you can comment on or make changes to this bug.