Closed Bug 1170117 Opened 5 years ago Closed 5 years ago

[B2G] Separate volume control settings for multiple audio profiles

Categories

(Firefox OS Graveyard :: AudioChannel, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(feature-b2g:2.2r+, b2g-v2.2 wontfix, b2g-v2.2r fixed, b2g-master verified)

RESOLVED FIXED
FxOS-S3 (24Jul)
feature-b2g 2.2r+
Tracking Status
b2g-v2.2 --- wontfix
b2g-v2.2r --- fixed
b2g-master --- verified

People

(Reporter: alwu, Assigned: alwu)

References

Details

(Whiteboard: [red-tai])

Attachments

(3 files, 10 obsolete files)

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.
Assignee: nobody → alwu
Attached patch Separate volume control (obsolete) — Splinter Review
Here is the preliminary patch with complete functionality.
Blocks: 1104972
Attached patch Separate volume control (obsolete) — Splinter Review
In this patch, we could change three different profiles - "primary", "headset" and "bluetooth".
Attachment #8613479 - Attachment is obsolete: true
Attached patch Separate volume control (obsolete) — Splinter Review
New patch and try-server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a404129633da
Attachment #8613951 - Attachment is obsolete: true
Because of the fails in the try-server, I'm still revising it.
Attached patch Separate volume control (obsolete) — Splinter Review
New patch and try-server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3cfd480fc0d
Attached patch Separate volume control (obsolete) — Splinter Review
Attachment #8614509 - Attachment is obsolete: true
Attachment #8616131 - Attachment is obsolete: true
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".
Attachment #8616134 - Flags: review?(amarchesini)
Blocks: 1172791
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.
Attachment #8616134 - Flags: review?(amarchesini) → review-
Attached patch Separate volume control (obsolete) — Splinter Review
Thanks! Here is my revised patch, and wait for the try-server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ba53435a245
Attachment #8616134 - Attachment is obsolete: true
Comment on attachment 8623016 [details] [diff] [review]
Separate volume control

Hi, Baku,
Here is my revised patch, very appreciate!
Attachment #8623016 - Flags: review?(amarchesini)
Whiteboard: [red-tai]
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.
Attachment #8623016 - Flags: review?(amarchesini) → review-
(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.
(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?
> 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?
> Could I use the NS_WARNING here?

Sure!
(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!
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?
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.
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 :)
Attached patch [WIP] Separate volume control (obsolete) — Splinter Review
This is the WIP patch, it still need to modify.
I think that I can upload the complete patch tomorrow and describe more details.
Attachment #8623016 - Attachment is obsolete: true
Depends on: 1179173
Depends on: 1179181
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.
Attached patch Separate volume control (obsolete) — Splinter Review
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
Attachment #8627645 - Attachment is obsolete: true
Attachment #8628606 - Flags: review?(amarchesini)
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
Attachment #8628606 - Flags: review?(amarchesini) → review-
Blocks: 1179181
No longer depends on: 1179181
Attached patch Separate volume control (obsolete) — Splinter Review
Hi, Baku,
The main changing of this patch is to modify variables to uint32_t.
Thanks!
Attachment #8628606 - Attachment is obsolete: true
Attachment #8629308 - Flags: review?(amarchesini)
feature-b2g: --- → 2.2r+
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 {'
Attachment #8629308 - Flags: review?(amarchesini) → review+
Attached patch Separate volume control. r=baku. (obsolete) — Splinter Review
Attachment #8629308 - Attachment is obsolete: true
Attachment #8633295 - Flags: review+
Try-server result is on the comment10.
Keywords: checkin-needed
Can you please post a Try link that's not over a month old? Things can change from underneath you :)
Keywords: checkin-needed
Sorry, Ryan,
Here is the new result, waiting for all green.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7b8ea94f17b0
Patch needs rebasing.
Keywords: checkin-needed
Attachment #8633295 - Attachment is obsolete: true
Keywords: checkin-needed
Depends on: 1185422
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.
Ok, I would rebase the patches after finishing other works.
Set Ni for reminding.
Flags: needinfo?(alwu)
This was merged to m-c.
http://hg.mozilla.org/mozilla-central/rev/f33885b67806
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S3 (24Jul)
Whiteboard: [red-tai] → [red-tai][ETA=8/31]
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
QA Whiteboard: [MGSEI-Triage+]
This needs rebasing for v2.2r uplift.
[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.
Flags: needinfo?(alwu)
Attachment #8650256 - Flags: approval‑mozilla‑b2g37_v2_2r?
Comment on attachment 8650256 [details] [diff] [review]
[v2.2r] Separate volume control. r=baku.

feature-b2g:2.2r+ have auto-approval
Attachment #8650256 - Flags: approval‑mozilla‑b2g37_v2_2r?
You need to log in before you can comment on or make changes to this bug.