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)
Tracking
(blocking-b2g:2.1+, 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)
(deleted),
text/html
|
justindarc
:
review+
fabrice
:
approval-gaia-v2.1+
|
Details |
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)
Comment 2•10 years ago
|
||
I checked camera app manifest. It does not have "audio-
Comment 3•10 years ago
|
||
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.
Comment 4•10 years ago
|
||
(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
Comment 5•10 years ago
|
||
(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.
Comment 6•10 years ago
|
||
(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.
Comment 7•10 years ago
|
||
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)
Comment 8•10 years ago
|
||
Confirmed in v2.0 we *are* receiving the 'visibilitychange' event once the power button is pressed.
Flags: needinfo?(kgrandon)
Comment 9•10 years ago
|
||
It appears the regression was introduced somewhere between v2.0 - v2.1.
Comment 10•10 years ago
|
||
[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?
Comment 11•10 years ago
|
||
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.
Comment 12•10 years ago
|
||
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.
Comment 13•10 years ago
|
||
FWIW we *are* getting the 'visibilitychange' event if the device is locked during video recording.
Comment 14•10 years ago
|
||
Blocking Reason: regression based on comments 9 and 11
blocking-b2g: 2.1? → 2.1+
Keywords: regressionwindow-wanted
Comment 15•10 years ago
|
||
Updating flags to reflect currently known branch state.
Comment 16•10 years ago
|
||
Can we get branch checks and a regression window here?
Keywords: regression
Updated•10 years ago
|
QA Contact: aalldredge
Assignee | ||
Comment 17•10 years ago
|
||
(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
Comment 18•10 years ago
|
||
(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'...?
Assignee | ||
Comment 19•10 years ago
|
||
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)
Comment 20•10 years ago
|
||
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)
Comment 21•10 years ago
|
||
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.
Comment 22•10 years ago
|
||
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.
Comment 23•10 years ago
|
||
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.
Comment 24•10 years ago
|
||
(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)
Comment 25•10 years ago
|
||
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)
Keywords: regression,
regressionwindow-wanted
Comment 26•10 years ago
|
||
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)
Assignee | ||
Comment 27•10 years ago
|
||
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)
Comment 28•10 years ago
|
||
(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.
Comment 29•10 years ago
|
||
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)
Comment 30•10 years ago
|
||
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.
Updated•10 years ago
|
Comment 31•10 years ago
|
||
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
Comment 32•10 years ago
|
||
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 33•10 years ago
|
||
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 34•10 years ago
|
||
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)
Updated•10 years ago
|
Assignee: nobody → hyuna.cho82
Updated•10 years ago
|
Status: NEW → ASSIGNED
Comment 35•10 years ago
|
||
Landed on master:
https://github.com/mozilla-b2g/gaia/commit/3c898380b47f298cd3b7a0dacb3a6529e94322d4
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 36•10 years ago
|
||
(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)
Updated•10 years ago
|
Updated•10 years ago
|
Target Milestone: --- → 2.1 S5 (26sep)
Updated•10 years ago
|
Attachment #8492959 -
Flags: approval-gaia-v2.1?(fabrice) → approval-gaia-v2.1+
Comment 37•10 years ago
|
||
Updated•10 years ago
|
Whiteboard: [CR 730893]
Updated•10 years ago
|
Whiteboard: [CR 730893] → [caf priority: p3][CR 730893]
Comment 38•10 years ago
|
||
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)
Updated•10 years ago
|
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.
Description
•