Closed Bug 995540 Opened 11 years ago Closed 11 years ago

[Sora][camera]video recording would not stop instantly when coming a call or alarm

Categories

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

defect

Tracking

(blocking-b2g:1.3+, b2g-v1.3 fixed, b2g-v1.3T wontfix, b2g-v1.4 fixed, b2g-v2.0 fixed, b2g-v2.2 verified, b2g-v2.5 verified, b2g-master verified)

VERIFIED FIXED
2.0 S1 (9may)
blocking-b2g 1.3+
Tracking Status
b2g-v1.3 --- fixed
b2g-v1.3T --- wontfix
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed
b2g-v2.2 --- verified
b2g-v2.5 --- verified
b2g-master --- verified

People

(Reporter: sync-1, Assigned: djf)

References

()

Details

(Keywords: regression, Whiteboard: [cert])

Attachments

(7 files)

Firefox OS v1.3 Mozilla build ID:20140404164003 DEFECT DESCRIPTION: video recording would not stop instantly when coming a call or alarm REPRODUCING PROCEDURES: 1.Launch camera; 2.Switch to camcorder; 3.Start recording video; 4.Coming a call or alarm during video recording.Video recording can't stop instantly.When preview video,you can hear ringtone of call or alarm.->KO Note:Beetle Lite FF don't have this problem. EXPECTED BEHAVIOUR: Video recording can stop instantly so that can't hear ringtone of call or alarm. ASSOCIATE SPECIFICATION: TEST PLAN REFERENCE: TOOLS AND PLATFORMS USED: USER IMPACT: REPRODUCING RATE:5/5
Can we confirm this on the Moz side?
Keywords: qawanted, regression
I was able to reproduce this issue on a Buri device running the latest Master build, both during an incoming call and while an alarm is going off. It takes a few seconds for the camera to stop recording after receiving a call/having an alarm go off, and the ring/alarm tone can be heard in the video Environmental Variables: Device: Buri BuildID: 20140418060928 Gaia: 76c94ac5dc3b8e17cc23d9cc3e2662b0d5d28b2e Gecko: 45ba19361b97 Version: 31.0a1 Base Image: V1.2-device.cfg
Keywords: qawanted
QA Contact: jzimbrick
Hi, please fix it in 1.3, thanks.
blocking-b2g: --- → 1.3?
Hi Jason - Could you help to triage this one? I think it should be a blocker for the following two reasons: 1. It is a regression 2. It will impact the end user since according to Comment#2, the camera stops recording after receiving a call/having an alarm go off, and the ring/alarm tone can be heard in the video. Please help to triage this one to see if it should be a blocker. Thanks
Flags: needinfo?(jsmith)
Is this a cert blocker?
Flags: needinfo?(jsmith) → needinfo?(vchen)
Yes, this is a cert blocker
Flags: needinfo?(vchen)
blocking-b2g: 1.3? → 1.3+
Whiteboard: [cert]
Hema - Can you find someone to look into this issue?
Flags: needinfo?(hkoka)
Jason, Is this a Sora specific issue? does QA have a Sora (either in SF or MV) that one of the media devs could use? Diego/Punam, will need help from one of you to look into this certification blocker. Thanks Hema
Flags: needinfo?(pdahiya)
Flags: needinfo?(jsmith)
Flags: needinfo?(hkoka)
Flags: needinfo?(dmarcos)
Making arrangement for Sora device so Diego can help investigate (but if someone from QA can confirm if this is just Sora specific, it would be great) thanks!
Flags: needinfo?(pdahiya)
Flags: needinfo?(jsmith)
(In reply to Hema Koka [:hema] from comment #9) > Making arrangement for Sora device so Diego can help investigate (but if > someone from QA can confirm if this is just Sora specific, it would be great) > > thanks! Hi Hema, per Comment#2, this issue can also be reproduced on Buri with master build Thanks Vance
Diego: Please take a look at this with the Buri device that you have in the meantime (Naoki is planning on dropping off a Sora tomorrow)
Assignee: nobody → dmarcos
This doesn't reproduce on Sora: Gaia 741869c9f0eaae242351f312c18fa6051715c086 Gecko 96b3233f80637206d502b52f238196fa86769ec9 BuildID 20140207155315 Version 28.0 ro.build.version.incremental=221 ro.build.date=Mon Jan 27 20:11:26 CST 2014
After investigation these are my findings: 1. Camera stops recording when the app receives the visibilitychange event. 2. When an incoming call arrives the callscreen app is launched and brought to foreground. 3. The visibilitychange event should be delivered to camera before callscreen takes over on the foreground but callscreen is notified before camera. We cannot stop recording before the ringtone starts playing. I think this is a system app bug. I filed bug 1006200
Flags: needinfo?(dmarcos)
Depends on: 1006200
For the record. I was able to reproduce the issue in the version referred on comment 12. It was a misunderstanding of the bug on my side.
It would be a really ugly hack, but could we fix this by setting the camera audio stream priority to be as high as the telephony and alarm priority when the camera is recording a video? Then, when the visibility change arrives and we're sent to the background we stop recording, release our audio priority and the telephone or alarm ringer starts playing... I don't know if that would work since I don't really understand the audio priority stuff, but a workaround like that seems more plausible for a late 1.3+ bug than a change to the system app.
Attached file patch for the 1.3 branch (deleted) —
This patch fixes the bug for me, though I've only tested it on a Hamachi. Video recording stops before any phone ringer or alarm sound is heard. I can see recording stop before I see the attention screen appear. Diego, would you review the camera changes? Alive, would you review the system app changes? This is the solution you proposed in bug 1006200 except that for simplicity I used the settings API instead of IAC. Alive: will you accept this use of the settings API on the 1.3 branch at least? Will you accept it in 1.4 and master as well? Diego: if Alive is okay with the use of settings, can you produce a version of this patch for master since you know that version of the camera better than I do?
Attachment #8419791 - Flags: review?(dmarcos)
Attachment #8419791 - Flags: review?(alive)
J Zimbrick, Could you test the attached patch to verify that it fixes the bug for you?
Flags: needinfo?(jzimbrick)
Comment on attachment 8419791 [details] patch for the 1.3 branch On the camera side the patch looks good to me. I let alive review the system app side.
Attachment #8419791 - Flags: review?(dmarcos) → review+
On second thought, I'm not even sure we could easily use the IAC API here. If the camera says it is interested in messages from the system app, doesn't it actually get launched when the system app sends a message? To avoid that, we'd have to have the camera app be the publisher and have the system app subscribe. That seems quite a bit harder to implement as a broadcast mechanism.
Even if djf patch goes through for 1.3 we still need a proper solution for 1.3+
The proper fix is either (1) Remove transition for dialer (AttentionScreen issue). (2) Keep rendering state of app even it's background (Decouple rendering and page visibility). (3) Move audio competing policy from gecko to gaia and let gaia stops the low priority channel. (In reply to Diego Marcos [:dmarcos] from comment #21) > Even if djf patch goes through for 1.3 we still need a proper solution for > 1.3+
Comment on attachment 8419791 [details] patch for the 1.3 branch Sad. I'd like to have different solution for 1.4+ but for 1.3 I think I should just grant this because IAC is also a hack..
Attachment #8419791 - Flags: review?(alive) → review+
We still need to decide what to do for v1.4 and master, but I'm going to close this bug since it was a blocker, and we can follow up in 1006200
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Did this get approval somewhere to land on v1.3?
Flags: needinfo?(dmarcos)
Comment on attachment 8419791 [details] patch for the 1.3 branch 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 #): I don't think this is actually a regression. I think we've always worked this way, but TCL is calling this a cert blocker now. [User impact] if declined: when video recording is stopped because of an incoming phone call or alarm, the user will hear some of the ringer or alarm sound before the recording stops. This is apparently a certification blocker for TCL. [Testing completed]: It works for me on Hamachi. Assuming TCL will test it on their devices. [Risk to taking this patch] (and alternatives if risky): Minimal. It adds two lines to the system app that set values in the settings db. The camera app listens for changes to those settings and stops recording if necessary. [String changes made]: no string changes
Attachment #8419791 - Flags: approval-gaia-v1.3?(bbajaj)
Keywords: qawanted
(In reply to David Flanagan [:djf] from comment #27) > Comment on attachment 8419791 [details] > patch for the 1.3 branch > > 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 #): I don't think this is actually a > regression. I think we've always worked this way, but TCL is calling this a > cert blocker now. Hmmm, the description, "Note:Beetle Lite FF don't have this problem" which is Buri equivalent and this may not be a 1.1. issue. > > [User impact] if declined: when video recording is stopped because of an > incoming phone call or alarm, the user will hear some of the ringer or alarm > sound before the recording stops. This is apparently a certification > blocker for TCL. > > [Testing completed]: It works for me on Hamachi. Assuming TCL will test it > on their devices. I'll NI QA to verify this issue. > > [Risk to taking this patch] (and alternatives if risky): Minimal. It adds > two lines to the system app that set values in the settings db. The camera > app listens for changes to those settings and stops recording if necessary. > > [String changes made]: no string changes
Keywords: qawanted
Keywords: qawantedverifyme
Attachment #8419791 - Flags: approval-gaia-v1.3?(bbajaj) → approval-gaia-v1.3+
Target Milestone: --- → 2.0 S1 (9may)
Flags: in-moztrap?
Attached file pull request for the branch 1.4 (deleted) —
Attachment #8425223 - Flags: review?(dflanagan)
Flags: needinfo?(dmarcos)
Flags: in-moztrap? → in-moztrap+
Comment on attachment 8425223 [details] pull request for the branch 1.4 The code looks good to me, though I did have one naming suggetion I mentioned on github. Also, it looks like a test is now failing because of this change.
Attachment #8425223 - Flags: review?(dflanagan) → review+
Changing v1.3T branch to affected - I can still reproduce this issue on the latest v1.3T Tarako build: v1.3T Environmental Variables: Device: Tarako v1.3T MOZ RIL BuildID: 20140530014002 Gaia: e68858693b71d917c9c5ee7e215f7ceea04635f7 Gecko: 1945abae19ff Version: 28.1 Firmware Version: SP6821a-Gonk-4.0-5-12 - 1) When calling the device or having an alarm go off while recording a video, playback of the video will have the recorded audio of the incoming phone calls ringer or alarm sound in the background.
This needs to land on trunk as well as a permanent workaround so that we can unblock bug 1006200, since bug 1006200 will not block any available release. Can you land this?
Flags: needinfo?(dmarcos)
I don't understand what you're requesting. We came up with a temporary workaround for 1.3 and 1.4 but a proper solution has to be implemented on the system app side. bug 1006200 describes the actual root of the issue. If bug 1006200 is fixed there's nothing we have to do on the camera app. (In reply to Jason Smith [:jsmith] from comment #33) > This needs to land on trunk as well as a permanent workaround so that we can > unblock bug 1006200, since bug 1006200 will not block any available release. > > Can you land this?
Flags: needinfo?(dmarcos) → needinfo?(jsmith)
This has never been a bug in camera. It's a bug in the system app that affects camera.
(In reply to Diego Marcos [:dmarcos] from comment #34) > I don't understand what you're requesting. We came up with a temporary > workaround for 1.3 and 1.4 but a proper solution has to be implemented on > the system app side. bug 1006200 describes the actual root of the issue. If > bug 1006200 is fixed there's nothing we have to do on the camera app. Until https://bugzilla.mozilla.org/show_bug.cgi?id=1006200 is resolved we should have the hack on 2.0 as well. Also it is best practice to have all the branches in sync along with master/central when trying to land hacks, else it becomes a tracking nightmare. > > (In reply to Jason Smith [:jsmith] from comment #33) > > This needs to land on trunk as well as a permanent workaround so that we can > > unblock bug 1006200, since bug 1006200 will not block any available release. > > > > Can you land this?
I don't feel comfortable implementing the hack in master. If we don't fix this properly there are security and privacy implications for every app (1st and 3rd party) (read https://bugzilla.mozilla.org/show_bug.cgi?id=1006200#c46) Why cannot bug 1006200 be fixed for 2.0?
It's not a good practice making permanent a hack that was implemented due to time constraints. (In reply to bhavana bajaj [:bajaj] from comment #36) > (In reply to Diego Marcos [:dmarcos] from comment #34) > > I don't understand what you're requesting. We came up with a temporary > > workaround for 1.3 and 1.4 but a proper solution has to be implemented on > > the system app side. bug 1006200 describes the actual root of the issue. If > > bug 1006200 is fixed there's nothing we have to do on the camera app. > > Until https://bugzilla.mozilla.org/show_bug.cgi?id=1006200 is resolved we > should have the hack on 2.0 as well. Also it is best practice to have all > the branches in sync along with master/central when trying to land hacks, > else it becomes a tracking nightmare. > > > > (In reply to Jason Smith [:jsmith] from comment #33) > > > This needs to land on trunk as well as a permanent workaround so that we can > > > unblock bug 1006200, since bug 1006200 will not block any available release. > > > > > > Can you land this?
If this isn't fixed on 1.3T, this presumably isn't on 1.3 either. What's the next step?
Flags: needinfo?(dmarcos)
Taking this since Diego doesn't have a working Tarako.
Assignee: dmarcos → dflanagan
The patch is correctly applied on Tarako, and I've verified that the code works. Before the attention screen comes up, the system app sets a setting in the settings db. When the camera is notified of the setting change it stops recording. But the problem is that everything is so slow on the Tarako that it can take 3 to 6 seconds before the camera app gets the event. And by that time the ringtone has already started playing. The workaround patch worked on 1.3 on a Hamachi, but Tarako just isn't fast enough to handle it. I think we may just not be able to fix this on Tarako. (And if not, we might want to remove the workaround from 1.3T since it isn't doing any good.) I have noticed (in 1007019 and 1018384, e.g.) that the camera app is really, really sluggish on Tarako when running as an activity. This is a similar case where the dialer starts up in the background and competes with the camera for memory. Maybe there is something wrong in gecko or gonk making the Tarako camera especially slow. Or maybe we just really don't have enough memory, and everything gets slow because we're thrashing on zmem. I'll attach the logcat I captured when I reproduced this bug.
This is the logcat output while reproducing this bug. Search for this string and note the timestamp that follows it. This message is emitted by the system app when it begins the workaround hack to send a message by the settings db system: attention screen opening Then search for this string and note that the timestamp is about 6 seconds later: camera: attention screen opening This second message is emitted when the camera is notified of the setting change. Mike: could you take a quick look at the log to see if anything stands out for you? Does it look like the spreadtrum camera driver is doing anything inefficient that would account for this profound sluggishness? Joe: this bug strikes me as sort of a corner case, and I wonder how badly it needs to be fixed for Tarako. If we can give up on it, then we should remove the workaround code since it doesn't work and will just make things slower when a call comes in. On the other hand, if this is something that is going to block 1.3T, then I don't think we can workaround it in gaia. We need some other faster communication channel to shut the camera down. Like we need gecko to send a system message to the camera app to make it stop when a call comes in or alarm starts playing. I have no idea how to do that, so you'll need to find a gecko engineer who can help with that. (Though I could add the gaia code to handle a system message like that.)
Flags: needinfo?(mhabicher)
Flags: needinfo?(jcheng)
David, thanks for helping with this. Clearing NI from me.
Flags: needinfo?(dmarcos)
Here's the workaround, cherry-picked into the master branch, and with merge conflicts resolved.
Attachment #8436187 - Flags: review+
No one likes the idea about continuing to use the workaround here, but we're out of time for fixing this right in 2.0, so we've just got to land it. When travis goes green, let's land this, close the bug, and use bug 1006200 for development of a real fix in 2.1
Setting checkin-needed in the hope that a sheriff working this weekend will see that Travis has gone green and land this before the FL deadline on Monday.
Keywords: checkin-needed
(In reply to David Flanagan [:djf] from comment #42) > I think we may just not be able to fix this on Tarako. Who's on the hook for making that decision?
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #49) > Who's on the hook for making that decision?
Flags: needinfo?(dmarcos)
At this moment, We are trying to close out Tarako since this patch doesn't help Tarako and this isn't a partner blocker for 1.3T, let's leave this out of 1.3T Thanks
Flags: needinfo?(jcheng)
Flags: needinfo?(mhabicher)
Flags: needinfo?(dmarcos)
Hi David, The patch on the branch 1.4 results in another issue, https://bugzilla.mozilla.org/show_bug.cgi?id=1069887. After removing the patch, this bug Bug995540 still doesnot reproduce. Is there any change of communication mechanism to speed response? Is it workable that removing the patch to fix Bug1069887 in Bug995540 unreproduce case? Thanks.
Flags: needinfo?(dflanagan)
Sorry, ignore Comment 52.
Flags: needinfo?(dflanagan)
Hi William, Could you help with this bug? Steps: 1.Launch camera; 2.Switch to camcorder; 3.Start recording video; 4.Coming a call or alarm during video recording.Video recording can't stop instantly.When preview video,you can hear ringtone of call or alarm. Actual result: I have verified this bug for 5 times, and if the alarm is coming while recording a video, the video will stop before alarm rings, you can't hear alarm while you play this video. But if a call is coming while recording, the video will stop after call ringtone plays for 1s. If you play back this video, you can hear the call ringtone too. See attachment: 1615.mp4
Flags: needinfo?(whsu)
Attached video 1615.MP4 (deleted) —
(In reply to Lance from comment #54) > Hi William, > Could you help with this bug? > > Steps: > 1.Launch camera; > 2.Switch to camcorder; > 3.Start recording video; > 4.Coming a call or alarm during video recording.Video recording can't stop > instantly.When preview video,you can hear ringtone of call or alarm. > > Actual result: > I have verified this bug for 5 times, and if the alarm is coming while > recording a video, the video will stop before alarm rings, you can't hear > alarm while you play this video. But if a call is coming while recording, > the video will stop after call ringtone plays for 1s. If you play back this > video, you can hear the call ringtone too. > > See attachment: 1615.mp4 Hi, Lance, I thought this was a regression. But, I was wrong. I used the older build to do the verification, and I saw the same symptom. May I have your help to file a follow-up? Many thanks. * Build info. - Gaia-Rev 8f4201a44676eb70926a3d2645d94bf92fcd6718 - Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/95ca6aefcdcb - Build-ID 20140525000202 (Comment 31 landed on v1.4) - Version 30.0 - Device-Name flame - FW-Release 4.3
Flags: needinfo?(whsu) → needinfo?(liuke)
We have upload new bug 1130966 to tract this bug.
Flags: needinfo?(liuke)
Hi William, If 1.4 should be affected, please mark this info. Thank you very much.
Flags: needinfo?(whsu)
(In reply to Lance from comment #58) > Hi William, > > If 1.4 should be affected, please mark this info. > Thank you very much. Added. Thanks for your reminder!
Flags: needinfo?(whsu)
It seems not fixed at v2.2. Could you try both v2.2 & v2.1?
Flags: needinfo?(liuke)
Attached file logcat_1449.txt (deleted) —
Hi Hermes, The bug still exists on latest Nightly Flame 2.1 & 2.2 and Nexus5_2.2 by STR as comment 0.The results are same as mentioned in comment 54. Found time:14:49 Fail rate:5/5 See attachment: 1449.3gp and logcat_1449.txt Device: Flame 2.1 version(Affected): Build ID 20150616161200 Gaia Revision f8b848c82d1ed589f7a1eb5cc099830c867ff1d4 Gaia Date 2015-06-08 09:48:23 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/0ebea88c344d Gecko Version 34.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150616.193647 Firmware Date Tue Jun 16 19:36:56 EDT 2015 Bootloader L1TC000118D0 Device: Flame 2.2 version(Affected): Build ID 20150616002505 Gaia Revision e7a0c6d5f4df04d45fb3f726efb9e8223600cb79 Gaia Date 2015-06-15 06:12:18 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/8045028bf400 Gecko Version 37.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150616.035412 Firmware Date Tue Jun 16 03:54:24 EDT 2015 Bootloader L1TC000118D0 Device: Nexus 5_2.2 version(Affected): Build ID 20150616002505 Gaia Revision e7a0c6d5f4df04d45fb3f726efb9e8223600cb79 Gaia Date 2015-06-15 06:12:18 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/8045028bf400 Gecko Version 37.0 Device Name hammerhead Firmware(Release) 5.1 Firmware(Incremental) eng.cltbld.20150616.034849 Firmware Date Tue Jun 16 03:49:07 EDT 2015 Bootloader HHZ12f
Flags: needinfo?(liuke) → needinfo?(hcheng)
Attached video 1449.3GP (deleted) —
QA Whiteboard: [MGSEI-Triage+]
Lance, thanks. Use bug 1175767 for following up.
Flags: needinfo?(hcheng)
I'm not able to repro the bug. The video stops when a call comes in or an alarm goes off. Environmental Variables: (not affected) Device: Flame 2.6 BuildID: 20151105030203 Gaia: 607b9c5db7fdbbafc16a572e7c319baa266a3372 Gecko: 59c648a3f95524cb1ee42f2306c1db2698d35258 Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a Version: 45.0a1 (2.6) Firmware Version: v18D Environmental Variables: (not affected) Device: Aries 2.6 BuildID: 20151030120435 Gaia: 91cac94948094cfdcd00cba5c6483e27e80cb3b0 Gecko: c2534acb485963331d67bbc5c07f0d862ed56bf5 Gonk: a19052e4389c3ae2d8fc3e7a74a475401baacc56 Version: 45.0a1 (2.6) Firmware Version: D5803_23.1.A.1.28_NCB.ftf Environmental Variables: (not affected) Device: Flame 2.2 BuildID: 20141231010205 Gaia: 26d479f0fccb7174e06255121e4e938c1b280d67 Gecko: 88037f94b7d7 Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a Version: 37.0a1 (2.2) Firmware Version: v18D Environmental Variables: (not affected) Device: Flame 2.5 BuildID: 20151027030203 Gaia: b6ede3d0fdec5fc922e9ca3401e60db461bf705c Gecko: 9a8f2342fb3116d23989087e026448d38a3768c5 Gonk: c4779d6da0f85894b1f78f0351b43f2949e8decd Version: 44.0a1 (2.5) Firmware Version: v18D
QA Whiteboard: [MGSEI-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(jmercado)
Keywords: verifyme
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmercado)
Flags: needinfo?(jzimbrick)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: