Closed Bug 656101 Opened 9 years ago Closed 3 years ago

lower or mute audio volume when Android asks us to

Categories

(Core :: Widget: Android, defect, P5)

defect

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
fennec + ---
firefox54 --- fixed

People

(Reporter: blassey, Assigned: adamtomasv, Mentored)

References

(Blocks 1 open bug)

Details

(Whiteboard: [lang=java])

Attachments

(1 file)

The OS and other apps can request that we either lower or mute the volumeduring long playback so they can play short chimes ("You've got mail!"), we should add support for honoring those requests
How these "requests" are performed ? We may be able to lower the volume globally (thanks to flash not working on Fennec), but how the other apps can notify us ?
I believe we could use this:
http://developer.android.com/reference/android/media/AudioManager.html

Paul, do you want to jump on this? :)
OS: Linux → All
Hardware: x86_64 → All
I'll work on this a soon as I've finished an API I'm working on to lower the volume off all the active media, but I guess I'll need some help on the Java-Androidish bridge thing.
tracking-fennec: --- → ?
tracking-fennec: ? → +
(In reply to Paul Adenot (:padenot) from comment #3)
> I'll work on this a soon as I've finished an API I'm working on to lower the
> volume off all the active media, but I guess I'll need some help on the
> Java-Androidish bridge thing.

Paul, still interested in working on this?
Whiteboard: [mentor=blassey][lang=java]
Wow, that was a long time ago !

Sorry, my plate is super full at the moment, but I can certainly help people when they reach the point where they actually have to duck the volume.
Mentor: blassey.bugs
Whiteboard: [mentor=blassey][lang=java] → [lang=java]
filter on [mass-p5]
Priority: -- → P5
Hi! Just posted first simple implementation of audio ducking to review.
Attachment #8810226 - Flags: review?(blassey.bugs) → review?(snorp)
Comment on attachment 8810226 [details]
Bug 656101 - support for audio ducking;

https://reviewboard.mozilla.org/r/92580/#review93090

::: mobile/android/base/java/org/mozilla/gecko/media/MediaControlService.java:48
(Diff revision 1)
> +    public static final String ACTION_START_AUDIO_DUCK      = "action_start_audio_duck";
> +    public static final String ACTION_STOP_AUDIO_DUCK       = "action_stop_audio_duck";
>  
>      private static final int MEDIA_CONTROL_ID = 1;
>      private static final String MEDIA_CONTROL_PREF = "dom.audiochannel.mediaControl";
> +    private static final int AUDIO_DUCK_STEPS = 3;

Why 3 here ? If this is abitrary, say so. It looks like this is the number of steps from 0 we want to drop the volume to when we need to duck the audio.

Is there any guarantees about the number of steps there is on a particular device ?

::: mobile/android/base/java/org/mozilla/gecko/media/MediaControlService.java:57
(Diff revision 1)
>      private MediaSession mSession;
>      private MediaController mController;
>  
> +    private enum AudioDucking { START, STOP };
> +    private boolean mSupportsDucking = false;
> +    private int mVolumeChangeLevels = 0;

The name of this should convey the meaning that this was the volume before the ducking event.
Attachment #8810226 - Flags: review?(padenot) → review+
Comment on attachment 8810226 [details]
Bug 656101 - support for audio ducking;

https://reviewboard.mozilla.org/r/92580/#review93090

Looks like this would work, I haven't tested it. Snorp or someone else that knows the code better than I do should have the final say here.

Thanks !
Comment on attachment 8810226 [details]
Bug 656101 - support for audio ducking;

https://reviewboard.mozilla.org/r/92580/#review93090

> Why 3 here ? If this is abitrary, say so. It looks like this is the number of steps from 0 we want to drop the volume to when we need to duck the audio.
> 
> Is there any guarantees about the number of steps there is on a particular device ?

Hi Paul, yes this is arbitrary and I chose number 3 to make a hearable difference in the audio volume. I tried different values: 1 would make the difference almost not recognizable, 5 was a big step. 

The correct name would probably be **AUDIO_DUCK_MAX_STEPS** as this is the maximum number of AudioManager.ADJUST_LOWER/ADJUST_RAISE which are performed.

There is a guard in the method handleAudioDucking which makes sure that the volume will never be set below 0 / over the max volume.

> The name of this should convey the meaning that this was the volume before the ducking event.

This is the number of steps which we lowered the current volume by. And also the number of steps we need to raise the volume back after the duck, to reach the original level.

Now I feel that a better name would be **mAudioDuckCurrentSteps** (to be similar to the macro AUDIO_DUCK_MAX_STEPS).
Assignee: nobody → adamtomasv
Please manually rebase your commits and try again. 
hg error in cmd: hg rebase -s d41ecb304d48 -d 46a1b53d651b: rebasing 346993:d41ecb304d48 "Bug 656101 - support for audio ducking; r=padenot,snorp" (tip) merging mobile/android/base/java/org/mozilla/gecko/media/MediaControlService.java warning: conflicts while merging mobile/android/base/java/org/mozilla/gecko/media/MediaControlService.java! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Keywords: checkin-needed
sorry I am not the expert to review your patch, maybe snorp would help with that?
Flags: needinfo?(adamtomasv)
Attachment #8810226 - Flags: review?(ihsiao) → review?(snorp)
Iris, snorp is the right person. Thank you for notifying me - I see you already updated the flags.
Flags: needinfo?(adamtomasv)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/128a1ad3ddff
support for audio ducking; r=padenot,snorp
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/128a1ad3ddff
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.