[LG] [Sound] - Volume control does not change the volume

RESOLVED FIXED in Firefox OS v2.0

Status

Firefox OS
Gaia
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: rdaub, Assigned: dkuo)

Tracking

(Blocks: 1 bug)

unspecified
2.0 S3 (6june)
ARM
Gonk (Firefox OS)
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(b2g-v2.0 fixed)

Details

(Whiteboard: [p=5])

Attachments

(2 attachments)

269.07 KB, text/x-vhdl
Details
46 bytes, text/x-github-pull-request
alive
: review+
jrburke
: feedback+
wilsonpage
: feedback+
mcav
: feedback+
etienne
: feedback+
salva
: feedback+
millermedeiros
: feedback+
arcturus
: feedback+
arthurcc
: feedback+
Details | Review | Splinter Review
(Reporter)

Description

4 years ago
Hi, 

The volume controls on my LG Fireweb device does not seem to change the volume. The animation of changing the volume shows up correctly, but the actual volume under Settings > Sound > Ringer & Notifications remains unchanged.

STEPS TO REPRODUCE:
1. Go to Settings > Sound and make sure "Ringer & Notifications" is not set at 0.
2. Use the volume knobs to lower the volume all the way to mute.
2. Send a text message to the phone.

EXPECTED RESULTS:
The phone should receive a visual notification on the notification bar, but there should be no sound.


ACTUAL RESULTS:
There is a sound notification.

I am including a logcat to this bug. 

Please let me know if there is any information I could provide to help with the investigation and troubleshooting of this issue.

Thanks!!
(Reporter)

Comment 1

4 years ago
Created attachment 831190 [details]
Volume-control knob does not change the actual volume
(Reporter)

Updated

4 years ago
Component: Gaia → Gaia::Settings
Where are you changing the sound when executing step #2? Are you doing that within the settings app?
(Reporter)

Comment 3

4 years ago
Hi Jason Smith,

I tried both within the Settings app, within the Sound menu, and also on the homescreen.
Flags: needinfo?(ofeng)
Blocks: 991026
See Bug 991026 for the Sound UX spec update.
Flags: needinfo?(ofeng)
(Assignee)

Updated

4 years ago
Target Milestone: --- → 2.0 S2 (23may)
(Assignee)

Updated

4 years ago
Whiteboard: [p=5]
Please see Bug 991026 for the latest UX spec.
(Assignee)

Updated

4 years ago
Assignee: nobody → dkuo
(Assignee)

Updated

3 years ago
Target Milestone: 2.0 S2 (23may) → 2.0 S3 (6june)
(Assignee)

Comment 6

3 years ago
Created attachment 8430668 [details] [review]
patch
(Assignee)

Updated

3 years ago
Component: Gaia::Settings → Gaia
(Assignee)

Comment 7

3 years ago
Comment on attachment 8430668 [details] [review]
patch

Hi owners,

The sound ux spec(attachment 8420872 [details], page 6) in bug 991026 describes which volume we should control in the apps and how the sound toast should be displayed while the users pressing the volumedown/volumeup buttons. To fit the ux requirements, we have to call the AudioChannelManager api to tell gecko that, our apps want some volume to be adjustable when they are in the foreground.

And because the default audio channel type for the web apps is "normal/content", so the default volume is |Media|, for the 3rd party apps they don't to do anything then they will be associated to the |Media| volume. But our core apps should control the |Ringer & Notifications| volume, except the media apps and clock app, so the patch modified the apps with |Ringer & Notifications| volume, plus the clock app with |Alarm| volume. Note that some core apps(like Usage and Calendar) don't really produce sounds, but still we call the AudioChannelManager api to associate the volume, hope you can understand.

I simply added the AudioChannelManager api in where the apps get started, or the code block that related to the sounds thing, and everyone please let me know if I am doing right or I should put it in another place, thanks!(I am working on the integration tests in system app, will request review later)

