Closed Bug 866036 Opened 7 years ago Closed 7 years ago

[Buri][Audio]Can't adjust game volume. (Poppit)

Categories

(Firefox OS Graveyard :: General, defect, P2)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:tef+, b2g18 verified, b2g18-v1.0.1 verified)

RESOLVED FIXED
1.0.1 Cert2 (21may)
blocking-b2g tef+
Tracking Status
b2g18 --- verified
b2g18-v1.0.1 --- verified

People

(Reporter: sync-1, Assigned: baku)

References

Details

(Whiteboard: [status: needs uplift][apps watch list][required_last_cert_round])

Attachments

(2 files, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #444928 +++
 
 AU_LINUX_GECKO_ICS_STRAWBERRY_V1.01.00.01.019.080
 Firefox os  v1.0.1
 Mozilla build ID:20130418070206
 
 DEFECT DESCRIPTION:
 [Audio]Can't adjust game volume.
 
  REPRODUCING PROCEDURES:
 Play games(poppit),use volume+/- to adjust volume to max/min/silence,the game volume keep the same.->ko
 
  EXPECTED BEHAVIOUR:
 Game volume change with setting.
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 B2G-7210 
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:
 3/3
 
  For FT PR, Please list reference mobile's behavior:
 
 ++++++++++ end of initial bug #444928 description ++++++++++
 
 
 
 CONTACT INFO (Name,Phone number):
 
  DEFECT DESCRIPTION:
 
  REPRODUCING PROCEDURES:
 
  EXPECTED BEHAVIOUR:
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:
 
  For FT PR, Please list reference mobile's behavior:
Can you try to adjust volume while audio is playing?
Whiteboard: [apps watch list1]
I'm unable to duplicate this on Unagi v1.0.1 build 2013-04-25 and Poppit v1.0.1 (which is still in review).
(In reply to Lisa Brewster [:adora] from comment #2)
> I'm unable to duplicate this on Unagi v1.0.1 build 2013-04-25 and Poppit
> v1.0.1 (which is still in review).

That probably means this could potentially be a Buri-specific bug if this is reproducible on Buri only.
Summary: [Buri][Audio]Can't adjust game volume. → [Buri][Audio]Can't adjust game volume. (Poppit)
(In reply to Randy Lin [:rlin] from comment #1)
> Can you try to adjust volume while audio is playing?

I can adjust volume while audio is playing.

So when the audio is not playing, what I adjust is the "notification" volume but not "content" volume even I stay on poppit screen, right?
The current behavior is similar as your description.
This is similar on the Game Pack e.g. Magic Stones and occurs on the ZTE.  Turning the volume up and down is random. Press it and the volume reduces e.g. to level 2. Wait. Press it up, and the volume is at 6 and goes to 7. Press it down and the volume starts at something else.

Turning the volume down to zero still produces sounds.
Component: Gaia → General
So let me clarify the issue of contention here based on discussion above:

The issue is the fact that adjusting volume within an app only changes the volume of the audio involved when it's playing. But when it's not playing, the volume for the audio that would play later would not have the sound changed for that app.

This is likely an audio channels bug.

Don't know if I'd block on this though given that the workaround for users is to adjust volume when audio is playing. Certainly awkward, though.

Andrew - Do you know who works on audio channels who would be able to look into this?
Flags: needinfo?(overholt)
Actually, I see that Andrea is working on some of these audio channel bugs. 

Andrea - Any thoughts on comment 7?
Flags: needinfo?(overholt) → needinfo?(amarchesini)
Actually, I'm changing opinion on the blocking call. This is nearly unusable in the preinstalled gaming apps - you can only adjust the volume of the sound while it plays and cannot adjust it otherwise. So with games, you'll constantly be having to play the waiting game for the sound to play, mash the volume down/up button to what you need, and then wait for the sound to play again.
blocking-b2g: --- → tef?
tracking-b2g18: ? → ---
Marco or Andrea are the best people to investigate here.
Talking to our partnership team, this seems to be pretty bad as it affects not only the pre-embedded apps but also any 3rd party app. Andrew, has anyone checked how complex would solving this be?
blocking-b2g: tef? → tef+
Flags: needinfo?(overholt)
(In reply to Daniel Coloma:dcoloma from comment #12)
> Talking to our partnership team, this seems to be pretty bad as it affects
> not only the pre-embedded apps but also any 3rd party app. Andrew, has
> anyone checked how complex would solving this be?

Andrea can probably answer this. Let me ping him in IRC to get an answer here.
Flags: needinfo?(overholt)
Andrea mentioned in IRC he'll look into this either today or tomorrow.
Hi Andrea,

May I know the volumeControlChannel in https://wiki.mozilla.org/WebAPI/AudioChannels is considered to solve this issue? (Bug 855655)

Thanks.
I think this bug can be fixed in gaia. Currently we emit audio-channel-changed with the name of the channel used. If no channel is used, we emit it with 'none'.

Currently the default channel for volume settings is "notification".
First of all, I would like to know why. Probably we should use 'normal'?
Then, maybe we can implement a timer, so that, when there is not audio-channel in use, for 5..10 secs, the volume still works with the old audio-channel.

I still don't like the volumeControlChannel, because any app should set it.. but in 99% of the apps it is 'normal'. Is it? Marco, give me a feedback about this.
Flags: needinfo?(amarchesini) → needinfo?(mchen)
Hi Andrea,

> Currently the default channel for volume settings is "notification".

I remember that this is decided by UX engineer(Casey?). The reason seems to be Firefox OS is targeted to "mobile phone" so the default volume control channel MUST include ringer channel and it is combined with notification. If you have concern on this, maybe you need UX's input.

> Then, maybe we can implement a timer, so that, when there is not audio-channel 
> in use, for 5..10 secs, the volume still works with the old audio-channel.

If poppit starts with no audio playing and normal channel is never used, then user still can't adjust normal volume. 

> I still don't like the volumeControlChannel, because any app should set it.. 
> but in 99% of the apps it is 'normal'. Is it?

I still no comment on using normal for default because you need input from UX not technical developer. 
By the way, I think no matter the default channel is normal or notification/ringer. It can't feet all apps's requirement so the "most flexible" way is chosen by app itself and limit it when app in the foreground.
Flags: needinfo?(mchen)
I would like to read a comment from Casey or some other UX engineer about this.
Casey, the question is: why the default audio channel is notification?
I think that 99% of the case the volume is related to:

1. what is currently playing audio (so the active audio-channel)
2. the current app - 99% of the time it's a normal app.

Thanks for feedback!
Flags: needinfo?(kyee)
Assignee: nobody → amarchesini
Whiteboard: [apps watch list1] → [status: needs patch][apps watch list1]
I had a meeting with Casey yesterday. The UX team is taking a decision based on comment 18. I hope we will have a roadmap asap.
Attached patch patch (obsolete) — Splinter Review
After discussing with Casey, we agreed that the new approach for the volume control should be:

1. if there is an active audio channel, the volume buttons should adjust that.
2. if there is a visible app, the volume buttons should adjust the content/normal audio channel.
3. otherwise (homescreen/lockscreen) the volume buttons adjust the ringer/notification audio channel.

This solution fixes this bug and maybe it's not perfect but it can be a good starting point for a discussion. Probably Casey wants to add few comments.
Attachment #747406 - Flags: review?(alive)
Duplicate of this bug: 866161
set milestone to partner bug
Target Milestone: --- → 1.0.1 Cert2 (28may)
The downsize of Comment 20 proposal is that you can only adjust ringer/notification channels while on home and lock-screens.  All other applications are treated as if they have content audio rather than applying some kind of detection logic to toggle the button behavior of the volume buttons.

There are a few exceptions to this:
- Dialer: Adjusting volume in the dialer should adjust DTMF tone volume
- Active call: Adjusting volume during a active call will adjust the telephony volume,
- Alarm app: Adjusting volume should adjust Alarm volume (content volume will have no effect when you test a alarm sound for example)

Ideally we have some kind of way for applications to register if they make sounds or not and if they are intermittent (GPS navigation directions) or continuous (like music).  This would allow us to better adjust the volume button behavior.
Flags: needinfo?(kyee)
Whiteboard: [status: needs patch][apps watch list1] → [status: needs review][apps watch list1]
Comment on attachment 747406 [details] [diff] [review]
patch

Review of attachment 747406 [details] [diff] [review]:
-----------------------------------------------------------------

Interesting idea, but 
1) I don't want to add this public attribute to Window Manager.
2) Why does an active notification channel also returns content?

::: apps/system/js/sound_manager.js
@@ +134,2 @@
>        case 'notification':
>        case 'ringer':

I am certain you forget to return 'notification' here.
Otherwise if any notification channel is playing, this will make it impossible to adjust it.

::: apps/system/js/window_manager.js
@@ +30,4 @@
>  //    kill(origin, callback): stop specified app
>  //    reload(origin): reload the given app
>  //    getDisplayedApp(): return the origin of the currently displayed app
> +//    isHomescreenDisplayed(): return true if the current displayed app is

Don't do this. 
Listen to home(for home displayed) and appopen(for an app is opened) event in sound manager.
Attachment #747406 - Flags: review?(alive) → review-
Hi Casey & Andrea,

Based on the conclusion here, may I confirm that
  Q: Is the default control channel still normal/content if apps is without any audio tags?

If the answer is yes, is it a strange thing of why can't I control ringer volume in calculator or others without audio's app because I think I am using a "phone".

Or as your opinion, we can consider to let App assign it's default channel by Web API or manifest. On the other way, this change will effect less scenario we already verified and  keep change on these app raised issue only.
> Based on the conclusion here, may I confirm that
>   Q: Is the default control channel still normal/content if apps is without
> any audio tags?

Unfortunately if an app doesn't have audio tags in the dom, it doesn't mean it cannot play.
Audio elements can be created in JS whenever the app wants.

I think it makes sense to change the ringer only when no apps are visible and nothing is playing. Right now, a smart-phone is 90% apps an 10% telephony (my perception)..
Exactly, there's always at least one or two visible apps. Homescreen is also an app.

The problem of "guess what's the channel this will use" is your guess may be wrong.
Let's consider the most strict way to win the game:
Check the audio-channel-* permission (By navigator.mozPermissionSettings) of an app and then make a guess.
You could still make error. 
Dialer is a good example: it has audio-channel-telephony + audio-channel-ringer permission,
but the keypad in dialer is using normale/content channel. You won't know what the first channel it would like to use.

(In reply to Andrea Marchesini (:baku) from comment #26)
> I think it makes sense to change the ringer only when no apps are visible
> and nothing is playing. Right now, a smart-phone is 90% apps an 10%
> telephony (my perception)..
Attached patch patchSplinter Review
Attachment #747406 - Attachment is obsolete: true
Attachment #747845 - Flags: review?(alive)
> Dialer is a good example: it has audio-channel-telephony +
> audio-channel-ringer permission,
> but the keypad in dialer is using normale/content channel. You won't know
> what the first channel it would like to use.

For this we have volumeDefaultChannel. is it? Default should be 'null' or 'normal' and this value as to be in sync with the audio-channel permissions.
Comment on attachment 747845 [details] [diff] [review]
patch

Review of attachment 747845 [details] [diff] [review]:
-----------------------------------------------------------------

r=alive, at least the code is correct now. 
For the UX, let's wait and see what's the effect after applying this.

::: apps/system/js/sound_manager.js
@@ +41,4 @@
>      }
>    });
>  
> +  // True if the homescreen or the lockscreen are visible.

