1170117 - [B2G] Separate volume control settings for multiple audio profiles

Status: RESOLVED FIXED

(Firefox OS Graveyard :: AudioChannel, defect)

Description

[Alastor Wu [:alwu]]

FxOS should remember the user listening habit of using different devices. Therefore, we need to support the separate volume control setting for different output devices. It would bring the comfortable sound volume to users when they switch devices.

Example,
I always listen music with 5 volume level on the speaker, using 8 volume level on the headset.
The volume level should be automatically changed by this feature when I switch the different devices.

Comment 1

[Alastor Wu [:alwu]]

Attachment: Separate volume control

Here is the preliminary patch with complete functionality.

Comment 2

[Alastor Wu [:alwu]]

Attachment: Separate volume control

In this patch, we could change three different profiles - "primary", "headset" and "bluetooth".

Comment 3

[Alastor Wu [:alwu]]

Try-server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=62873ddbbd61

Comment 4

[Alastor Wu [:alwu]]

Attachment: Separate volume control

New patch and try-server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a404129633da

Comment 5

[Alastor Wu [:alwu]]

Because of the fails in the try-server, I'm still revising it.

Comment 6

[Alastor Wu [:alwu]]

Attachment: Separate volume control

New patch and try-server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3cfd480fc0d

Comment 7

[Alastor Wu [:alwu]]

Attachment: Separate volume control

Comment 8

[Alastor Wu [:alwu]]

Comment on attachment 8616134 [details] [diff] [review]
Separate volume control

Hi, Baku,
Could you help me review this patch?
Very appreciate :)

[Description]
This patch is about the new feature that the FxOS can adjust the volume setting according to different output profiles. 

For example, if the users want the louder sound on the speaker, but the lower sound on the headset. They might need the different volume setting on these output device.

However, we have the same volume setting on all devices. Users need to adjust the volume everytime when they change the devices. It's very inconvenience. I believe this feature can bring a better experience when users use the FxOS :)

[Volume setting modes]
1) Primary (output devices on the phone)
2) Wired device (headset/headphone)
3) Bluetooth device (SCO/A2DP)

[Notice]
In Android design, when the wired device and bluetooth device are both connected, we would choose the lastest connected one as output devices. It's the same as the output volume mode.

Example, 
Connects BT first, then connects wired headset, the volume setting mode would be "wired device".

Comment 9

[Andrea Marchesini [:baku]]

Comment on attachment 8616134 [details] [diff] [review]
Separate volume control

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

I would like to read this patch again when these comments are applied.

