Currently gaia UI has 10 steps as default for adjusting volume (under all scenarios) However, under different scenarios, the audio volume steps are actually different base on the underlying system's capability Below are different volume steps capability, the UI should sync to the system capability 5 steps, // STREAM_VOICE_CALL <-- can be adjusted during voice call 7 steps, // STREAM_SYSTEM <-- I think it can sync with ringer 7 steps, // STREAM_RING <-- ringer volume, can be adjusted by vol-key on home screen 15 steps, // STREAM_MUSIC <-- music playback volume, can be adjusted by vol-key when music is playing 7 steps, // STREAM_ALARM <-- alarm sound volume, can be changed on setting 7 steps, // STREAM_NOTIFICATION <-- sync with ringer 15 steps, // STREAM_BLUETOOTH_SCO <-- voice call on and can be adjusted by volume key or bt handset-free. 7 steps, // STREAM_SYSTEM_ENFORCED <-- force to output camera shutter sound 15 steps, // STREAM_DTMF <-- this tone volume can be adjust with ring 15 steps, // STREAM_TTS X no use 15 steps, // STREAM FM <-- I think this can sync with music.

setting can get max volume step by audioManager.getMaxStreamVolumeIndex API and those values are defined in AudioManager.cpp

gaia can't access the audioManager object, maybe we need new api for sound_manager to query the default maximum value.

