Closed
Bug 990761
Opened 10 years ago
Closed 10 years ago
[Camera][Madai][QRD Only] Camera preview freezes after taking 1st picture when HDR is ON
Categories
(Firefox OS Graveyard :: Gaia::Camera, defect)
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+ |
People
(Reporter: hkoka, Assigned: mikeh)
References
()
Details
(Whiteboard: [m+])
Attachments
(1 file, 1 obsolete file)
6.69 KB,
patch
|
dhylands
:
review+
|
Details | Diff | Splinter Review |
Issue: Camera preview freezes after taking 1st picture when HDR is ON. Device: QRD (Qualcomm) Build: Gecko rev: 38bc15c969fb1b5d435fa943f488bb4bd8918397 Gaia rev: eee964fc7e8dc519218dee6b72595ad0f7dd8231
Reporter | ||
Comment 1•10 years ago
|
||
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)
Comment 2•10 years ago
|
||
Dear Ashish, Please check it whether it's reproduced on Nexus4.
Assignee: nobody → singhashish1887
Flags: needinfo?(jjoons79)
Comment 3•10 years ago
|
||
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)
Assignee | ||
Comment 4•10 years ago
|
||
I can also confirm that this does not reproduce on a nexus-4, running: - gecko: b2g-inbound:275feb8db98a - gaia: camera-new-features:bb7e44e428640f15e1c07b0ab89c712e797fb9ea
Reporter | ||
Updated•10 years ago
|
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
Reporter | ||
Updated•10 years ago
|
Assignee: singhashish1887 → nobody
Reporter | ||
Updated•10 years ago
|
blocking-b2g: --- → 1.4+
Reporter | ||
Comment 5•10 years ago
|
||
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)
Assignee | ||
Comment 6•10 years ago
|
||
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)
Assignee | ||
Comment 7•10 years ago
|
||
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
Assignee | ||
Comment 8•10 years ago
|
||
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
Assignee | ||
Comment 9•10 years ago
|
||
Bhargav, Inder: is the snippet in comment 8 a bug in your CameraStateMachine?
Flags: needinfo?(ikumar)
Flags: needinfo?(bhargavg1)
Assignee | ||
Comment 10•10 years ago
|
||
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?
Assignee | ||
Comment 11•10 years ago
|
||
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().
Assignee | ||
Comment 12•10 years ago
|
||
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?
Comment 13•10 years ago
|
||
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)
Assignee | ||
Comment 14•10 years ago
|
||
I'll 'take' this so that someone is working on it, but am waiting for info from CAF.
Assignee: nobody → mhabicher
Comment 15•10 years ago
|
||
(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
Comment 16•10 years ago
|
||
MikeH -- please let us know if you need anything else.
Flags: needinfo?(ikumar)
Assignee | ||
Comment 17•10 years ago
|
||
Bhargav, is the second photo the HDR one? Always? If so, we'll probably just discard the first one.
Flags: needinfo?(bhargavg1)
Comment 18•10 years ago
|
||
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?
Assignee | ||
Comment 19•10 years ago
|
||
(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?
Comment 20•10 years ago
|
||
(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)
Comment 21•10 years ago
|
||
(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
Assignee | ||
Comment 22•10 years ago
|
||
(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.
Assignee | ||
Comment 23•10 years ago
|
||
Attachment #8404884 -
Attachment is obsolete: true
Attachment #8407901 -
Flags: review?(dhylands)
Assignee | ||
Comment 24•10 years ago
|
||
try-server push: https://tbpl.mozilla.org/?tree=Try&rev=46f160362e6b
Comment 25•10 years ago
|
||
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+
Assignee | ||
Comment 26•10 years ago
|
||
(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. :)
Assignee | ||
Comment 27•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/4953b30842cd
Status: NEW → ASSIGNED
Comment 28•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4953b30842cd
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 29•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/ea3724cf62ea
status-b2g-v1.4:
--- → fixed
status-b2g-v2.0:
--- → fixed
status-firefox29:
--- → wontfix
status-firefox30:
--- → fixed
status-firefox31:
--- → fixed
Comment 31•10 years ago
|
||
Test case needs to be updated. https://moztrap.mozilla.org/manage/case/8471/
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Flags: in-moztrap?(rmead)
Comment 32•10 years ago
|
||
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+
You need to log in
before you can comment on or make changes to this bug.
Description
•