All users were logged out of Bugzilla on October 13th, 2018

Front camera preview image upside down

VERIFIED FIXED

Status

VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: Dafeng.Xu, Assigned: rexboy)

Tracking

unspecified
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:hd+, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Front camera preview image upside down
(Reporter)

Comment 1

6 years ago
Created attachment 727057 [details] [diff] [review]
camera.js patch
Comment on attachment 727057 [details] [diff] [review]
camera.js patch

I'm f-minusing this patch because it seems to have some extra stuff in it from statusbar.js; also because I think the correct way to address this issue is by specifying a cam.1.sensor_offset value for tara.  See [1] for an example.

1. https://github.com/mozilla-b2g/android-device-unagi/blob/master/full_unagi.mk#L16
Attachment #727057 - Flags: feedback-

Comment 3

5 years ago
The problem happens on Helix device.
Per comment 2, ni marco to check the settings in gecko/gonk.
We will also check the code segment in Camera app.
Status: UNCONFIRMED → NEW
blocking-b2g: --- → hd?
Ever confirmed: true
Flags: needinfo?(mchen)

Updated

5 years ago
Summary: [tara]Front camera preview image upside down → Front camera preview image upside down

Comment 4

5 years ago
(In reply to Mike Habicher [:mikeh] from comment #2)
> 
> 1.
> https://github.com/mozilla-b2g/android-device-unagi/blob/master/full_unagi.
> mk#L16

Currently we found that property will be used for "takePicture" only not for preview.
And we are digging to the flow of rotation on case of preview.
Flags: needinfo?(mchen)

Comment 5

5 years ago
And camera.js already set rotation to 90 degree in camera.js.

https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/camera.js#L787
Patch:
https://github.com/rexboy7/gaia/commit/ed19a5710f10425c9cd7c59e4f75a9c68e5ace47

This is more like a regression because |camera| is an object which can never equals to 1. When it works correctly, the preview for front camera is flipped upside-down and mirrored, which is exactly what we want.

Seems Gaia flips the preview for front camera since very long ago. I'm not sure whether this should be done in Gaia or Gecko though.
Flags: needinfo?(dale)
The code referenced for the regression should most definitely be taken out, however the current code works on the peak (I will hopefully be able to test with an sgs2 shortly) 

So I suspect we should be fixing this as mikeh referenced with cam.1.sensor_offset fixes
Flags: needinfo?(dale)

Comment 8

5 years ago
(In reply to Dale Harvey (:daleharvey) from comment #7)
> So I suspect we should be fixing this as mikeh referenced with
> cam.1.sensor_offset fixes

Please refer to comment 4 for the usage of cam.x.sensor_offset currently.
It seems to not related to preview.

Comment 9

5 years ago
(In reply to KM Lee [:rexboy] from comment #6)
> Patch:
> https://github.com/rexboy7/gaia/commit/
> ed19a5710f10425c9cd7c59e4f75a9c68e5ace47
> 
> This is more like a regression because |camera| is an object which can never
> equals to 1. When it works correctly, the preview for front camera is
> flipped upside-down and mirrored, which is exactly what we want.
> 
> Seems Gaia flips the preview for front camera since very long ago. I'm not
> sure whether this should be done in Gaia or Gecko though.

Discussed with Marco offline, we think the flip work in Gaia is reasonable.
I test the patch and it works, so if it get r+ from reviewer, we can merge it and close this issue.
Rex, please set review flag to a proper reviewer. Thanks.
Created attachment 765866 [details]
patch

Per Comment #9, let's try on this patch.
I've tested it on the target device and it works well.
Dale, may you help review it? Thank you!
Attachment #765866 - Flags: review?(dale)
Attachment #765866 - Flags: review?(dale) → review+
Merged in: https://github.com/mozilla-b2g/gaia/commit/8012351787d24bfcbf21126aeca9eb4803675af0

Thanks, So the peak branch I was testing (v1.0.1) didnt have this regression, master did though (and this fixed it)
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 12

5 years ago
Rex: it seems we don't have hd+ triage yet, please help on uplifting this patch to v1.1.0hd. Thanks!
status-b2g18: --- → wontfix
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → wontfix
status-b2g-v1.1hd: --- → affected

Updated

5 years ago
Assignee: nobody → rexboy
v1.1.0hd: 036bcaca5ed58dbb0360594a8876cb9819436966
v1.1.0hd: 170f62c5451c0de960cf18762a5d673a0b3d8443
status-b2g18: wontfix → fixed
hd+ as device specific requirement
blocking-b2g: hd? → hd+
v1.1.0hd: 036bcaca5ed58dbb0360594a8876cb9819436966
v1.1.0hd: 170f62c5451c0de960cf18762a5d673a0b3d8443

Comment 17

5 years ago
Thanks for your help!
I cannot reproduce this case by following test steps.
 1.Open the camera application.
 2.Switch to the front camera.

* Test build:(Mozilla-b2g18_v1_1_0_hd-helix/2013-08-15-04-22-01)
  + Mercurial-Information
    - Gecko revision="bd0864949c0f"
  + Git-information
    - Gaia revision="d04d2d3969e4366fabafe9659f9b2705b7879ac2"

Marked as "Verified"+"Fixed"
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.