Closed Bug 1067205 Opened 10 years ago Closed 10 years ago

[Camera] Preview gallery video playback doesn't pause when device is locked

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.1+, b2g-v2.0 affected, b2g-v2.1 verified, b2g-v2.2 verified)

VERIFIED FIXED
2.1 S5 (26sep)
blocking-b2g 2.1+
Tracking Status
b2g-v2.0 --- affected
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: hyuna.cho82, Assigned: hyuna.cho82)

References

Details

(Whiteboard: [caf priority: p3][CR 730893])

Attachments

(1 file)

Repros. 1) record video content in Camera app 2) click thumbnail image 3) play video file 4) press power key 5) unlock lock screen Observe video file. video content is playing after unlock lock screen. In Gallery app, video file is paused.
Hi, Diego and Wilson At this case, video player can't receive 'visibilitychange' event. I checked it on nexus4 and it's reproduced. I didn't check on flame device. Plz check it.
Flags: needinfo?(wilsonpage)
Flags: needinfo?(dmarcos)
I checked camera app manifest. It does not have "audio-
Oh, I'm sorry for the incomplete comments. It's uploaded while writing my comment. :-( Camera app manifest does not have "audio-channel-content" permission. So it looks like that only "normal" audio channel type is being transferred to gecko-side. I think it makes the problem. And I verified that it works fine when "audio-channel-content" permission is added to camera app manifest.
(In reply to hyuna.cho from comment #1) > Hi, Diego and Wilson > > At this case, video player can't receive 'visibilitychange' event. > > I checked it on nexus4 and it's reproduced. I didn't check on flame device. > Plz check it. I just checked and it seems we are receiving no 'visibilitychange' event when the phone is locked. This is a serious issue, as it means that the camera will continue to run in the background when the phone is locked, which will drain the battery very quickly!
Flags: needinfo?(wilsonpage)
Summary: [Camera] Can't pause when press power key during playing video content in preview gallery → [Camera] Not receiving 'visibilitychange' event when device is locked
(In reply to Wilson Page [:wilsonpage] from comment #4) > I just checked and it seems we are receiving no 'visibilitychange' event > when the phone is locked. This is a serious issue, as it means that the > camera will continue to run in the background when the phone is locked, > which will drain the battery very quickly! If that's the case, it sounds like this could be the same root cause as Bug 1066015.
(In reply to Justin D'Arcangelo [:justindarc] from comment #5) > (In reply to Wilson Page [:wilsonpage] from comment #4) > If that's the case, it sounds like this could be the same root cause as Bug > 1066015. I'm sure we used to have code in camera that dealt with lockscreen events. I can't see them anymore. I know dmarcos did some work with them.
kgrandon: Do you know why we wouldn't be receiving 'visibilitychange' events when the device is locked. Should we be? If not, how can we detect when the device is locked whilst our app is open?
Flags: needinfo?(kgrandon)
Confirmed in v2.0 we *are* receiving the 'visibilitychange' event once the power button is pressed.
Flags: needinfo?(kgrandon)
It appears the regression was introduced somewhere between v2.0 - v2.1.
[Blocking Requested - why for this release]: Prevents camera hardware from being released, meaning camera continues to run the background, and battery will deplete rapidly.
blocking-b2g: --- → 2.1?
This bug only affects camera app; I can confirm that Music app *does* receive the expected events. This suggests some operation Camera is running must be preventing the 'visibilitychange' events from firing. I believe this to be a Gecko regression as flashing v2.0 Gaia (working with v2.0 Gecko) on latest Gecko still exhibits the bug.
UPDATED STEPS: 1. Open Camera 2. Switch to 'video' mode 3. Record a ~10sec video 4. Stop recording 5. Tap the thumbnail in the bottom right 6. Play the video 7. Press power button to lock device 8. Unlock device EXPECTED: Preview gallery should have closed ACTUAL: Preview gallery is open and video is still playing --- The consequences of this bug are a lot more severe than symptom outlined above. But these steps is the only way I could think of to visually detect the regression.
FWIW we *are* getting the 'visibilitychange' event if the device is locked during video recording.
Blocking Reason: regression based on comments 9 and 11
blocking-b2g: 2.1? → 2.1+
Updating flags to reflect currently known branch state.
Can we get branch checks and a regression window here?
Keywords: regression
QA Contact: aalldredge
(In reply to Youngwoo Jo from comment #3) > Oh, I'm sorry for the incomplete comments. > It's uploaded while writing my comment. > :-( > > Camera app manifest does not have "audio-channel-content" permission. > So it looks like that only "normal" audio channel type is being transferred > to gecko-side. > I think it makes the problem. > > And I verified that it works fine when "audio-channel-content" permission is > added to camera app manifest. Hi, Wilson If we add "audio-channel-content" in manifest.webapp, it's not reproduced. Our gecko/audio engineers said, Audio object created as 'normal' but changed to ' notification' due to the shutter sound. plz check it. Thanks Hyuna
(In reply to hyuna.cho from comment #17) > Hi, Wilson > > If we add "audio-channel-content" in manifest.webapp, it's not reproduced. > Our gecko/audio engineers said, Audio object created as 'normal' but changed > to ' notification' due to the shutter sound. > > plz check it. > > Thanks > Hyuna I'm confused what audio-channel config has to do with the firing or 'visibilitychange'...?
Gecko engineer said, if audio channel is normal, do change he app to hidden like music. I will request our gecko engineer to explain more information. To. Youngwoo Could you explain relationship between visibility and audio channel.
Flags: needinfo?(hiro7998)
http://lxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.js#482 => if shell.visibleNormalAudioActive is set, it does not set visibility to false('hidden'). That is, it does not change the current visibility state to 'hidden'. As a result, it does not trigger 'visibilitychange' event, because old value and new value are same as 'shown' state. http://lxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.js#1025 => shell.visibleNormalAudioActive is set to true, if visible-audio-channel is 'normal'. This means that, if 'normal' audio channel is set, 'visibilitychange' event is not triggered. So we have to check why 'normal' channel is set in the camera-preview-play scene.
Flags: needinfo?(hiro7998)
Camera app preview player is setting audio channel to 'content'. https://github.com/mozilla-b2g/gaia/blob/master/shared/js/media/video_player.js#L56 However, camera app does not have 'audio-channel-content' permission. So I expect that gecko changes the audio channel of the video element to the default value('normal'). Anyway, camera app is using 'content' audio channel, but it does not have the permission. So we have to add 'audio-channel-content' permission to camera app manifest.
This seems like a serious mix of concerns. Audio channels shouldn't be affecting page visibility events. I can't think of any cases where this would be useful. If an app wanted to be able to continue playing in the background (I'm guessing that's what the intention is with this mix-up), then the app could simply ignore the visibility events.
This issue is occurring on 2.2 Flame, 2.2 Open_C, 2.1 Flame, 2.0 Flame, and 2.0 Flame Base. Environmental Variables: Device: Flame 2.2 Master BuildID: 20140918150023 Gaia: d37950eb09e28aa18d0e01df9ff90574bd4337e0 Gecko: d6f78faaefcc Version: 35.0a1 (2.2 Master) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0 Device: Open_C 2.2 Master BuildID: 20140919052449 Gaia: d170091ba1b5597b05f37fb259f6b8eb02568798 Gecko: 3475e6a1665a Version: 35.0a1 (2.2 Master) Firmware: P821A10v1.0.0B06_LOG_DL User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0 Device: Flame 2.1 BuildID: 20140918145021 Gaia: 9f3588e8fdd37f6f354b5518dbeab7299b118a9d Gecko: 6f26d4fce19c Version: 34.0a2 (2.1) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Device: Flame 2.0 BuildID: 20140918082321 Gaia: 31434a3949556171f3565ca47ac2b44e810e95e6 Gecko: 5cf783171d5c Version: 32.0 (2.0) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 Device: Flame 2.0 BuildID: 20140904160718 (Base) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 Result: Video plays while device is locked.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
(In reply to Youngwoo Jo from comment #20) > http://lxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.js#482 > => if shell.visibleNormalAudioActive is set, it does not set visibility to > false('hidden'). > That is, it does not change the current visibility state to 'hidden'. > As a result, it does not trigger 'visibilitychange' event, because old value > and new value are same as 'shown' state. > > http://lxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell. > js#1025 > => shell.visibleNormalAudioActive is set to true, if visible-audio-channel > is 'normal'. > > This means that, if 'normal' audio channel is set, 'visibilitychange' event > is not triggered. > > So we have to check why 'normal' channel is set in the camera-preview-play > scene. I agree completely with Justin, 'visibilitychange' should always fire when the app is no longer visible on the screen aside from what audio-channel is in use. Youngwoo: Can you please advise a solution and arrange the necessary Gaia/Gecko patches to fix?
Flags: needinfo?(hiro7998)
Just to be sure - I checked the EARLIEST 2.0 we had as well and this bug repro'd there - this and the prior comments' branch check indicate that this in not a regression striking regression window wanted flag. Device: Flame 2.0 BuildID: 20140904114640 Gaia: 13978cf2230652274969536322378d448fd142a4 Gecko: 2537ab191112 Version: 32.0 (2.0) Firmware: User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
I agree the coupling of "visibilitychange" event and audio channel is not a good design. But the cause of this issue is from the wrong camera app permission. So now Hyuna is attaching the patch. To Hyuna, Can you upload your patch? And I'm not a proper person to comment about modifying the coupling issue, because I don't know history of the audio channel and the visibility issues and I'm not a person of medai/audio/video part.
Flags: needinfo?(hiro7998)
Attached file PR-1067205.html (deleted) —
Hi, Wilson We can fix it if we add permissions for content audio channel in camera app. Plz check it. Thanks
Attachment #8492959 - Flags: review?(wilsonpage)
(In reply to hyuna.cho from comment #27) > Created attachment 8492959 [details] > PR-1067205.html > > Hi, Wilson > > We can fix it if we add permissions for content audio channel in camera app. > Plz check it. > > Thanks This patch is not fixing the issue for me on Flame. I still am not receiving the `visibilitychange` event with the patch applied.
Blocks: 1066015
Tim, we need visibilitychange to be always fired when the app is not visible. It should not depend on the audio channel permissions. Is anybody looking into this or any other bug tracking the issue?
Flags: needinfo?(dmarcos) → needinfo?(timdream)
On Flame (JB v123), the `visibilitychange` event is not firing on master (v2.2) when the lock-button is pressed. All other branches appear to be unaffected. Here's the regression window I've identified: Last Working: Gaia-Rev 8e02f689b0fc39cb6ccdc22d02ed7e219c58faa7 Gecko-Rev https://hg.mozilla.org/mozilla-central/rev/6b3948d3413a Build-ID 20140909160207 Version 35.0a1 First Broken: Gaia-Rev 8e02f689b0fc39cb6ccdc22d02ed7e219c58faa7 Gecko-Rev https://hg.mozilla.org/mozilla-central/rev/152ef25e89ae Build-ID 20140910040203 Version 35.0a1 NOTE: I've noticed that Comment 0 appears to be referring to an issue regarding the playback of recorded video content. However, the title of this bug is simply referring to the absence of the `visibilitychange` event when pressing the 'lock' button. So, I'm not sure if these are two separate issues or not. My concern is with the missing `visibilitychange` events because this has serious negative consequences with the Camera app. The easiest way to tell if we're getting the `visibilitychange` events is to have the Flash set to "Auto" or "On" and point the camera in a dim/dark area which will cause the auto-focus to continuously trigger the flash to turn on. When you press the 'lock' button, the camera should stop focusing and you should no longer see the flash turning on and off. In Build 20140910040203 and onward, the camera continues to try and focus and trigger the flash even after the 'lock' button is pressed.
No longer blocks: 1066015
Re-naming this bug to clarify. There are two separate issues here.
Summary: [Camera] Not receiving 'visibilitychange' event when device is locked → [Camera] Preview gallery video playback doesn't pause when device is locked
Moving discussion specifically surrounding the absence of the `visibilitychange` event upon pressing the 'lock' button to Bug 1071198. This bug will continue to track the specific issue in Comment 0 regarding the preview gallery video playback.
Comment on attachment 8492959 [details] PR-1067205.html Taking this review from Wilson :-) I'll check and see if this permission change fixes the specific issue from Comment 0 on the v2.1 branch. If it does, we can land and uplift this change to v2.1.
Attachment #8492959 - Flags: review?(wilsonpage) → review?(jdarcangelo)
Comment on attachment 8492959 [details] PR-1067205.html [Approval Request Comment] [Bug caused by] (feature/regressing bug #): n/a [User impact] if declined: Video playback within the Camera's preview gallery would continue playing after the 'lock' button is pressed. [Testing completed]: Tested on Flame (JB-v123) [Risk to taking this patch] (and alternatives if risky): none; just adds a permission for 'audio-channel-content' [String changes made]: none Hyuna, This definitely causes video playback in the Camera preview gallery to stop when the 'lock' button is pressed. Thank you for your patch! Tim, Can you explain why the `visibilitychange` event is conditional depending on the type of audio-channel? It seems that something should be done to address the separation of these concerns.
Attachment #8492959 - Flags: review?(jdarcangelo)
Attachment #8492959 - Flags: review+
Attachment #8492959 - Flags: approval-gaia-v2.1?(fabrice)
Assignee: nobody → hyuna.cho82
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
(In reply to Diego Marcos [:dmarcos] from comment #29) > Tim, we need visibilitychange to be always fired when the app is not > visible. It should not depend on the audio channel permissions. Is anybody > looking into this or any other bug tracking the issue? Diego, the bug to look is bug 1015073, specifically the last comment. Again, unfortunately, this is nothing Gaia System can do at this point. I plan to make sure we have proper Gecko help address this soon.
Flags: needinfo?(timdream)
Target Milestone: --- → 2.1 S5 (26sep)
Attachment #8492959 - Flags: approval-gaia-v2.1?(fabrice) → approval-gaia-v2.1+
Whiteboard: [CR 730893]
Whiteboard: [CR 730893] → [caf priority: p3][CR 730893]
This issue is verified fixed on the latest 2.1 and 2.2 Flame builds: Environmental Variables: ---------------------------------------- Device: Flame 2.1 BuildID: 20141012001201 Gaia: d18e130216cd3960cd327179364d9f71e42debda Gecko: 610ee0e6a776 Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf Version: 34.0a2 (2.1) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Environmental Variables: ---------------------------------------- Device: Flame 2.2 Master BuildID: 20141012040203 Gaia: 717ad4e8b7fc10ab8248500d00ba5ba0977fa8ab Gecko: 44168a7af20d Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf Version: 35.0a1 (2.2 Master) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0 The video is paused when user presses the power button to lock device and replays at correct location when resumed.
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: