1069200 - [Camera]Rapidly click take button and switch picture/video mode will cause video recording and picture mode in the same time

Status: RESOLVED FIXED

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

Description

[Mike Lien[:mlien]]

Attachment: logcat.log

[Blocking Requested - why for this release]:

[Build Information]
Device: Flame
Gaia-Rev        31434a3949556171f3565ca47ac2b44e810e95e6
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/cfedc95605d4
Build ID        20140917140111
Version         32.0
Device Name     flame
FW-Release      4.4.2
FW-Incremental  27
FW-Date         Thu Sep  4 14:59:02 CST 2014
Bootloader      L1TC10011800

[Description]
Rapidly click take button and switch picture/video mode will cause video recording and picture mode in the same time
After that, re-launch will get black viewfinder

[Steps to Reproduce]
  1. Launch Camera app
  2. Rapidly click take button and switch picture/video mode
  3. Repeat step 2

[Expected Results]
Keep in picture mode or video mode

[Actual Results]
Video recording and picture mode in the same time
Left-top show recording timer and bottom show take picture button
After that, re-launch camera app will get black viewfinder

[Reproduction Frequency]
80%

Comment 1

[Mike Lien[:mlien]]

Attachment: screenshot.png

Comment 2

[Mike Lien[:mlien]]

[Blocking Requested - why for this release]:

v2.1 can also reproduce this problem

Gaia-Rev        379e68fe729a684fa2fcddb30ea1e65508db73e1
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-aurora/rev/7ff763eb328b
Build ID        20140917183806
Version         34.0a2
Device Name     flame
FW-Release      4.4.2
FW-Incremental  27
FW-Date         Thu Sep  4 14:59:02 CST 2014
Bootloader      L1TC10011800

Comment 3

[Hema Koka [:hema]]

I am also able to reproduce this easily. Following the STR, when we get to the state where video recording time seems to be progressing and we are able to take a snapshot also -- only the pics are saved and video is not (sometimes I see an error message when I re-open camera that says "video was not recorded" and sometimes a black screen with touch to focus ring is displayed). It looks like we don't clear out the previous recording time indicator cleanly when the switch happens to pic mode.

In the first place the functionality is supposed to be a toggle of being able to take just pics or shoot video (taking pictures while recording video is an enhancement pending in our backlog). 

Wilson, can you please investigate. 

Thanks
Hema

Comment 4

[Wilson Page [:wilsonpage]]

Attachment: pull-request (master)

Added a debounce to Camera's 'ready' event. This means that we better cope with rapid 'ready' -> 'busy' -> 'ready' scenarios and our app becomes usable shortly after the camera is actually ready.

Overall we're more defensive against this aggressive button bashing.

Comment 5

[Wilson Page [:wilsonpage]]

Also made sure that recording is stopped when the camera is being configured. This prevents the camera being able to switch to 'picture' mode whilst still recording a video.

Comment 6

[Justin D'Arcangelo [:justindarc]]

(In reply to Wilson Page [:wilsonpage] from comment #4)
> Created attachment 8491933 [details] [review]
> pull-request (master)
> 
> Added a debounce to Camera's 'ready' event. This means that we better cope
> with rapid 'ready' -> 'busy' -> 'ready' scenarios and our app becomes usable
> shortly after the camera is actually ready.
> 
> Overall we're more defensive against this aggressive button bashing.

Question in the PR about the 300ms value. It seems a bit high. For zoom we throttle with 150ms. Is this the lowest possible throttle timeout required for this to be effective?

Comment 7

[Wilson Page [:wilsonpage]]

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

LANDED https://github.com/mozilla-b2g/gaia/commit/25e5384e5df4d6265c5db89041845e517a6b4fdb

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Please request Gaia v2.1 approval on this when you get a chance. Thanks :)

Comment 9

[Wilson Page [:wilsonpage]]

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

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: Camera could appear to be in opposite mode after aggressive button bashing
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky): Minimal. Camera 'ready' events debounced to make UI more defensive.
[String changes made]:

Comment 10

[Mike Lien[:mlien]]

please also uplift this patch to v2.0 because this issue is a partner blocker

Comment 11

[Mike Lien[:mlien]]

ni wilson to help on this

Comment 12

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/1e554253d11b0cc932de39c0a4510d1abb176550

Comment 13

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Triage: 2.0+ per Comment#10. Please help to uplift to 2.0 since it is a partner test blocker

Thanks

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Needs Gaia v2.0 approval before it can be uplifted.

Comment 15

[Wilson Page [:wilsonpage]]

Attachment: pull-request (v2.0)

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Missing a v2.0 approval request?

Comment 17

[Wilson Page [:wilsonpage]]

Comment on attachment 8495203 [details] [review]
pull-request (v2.0)

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: Button bashing can get app into strange state
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky): 
[String changes made]:

Comment 18

[bhavana bajaj [:bajaj]]

(In reply to Wilson Page [:wilsonpage] from comment #17)
> Comment on attachment 8495203 [details] [review]
> pull-request (v2.0)
> 
> NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to
> better understand the B2G approval process and landings.
> 
> [Approval Request Comment]
> [Bug caused by] (feature/regressing bug #):
> [User impact] if declined: Button bashing can get app into strange state
> [Testing completed]:
Any automated/manual tesing done here?
> [Risk to taking this patch] (and alternatives if risky): 
Can you explain what sort of risk this patch introduces to the release?
> [String changes made]:

Comment 19

[Wilson Page [:wilsonpage]]

(In reply to bhavana bajaj [:bajaj] from comment #18)
> (In reply to Wilson Page [:wilsonpage] from comment #17)
> > Comment on attachment 8495203 [details] [review]
> > pull-request (v2.0)
> > 
> > NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to
> > better understand the B2G approval process and landings.
> > 
> > [Approval Request Comment]
> > [Bug caused by] (feature/regressing bug #):
> > [User impact] if declined: Button bashing can get app into strange state
> > [Testing completed]:
> Any automated/manual tesing done here?

Added unit tests.

> > [Risk to taking this patch] (and alternatives if risky): 

Minimal. Adds a small debounce to the camera's 'ready' event.

> Can you explain what sort of risk this patch introduces to the release?
> > [String changes made]:

None

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Looks like the v2.0 PR needs rebasing :(

Comment 21

[Wilson Page [:wilsonpage]]

Done

Comment 22

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/092d2b7678774c8b0b06dca0e0a8119e9eafdec3

Comment 23

[Kai-Zhen Li [:seinlin][:kli]]

v2.0m: https://github.com/mozilla-b2g/gaia/commit/aa6e1fa38251b15f22b4bec6c72deee2e782d806

Comment 24

[Lancy(Leave from Mozilla)]

Attachment: Video.MP4

Verify passed on Woodduck and Flame, this issue can't be repro on Woodduck 2.0m, Flame 2.0& 2.1& 2.2.

Woodduck v2.0 version:
Gaia-Rev        3a98f1287fa7b604891220ba5d86982ae8f9971e
Gecko-Rev       03d3ab62d5b07b915434f2d1d68495ad5915ecd2
Build-ID        20141120103003
Version         32.0

Flame 2.0 version:
Gaia-Rev        1ede2666f1e6c1b3fd3b282011caf0cbc59544b0
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/54f1b0ee07a6
Build-ID        20141120000206
Version         32.0

Flame 2.1 version:
Gaia-Rev        f8d3bf44029e0afc0124600a4bb34dba8fc1ad21
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/f70a67a7f846
Build-ID        20141120001207
Version         34.0

Flame 2.2 version:
Gaia-Rev        1abe09b4925547699dfdb2d358aed019137c3aa6
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/6ce1b906c690
Build-ID        20141120040205
Version         36.0a1