::: dom/system/gonk/AudioManager.cpp
@@ +94,5 @@
>  namespace mozilla {
>  namespace dom {
>  namespace gonk {
> +
> +class AudioProfileData

final

@@ +97,5 @@
> +
> +class AudioProfileData
> +{
> +public:
> +  AudioProfileData(AudioOutputProfiles aProfile)

explicit

@@ +104,5 @@
> +  {
> +    mVolumeTable.SetLength(VOLUME_TOTAL_NUMBER);
> +  };
> +
> +  AudioOutputProfiles GetProfile()

const

@@ +114,5 @@
> +  {
> +    mActive = aActive;
> +  }
> +
> +  bool GetActive()

const

@@ +121,5 @@
> +  }
> +
> +  nsTArray<int> mVolumeTable;
> +private:
> +  AudioOutputProfiles mProfile;

const

@@ +179,5 @@
>  {
>  public:
>    NS_DECL_ISUPPORTS
>  
> +  AudioChannelVolInitCallback(AudioManager* aAudioManager) :

explicit

@@ +180,5 @@
>  public:
>    NS_DECL_ISUPPORTS
>  
> +  AudioChannelVolInitCallback(AudioManager* aAudioManager) :
> +    mAudioManager(aAudioManager) {}

{} in a new line.

@@ +223,5 @@
>    }
>  
>  protected:
>    ~AudioChannelVolInitCallback() {}
> +  AudioManager* mAudioManager;

Who keeps this alive? write a comment please.

@@ +958,5 @@
> +
> +void
> +AudioManager::CreateAudioProfilesData()
> +{
> +  mAudioProfiles.SetLength(0);

this should not be needed.
MOZ_ASSERT(mAudioProfiles.IsEmpty());

@@ +959,5 @@
> +void
> +AudioManager::CreateAudioProfilesData()
> +{
> +  mAudioProfiles.SetLength(0);
> +  MOZ_ASSERT(mAudioProfiles.Length() == 0, "mAudioProfiles should be empty!");

IsEmpty()

@@ +961,5 @@
> +{
> +  mAudioProfiles.SetLength(0);
> +  MOZ_ASSERT(mAudioProfiles.Length() == 0, "mAudioProfiles should be empty!");
> +  for (int idx = 0; idx < DEVICE_TOTAL_NUMBER; idx++) {
> +    AudioProfileData* profile = new AudioProfileData(static_cast<AudioOutputProfiles>(idx));

what you want here is mAudioProfiles as array of nsAutoPtr<AudioProfileData>

@@ +969,5 @@
> +  UpdateProfileState(DEVICE_PRIMARY, true);
> +}
> +
> +void
> +AudioManager::InitProfilesVolume(AudioVolumeCategories aCatogory, int32_t aIndex)

Category

@@ +977,5 @@
> +  }
> +  int profilesNum = mAudioProfiles.Length();
> +  MOZ_ASSERT(profilesNum == DEVICE_TOTAL_NUMBER, "Error profile numbers!");
> +  for (int idx = 0; idx < profilesNum; idx++) {
> +    mAudioProfiles[idx]->mVolumeTable[aCatogory] = aIndex;

Category

@@ +985,5 @@
> +void
> +AudioManager::SwitchProfileData(AudioOutputProfiles aProfile, bool aActive)
> +{
> +  if (!mIsInitProfiles) {
> +    return;

no else after a return.
Then... the profiles are created in the constructor. How is it possible that they are not initialized?

@@ +986,5 @@
> +AudioManager::SwitchProfileData(AudioOutputProfiles aProfile, bool aActive)
> +{
> +  if (!mIsInitProfiles) {
> +    return;
> +  } else if (aProfile < DEVICE_PRIMARY && DEVICE_TOTAL_NUMBER <= aProfile) {

convert this in a MOZ_ASSERT

@@ +1023,5 @@
> +void
> +AudioManager::UpdateProfileState(AudioOutputProfiles aProfile, bool aActive)
> +{
> +  if (!mIsInitProfiles) {
> +    return;

same here.

@@ +1027,5 @@
> +    return;
> +  } else if (aProfile == DEVICE_PRIMARY && !aActive) {
> +    NS_WARNING("Can't turn off the primary profile!");
> +    return;
> +  } else if (aProfile < DEVICE_PRIMARY && DEVICE_TOTAL_NUMBER <= aProfile) {

same here.

::: dom/system/gonk/AudioManager.h
@@ +71,3 @@
>  class RecoverTask;
>  class AudioChannelVolInitCallback;
> +class AudioProfileData;

put a new line here.

Comment 10

[Alastor Wu [:alwu]]

Attachment: Separate volume control

Thanks! Here is my revised patch, and wait for the try-server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ba53435a245

Comment 11

[Alastor Wu [:alwu]]

Comment on attachment 8623016 [details] [diff] [review]
Separate volume control

Hi, Baku,
Here is my revised patch, very appreciate!

Comment 12

[Andrea Marchesini [:baku]]

Comment on attachment 8623016 [details] [diff] [review]
Separate volume control

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

r- just because I think we can make this code more readable.

::: dom/system/gonk/AudioManager.cpp
@@ +101,5 @@
> +  explicit AudioProfileData(AudioOutputProfiles aProfile)
> +    : mProfile(aProfile)
> +    , mActive(false)
> +  {
> +    mVolumeTable.SetLength(VOLUME_TOTAL_NUMBER);

have you checked that mVolumeTable contains 0 after this line?
Double check this, please.

@@ +104,5 @@
> +  {
> +    mVolumeTable.SetLength(VOLUME_TOTAL_NUMBER);
> +  };
> +
> +  const AudioOutputProfiles GetProfile()

I meant:

AudioOutputProfiles GetProfile() const

@@ +114,5 @@
> +  {
> +    mActive = aActive;
> +  }
> +
> +  const bool GetActive()

same here.

@@ +191,1 @@
>      int32_t volIndex = aResult.toInt32();

hold on... this volIndex, is not a index, correct? It's a volume level.
Plus: do we have a range? Or any possible volIndex is ok?

@@ +211,2 @@
>      } else {
>        MOZ_ASSERT_UNREACHABLE("unexpected audio channel for initializing "

Maybe this is a bit extreme... can you just add a NS_WARN_IF ?

@@ +927,5 @@
>  }
> +
> +void
> +AudioManager::SendVolumeChangeNotification(AudioProfileData* aProfileData)
> +{

MOZ_ASSERT(aProfileData);

@@ +942,5 @@
> +  }
> +
> +  // Send events to update the Gaia volume
> +  mozilla::AutoSafeJSContext cx;
> +  JS::Rooted<JS::Value> value(cx, JS::Int32Value(aProfileData->mVolumeTable[VOLUME_MEDIA]));

I see you have a list of values-strings twice in this code. I guess we should improve this code.

What about if we do this:

struct VolumeNameToValue /* choose a better name please */ {
  const char* mName;
  uint32_t mValue; // or index...
} namesAndValues[] = {
  { "audio.volume.content", VOLUME_MEDIA },
  { "audio.volume.alarm", VOLUME_ALARM },
  ...,
  { nullptr, 0 }
};

And then here you do:

for (uint32_t i = 0; namesAndValues[i].mName; ++i) {
  value.setInt32(aProfileData->mVolumeTable[namesAndValues[i].mValue]);
  lock->Set(namesAndValues[i].mName, value, nullptr, nullptr);
}

and the same at line 192 where you have: if (name.Equals(...))..

@@ +966,5 @@
> +  UpdateProfileState(DEVICE_PRIMARY, true);
> +}
> +
> +void
> +AudioManager::InitProfilesVolume(AudioVolumeCategories aCategory, int32_t aIndex)

80chars

@@ +971,5 @@
> +{
> +  int profilesNum = mAudioProfiles.Length();
> +  MOZ_ASSERT(profilesNum == DEVICE_TOTAL_NUMBER, "Error profile numbers!");
> +  for (int idx = 0; idx < profilesNum; idx++) {
> +    mAudioProfiles[idx]->mVolumeTable[aCategory] = aIndex;

are we sure you are setting a index here?
can we check the range of this value?

@@ +976,5 @@
> +  }
> +}
> +
> +void
> +AudioManager::SwitchProfileData(AudioOutputProfiles aProfile, bool aActive)

80chars

@@ +978,5 @@
> +
> +void
> +AudioManager::SwitchProfileData(AudioOutputProfiles aProfile, bool aActive)
> +{
> +  MOZ_ASSERT(DEVICE_PRIMARY <= aProfile && aProfile < DEVICE_TOTAL_NUMBER,

here too.

@@ +987,5 @@
> +  int profilesNum = mAudioProfiles.Length();
> +  MOZ_ASSERT(profilesNum == DEVICE_TOTAL_NUMBER, "Error profile numbers!");
> +  for (int idx = 0; idx < profilesNum; idx++) {
> +    AudioProfileData* profileData = mAudioProfiles[idx];
> +    if (profileData->GetProfile() == oldProfile) {

you do this forloop in several places in your code. Can you do an helper?
Something like:

AudioProfileData* FindAudioProfileData(AudioOuputProfiles aOldProfile);

then you can use it everywhere just simply doing:

AudioProfileData* profileData = FindAudioProfileData(mOldProfile);
MOZ_ASSERT(profileData);

@@ +1040,5 @@
> +
> +void
> +AudioManager::UpdateVolumeToProfile(AudioProfileData* aProfileData)
> +{
> +  aProfileData->mVolumeTable[VOLUME_MEDIA] = mCurrentStreamVolumeTbl[AUDIO_STREAM_MUSIC];

Maybe also here you can use the nameAndValues data.

@@ +1050,5 @@
> +
> +void
> +AudioManager::UpdateVolumeFromProfile(AudioProfileData* aProfileData)
> +{
> +  SetStreamVolumeIndex(AUDIO_STREAM_MUSIC, aProfileData->mVolumeTable[VOLUME_MEDIA]);

same here.

Comment 13

[Alastor Wu [:alwu]]

(In reply to Andrea Marchesini (:baku) from comment #12)
> r- just because I think we can make this code more readable.
Thanks! I'll modify the code!

> @@ +191,1 @@
> hold on... this volIndex, is not a index, correct? It's a volume level.
> Plus: do we have a range? Or any possible volIndex is ok?

Yes, the index means the volume level, it's the Android naming.
Ex. AudioSystem::setStreamVolumeIndex()

We'll check the index range in AudioManager::SetStreamVolumeIndex(). It should be large than zero, and smaller than sMaxStreamVolumeTbl[aStream_type].

> @@ +942,5 @@
> I see you have a list of values-strings twice in this code. I guess we
> should improve this code.
> 
> What about if we do this:
> 
> struct VolumeNameToValue /* choose a better name please */ {
>   const char* mName;
>   uint32_t mValue; // or index...
> } namesAndValues[] = {
>   { "audio.volume.content", VOLUME_MEDIA },
>   { "audio.volume.alarm", VOLUME_ALARM },
>   ...,
>   { nullptr, 0 }
> };
> 
> And then here you do:
> 
> for (uint32_t i = 0; namesAndValues[i].mName; ++i) {
>   value.setInt32(aProfileData->mVolumeTable[namesAndValues[i].mValue]);
>   lock->Set(namesAndValues[i].mName, value, nullptr, nullptr);
> }
> 
> and the same at line 192 where you have: if (name.Equals(...))..

You're right, It can make the code more readable!

> @@ +971,5 @@
> are we sure you are setting a index here?
> can we check the range of this value?

Because we'll check the index in AudioManager::SetStreamVolumeIndex(), I think we have no need to check its value here?

If the value is wrong here, that means we got the error index value from the settings service.

Comment 14

[Alastor Wu [:alwu]]

(In reply to Andrea Marchesini (:baku) from comment #12)
> @@ +211,2 @@
> >      } else {
> >        MOZ_ASSERT_UNREACHABLE("unexpected audio channel for initializing "
> 
> Maybe this is a bit extreme... can you just add a NS_WARN_IF ?

Could I use the NS_WARNING here?

Comment 15

[Andrea Marchesini [:baku]]

> We'll check the index range in AudioManager::SetStreamVolumeIndex(). It
> should be large than zero, and smaller than
> sMaxStreamVolumeTbl[aStream_type].

Can we have a macro or something to validate it when the input is received instead propagate it into the chain of methods?

Comment 16

[Andrea Marchesini [:baku]]

> Could I use the NS_WARNING here?

Sure!

Comment 17

[Alastor Wu [:alwu]]

(In reply to Andrea Marchesini (:baku) from comment #15)
> Can we have a macro or something to validate it when the input is received
> instead propagate it into the chain of methods?

OK!

Comment 18

[Marco Chen [:mchen]]

Hi Alastor,

Thanks for improving feature of audio control continuously.

And one question regarding to your patch the what behavior you need.
Q: Please consider  this scenario 
  1. user sets volume of content type to 10 while output device is speaker.
  2. user inserts the headset then adjust volume of content type to 5.
  3. after rebooting the device, user may expect that a. volume would be 5 if user keeps the headset there. b. volume would be 10 if user pull the headset out.

  The result of the patch now: No matter the what output device is, the volume will be always 5 even output device is speaker.

Is this really what you need?

Comment 19

[Marco Chen [:mchen]]

And other point is about the implementation in AudioManager::SetStreamVolumeIndex.

For Android version is not small then 17, we need to specify the output device type when calling AudioSystem::setStreamVolumeIndex(...). It means the audio system from gonk already supported different volume index of each output devices.

Recall the old implementation, we set one volume index to all output devices because we don't plan to support what you want to do now. So maybe you can set volume index to a specified output device only not all of them now.

Comment 20

[Alastor Wu [:alwu]]

Hi, Marco,
Very appreciate to remind these points. 
I didn't consider the rebooting condition, I'll modify it.
It sounds well that to set the volume on specific deivce instead of all devices, I'll also modify this function.
Thanks :)

Comment 21

[Alastor Wu [:alwu]]

Attachment: [WIP] Separate volume control

This is the WIP patch, it still need to modify.
I think that I can upload the complete patch tomorrow and describe more details.

Comment 22

[Alastor Wu [:alwu]]

In the latest patch, 

I remove the volume control in the AudioChannelService, and move them into AudioManager. That is because actually we change the volume value by AudioVolumeCategories(Media/Notification/Alarm/...) instead of audio channel types. I think moving them to AudioManager is more reasonable.

However, I keep the GetAudioChannelVolume related functions in AudioManager. Because these function can be used when someday we want to implement the new audio channel volume control API [1].

[1] https://gist.github.com/evanxd/41d8e2d91c5201a42bfa
---

The problem in comment18 can't be solved by only Gecko part, we also need to modify Gaia.
See bug1179181.

Comment 23

[Alastor Wu [:alwu]]

Attachment: Separate volume control

Hi, Baku,
Here is my revised patch and there are some important changing,
(1) Move all volume control codes to AudioManager
(2) Volume value can be controlled by volume categories or audio channel types
(3) The volume control functions by audio channel type will be used for new volume control API [1].

The problem mentioned in comment18 will be solved in Bug1179181. Because the problem will only happen on switching devices after shutdown the phone, this isn't very common scenario. I think that we can land this patch first, and then land the Bug1179181.

How do you think?
Thanks :)

[1] https://gist.github.com/evanxd/41d8e2d91c5201a42bfa

Comment 24

[Andrea Marchesini [:baku]]

Comment on attachment 8628606 [details] [diff] [review]
Separate volume control

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

lgtm

::: dom/system/gonk/AudioManager.cpp
@@ +199,4 @@
>      int32_t volIndex = aResult.toInt32();
> +    for (int idx = 0; idx < VOLUME_TOTAL_NUMBER; idx++) {
> +      if (aName.EqualsASCII(gVolumeData[idx].mChannelName)) {
> +        nsresult rv = audioManager->ValidateVolumeIndex(gVolumeData[idx].mCategory,

80chars

@@ +532,5 @@
>    nsresult rv = settingsService->CreateLock(nullptr, getter_AddRefs(lock));
>    NS_ENSURE_SUCCESS_VOID(rv);
>    nsCOMPtr<nsISettingsServiceCallback> callback = new AudioChannelVolInitCallback();
>    NS_ENSURE_TRUE_VOID(callback);
> +  for (int idx = 0; idx < VOLUME_TOTAL_NUMBER; idx++) {

int32_t here and in any other for loop.

@@ +1062,5 @@
> +
> +void
> +AudioManager::InitProfilesVolume(uint32_t aCategory, int32_t aIndex)
> +{
> +  int profilesNum = mAudioProfiles.Length();

uint32_t

@@ +1099,5 @@
> +
> +void
> +AudioManager::UpdateProfileState(AudioOutputProfiles aProfile, bool aActive)
> +{
> +

extra line.

@@ +1113,5 @@
> +    mPresentProfile = aProfile;
> +  } else {
> +    // The primary profile has the lowest priority. We will check whether there
> +    // are other profiles. The bluetooth and headset have the same priotity.
> +    int profilesNum = mAudioProfiles.Length();

int32_t

@@ +1115,5 @@
> +    // The primary profile has the lowest priority. We will check whether there
> +    // are other profiles. The bluetooth and headset have the same priotity.
> +    int profilesNum = mAudioProfiles.Length();
> +    MOZ_ASSERT(profilesNum == DEVICE_TOTAL_NUMBER, "Error profile numbers!");
> +    for (int idx = profilesNum - 1; idx >= 0; idx--) {

int32_t

@@ +1128,5 @@
> +void
> +AudioManager::UpdateVolumeToProfile(AudioProfileData* aProfileData)
> +{
> +  MOZ_ASSERT(aProfileData);
> +  for (int idx = 0; idx < VOLUME_TOTAL_NUMBER; idx++) {

uint32_t

@@ +1138,5 @@
> +void
> +AudioManager::UpdateVolumeFromProfile(AudioProfileData* aProfileData)
> +{
> +  MOZ_ASSERT(aProfileData);
> +  for (int idx = 0; idx < VOLUME_TOTAL_NUMBER; idx++) {

uint32_t

Comment 25

[Alastor Wu [:alwu]]

Attachment: Separate volume control

Hi, Baku,
The main changing of this patch is to modify variables to uint32_t.
Thanks!

Comment 26

[Andrea Marchesini [:baku]]

Comment on attachment 8629308 [details] [diff] [review]
Separate volume control

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

would be nice if we can land this code in m-i only when we have landed the refactoring of the AudioChannelService.

::: dom/system/gonk/AudioManager.cpp
@@ +196,5 @@
>      NS_ENSURE_TRUE(aResult.isInt32(), NS_OK);
> +    nsRefPtr<AudioManager> audioManager = AudioManager::GetInstance();
> +    MOZ_ASSERT(audioManager);
> +    uint32_t volIndex = aResult.toInt32();
> +    for (uint32_t idx = 0; idx < VOLUME_TOTAL_NUMBER; idx++) {

doesn't change the logic of your code, but can you do:
++idx in the for loop? here an in the other for loop?

@@ +1108,5 @@
> +  }
> +
> +  mAudioProfiles[aProfile]->SetActive(aActive);
> +  if (aActive) {
> +    mPresentProfile = aProfile;

return;

and remove the '} else {'

Comment 27

[Alastor Wu [:alwu]]

Attachment: Separate volume control. r=baku.

Comment 28

[Alastor Wu [:alwu]]

Try-server result is on the comment10.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

Can you please post a Try link that's not over a month old? Things can change from underneath you :)

Comment 30

[Alastor Wu [:alwu]]

Sorry, Ryan,
Here is the new result, waiting for all green.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7b8ea94f17b0

Comment 31

[Alastor Wu [:alwu]]

New try-result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=01ad108ca55c

Comment 32

[Ryan VanderMeulen [:RyanVM]]

Patch needs rebasing.

Comment 33

[Alastor Wu [:alwu]]

Attachment: Separate volume control. r=baku.

Comment 34

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/f33885b67806

Comment 35

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Reminder: we'll need it to uplift 2.2r branch which is RedTai's base repo.
Feel free to contact me if there's any question.

Comment 36

[Alastor Wu [:alwu]]

Ok, I would rebase the patches after finishing other works.
Set Ni for reminding.

Comment 37

[Ryan VanderMeulen [:RyanVM]]

This was merged to m-c.
http://hg.mozilla.org/mozilla-central/rev/f33885b67806

Comment 38

[Shally Li（leave from Mozilla）]

This bug has been verified as "pass" on the latest nightly build of Flame kk 2.5 and Aries kk 2.5 by the STR in comment 0.

Actual results: 
Listening to music with 11 volume level on the speaker, and 4 volume level on the headset, and 10 volume level on BT headset:
a. After changing between the headset/speaker/other device, the volume level of headset/speaker still have the same volume setting (headset is 4 and speaker is 11).
b. If user connects BT headset first, then connects wired headset, the volume setting mode is "wired device".

See attachment: verified_Flame_v2.5.3gp
Reproduce rate: 0/10


Device: Flame KK 2.5 (Pass)
Build ID               20150729150205
Gaia Revision          088f350b39baf8f86c7c1161fd4be178ce822b7b
Gaia Date              2015-07-29 15:33:30
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/62cd40885e93
Gecko Version          42.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150729.183952
Firmware Date          Wed Jul 29 18:40:04 EDT 2015
Bootloader             L1TC000118D0
Firmware Version       v18D v4

Device: Aries KK 2.5(Pass)
Build ID               20150728035120
Gaia Revision          14e32276025b0310d3e89027320cf4b2a24cedfb
Gaia Date              2015-07-27 16:43:18
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/33dc8a83cfc0
Gecko Version          42.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150728.033538
Firmware Date          Tue Jul 28 03:35:46 UTC 2015
Bootloader             s1

Comment 39

[Shally Li（leave from Mozilla）]

Attachment: verified_Flame_v2.5.3gp

Comment 40

[Ryan VanderMeulen [:RyanVM]]

This needs rebasing for v2.2r uplift.

Comment 41

[Alastor Wu [:alwu]]

Attachment: [v2.2r] Separate volume control. r=baku.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Feature, enable the separate volume control setting.
User impact if declined: The users would only have one volume setting.
Testing completed: Yes.
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: Yes, changing the UUID of the nsIAudioManager.

Comment 42

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8650256 [details] [diff] [review]
[v2.2r] Separate volume control. r=baku.

feature-b2g:2.2r+ have auto-approval

Comment 43

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/48d5719e2bdc