(If anyone does not know about audio channels, please read: https://wiki.mozilla.org/WebAPI/AudioChannels)
Attachment #8430668 - Flags: feedback?(salva)
Attachment #8430668 - Flags: feedback?(m)
Attachment #8430668 - Flags: feedback?(jrburke)
Attachment #8430668 - Flags: feedback?(jlal)
Attachment #8430668 - Flags: feedback?(francisco)
Attachment #8430668 - Flags: feedback?(etienne)
Attachment #8430668 - Flags: feedback?(dflanagan)
Attachment #8430668 - Flags: feedback?(arthur.chen)
Attachment #8430668 - Flags: feedback?(m) → feedback+

Updated

3 years ago
Attachment #8430668 - Flags: feedback?(jrburke) → feedback+
Comment on attachment 8430668 [details] [review]
patch

Please check my comment in github.
Attachment #8430668 - Flags: feedback?(arthur.chen)
Comment on attachment 8430668 [details] [review]
patch

Just clarification, is 'notification' the same as |Ringer & Notifications|? If so, no problem from my part.
Attachment #8430668 - Flags: feedback?(salva) → feedback+
Flags: needinfo?(dkuo)
(Assignee)

Comment 10

3 years ago
(In reply to Salvador de la Puente González [:salva] from comment #9)
> Comment on attachment 8430668 [details] [review]
> WIP
> 
> Just clarification, is 'notification' the same as |Ringer & Notifications|?
> If so, no problem from my part.

Yes, Ringer & Notifications share the same volume and you are correct :)
Flags: needinfo?(dkuo)
(Assignee)

Comment 11

3 years ago
Comment on attachment 8430668 [details] [review]
patch

Arthur, I have addressed the issue on github and please take a look again, thanks!
Attachment #8430668 - Flags: feedback?(arthur.chen)
Attachment #8430668 - Flags: feedback?(arthur.chen) → feedback+
(Assignee)

Comment 12

3 years ago
Comment on attachment 8430668 [details] [review]
patch

Looks like James is busy on some other things, Miller, would you please take a look on the Calendar changes that I made? thanks(Please read comment 7, it should help you to understand this bug).
Attachment #8430668 - Flags: feedback?(jlal) → feedback?(mmedeiros)
For Calendar app, the changes look good to me.
Attachment #8430668 - Flags: feedback?(francisco) → feedback+
Attachment #8430668 - Flags: feedback?(mmedeiros) → feedback+
Do we need a |navigator.mozAudioChannelManager.volumeControlChannel = 'telephony';| in the callscreen app ?
Flags: needinfo?(dkuo)
Comment on attachment 8430668 [details] [review]
patch

r+ for the dialer part
Attachment #8430668 - Flags: feedback?(etienne) → feedback+
(Assignee)

Comment 16

3 years ago
(In reply to Etienne Segonzac (:etienne) from comment #14)
> Do we need a |navigator.mozAudioChannelManager.volumeControlChannel =
> 'telephony';| in the callscreen app ?

I think probably not because ux wants the Telephony volume to be adjustable in call. Before the user answered the call, we still let the user control the Ringer & Notifications volume, after answered it, the Telephony channel should be active and so does the volume, the sound manager in system app will handle it so callscreen app does not need to call volumeControlChannel = 'telephony' to notify the mozAudioChannelManager.

(In reply to Etienne Segonzac (:etienne) from comment #15)
> Comment on attachment 8430668 [details] [review]
> WIP
> 
> r+ for the dialer part

Thanks Etienne!
Flags: needinfo?(dkuo)
(Assignee)

Comment 17

3 years ago
Comment on attachment 8430668 [details] [review]
patch

Looks like djf is busy, Wilson, would you please read comment 7 and see if I am modifying the Camera app correctly? thanks.
Attachment #8430668 - Flags: feedback?(dflanagan) → feedback?(wilsonpage)
Comment on attachment 8430668 [details] [review]
patch

Thanks Dominic! Just a small nit (see Github) :)
Attachment #8430668 - Flags: feedback?(wilsonpage) → feedback+
(In reply to Dominic Kuo [:dkuo] from comment #16)
> (In reply to Etienne Segonzac (:etienne) from comment #14)
> > Do we need a |navigator.mozAudioChannelManager.volumeControlChannel =
> > 'telephony';| in the callscreen app ?
> 
> I think probably not because ux wants the Telephony volume to be adjustable
> in call. Before the user answered the call, we still let the user control
> the Ringer & Notifications volume, after answered it, the Telephony channel
> should be active and so does the volume, the sound manager in system app
> will handle it so callscreen app does not need to call volumeControlChannel
> = 'telephony' to notify the mozAudioChannelManager.

Makes sense, thanks.
(Assignee)

Comment 20

3 years ago
Comment on attachment 8430668 [details] [review]
patch

Alive,

This is the patch that I fixed base on the ux spec(Please read comment 7), and with integration tests on sound manager in system app, would you please review this? thanks.
Attachment #8430668 - Attachment description: WIP → patch
Attachment #8430668 - Flags: review?(alive)
Comment on attachment 8430668 [details] [review]
patch

test+++++
Attachment #8430668 - Flags: review?(alive) → review+
(Assignee)

Comment 22

3 years ago
Thanks everyone!

Landed on master: 6dac2429c49536d372d9cf5be0ac86356656ca46
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-b2g-v2.0: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.