1020560 - [B2G][Camera]Quickly switching between camera and video mode causes the camera to freeze

Status: VERIFIED FIXED

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

Description

[Andrew Stole [:astole]]

Attachment: logcat

When quickly switching between camera mode and video mode, the camera app freezes. It is possible to unfreeze the app if it is force closed, however sometimes it has to be force closed, opened, then force closed again for it to work as expected.

Repro Steps:
1) Update a Flame to BuildID: 20140604040204
2) Open the camera app
3) Repeatedly switch between camera and video mode

Actual:
Camera app is unresponsive

Expected:
Camera app correctly switches between camera and video mode without freezing

2.0 Environmental Variables:
Device: Flame 2.0 MOZ
BuildID: 20140604040204
Gaia: 1d4f6f7312882e78b57971152de75d1281a26187
Gecko: 668f29cd71b3
Version: 32.0a1
Firmware Version: v10G-2

Repro frequency: 3/3, 100%
See attached: logcat, video

Comment 1

[Andrew Stole [:astole]]

Attaching video.

Adding QA Wanted to check 1.4 and to check 2.0 on Buri.

Comment 2

[Mike Habicher [:mikeh] (high bugzilla latency)]

Did something change/regress in mode-switching recently?

Comment 3

[Justin D'Arcangelo [:justindarc]]

Mike: In the video (and when I just tried reproducing on my Flame), I noticed that the "touch-to-focus" ring starts appearing in the lower-right corner around the mode-switch button. I'm wondering if we're focusing when the camera is in-between states and that is causing this?

I'm getting the following error:
E/GeckoConsole( 1140): [JavaScript Error: "NS_ERROR_FAILURE: " {file: "app://camera.gaiamobile.org/js/main.js" line: 4045}]

...which corresponds to the following line:

https://github.com/justindarc/gaia/blob/4d35bde872a62c778a09ab11b0a37423870c7355/apps/camera/js/lib/camera/focus.js#L218

Comment 4

[Justin D'Arcangelo [:justindarc]]

Attachment: pull-request (master)

Comment 5

[Diego Marcos [:dmarcos]]

I don't like this patch. Even if it fixes the symptoms it doesn't address the core of the issue. Problems I see here:

1. The full screen preview as currently designed never receives touch events underneath the control bar. The user is not allowed to touch to focus on that area of the image. I never liked the full screen viewfinder but it is what it is. 
2. When the camera is busy (for instance when changing modes) we disable the controls bar: This means that we set pointer-events: none in the CSS. As a side effect the events that would be captured by the controls bar bubble up to viewfinder. This breaks touch to focus consistency and should never happen.
3. Even if the touch event bubbles up, the camera should focus in that area but it crashes instead. There's something wrong going on the camera controller side when calling mozCamera.setConfiguration. I'm guessing that even if the success callback is invoked the camera is still not ready to receive autoFocus calls.

mozCamera API is becoming increasingly frustrating to reason about. If we cannot improve the API in the short term we need at least MDN documentation describing its behavior. There's a lot of nuances that are now in people's heads that make difficult to investigate and fix bugs. Thinks like:

- onAutoFocusMoving event doesn't return the state of the focus we have to call mozCamera.autoFocus to query for the state. 
- After calling setFocusAreas and setMeteringAreas we have to call resumeContinuousFocus if we want continuous auto focus to work again. 

These two tings are just in the top of my head but there are more gotchas that bit me in the past.

Comment 6

[Justin D'Arcangelo [:justindarc]]

I agree with Diego that this patch is masking the root of the problem. I think what's happening is a call to focus while the camera is in mid-configuration, and that's causing a strange error to appear in the console that sometimes leads to the camera never completing it's configuration from the mode change (app freezes).

Comment 7

[Diego Marcos [:dmarcos]]

Attachment: Alternative Pull Request

This pull request prevents the click events to bubble up even if the controls bar is disabled. Now the user can never focus on the region underneath regardless of the controls being enabled or disabled. We have UI consistency but as the original patch it just masks the underlying issue of the camera crashing when quickly calling mozCamera.autoFocus right after the success callback of mozCamera.setConfiguration has been invoked.

Comment 8

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8436584 [details] [review]
Alternative Pull Request

I like this better. Plus, I adds a getter to our base View class that we probably should've had. Before we close this bug though, let's see if Mike can find the source of the issue in the Camera Control API.

Comment 9

[Mike Habicher [:mikeh] (high bugzilla latency)]

(In reply to Diego Marcos [:dmarcos] from comment #5)
> 
> mozCamera API is becoming increasingly frustrating to reason about. If we
> cannot improve the API in the short term we need at least MDN documentation
> describing its behavior. There's a lot of nuances that are now in people's
> heads that make difficult to investigate and fix bugs. Thinks like:
> 
> - onAutoFocusMoving event doesn't return the state of the focus we have to
> call mozCamera.autoFocus to query for the state. 
> - After calling setFocusAreas and setMeteringAreas we have to call
> resumeContinuousFocus if we want continuous auto focus to work again. 
> 
> These two things are just in the top of my head but there are more gotchas
> that bit me in the past.

If there are any suggestions for improving the CameraControl API, please file new bugs and make bug 994912 block on them. I'm collecting a list of improvements. Things like the focus API weirdness are largely inherited from Android and I agree they could use some "Webification". 

One thing to keep in mind is that Gecko is constrained by what is exposed by the camera library: particularly the inability to know whether or not we're focused without risking triggering an auto-focus cycle, with the side-effect of cancelling the continuous auto-focus setting.

(In reply to Justin D'Arcangelo [:justindarc] from comment #8)
> 
> I like this better. Plus, I adds a getter to our base View class that we
> probably should've had. Before we close this bug though, let's see if Mike
> can find the source of the issue in the Camera Control API.

Justin, from reading through this bug, do you think reproducing is as simple as:

  mozCamera.setConfiguration(settings, onSuccess);
  function onSuccess(...) {
    mozCamera.autoFocus(...);
  }

...?

Comment 10

[Justin D'Arcangelo [:justindarc]]

(In reply to Mike Habicher [:mikeh] from comment #9)
> Justin, from reading through this bug, do you think reproducing is as simple
> as:
> 
>   mozCamera.setConfiguration(settings, onSuccess);
>   function onSuccess(...) {
>     mozCamera.autoFocus(...);
>   }
> 
> ...?

No, as I think the problem is coming from the configuration *not* completing before the call to focus is made. So, it would be closer to something like:

// Execution order: 1
mozCamera.setConfiguration(settings, onConfigSuccess);

// Execution order: 2
//
// Note that these following lines can potentially be
// running *before* the onSuccess callback is made
mozCamera.setFocusAreas([rect]);
mozCamera.setMeteringAreas([rect]);
if (mozCamera.focusMode === 'continuous-picture') {
  mozCamera.resumeContinuousFocus();
}
mozCamera.autoFocus(...);

// Execution order: 3
function onConfigSuccess() {
  ...
}

Comment 11

[Diego Marcos [:dmarcos]]

Landed on master:

https://github.com/mozilla-b2g/gaia/commit/68783ed9bca1b06a7aaf3bb5650ed0ebabeed714

The core issue will be addressed on a following bug. 

Justin, do we already have a bug for it?

Comment 12

[Eric Kramer [:Kramer]]

This issue does NOT repro on 1.4 Flame

Environmental Variables:
Device: Flame 1.4
Build ID: 20140609000201
Gaia: 8b239e41bbd85aa7b6a2c5d388e775ba7de6fb2b
Gecko: e3f85877db29
Version: 30.0 (1.4) 
Firmware Version: v10G-2

================================================

This issue does NOT repro on Buri 2.0

Environmental Variables:
Device: Buri 2.0
Build ID: 20140609074621
Gaia: d283b742a12ac43ec087f45b02d3817cf7ddab69
Gecko: 68ac46c1b1f7
Version: 32.0a1 (2.0) 
Firmware Version: v1.2device.cfg

Comment 13

[Mike Habicher [:mikeh] (high bugzilla latency)]

(In reply to Diego Marcos [:dmarcos] from comment #11)
> 
> The core issue will be addressed on a following bug. 
> 
> Justin, do we already have a bug for it?

See bug 1022766.

Comment 14

[Diego Marcos [:dmarcos]]

The underlying issue is going to be addressed in bug 1022766. 

Nothing remains to be done here. I'm closing this.

Comment 15

[Lancy(Leave from Mozilla)]

Attachment: Verify_Video_Flame.MP4

This issue has been verified successfully on Flame 2.0 & 2.1.
See attachment: Verify_Video_Flame.MP4
Reproducing rate: 0/10

Flame v2.0 version:
Gaia-Rev        ead3b72a84512750bc5faff4e9e8faa1715c0d05
Gecko-Rev       c715df57d1d1593ede48140ebc88e101f8c3f7da
Build-ID        20141208050313
Version         32.0
Device-Name     jrdhz72_w_ff
FW-Release      4.4.2

Flame v2.1 version:
Gaia-Rev        38e17b0219cbc50a4ad6f51101898f89e513a552
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/8b92c4b8f59a
Build-ID        20141205001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141205.035305
FW-Date         Fri Dec  5 03:53:16 EST 2014
Bootloader      L1TC00011880