991953 - [Camera][Buri] Timer turns sideways or upside down when filming video

Status: RESOLVED FIXED

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

Description

[Marcia Knous [:marcia]]

Attachment: 2014-04-03-16-59-31.png

Buri, while running:

Gaia   0e974ff33ba47f3d1e59df1e0ad534f1bbe3ef8a
SourceStamp 91be2828f17e
BuildID 20140403040201
Version 31.0a1
Base Image: 1.2cfg

and:

Gaia   0e974ff33ba47f3d1e59df1e0ad534f1bbe3ef8a
SourceStamp 91be2828f17e
BuildID 20140403040201
Version 31.0a1

STR:
1. Open the camera app and start recording a video
2. Turn the camera.
3. Observe the attached screenshots

Comment 1

[Marcia Knous [:marcia]]

Attachment: Timer in upside down position

Comment 2

[Jason Smith [:jsmith]]

Switching to qawanted to check on 1.4.

Comment 3

[Cody Roesch [:croesch]]

In the latest Buri V1.4, based on the STR above I can get the timer to rotate in place if i tilt the phone while recording. Any slight tilt of the phone and the timer will rotate sideways or upside down based on orientation. The Record/Record Stop button does not rotate or change however. So it seems the whole UI isn't rotating to match phone orientation.

Also when not recording, the Camera/Video buttons will also rotate in place in a similar fashion to the timer.

Let me know if any other info is needed.

Environmental Variables:
Device: Buri V1.4 MoZ Ril
BuildID: 20140404000202
Gaia: b4f3b84ec68233a99fd5865c15cfe28aebe26531
Gecko: 3186bbc50050
Version: 30.0a2
RIL Version:
Firmware Version: V1.2-device.cfg

Comment 4

[Jason Smith [:jsmith]]

comment 3 isn't crystal clear here. I need to know if this issue reproduces on 1.4 or not.

Comment 5

[Cody Roesch [:croesch]]

Yes, this bug DOES happen on Buri V1.4.

Environmental Variables:
Device: Buri V1.4 MoZ Ril
BuildID: 20140404000202
Gaia: b4f3b84ec68233a99fd5865c15cfe28aebe26531
Gecko: 3186bbc50050
Version: 30.0a2
RIL Version:
Firmware Version: V1.2-device.cfg

Comment 6

[Jason Smith [:jsmith]]

Note for window - use master to do the window.

Comment 7

[Marcia Knous [:marcia]]

Attachment: Another example of upside down content

Note that this happens when you view a single picture in gallery in as well - when I tip the camera forward, the picture reverses. So this is an overall problem of some sort when the device is tipped to different angles.

Comment 8

[Hema Koka [:hema]]

This looks bad regardless of whether it is regression or not. Blocking it

Comment 9

[Justin D'Arcangelo [:justindarc]]

It sounds from the comments like the issue here is that the app seems to be too sensitive to orientation changes, especially when the camera is facing downward (pointed towards the floor). Setting NI for David since our orientation events are coming from `vendor/orientation.js` and I believe the code in that module has existed within the Camera app for quite some time now.

David: Are you familiar with the `vendor/orientation.js` code in the Camera app? If so, it looks like there are some numbers in that file that can be used to tweak the sensitivity.

Comment 10

[mkruml]

B2G Inbound Regression Window:

First problem build:
1.5 Environmental Variables:
Device: Buri 1.5 MOZ
BuildID: 20140326113652
Gaia: e1c443d9ff62a21920d389be097d0e666f38e70b
Gecko: 999f90b84990
Version: 31.0a1
Firmware Version: v1.2-device.cfg

Last working build: 
1.5 Environmental Variables:
Device: Buri 1.5 MOZ
BuildID: 20140326103839
Gaia: b3c98000d6424153dc4ea5fbab2c4c2876ac9412
Gecko: 3faf66dc140f
Version: 31.0a1
Firmware Version: v1.2-device.cfg

Last working gaia/first broken gecko: Issue does not reproduce
Gaia: b3c98000d6424153dc4ea5fbab2c4c2876ac9412
Gecko: 999f90b84990

First broken gaia/last working gecko: Issue does reproduce
Gaia: e1c443d9ff62a21920d389be097d0e666f38e70b
Gecko: 3faf66dc140f


Gaia Pushlog: https://github.com/mozilla-b2g/gaia/compare/b3c98000d6424153dc4ea5fbab2c4c2876ac9412...e1c443d9ff62a21920d389be097d0e666f38e70b

Comment 11

[David Flanagan [:djf]]

Marcia: I think you're reporting two separate bugs here. Could you open a separate bug for comment #7 and leave this one focused on the timer issue? Also note that camera screenshots are always going to come out in portrait mode even if you're holding the phone in some other way. So an upside down screenshot by itself isn't enough to tell us anything. There is a general problem when a phone is flat on a table (or the camera is pointed straight up or down): we don't know what orientation the user wants.  If we're switching orientation to eagerly here that is something we should fix, but please give explicit STR in the bug you file, and please compare what we do to what Android or iOS does in the same case.

Justin: comment 7 might involve the orientation library, and yes I can probably help with that, if there is actually a bug there.

But let's start with the original bug that was reported here.  When nothing is being recorded, we want all of the onscreen elements to rotate as the user rotates the device. But as soon as they've started recording, they've locked in the orientation of the video they are creating, so at that point we should stop rotating things. While recording nothing should rotate, I think. In addition to the recording timer, this would include the low battery indicators (or anything else on the screen). Maybe you can fix this in CSS by adding a :not(.recording) to various selectors.  (And by setting and clearing the .recording class at appropriate times as well, of course.)

Comment 12

[Tiffanie Shakespeare [:tif]]

Hey guys! I'm a little fuzzy on the core issue. 

When the user taps the record button in portrait and purposefully rotates to landscape (let's ignore the sensitivity issue for the moment), what happens?

If the counter is rotating so that it is never sideways or upside down - that is correct.

If the counter is locked and appears upside down in 180 - that is incorrect.

The sensitivity issue that everyone is mentioning is an issue I've noticed as well. Please add me to that bug when it is filed.

Amy - can you please attach your visual design spec for record so people can see this? I can't find it in Box

Thanks!

Comment 13

[Justin D'Arcangelo [:justindarc]]

Related:

Bug 993577 - [Camera] Orientation changes are too sensitive

Comment 14

[Amy Lee [:amylee] UX]

Attachment: Dual_Shutter_Orientation.png

Hi Tiff, 

Here's the spec for orientation. It's dual shutter but the same rules apply to single shutter.

Comment 15

[Tiffanie Shakespeare [:tif]]

Alright guys - had some IRC/Video chats with dev and Rob/Amy. To sum:

David makes some really good points and everyone agrees that when the user hits the record button, the orientation should be locked such that the counter does NOT rotate on device rotation.

The timer should be in the correct orientation when the video starts, however.

E.g. Device starts in landscape, user taps record and the counter is oriented correctly. User rotates the device to portrait and since the timer is locked, the timer appears sideways.

Hopefully this makes sense and I'll make an update to the spec about this today. Thanks all and nice job!

Comment 16

[Justin D'Arcangelo [:justindarc]]

Attachment: pull-request (master)

Pull request for patch to lock app orientation after video recording has started.

Comment 17

[Tiffanie Shakespeare [:tif]]

Comment on attachment 8403508 [details] [review]
pull-request (master)

Patch looks good to me! Thanks guys! :)

Adding Amy to make sure she's happy with the positioning and how it looks.

Comment 18

[David Flanagan [:djf]]

Comment on attachment 8403508 [details] [review]
pull-request (master)

This fix is even simpler than I thought it would be.

In bug 987569 I actually removed the start/stop functions from the orientation library because they were unused (and having them public made it slightly harder for me to add lock/unlock methods). I haven't landed that change yet, and will have to remember to take that out.  You can ensure I don't screw it up by adding unit tests that just assert the presence of the start and stop methods on the orientation object.  (Or maybe that test is already there)

If I start recording a video, rotate the phone, and then stop recording, there is a fraction of a second when the items in the viewfinder are displayed in the wrong orientation.  I think one of my changes in 987569 (in vendor/orientation.js) will actually speed up this first orientation change event after calling start, so no concerns there.

Comment 19

[Justin D'Arcangelo [:justindarc]]

Landed on master:

https://github.com/mozilla-b2g/gaia/commit/140b61875f29040909bea9911eb862c35b882414

Comment 20

[Amy Lee [:amylee] UX]

Comment on attachment 8403508 [details] [review]
pull-request (master)

Looks good to me!

Comment 21

[Ryan VanderMeulen [:RyanVM]]

v1.4: https://github.com/mozilla-b2g/gaia/commit/714e05294f01f6cb8451fe829b6f7bcdc36b72db

Comment 22

[Marcia Knous [:marcia]]

(In reply to David Flanagan [:djf] from comment #11)
> Marcia: I think you're reporting two separate bugs here. Could you open a
> separate bug for comment #7 and leave this one focused on the timer issue?
> Also note that camera screenshots are always going to come out in portrait
> mode even if you're holding the phone in some other way. So an upside down
> screenshot by itself isn't enough to tell us anything. There is a general
> problem when a phone is flat on a table (or the camera is pointed straight
> up or down): we don't know what orientation the user wants.  If we're
> switching orientation to eagerly here that is something we should fix, but
> please give explicit STR in the bug you file, and please compare what we do
> to what Android or iOS does in the same case.
> 
> Justin: comment 7 might involve the orientation library, and yes I can
> probably help with that, if there is actually a bug there.
> 
> But let's start with the original bug that was reported here.  When nothing
> is being recorded, we want all of the onscreen elements to rotate as the
> user rotates the device. But as soon as they've started recording, they've
> locked in the orientation of the video they are creating, so at that point
> we should stop rotating things. While recording nothing should rotate, I
> think. In addition to the recording timer, this would include the low
> battery indicators (or anything else on the screen). Maybe you can fix this
> in CSS by adding a :not(.recording) to various selectors.  (And by setting
> and clearing the .recording class at appropriate times as well, of course.)

New bug filed as Bug 994893 for this issue.

Comment 23

[Wilson Page [:wilsonpage]]

This is has caused regression bug 1024278. By stopping orientation listeners altogether we have prevented the recording-timer from ever rotating. NI justindarc for some more insight.

Comment 24

[Wilson Page [:wilsonpage]]

Let's continue discussion over at bug 1024278 :)

Comment 25

[Justin D'Arcangelo [:justindarc]]

(In reply to Wilson Page [:wilsonpage] from comment #23)
> This is has caused regression bug 1024278. By stopping orientation listeners
> altogether we have prevented the recording-timer from ever rotating. NI
> justindarc for some more insight.

Comment 15 here explains the decision that was made to lock the recording timer in place. Continuing discussion at Bug 1024278...