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

Status: RESOLVED FIXED

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

Description

[sync-1]

+++ 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:

Comment 1

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Can you try to adjust volume while audio is playing?

Comment 2

[Lisa Brewster [:adora]]

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).

Comment 3

[Jason Smith [:jsmith]]

(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.

Comment 4

[buri.blff]

(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?

Comment 5

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

The current behavior is similar as your description.

Comment 6

[Simon Callan, Telefonica]

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.

Comment 7

[Jason Smith [:jsmith]]

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?

Comment 8

[Jason Smith [:jsmith]]

Actually, I see that Andrea is working on some of these audio channel bugs. 

Andrea - Any thoughts on comment 7?

Comment 9

[Jason Smith [:jsmith]]

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.

Comment 10

[Andrew Overholt [:overholt]]

Marco or Andrea are the best people to investigate here.

Comment 11

[Simon Callan, Telefonica]

Attachment: Pressing volume button is random adjustments

Comment 12

[Daniel Coloma:dcoloma]

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?

Comment 13

[Jason Smith [:jsmith]]

(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.

Comment 14

[Jason Smith [:jsmith]]

Andrea mentioned in IRC he'll look into this either today or tomorrow.

Comment 15

[Marco Chen [:mchen]]

Hi Andrea,

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

Thanks.

Comment 16

[Andrea Marchesini [:baku]]

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.

Comment 17

[Marco Chen [: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.

Comment 18

[Andrea Marchesini [:baku]]

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!

Comment 19

[Andrea Marchesini [:baku]]

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.

Comment 20

[Andrea Marchesini [:baku]]

Attachment: patch

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.

Comment 22

[Joe Cheng [:jcheng] (please needinfo)]

set milestone to partner bug

Comment 23

[Casey Yee]

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.

Comment 24

[(inactive after 6/18) Alive Kuo [:alive]]

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.

Comment 25

[Marco Chen [:mchen]]

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.

Comment 26

[Andrea Marchesini [:baku]]

> 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)..

Comment 27

[(inactive after 6/18) Alive Kuo [:alive]]

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)..

Comment 28

[Andrea Marchesini [:baku]]

Attachment: patch

Comment 29

[Andrea Marchesini [:baku]]

> 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 30

[(inactive after 6/18) Alive Kuo [:alive]]

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.

Comment 31

[Mike Habicher [:mikeh] (high bugzilla latency)]

(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?

Comment 32

[Andrea Marchesini [:baku]]

> 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.

Comment 33

[Johnny Stenback (:jst)]

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.

Comment 34

[Daniel Coloma:dcoloma]

Can we land this asap? Our partners are going to generate a new build for starting internal testing

Comment 35

[Andrea Marchesini [:baku]]

Yes, we can land it. I'm creating a pull-request.

Comment 36

[Andrea Marchesini [:baku]]

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?

Comment 37

[[:fabrice] Fabrice Desré]

Pushed to master:
https://github.com/mozilla-b2g/gaia/commit/7c87f0b247f04996c7173a22be249f9304e2ad9c

Comment 38

[Casey Yee]

(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.

Comment 39

[Casey Yee]

(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.

Comment 40

[(inactive after 6/18) Alive Kuo [:alive]]

(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.

Comment 41

[(inactive after 6/18) Alive Kuo [:alive]]

Change status per comment 37.

Comment 42

[(inactive after 6/18) Alive Kuo [:alive]]

All, please file another bug if you're questioning the fix in this bug.

Comment 43

[Andrea Marchesini [:baku]]

(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.

Comment 44

[John Ford [:jhford] CET/CEST Berlin Time]

[v1-train 7b40365]
[v1.0.1 2d3c0dd]

Comment 45

[Jason Smith [:jsmith]]

Verified on b2g18 and 1.01 builds for 5/20 by playing the game switching around the volume while using it.

Comment 46

[Joe Cheng [:jcheng] (please needinfo)]

created Bug 875262 - audio volume behavior in lockscreen not implemented per UX in bug 866036

Comment 47

[Leo]

(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

Comment 48

[(inactive after 6/18) Alive Kuo [: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.

Comment 49

[Leo]

(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

Comment 50

[Casey Yee]

I'm unclear of what the question is in Comment 48.    

I've commented on 876340.   Definitely something we should fix eventually.

Comment 51

[Casey Yee]

Flagging Alive for further clarification

Comment 52

[(inactive after 6/18) Alive Kuo [:alive]]

Replied there.