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

RESOLVED FIXED in 2.1 S5 (26sep)

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mlien, Assigned: wilsonpage)

Tracking

unspecified
2.1 S5 (26sep)
ARM
Linux

Firefox Tracking Flags

(blocking-b2g:2.0+, b2g-v2.0 verified, b2g-v2.0M verified, b2g-v2.1 verified, b2g-v2.2 verified)

Details

Attachments

(5 attachments)

(Reporter)

Description

4 years ago
Created attachment 8491357 [details]
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%
(Reporter)

Comment 1

4 years ago
Created attachment 8491364 [details]
screenshot.png
(Reporter)

Comment 2

4 years ago
[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
blocking-b2g: --- → 2.1?
status-b2g-v2.0: --- → affected
status-b2g-v2.1: --- → affected

Comment 3

4 years ago
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
Assignee: nobody → wilsonpage
blocking-b2g: 2.1? → 2.1+
(Assignee)

Comment 4

4 years ago
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.
Attachment #8491933 - Flags: review?(jdarcangelo)
(Assignee)

Comment 5

4 years ago
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.
(Reporter)

Updated

4 years ago
Blocks: 1069746
(Reporter)

Updated

4 years ago
See Also: → bug 1065199
(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?
Flags: needinfo?(wilsonpage)
Attachment #8491933 - Flags: review?(jdarcangelo) → review+
(Assignee)

Updated

4 years ago
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Please request Gaia v2.1 approval on this when you get a chance. Thanks :)
status-b2g-v2.2: --- → fixed
Flags: needinfo?(wilsonpage)
Target Milestone: --- → 2.1 S5 (26sep)
(Assignee)

Comment 9

4 years ago
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]:
Attachment #8491933 - Flags: approval-gaia-v2.1?
Flags: needinfo?(wilsonpage)
(Reporter)

Comment 10

4 years ago
please also uplift this patch to v2.0 because this issue is a partner blocker
(Reporter)

Comment 11

4 years ago
ni wilson to help on this
Flags: needinfo?(wilsonpage)
Attachment #8491933 - Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
Triage: 2.0+ per Comment#10. Please help to uplift to 2.0 since it is a partner test blocker

Thanks
blocking-b2g: 2.1+ → 2.0+
Needs Gaia v2.0 approval before it can be uplifted.
(Assignee)

Comment 15

4 years ago
Created attachment 8495203 [details] [review]
pull-request (v2.0)
Flags: needinfo?(wilsonpage)
Missing a v2.0 approval request?
Flags: needinfo?(wilsonpage)
(Assignee)

Comment 17

4 years ago
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]:
Attachment #8495203 - Flags: approval-gaia-v2.0?
Flags: needinfo?(wilsonpage)
(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]:
Flags: needinfo?(wilsonpage)
(Assignee)

Comment 19

4 years ago
(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
Flags: needinfo?(wilsonpage)
Attachment #8495203 - Flags: approval-gaia-v2.0? → approval-gaia-v2.0+
Looks like the v2.0 PR needs rebasing :(
Flags: needinfo?(wilsonpage)
(Assignee)

Comment 21

4 years ago
Done
Flags: needinfo?(wilsonpage)
Created attachment 8526512 [details]
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
status-b2g-v2.0: fixed → verified
status-b2g-v2.0M: fixed → verified
status-b2g-v2.1: fixed → verified
status-b2g-v2.2: fixed → verified
You need to log in before you can comment on or make changes to this bug.