(In reply to Randy Lin [:rlin] from comment #2) > gaia can't access the audioManager object, maybe we need new api for > sound_manager to query the default maximum value. Can you file a bug for that and link to this bug and bug 810780? Thanks. Audio scope is getting bigger and bigger. :/

From mail discussed before, settings app can group those volumes into. - content volume (music/fm streams) - Ringer volume (ringer and notification and system streams) - Alarm volume (alarm stream) The voice /bt sco volume can be adjusted by hw volume key during the voice call. The maximal volume should be queried by this Bug 815024

Dear Alive, After discuss with Jonas and we think it's better to hard-code the maximal value in the setting/system apps first at this stage.

suggest group into 5 types gaia setting strings (maximal volume step) ->control those stream volume ======================================================================== audio.volume.content (15) ->control the SYSTEM, MUSIC, FM stream volume audio.volume.notification (7) ->control the RING, NOTIFICATION audio.volume.alarm (7)-> ALARM audio.volume.telephony (5)->VOICE_CALL audio.volume.bt_sco-(15)>BT_SCO

enlarge the volume step to 15 and change the naming of volume setting

suggestion change:

Miss press the save changes button, please ignore upper comment.

Randy: Please test your gecko with https://github.com/mozilla-b2g/gaia/pull/6682 Tell me if any problem, thanks!

(In reply to Randy Lin [:rlin] from comment #11) > Created attachment 686402 [details] [diff] [review] > gecko part patch 1 try server https://tbpl.mozilla.org/?tree=Try&rev=5aca8af1beaa

Comment on attachment 686402 [details] [diff] [review] gecko part patch 1 Review of attachment 686402 [details] [diff] [review]: ----------------------------------------------------------------- The code looks good, but I want to know why we don't use the same values as android. ::: b2g/chrome/content/settings.js @@ +83,5 @@ > let audioManager = Services.audioManager; > if (!audioManager) > return; > > + for each(let st in t) { I know this is not your fault, but can you use more descriptive variable names? ::: dom/system/gonk/AudioManager.cpp @@ +36,5 @@ > > // Refer AudioService.java from Android > static int sMaxStreamVolumeTbl[AUDIO_STREAM_CNT] = { > + 5, // voice call > + 15, // system this is different from http://androidxref.com/4.0.4/xref/frameworks/base/media/java/android/media/AudioService.java#178 why?

(In reply to Fabrice Desré [:fabrice] from comment #13) > Comment on attachment 686402 [details] [diff] [review] > gecko part patch 1 > > Review of attachment 686402 [details] [diff] [review]: > ----------------------------------------------------------------- > > The code looks good, but I want to know why we don't use the same values as > android. > > ::: b2g/chrome/content/settings.js > @@ +83,5 @@ > > let audioManager = Services.audioManager; > > if (!audioManager) > > return; > > > > + for each(let st in t) { > > I know this is not your fault, but can you use more descriptive variable > names? OK, I will change it. > > ::: dom/system/gonk/AudioManager.cpp > @@ +36,5 @@ > > > > // Refer AudioService.java from Android > > static int sMaxStreamVolumeTbl[AUDIO_STREAM_CNT] = { > > + 5, // voice call > > + 15, // system > > this is different from > http://androidxref.com/4.0.4/xref/frameworks/base/media/java/android/media/ > AudioService.java#178 > > why? We want to group the system sound into content, so we should align those maximal value to the same.

*Change the naming of variable and voice maximal volume index, *Another change has increased the maximal index of System /FM to 15. https://hg.mozilla.org/integration/mozilla-inbound/rev/d2a0cc53acdc The reason of system volume sync with content volume is avoid sound shack issue. (low content volume mix with high volume of system sound)

I don't understand comment 0 here. All channels except for telephony and fmradio are ones that are played by gecko and could be mixed by gecko. So I don't understand why we couldn't have an arbitrary number of steps in the volume of those channels. Randy mention that we use audioflinger to mix those channels. I'm not sure I understand why we're not mixing them in gecko, but that's outside my area of expertise. But I was told that audioflinger has 15 volume settings. Is that not true?

Randy will be a better person to explain comment 0. posted the bug for Randy for tracking purpose.

this is related to the latest updated sound spec from lco. adding lco

The magic number 15 comes from AudioService.java. android phone used it as music/fm volume step. We can also modify it in gonk:AudioManager.cpp:sMaxStreamVolumeTbl.

(In reply to Randy Lin [:rlin] from comment #19) > The magic number 15 comes from AudioService.java. android phone used it as > music/fm volume step. > We can also modify it in gonk:AudioManager.cpp:sMaxStreamVolumeTbl. Don't we need to make sure that this table is kept in sync with the one in settings.js ?

(In reply to Fabrice Desré [:fabrice] from comment #20) > (In reply to Randy Lin [:rlin] from comment #19) > > The magic number 15 comes from AudioService.java. android phone used it as > > music/fm volume step. > > We can also modify it in gonk:AudioManager.cpp:sMaxStreamVolumeTbl. > > Don't we need to make sure that this table is kept in sync with the one in > settings.js ? refer to https://hg.mozilla.org/mozilla-central/file/1b7103181091/dom/system/gonk/AudioManager.cpp 38 // Refer AudioService.java from Android 39 static int sMaxStreamVolumeTbl[AUDIO_STREAM_CNT] = { 40 10, // voice call 41 15, // system 42 7, // ring 43 15, // music 44 7, // alarm 45 7, // notification 46 15, // BT SCO 47 7, // enforced audible 48 15, // DTMF 49 15, // TTS 50 15, // FM 51 }; This patch would change voice call to 5, others are sync to settings.js

Fix conflict.

gaia part would land after attachment 687091 [details] [diff] [review] land.

lco 's proposed settings app change is here: http://people.mozilla.com/~lco/FFOS/Sound/R1_Sound%20Spec_v1%20DRAFT.pdf But for v1 c2, 1. I won't add ringtone selection in this bug because bug 807431 is covering that. 2. The content volume adjust will fail until 811222 and 810780 lands

follow by sicking suggestion.

Comment on attachment 688192 [details] [diff] [review] correct volume step Review of attachment 688192 [details] [diff] [review]: ----------------------------------------------------------------- Sweet! Thanks!

final check-in patch

This patch does: 1. Try to align the spec as much as possible. 2. Resolve a C2 bug 815705 3. Set the segments of all channels except voice to 15. This patch does not: 1. Remove the content slider. Because this is based on bug 810780 and bug 811222's work and if I removed it, regression will come. 2. Add final ringtones. Leave it to bug 807431 The remaining volume work will be done in bug 810780, another c2 bug if 811222 landed.

Set late-l10n and reviewer to stas for l10n touch. Reset r? to timdream coz seems kaze doesn't have time to review :)

Sorry we still have relevant gaia patch reviewing :(

Comment on attachment 688206 [details] PR 6682 See Github, please re-request for review after explain to me the desired behavior. Also, please file separate bugs for Gaia and Gecko fixes next time. :) Good job; thanks for cleaning up style sheets.

since it blocks C2 bugs, change to P2 & C2 to be consistent

Comment on attachment 688206 [details] PR 6682 r=me, thanks. Great work!

I propose to land this ASAP because of gecko has landed, so I am going to remove the l10n change in this patch, open another bug to track it.

Comment on attachment 688206 [details] PR 6682 There seems to be something for me to review, I guess, but not this?

Already landed https://github.com/mozilla-b2g/gaia/commit/bffc1fa13724d9cd37f9fcc44f90839d0e944eb9

https://hg.mozilla.org/releases/mozilla-aurora/rev/0978db919b27 https://hg.mozilla.org/releases/mozilla-beta/rev/b675946f1fd3