[Camera] refactor .sensorAngle patch in bug 947956 for new camera app

RESOLVED FIXED in Firefox OS v1.4

Status

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

People

(Reporter: rexboy, Assigned: justindarc)

Tracking

unspecified
1.4 S5 (11apr)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:1.4+, b2g-v1.4 fixed, b2g-v2.0 fixed)

Details

(Whiteboard: [caf priority: p2][CR 621217] [landed-in-master])

Attachments

(1 attachment)

+++ This bug was initially created as a clone of Bug #947956 +++

We've landed the .sensorAngle patch on bug 947956 for v1.3.
Now we can try to refactor the patch to gaia master for new camera app.

Updated

4 years ago
Blocks: 957957
Assignee: nobody → dmarcos

Updated

4 years ago
Blocks: 960372
blocking-b2g: --- → 1.4?

Comment 1

4 years ago
Preeti / Diego -- this is causing the camera preview to be upside down on QRD devices. This is a blocker for us and we need it fixed by 3/24.

Thanks.
Flags: needinfo?(praghunath)
Hema

Can you please review this bug for 1.4?
Flags: needinfo?(praghunath) → needinfo?(hkoka)

Comment 3

4 years ago
rexboy: since you already have a patch for master on bug: https://bugzilla.mozilla.org/show_bug.cgi?id=947956 that has some comments that you need to address. Can you take this bug and fix it?

Thanks
Hema
Assignee: dmarcos → nobody
Flags: needinfo?(hkoka) → needinfo?(rexboy)
(Assignee)

Updated

4 years ago
Assignee: nobody → jdarcangelo
I’m currently working on another project and I’m afraid that I can’t put a patch before 3/24. If someone can work on this, I can help him / try to describe how the patch is working.
Flags: needinfo?(rexboy)
(Assignee)

Comment 5

4 years ago
KM: Do you have a patch for this already? I assigned this to myself and was about to start looking into it.
Flags: needinfo?(rexboy)
What I have are just the same with patches on bug 947956.
https://github.com/mozilla-b2g/gaia/pull/15188 The patch committed to v1.3.

What I did before for refactoring to v1.4 (on bug 947956; and it's not committed in since wilsonpage told me there would have following refactors) was almost just move the setPreviewSize function to where it should be in v1.4; but there should be refactors since then so you may need to take a look on the new codebase.
Flags: needinfo?(rexboy)
blocking-b2g: 1.4? → 1.4+
(Assignee)

Comment 7

4 years ago
Created attachment 8393733 [details] [review]
pull-request (camera-new-features)
Attachment #8393733 - Flags: review?(dflanagan)

Comment 8

4 years ago
:justin -- I was trying out the patch (comment 7) and noticed it doesn't apply cleanly. Can you please upload an updated patch. We can then verify if it fixes our issue or not.
Thanks.
Flags: needinfo?(jdarcangelo)
(Assignee)

Comment 9

4 years ago
Inder: The patch needs to be applied on top of the `camera-new-features` branch:

https://github.com/mozilla-b2g/gaia/tree/camera-new-features
Flags: needinfo?(jdarcangelo)
Comment on attachment 8393733 [details] [review]
pull-request (camera-new-features)

Clearing the review request and setting feedback+.  With github issues addressed this looks good to me, but since we're both flying blind, let's ask QC to try the patch on a QRD before we r+ and land it.
Attachment #8393733 - Flags: review?(dflanagan) → feedback+
Inder: are you able to test by applying Justin's patch on top of the camera-new-features branch?  That is the branch that Leyla is currently testing on.  We hope to have that merged to master soon
Flags: needinfo?(lcelebi)
Flags: needinfo?(ikumar)

Updated

4 years ago
Whiteboard: [CR 621217]

Comment 12

4 years ago
Inder/Leyla, please help test the patch

Comment 13

4 years ago
Eric, 

Would you be able to help test this patch out from "camera-new-features" branch using the QRD that you have so we can move forward with this?

Thanks
Hema
Flags: needinfo?(echang)
Currently I have no working QRD to test with, there is some issue my gecko on my device, so I have tried to reproduce bugs with Fugu and Buri.
Flags: needinfo?(echang)

Comment 15

4 years ago
Tried out this patch on camera-new-features branch and it fixes the camera preview upside down issue on QRD!

Justin/Hema - can we get a version of the patch which works for current gaia v1.4 branch so we can land in the tree temporarily and remove the FC blocker.
Flags: needinfo?(lcelebi)
Flags: needinfo?(jdarcangelo)
Flags: needinfo?(ikumar)

Comment 16

4 years ago
inder: the camera guys are also actively working on addressing bugs on the new features branch to move a bunch of madai features to master/1.4 -- just context switching back and forth for specific bugs will cause some inefficiencies especially when we are racing to cram a bunch of camera features into 1.4. 

since you have verified this on branch that it works - can removing the FC blocker tag off of this/landing on master/1.4 wait for a few more days please (mid next week). Does this block any further testing for 1.4? If yes, we will get on figuring out a version of the patch landed first on master/1.4 (justin is also feeling sick today and needs to take a break!)
Flags: needinfo?(ikumar)

Comment 17

4 years ago
Hema: I understand that most of your team is busy with camera-new-features branch but the test teams have put it as a FC blocker as I mentioned earlier so we need a patch for 1.4.
Justin: sorry to hear this. Get well soon.

Thanks,
-Inder
Flags: needinfo?(ikumar)
David gave an r+ with the condition that it had to be verified on the QRD device. Comment #15 confirms that the patch fixes the bug on QRD
Attachment #8393733 - Flags: review+
(Assignee)

Comment 19

4 years ago
Landed on camera-new-features:

https://github.com/mozilla-b2g/gaia/commit/fc797effd2457d3ba48159a4087b91c3721d59fc
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: needinfo?(jdarcangelo)
Resolution: --- → FIXED
The branch this patch was filed against was master, so this isn't closed until the patch lands on master.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The fix for this bug is on top of a number of other patches on our camera-new-features branch and cannot easily be uplifted to 1.4. 

Since no one at Mozilla is able to get the camera app to run at all on our QRD devices, we won't be able to test the patch if we tried to create a version that applied cleanly to 1.4.

Therefore we think that this fix will have to wait until Wednesday.  We'll land camera-new-features to master early Tuesday morning, and then uplift to 1.4 about 24 hours after that.

Updated

4 years ago
Blocks: 987489

Updated

4 years ago
No longer blocks: 957957

Updated

4 years ago
Whiteboard: [CR 621217] → [CR 621217] [branch-camera-new-features fixed]

Updated

4 years ago
Blocks: 903304

Comment 22

4 years ago
landed on master along with other camera features
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Whiteboard: [CR 621217] [branch-camera-new-features fixed] → [CR 621217] [landed-in-master]
Bulk edit for camera bugs.

If earlier comments do not show how this bug landed to master, it probably landed as part of https://github.com/mozilla-b2g/gaia/pull/17599 which merged the camera-new-features branch into master.

This bug was uplifted from master to v1.4 as part of https://github.com/mozilla-b2g/gaia/commit/a8190d08e61316a86bba572ba8d894d081a20530
status-b2g-v1.4: --- → fixed
status-b2g-v2.0: --- → fixed
Target Milestone: --- → 1.4 S5 (11apr)

Updated

3 years ago
Whiteboard: [CR 621217] [landed-in-master] → [caf priority: p2][CR 621217] [landed-in-master]
You need to log in before you can comment on or make changes to this bug.