There's nothing done to deal with lockscreen case here.
Update the comment or do something else.

IMO we don't need to deal with lockscreen on case.
Attachment #747845 - Flags: review?(alive) → review+
(In reply to Andrea Marchesini (:baku) from comment #20)
> 
> This solution fixes this bug and maybe it's not perfect but it can be a good
> starting point for a discussion. Probably Casey wants to add few comments.

What about when there is music playing in the background (via the music app, or something similar)?

What if the screen is off?
> What about when there is music playing in the background (via the music app,
> or something similar)?
> What if the screen is off?

If there is music playing, the user adjusts the volume of that audio channel. In this case, content audio channel.
Whiteboard: [status: needs review][apps watch list1] → [status: needs review][apps watch list]
Andrea, seems this can land, and per our irc conversation you believed so as well even though there's more discussion going on about this topic. Let's get this one in, and file followups appropriately for further work.
Whiteboard: [status: needs review][apps watch list] → [status: needs landing][apps watch list]
Can we land this asap? Our partners are going to generate a new build for starting internal testing
Flags: needinfo?(amarchesini)
Yes, we can land it. I'm creating a pull-request.
Flags: needinfo?(amarchesini)
Unfortunately github doesn't allow me to create a pull-request. They have some problem with storage servers. Can someone else create it using the patch in attach?
Flags: needinfo?(alive)
(In reply to Marco Chen [:mchen] from comment #25)
> Hi Casey & Andrea,
> 
> Based on the conclusion here, may I confirm that
>   Q: Is the default control channel still normal/content if apps is without
> any audio tags?
> 
> If the answer is yes, is it a strange thing of why can't I control ringer
> volume in calculator or others without audio's app because I think I am
> using a "phone".

This is correct.  It's not ideal but it is more predictable from a UX perspective than what we currently have in place.

Adjusting volume on home and lock-screen will adjust the volume on ringer/notification.  All other areas it will adjust content.


> Or as your opinion, we can consider to let App assign it's default channel
> by Web API or manifest. On the other way, this change will effect less
> scenario we already verified and  keep change on these app raised issue only.

This would be the ideal mid/long-term solution.
(In reply to Mike Habicher [:mikeh] from comment #31)
> (In reply to Andrea Marchesini (:baku) from comment #20)
> > 
> > This solution fixes this bug and maybe it's not perfect but it can be a good
> > starting point for a discussion. Probably Casey wants to add few comments.
> 
> What about when there is music playing in the background (via the music app,
> or something similar)?

Only FM and Radio apps have the ability to play in the background.   Adjusting the volume controls while these applications are playing in the background will adjust the content volume regardless of where you are at.  Including Home and Lock-screen.

> 
> What if the screen is off?

If FM Radio, Music or Foreground application is actively playing audio, the controls will adjust the volume for content.

If there is no active audio, the buttons should be inactive to any input.
Whiteboard: [status: needs landing][apps watch list] → [status: needs uplift][apps watch list]
(In reply to Andrea Marchesini (:baku) from comment #36)
> Unfortunately github doesn't allow me to create a pull-request. They have
> some problem with storage servers. Can someone else create it using the
> patch in attach?

Seems fabrice did it.
Flags: needinfo?(alive)
Change status per comment 37.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
All, please file another bug if you're questioning the fix in this bug.
(In reply to Alive Kuo [:alive] from comment #42)
> All, please file another bug if you're questioning the fix in this bug.

I agree with Alive. This bug is not meant to be the final solution for the volume adjustment policy. The discussion is still going on, but in the meantime this issue (tef+) is fixed.
Whiteboard: [status: needs uplift][apps watch list] → [status: needs uplift][apps watch list][required_last_cert_round]
Keywords: verifyme
QA Contact: jsmith
Verified on b2g18 and 1.01 builds for 5/20 by playing the game switching around the volume while using it.
Keywords: verifyme
created Bug 875262 - audio volume behavior in lockscreen not implemented per UX in bug 866036
(In reply to Alive Kuo [:alive] from comment #30)

I came across this bug, while tracking down the following issue and this happens only when FTU is launched.

"Vibrate option is not available on volume indicator when volume down button is pressed."


Findings from debugging:
'appopen' event is received with "evt.detail.origin=app://communications.gaiamobile.org/ftu/index.html"
and the 'home' event is never received when FTU is completed and homescreen is displayed.
Therefore, homescreenVisible boolean stays false. It seems this causes the vibrate option disappear. And when phone is rebooted, this doesn't happen. (I believe, 'home' event is received.)


Alive, do we need to ignore 'appopen' event when the origin is FTU? 
Let me know, I'll create PR for it. Thanks

Hanjkim
Flags: needinfo?(alive)
No, don't do that...

at least don't hardcode FTU stuff in sound manager.
Please open another bug on this and let's discuss there.

My 2cent is: let's not fix the special FTU case. Let it be because the user wouldn't always stay at FTU.

I wonder there would be a long hard-coded black-list in the future...I don't want to see that.
Flags: needinfo?(alive) → needinfo?(kyee)
(In reply to Alive Kuo [:alive] from comment #48)

Ok, I confirmed the following works. But anyways.. By the way, the thing is the problem is still there even after the user exits FTU.

  window.addEventListener('appopen', function(evt) {
    if (FtuLauncher.isFtuRunning())
      return;

    homescreenVisible = false;
  });

Let me create a new bug. Thanks
Hanjkim
See Also: → 876340
I'm unclear of what the question is in Comment 48.    

I've commented on 876340.   Definitely something we should fix eventually.
Flags: needinfo?(kyee)
Flagging Alive for further clarification
Flags: needinfo?(alive)
Replied there.
Flags: needinfo?(alive)
You need to log in before you can comment on or make changes to this bug.