From 7807f46cd1b8f58ee8f2e5f6d7240d7ca7502311 Mon Sep 17 00:00:00 2001 From: Uoti Urpala Date: Mon, 9 Apr 2012 17:39:01 +0300 Subject: audio: keep volume level internally (not only in AO) Current volume was always queried from the the audio output driver (or filter in case of --softvol). The only case where it was stored on mixer level was that when turning off mute, volume was set to the value it had before mute was activated. Change the mixer code to always store the current target volume internally. It still checks for significant changes from external sources and resets the internal value in that case. The main functionality changes are: Volume will now be kept separately from mute status. Increasing or decreasing volume will now change it relative to the original value before mute, even if mute is implemented by setting AO level volume to 0. Volume changes no longer automatically disable mute. The exception is relative changes up (like the volume increase key in default keybindings); that's the only case which still disables mute. Keeping the value internally avoids problems with granularity of possible volume values supported by AO. Increase/decrease keys could work unsymmetrically, or when specifying a smaller than default --volstep, even fail completely. In one case occurring in practice, if the AO only supports changing volume in steps of about 2 and rounds down the requested volume, then volume down key would decrease by 4 but volume up would increase by 2 (previous volume plus or minus the default change of 3, rounded down to a multiple of 2). Now, the internal value will keep full precision. --- mixer.c | 110 ++++++++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 66 insertions(+), 44 deletions(-) (limited to 'mixer.c') diff --git a/mixer.c b/mixer.c index f28fea7b09..25fb79f76c 100644 --- a/mixer.c +++ b/mixer.c @@ -18,6 +18,8 @@ #include +#include + #include "config.h" #include "libao2/audio_out.h" #include "libaf/af.h" @@ -25,16 +27,16 @@ #include "mixer.h" -void mixer_getvolume(mixer_t *mixer, float *l, float *r) +static void checkvolume(struct mixer *mixer) { - *l = 0; - *r = 0; if (!mixer->ao) return; ao_control_vol_t vol; if (mixer->softvol || CONTROL_OK != ao_control(mixer->ao, AOCONTROL_GET_VOLUME, &vol)) { + if (!mixer->afilter) + return; float db_vals[AF_NCH]; if (!af_control_any_rev(mixer->afilter, AF_CONTROL_VOLUME_LEVEL | AF_CONTROL_GET, db_vals)) @@ -44,19 +46,34 @@ void mixer_getvolume(mixer_t *mixer, float *l, float *r) vol.left = (db_vals[0] / (mixer->softvol_max / 100.0)) * 100.0; vol.right = (db_vals[1] / (mixer->softvol_max / 100.0)) * 100.0; } - *r = vol.right; - *l = vol.left; + float l = mixer->vol_l; + float r = mixer->vol_r; + if (mixer->muted) + l = r = 0; + /* Try to detect cases where the volume has been changed by some external + * action (such as something else changing a shared system-wide volume). + * We don't test for exact equality, as some AOs may round the value + * we last set to some nearby supported value. 3 has been the default + * volume step for increase/decrease keys, and is apparently big enough + * to step to the next possible value in most setups. + */ + if (FFABS(vol.left - l) >= 3 || FFABS(vol.right - r) >= 3) { + mixer->vol_l = vol.left; + mixer->vol_r = vol.right; + mixer->muted = false; + } } -void mixer_setvolume(mixer_t *mixer, float l, float r) +void mixer_getvolume(mixer_t *mixer, float *l, float *r) { - if (!mixer->ao) { - mixer->muted = 0; - return; - } - ao_control_vol_t vol; - vol.right = r; - vol.left = l; + checkvolume(mixer); + *l = mixer->vol_l; + *r = mixer->vol_r; +} + +static void setvolume_internal(mixer_t *mixer, float l, float r) +{ + struct ao_control_vol vol = {.left = l, .right = r}; if (mixer->softvol || CONTROL_OK != ao_control(mixer->ao, AOCONTROL_SET_VOLUME, &vol)) { // af_volume uses values in dB @@ -81,33 +98,16 @@ void mixer_setvolume(mixer_t *mixer, float l, float r) } } } - mixer->muted = 0; } -void mixer_incvolume(mixer_t *mixer) -{ - float mixer_l, mixer_r; - mixer_getvolume(mixer, &mixer_l, &mixer_r); - mixer_l += mixer->volstep; - if (mixer_l > 100) - mixer_l = 100; - mixer_r += mixer->volstep; - if (mixer_r > 100) - mixer_r = 100; - mixer_setvolume(mixer, mixer_l, mixer_r); -} - -void mixer_decvolume(mixer_t *mixer) +void mixer_setvolume(mixer_t *mixer, float l, float r) { - float mixer_l, mixer_r; - mixer_getvolume(mixer, &mixer_l, &mixer_r); - mixer_l -= mixer->volstep; - if (mixer_l < 0) - mixer_l = 0; - mixer_r -= mixer->volstep; - if (mixer_r < 0) - mixer_r = 0; - mixer_setvolume(mixer, mixer_l, mixer_r); + checkvolume(mixer); // to check mute status + mixer->vol_l = av_clip(l, 0, 100); + mixer->vol_r = av_clip(r, 0, 100); + if (!mixer->ao || mixer->muted) + return; + setvolume_internal(mixer, mixer->vol_l, mixer->vol_r); } void mixer_getbothvolume(mixer_t *mixer, float *b) @@ -117,17 +117,39 @@ void mixer_getbothvolume(mixer_t *mixer, float *b) *b = (mixer_l + mixer_r) / 2; } -void mixer_mute(mixer_t *mixer) +void mixer_setmute(struct mixer *mixer, bool mute) { - if (mixer->muted) { - mixer_setvolume(mixer, mixer->last_l, mixer->last_r); - } else { - mixer_getvolume(mixer, &mixer->last_l, &mixer->last_r); - mixer_setvolume(mixer, 0, 0); - mixer->muted = 1; + checkvolume(mixer); + if (mute != mixer->muted) { + mixer->muted = mute; + setvolume_internal(mixer, mixer->vol_l * !mute, mixer->vol_r * !mute); } } +bool mixer_getmute(struct mixer *mixer) +{ + checkvolume(mixer); + return mixer->muted; +} + +static void addvolume(struct mixer *mixer, float d) +{ + checkvolume(mixer); + mixer_setvolume(mixer, mixer->vol_l + d, mixer->vol_r + d); + if (d > 0) + mixer_setmute(mixer, false); +} + +void mixer_incvolume(mixer_t *mixer) +{ + addvolume(mixer, mixer->volstep); +} + +void mixer_decvolume(mixer_t *mixer) +{ + addvolume(mixer, -mixer->volstep); +} + void mixer_getbalance(mixer_t *mixer, float *val) { *val = 0.f; -- cgit v1.2.3 From 157a6c1e8343cf0d174e6f9edee441bfefebe578 Mon Sep 17 00:00:00 2001 From: Uoti Urpala Date: Wed, 11 Apr 2012 02:48:18 +0300 Subject: audio: mixer: change logic for AOs with no volume control The volume filter was automatically inserted if setting AO volume failed. Remove that logic, and instead enable softvol mode fully if querying current volume (which will happen before any set attempts) fails. Fully switching to softvol mode is more robust, and any case where the behavior would differ (the behavior is neither that both querying/setting always work nor that both always fail) would have been buggy. --- mixer.c | 51 +++++++++++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 24 deletions(-) (limited to 'mixer.c') diff --git a/mixer.c b/mixer.c index 25fb79f76c..0c92447c62 100644 --- a/mixer.c +++ b/mixer.c @@ -35,6 +35,7 @@ static void checkvolume(struct mixer *mixer) ao_control_vol_t vol; if (mixer->softvol || CONTROL_OK != ao_control(mixer->ao, AOCONTROL_GET_VOLUME, &vol)) { + mixer->softvol = true; if (!mixer->afilter) return; float db_vals[AF_NCH]; @@ -74,35 +75,37 @@ void mixer_getvolume(mixer_t *mixer, float *l, float *r) static void setvolume_internal(mixer_t *mixer, float l, float r) { struct ao_control_vol vol = {.left = l, .right = r}; - if (mixer->softvol || CONTROL_OK != ao_control(mixer->ao, - AOCONTROL_SET_VOLUME, &vol)) { - // af_volume uses values in dB - float db_vals[AF_NCH]; - int i; - db_vals[0] = (l / 100.0) * (mixer->softvol_max / 100.0); - db_vals[1] = (r / 100.0) * (mixer->softvol_max / 100.0); - for (i = 2; i < AF_NCH; i++) - db_vals[i] = ((l + r) / 100.0) * (mixer->softvol_max / 100.0) / 2.0; - af_to_dB(AF_NCH, db_vals, db_vals, 20.0); - if (!af_control_any_rev(mixer->afilter, - AF_CONTROL_VOLUME_LEVEL | AF_CONTROL_SET, db_vals)) { - mp_tmsg(MSGT_GLOBAL, MSGL_INFO, - "[Mixer] No hardware mixing, inserting volume filter.\n"); - if (af_add(mixer->afilter, "volume")) { - if (!af_control_any_rev(mixer->afilter, - AF_CONTROL_VOLUME_LEVEL|AF_CONTROL_SET, db_vals)) { - mp_tmsg(MSGT_GLOBAL, MSGL_ERR, - "[Mixer] No volume control available.\n"); - return; - } - } - } + if (!mixer->softvol) { + if (ao_control(mixer->ao, AOCONTROL_SET_VOLUME, &vol) != CONTROL_OK) + mp_tmsg(MSGT_GLOBAL, MSGL_ERR, + "[Mixer] Failed to change audio output volume.\n"); + return; + } + // af_volume uses values in dB + float db_vals[AF_NCH]; + int i; + db_vals[0] = (l / 100.0) * (mixer->softvol_max / 100.0); + db_vals[1] = (r / 100.0) * (mixer->softvol_max / 100.0); + for (i = 2; i < AF_NCH; i++) + db_vals[i] = ((l + r) / 100.0) * (mixer->softvol_max / 100.0) / 2.0; + af_to_dB(AF_NCH, db_vals, db_vals, 20.0); + if (!af_control_any_rev(mixer->afilter, + AF_CONTROL_VOLUME_LEVEL | AF_CONTROL_SET, + db_vals)) { + mp_tmsg(MSGT_GLOBAL, MSGL_INFO, + "[Mixer] No hardware mixing, inserting volume filter.\n"); + if (!(af_add(mixer->afilter, "volume") + && af_control_any_rev(mixer->afilter, + AF_CONTROL_VOLUME_LEVEL | AF_CONTROL_SET, + db_vals))) + mp_tmsg(MSGT_GLOBAL, MSGL_ERR, + "[Mixer] No volume control available.\n"); } } void mixer_setvolume(mixer_t *mixer, float l, float r) { - checkvolume(mixer); // to check mute status + checkvolume(mixer); // to check mute status and AO support for volume mixer->vol_l = av_clip(l, 0, 100); mixer->vol_r = av_clip(r, 0, 100); if (!mixer->ao || mixer->muted) -- cgit v1.2.3 From 87dad2a4704b2fb0f983d5cb665a065437288d35 Mon Sep 17 00:00:00 2001 From: Uoti Urpala Date: Mon, 9 Apr 2012 21:02:27 +0300 Subject: audio: restore volume setting after AO reinit if needed MPlayer volume control was originally implemented with the assumption that it controls a system-wide volume setting which keeps its value even if a process closes and reopens the audio device. However, this is not actually true for --softvol mode or some audio output APIs that only consider volume as a per-client setting for software mixing. This could have annoying results, as the volume would be reset to a default value if the AO was closed and reopened, for example whem moving to a new file or crossing ordered chapter boundaries. Add code to set the previous volume again after audio reinitialization if the current audio chain is known to behave this way (softvol active or the AO driver is known to not keep persistent volume externally). This also avoids an inconsistency with the mute flag. The frontend assumed the mute status is persistent across file changes, but it could be similarly lost. The audio drivers that are assumed to not keep persistent volume are: coreaudio, dsound, esd, nas, openal, sdl. None of these changes have been tested. I'm guessing that ESD and NAS do per-connection non-persistent volume settings. Partially based on code by wm4. --- mixer.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) (limited to 'mixer.c') diff --git a/mixer.c b/mixer.c index 0c92447c62..b231563026 100644 --- a/mixer.c +++ b/mixer.c @@ -76,11 +76,17 @@ static void setvolume_internal(mixer_t *mixer, float l, float r) { struct ao_control_vol vol = {.left = l, .right = r}; if (!mixer->softvol) { + // relies on the driver data being permanent (so ptr stays valid) + mixer->restore_volume = mixer->ao->no_persistent_volume ? + mixer->ao->driver->info->short_name : NULL; if (ao_control(mixer->ao, AOCONTROL_SET_VOLUME, &vol) != CONTROL_OK) mp_tmsg(MSGT_GLOBAL, MSGL_ERR, "[Mixer] Failed to change audio output volume.\n"); return; } + mixer->restore_volume = "softvol"; + if (!mixer->afilter) + return; // af_volume uses values in dB float db_vals[AF_NCH]; int i; @@ -197,3 +203,24 @@ void mixer_setbalance(mixer_t *mixer, float val) af_pan_balance->control(af_pan_balance, AF_CONTROL_PAN_BALANCE | AF_CONTROL_SET, &val); } + +// Called after the audio filter chain is built or rebuilt. +void mixer_reinit(struct mixer *mixer, struct ao *ao) +{ + mixer->ao = ao; + /* Use checkvolume() to see if softvol needs to be enabled because of + * lacking AO support, but first store values it could overwrite. */ + float left = mixer->vol_l, right = mixer->vol_r; + bool muted = mixer->muted; + checkvolume(mixer); + /* Try to avoid restoring volume stored from one control method with + * another. Especially, restoring softvol volume (typically high) on + * system mixer could have very nasty effects. */ + const char *restore_reason = mixer->softvol ? "softvol" : + mixer->ao->driver->info->short_name; + if (mixer->restore_volume && !strcmp(mixer->restore_volume, + restore_reason)) { + mixer_setvolume(mixer, left, right); + mixer_setmute(mixer, muted); + } +} -- cgit v1.2.3 From e29cb8f323031b32369bc2104ea1fd4422dd2945 Mon Sep 17 00:00:00 2001 From: Uoti Urpala Date: Mon, 9 Apr 2012 21:06:10 +0300 Subject: audio: restore balance setting after reinit Restore the audio balance setting when the audio chain is reinitialized (also after switching to another file). Also add a note about the balance code being seriously buggy. --- mixer.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) (limited to 'mixer.c') diff --git a/mixer.c b/mixer.c index b231563026..e694253b11 100644 --- a/mixer.c +++ b/mixer.c @@ -161,13 +161,24 @@ void mixer_decvolume(mixer_t *mixer) void mixer_getbalance(mixer_t *mixer, float *val) { - *val = 0.f; - if (!mixer->afilter) - return; - af_control_any_rev(mixer->afilter, AF_CONTROL_PAN_BALANCE | AF_CONTROL_GET, - val); + if (mixer->afilter) + af_control_any_rev(mixer->afilter, + AF_CONTROL_PAN_BALANCE | AF_CONTROL_GET, + &mixer->balance); + *val = mixer->balance; } +/* NOTE: Currently the balance code is seriously buggy: it always changes + * the af_pan mapping between the first two input channels and first two + * output channels to particular values. These values make sense for an + * af_pan instance that was automatically inserted for balance control + * only and is otherwise an identity transform, but if the filter was + * there for another reason, then ignoring and overriding the original + * values is completely wrong. In particular, this will break + * automatically inserted downmix filters; the original coefficients that + * are significantly below 1 will be overwritten with much higher values. + */ + void mixer_setbalance(mixer_t *mixer, float val) { float level[AF_NCH]; @@ -175,6 +186,8 @@ void mixer_setbalance(mixer_t *mixer, float val) af_control_ext_t arg_ext = { .arg = level }; af_instance_t *af_pan_balance; + mixer->balance = val; + if (!mixer->afilter) return; @@ -182,6 +195,9 @@ void mixer_setbalance(mixer_t *mixer, float val) AF_CONTROL_PAN_BALANCE | AF_CONTROL_SET, &val)) return; + if (val == 0 || mixer->ao->channels < 2) + return; + if (!(af_pan_balance = af_add(mixer->afilter, "pan"))) { mp_tmsg(MSGT_GLOBAL, MSGL_ERR, "[Mixer] No balance control available.\n"); @@ -223,4 +239,6 @@ void mixer_reinit(struct mixer *mixer, struct ao *ao) mixer_setvolume(mixer, left, right); mixer_setmute(mixer, muted); } + if (mixer->balance != 0) + mixer_setbalance(mixer, mixer->balance); } -- cgit v1.2.3 From 9624f10aa85039c73d4bdb70e8062daeabaa90c6 Mon Sep 17 00:00:00 2001 From: Uoti Urpala Date: Mon, 9 Apr 2012 22:11:49 +0300 Subject: audio: fix unmute-at-end logic The player tried to disable mute before exiting, so that if mute is emulated by setting volume to 0 and the volume setting is a system-global one, we don't leave it at 0. However, the logic doing this at process exit was flawed, as volume settings are handled by audio output instances and the audio output that set the mute state may have been closed earlier. Trying to write reliably working logic that restores volume at exit only would be tricky, so change the code to always unmute an audio driver before closing it and restore mute status if one is opened again later. --- mixer.c | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) (limited to 'mixer.c') diff --git a/mixer.c b/mixer.c index e694253b11..793a6ac58e 100644 --- a/mixer.c +++ b/mixer.c @@ -235,10 +235,32 @@ void mixer_reinit(struct mixer *mixer, struct ao *ao) const char *restore_reason = mixer->softvol ? "softvol" : mixer->ao->driver->info->short_name; if (mixer->restore_volume && !strcmp(mixer->restore_volume, - restore_reason)) { + restore_reason)) mixer_setvolume(mixer, left, right); - mixer_setmute(mixer, muted); - } + mixer_setmute(mixer, muted); if (mixer->balance != 0) mixer_setbalance(mixer, mixer->balance); } + +/* Called before uninitializing the audio output. The main purpose is to + * turn off mute, in case it's a global/persistent setting which might + * otherwise be left enabled even after this player instance exits. + */ +void mixer_uninit(struct mixer *mixer) +{ + checkvolume(mixer); + if (mixer->muted) { + /* Current audio output API combines playing the remaining buffered + * audio and uninitializing the AO into one operation, even though + * ideally unmute would happen between those two steps. We can't do + * volume changes after uninitialization, but we don't want the + * remaining audio to play at full volume either. Thus this + * workaround to drop remaining audio first. */ + ao_reset(mixer->ao); + mixer_setmute(mixer, false); + /* We remember mute status and re-enable it if we play more audio + * in the same process. */ + mixer->muted = true; + } + mixer->ao = NULL; +} -- cgit v1.2.3 From 39aa7d9846a8a04e8f08acc0ea9e2ce38336e523 Mon Sep 17 00:00:00 2001 From: Uoti Urpala Date: Tue, 10 Apr 2012 04:45:53 +0300 Subject: mixer: support native audio driver mute Make mixer support setting the mute attribute at audio driver level, if one exists separately from volume. As of this commit, no libao2 driver exposes such an attribute yet; that will be added in later commits. Since the mute status can now be set externally, it's no longer completely obvious when the player should automatically disable mute when uninitializing an audio output. The implemented behavior is to turn mute off at uninitialization if we turned it on and haven't noticed it turn off (by external means) since. --- mixer.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) (limited to 'mixer.c') diff --git a/mixer.c b/mixer.c index 793a6ac58e..7be2179384 100644 --- a/mixer.c +++ b/mixer.c @@ -49,7 +49,7 @@ static void checkvolume(struct mixer *mixer) } float l = mixer->vol_l; float r = mixer->vol_r; - if (mixer->muted) + if (mixer->muted_using_volume) l = r = 0; /* Try to detect cases where the volume has been changed by some external * action (such as something else changing a shared system-wide volume). @@ -61,8 +61,14 @@ static void checkvolume(struct mixer *mixer) if (FFABS(vol.left - l) >= 3 || FFABS(vol.right - r) >= 3) { mixer->vol_l = vol.left; mixer->vol_r = vol.right; - mixer->muted = false; + if (mixer->muted_using_volume) + mixer->muted = false; } + if (!mixer->softvol) + // Rely on the value not changing if the query is not supported + ao_control(mixer->ao, AOCONTROL_GET_MUTE, &mixer->muted); + mixer->muted_by_us &= mixer->muted; + mixer->muted_using_volume &= mixer->muted; } void mixer_getvolume(mixer_t *mixer, float *l, float *r) @@ -130,8 +136,15 @@ void mixer_setmute(struct mixer *mixer, bool mute) { checkvolume(mixer); if (mute != mixer->muted) { + if (!mixer->softvol && !mixer->muted_using_volume && ao_control( + mixer->ao, AOCONTROL_SET_MUTE, &mute) == CONTROL_OK) { + mixer->muted_using_volume = false; + } else { + setvolume_internal(mixer, mixer->vol_l*!mute, mixer->vol_r*!mute); + mixer->muted_using_volume = mute; + } mixer->muted = mute; - setvolume_internal(mixer, mixer->vol_l * !mute, mixer->vol_r * !mute); + mixer->muted_by_us = mute; } } @@ -227,7 +240,7 @@ void mixer_reinit(struct mixer *mixer, struct ao *ao) /* Use checkvolume() to see if softvol needs to be enabled because of * lacking AO support, but first store values it could overwrite. */ float left = mixer->vol_l, right = mixer->vol_r; - bool muted = mixer->muted; + bool muted = mixer->muted_by_us; checkvolume(mixer); /* Try to avoid restoring volume stored from one control method with * another. Especially, restoring softvol volume (typically high) on @@ -237,7 +250,11 @@ void mixer_reinit(struct mixer *mixer, struct ao *ao) if (mixer->restore_volume && !strcmp(mixer->restore_volume, restore_reason)) mixer_setvolume(mixer, left, right); - mixer_setmute(mixer, muted); + /* We turn mute off at AO uninit, so it has to be restored (unless + * we're reinitializing filter chain while keeping AO); but we only + * enable mute, not turn external mute off. */ + if (muted) + mixer_setmute(mixer, true); if (mixer->balance != 0) mixer_setbalance(mixer, mixer->balance); } @@ -249,7 +266,7 @@ void mixer_reinit(struct mixer *mixer, struct ao *ao) void mixer_uninit(struct mixer *mixer) { checkvolume(mixer); - if (mixer->muted) { + if (mixer->muted_by_us) { /* Current audio output API combines playing the remaining buffered * audio and uninitializing the AO into one operation, even though * ideally unmute would happen between those two steps. We can't do @@ -260,7 +277,7 @@ void mixer_uninit(struct mixer *mixer) mixer_setmute(mixer, false); /* We remember mute status and re-enable it if we play more audio * in the same process. */ - mixer->muted = true; + mixer->muted_by_us = true; } mixer->ao = NULL; } -- cgit v1.2.3