1147076 - [Camera] Face tracking circle is aligned to the right in Portrait mode

Status: VERIFIED FIXED

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

Description

[Anshul]

STR

1. Launch the camera(front or back)
2. Focus on a face

Observed: In the portrait mode (vertical) the face tracking circle is aligned to the right of the face.
Expected: The trace tracking circle should be centered on the face

Note that this issue is not seen if the phone is held in a landscape mode.

Comment 1

[Hema Koka [:hema]]

QA,

Please check this on flame. There were no camera UI changes that went into 2.2 that would have changed or broken the face tracking circle alignment. But can we first get this to reproduce on Flame.

NI Diego in the meantime as well.

Thanks
Hema

Comment 2

[Anshul]

I can reproduce the issue on flame device as well.

Comment 3

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Attachment: Face tracking off to the right

I'm uploading a screenshot of this issue.

Comment 4

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

This issue occurs on Flame 3.0 and Flame 2.2.

Actual Results:  The face is tracked from the right.

Environmental Variables:
Device: Flame 3.0
BuildID: 20150325130912
Gaia: 508b8d48fb5ecf08bf0e5b4fef42bc48b770e7f2
Gecko: f40ee067d081
Version: 39.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Environmental Variables:
Device: Flame 2.2
BuildID: 20150325031209
Gaia: 48b93b6929c508f016e6f8254ddfa851f9feed59
Gecko: c0927c8ec63d
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

This issue does NOT occur on Flame 2.1.  

Actual Results: The face is tracked from the center.

Environmental Variables:
Device: Flame 2.1
BuildID: 20150325043108
Gaia: 6f39e4e876152de1dcdcc0e7656197f22f105e4b
Gecko: c411ee193eed
Version: 34.0 (2.1) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 5

[Diego Marcos [:dmarcos]]

The code that renders the face tracked has not changed in a while. Is the driver reporting different coordinates on the latest firmware? Where can I see a firmware change log? If tracked face coordinates have changed what's the rational behind it? Does this bug reproduces on any other device besides flame?

Comment 6

[Pi Wei Cheng [:piwei] (inactive)]

b2g-inbound regression window:

Last Working Environmental Variables:
Device: Flame
BuildID: 20141217182050
Gaia: 7a865bd6163e9a0372861d27450b3434875ef5c1
Gecko: 655536bc9ccd
Version: 37.0a1 (2.2 Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

First Broken Environmental Variables:
Device: Flame
BuildID: 20141217184854
Gaia: 7a865bd6163e9a0372861d27450b3434875ef5c1
Gecko: bea91a0b4dfb
Version: 37.0a1 (2.2 Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Gaia is the same so it's a Gecko issue.

Gecko pushlog:
http://hg.mozilla.org/integration/b2g-inbound/pushloghtml?fromchange=655536bc9ccd&tochange=bea91a0b4dfb

Caused by patches for Bug 1104913.

Comment 7

[Diego Marcos [:dmarcos]]

It's either a gecko or a firmware issue (v18D-1?). If the problem doesn't reproduce on any other phone than flame is almost sure that it's a driver change.

Comment 8

[Pi Wei Cheng [:piwei] (inactive)]

I thought it has been established that this occurs on multiple devices since this was originally reported on a non-Flame device.

Comment 9

[KTucker [:KTucker][Inactive 3/4/2016]]

Andrew, can you take a look at this please? Looks like the work done on bug 1104913 might be the cause here.

Comment 10

[Hema Koka [:hema]]

Blocking for regression

Comment 11

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

PiWei seems to have covered everything you wanted from me.  Of note, this issue is not likely related to a base since the same base was used to reproduce no 3.0/2.2 but not on 2.1.

Comment 12

[GitHub Autolander Bot]

Attachment: [gaia] aosmond:bug1147076 > mozilla-b2g:master

Comment 13

[Andrew Osmond [:aosmond] (he/him)]

I was able to reproduce but it happens to me on all gecko/gaia branches, so I'm not sure if I actually fixed the problem. The calculation for where to put the face detection circle is off in gaia -- the top/left coordinate of the circle needs to be adjusted in order for the centre of that circle to be the same as the bounding rect given by the camera.

Note that the problem is not limited to portrait mode. Due to the rotation, in landscape mode the circle should have a vertical bias. You will therefore see the problem moving the face up and down the viewfinder instead of left and right.

piwei: Can you try out the attached gaia patch and confirm if this fixes the problem (and confirm again on which branches :))? If you are unable to reproduce the issue on 2.1 without this patch, then could you please provide a reproduction video for both 2.1 and 2.2/3.0 so I can see the difference between them? Thanks!

Comment 14

[Pi Wei Cheng [:piwei] (inactive)]

Video of issue NOT reproducing on v2.1:

https://www.youtube.com/watch?v=Dy87-hAl3dA

This is tested on:
Device: Flame 2.1 (KK, full flash, 319MB mem)
BuildID: 20150327001204
Gaia: 6f39e4e876152de1dcdcc0e7656197f22f105e4b
Gecko: f4d7c0e7292e
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 34.0 (2.1) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

------

Video of issue reproducing on v3.0:

https://www.youtube.com/watch?v=yHtw2Mlkyy4

This is tested on:
Device: Flame 3.0 Master (KK, full flash, 319MB mem)
BuildID: 20150330010204
Gaia: be25b16efa19bab8d54be08f8fe45dcc93bf93d0
Gecko: dfe60814eda7
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 39.0a1 (3.0 Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

------

I'll post my result after applying the patch in the next comment. Keeping NI.

Comment 15

[Andrew Osmond [:aosmond] (he/him)]

With respect to gecko change in bug 1104913, I believe I now may see the connection. If the aspect ratio of the face bounding box given by the camera hardware is controlled by the selected video and/or preview size, it is possible that change exaggerated the flaw in the app rendering (two wrongs make a right!). While I am able to hit the problem in 2.1, different versions of flame support different preview/video sizes, which will affect the degree to which one is able to reproduce the problem.

Comment 16

[Pi Wei Cheng [:piwei] (inactive)]

Attachment: screenshot of issue reproducing after patch

The patch seems to have NOT fully fixed the issue. Most of the time the face tracking circle seems to put the circle fully centering the face, but sometimes it appears a little off to the left. Attaching a screenshot.

This behavior is observed on both 3.0 and 2.2.

Comment 17

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 8585744 [details] [review]
[gaia] aosmond:bug1147076 > mozilla-b2g:master

dmarcos: I know we discussed changing this to a square, but rather than deal with UX changes for 2.2, I think we should leave it as a circle for now and just fix the positioning.

(In reply to Pi Wei Cheng [:piwei] from comment #16)
> Created attachment 8586911 [details]
> screenshot of issue reproducing after patch
> 
> The patch seems to have NOT fully fixed the issue. Most of the time the face
> tracking circle seems to put the circle fully centering the face, but
> sometimes it appears a little off to the left. Attaching a screenshot.
> 
> This behavior is observed on both 3.0 and 2.2.

Pi Wei, unfortunately, the face detection algorithm is not perfect. That is the right location according to the vendor camera code. From your screenshots and my own testing, it is a marked improvement from before.

Comment 18

[Hema Koka [:hema]]

(In reply to Pi Wei Cheng [:piwei] from comment #16)
> Created attachment 8586911 [details]
> screenshot of issue reproducing after patch
> 
> The patch seems to have NOT fully fixed the issue. Most of the time the face
> tracking circle seems to put the circle fully centering the face, but
> sometimes it appears a little off to the left. Attaching a screenshot.
> 
> This behavior is observed on both 3.0 and 2.2.

This attachment looks much better compared to the original issue without Andrew's patch. For 2.2, I think we can go with this fix. 

Andrew, since you will be out on jury duty, please work on getting the patch reviewed and getting it in for approval today. Also include Tif for a quick ux review. 

Thanks
Hema

Comment 19

[Diego Marcos [:dmarcos]]

Comment on attachment 8585744 [details] [review]
[gaia] aosmond:bug1147076 > mozilla-b2g:master

It looks good to me. Can we add unit tests for the calculateFaceCircle method?

Comment 20

[GitHub Autolander Bot]

Attachment: [gaia] aosmond:bug1147076-v2.2 > mozilla-b2g:v2.2

Comment 21

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 8588843 [details] [review]
[gaia] aosmond:bug1147076-v2.2 > mozilla-b2g:v2.2

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): 1104913
[User impact] if declined: Detected face circles will be poorly placed in take picture mode.
[Testing completed]: Manually ensured face circle is better placed on screen in landscape/portrait modes on the front and back facing cameras for flame. Added unit tests verifying the result of the circle calculation.
[Risk to taking this patch] (and alternatives if risky): Minor; the math choosing the location/diameter of the circle is slightly tweaked, and well understood why it was wrong originally.
[String changes made]: N/A

Comment 22

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 8588843 [details] [review]
[gaia] aosmond:bug1147076-v2.2 > mozilla-b2g:v2.2

Carry r+ from master pull request.

Comment 23

[Andrew Osmond [:aosmond] (he/him)]

Set in-testsuite, added gaia unit tests for circle parameter calculations.

Comment 24

[bhavana bajaj [:bajaj]]

waiting for master landing here before acting on branch approval.

Comment 25

[GitHub Autolander Bot]

Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.

Comment 26

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/80699440e70da06b47f373ec0ae2a4bf5372b864

Comment 27

[Kevin Grandon :kgrandon]

I thought that we had some failures crop up on taskcluster after this landed. I reverted this commit, but was mistaken and ended up re-landing it. Sorry about the churn here, re-landed: https://github.com/mozilla-b2g/gaia/commit/a8bc2fb65d0174fe24385ca650ec02d0f1117903

Comment 28

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/daa518229385edb56826dae0ef4ae670fba40845

Comment 29

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

This issue is verified fixed on the latest Nightly 3.0 and 2.2 builds.

Actual Results: Face tracking is centered on the face and not severely off to either side.

Environmental Variables:
Device: Flame 3.0
BuildID: 20150408010203
Gaia: 84cbd4391fb7175d5380fa72c04d68873ce77e6d
Gecko: 078128c2600a
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Environmental Variables:
Device: Flame 3.0
BuildID: 20150408010203
Gaia: 84cbd4391fb7175d5380fa72c04d68873ce77e6d
Gecko: 078128c2600a
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0