From c96dc1fcffb2b06a66f38c9d98d16c349e21db70 Mon Sep 17 00:00:00 2001 From: ejurgensen Date: Tue, 25 May 2021 23:39:11 +0200 Subject: [PATCH] [dacp] Fix support for device-volume (speaker volume buttons) Closes #613 --- src/httpd_dacp.c | 2 +- src/outputs/airplay.c | 93 ++++++++++++++++++++++++++++--------------- src/outputs/raop.c | 27 +++++++------ src/player.c | 13 +++--- src/player.h | 2 +- 5 files changed, 85 insertions(+), 52 deletions(-) diff --git a/src/httpd_dacp.c b/src/httpd_dacp.c index 164a8c04..07c61ba4 100644 --- a/src/httpd_dacp.c +++ b/src/httpd_dacp.c @@ -1002,7 +1002,7 @@ dacp_propset_devicevolume(const char *value, struct httpd_request *hreq) if (speaker_get(&speaker_info, hreq, "device-volume") < 0) return; - player_volume_update_speaker(speaker_info.id, value); + player_volume_setraw_speaker(speaker_info.id, value); } // See player.c:speaker_prevent_playback_set() for comments regarding diff --git a/src/outputs/airplay.c b/src/outputs/airplay.c index 8b6356ce..3b5f8896 100644 --- a/src/outputs/airplay.c +++ b/src/outputs/airplay.c @@ -1744,14 +1744,11 @@ airplay_metadata_send(struct output_metadata *metadata) /* ------------------------------ Volume handling --------------------------- */ -static float -airplay_volume_from_pct(int volume, char *name) +static int +volume_max_get(const char *name) { - float airplay_volume; + int max_volume = AIRPLAY_CONFIG_MAX_VOLUME; cfg_t *airplay; - int max_volume; - - max_volume = AIRPLAY_CONFIG_MAX_VOLUME; airplay = cfg_gettsec(cfg, "airplay", name); if (airplay) @@ -1760,15 +1757,25 @@ airplay_volume_from_pct(int volume, char *name) if ((max_volume < 1) || (max_volume > AIRPLAY_CONFIG_MAX_VOLUME)) { DPRINTF(E_LOG, L_AIRPLAY, "Config has bad max_volume (%d) for device '%s', using default instead\n", max_volume, name); - - max_volume = AIRPLAY_CONFIG_MAX_VOLUME; + return AIRPLAY_CONFIG_MAX_VOLUME; } + return max_volume; +} + +static float +airplay_volume_from_pct(int volume, const char *name) +{ + float airplay_volume; + int max_volume; + + max_volume = volume_max_get(name); + /* RAOP volume - * -144.0 is off - * 0 - 100 maps to -30.0 - 0 + * -144.0 is off (not really used since we have no concept of muted/off) + * 0 - 100 maps to -30.0 - 0 (if no max_volume set) */ - if (volume > 0 && volume <= 100) + if (volume >= 0 && volume <= 100) airplay_volume = -30.0 + ((float)max_volume * (float)volume * 30.0) / (100.0 * AIRPLAY_CONFIG_MAX_VOLUME); else airplay_volume = -144.0; @@ -1777,38 +1784,52 @@ airplay_volume_from_pct(int volume, char *name) } static int -airplay_volume_to_pct(struct output_device *rd, const char *volume) +airplay_volume_to_pct(struct output_device *rd, const char *volstr) { float airplay_volume; - cfg_t *airplay; + float volume; int max_volume; - airplay_volume = atof(volume); + airplay_volume = atof(volstr); - // Basic sanity check - if (airplay_volume == 0.0 && volume[0] != '0') + if ((airplay_volume == 0.0 && volstr[0] != '0') || airplay_volume > 0.0) { - DPRINTF(E_LOG, L_AIRPLAY, "AirPlay device volume is invalid: '%s'\n", volume); + DPRINTF(E_LOG, L_AIRPLAY, "AirPlay device volume is invalid: '%s'\n", volstr); return -1; } - max_volume = AIRPLAY_CONFIG_MAX_VOLUME; - - airplay = cfg_gettsec(cfg, "airplay", rd->name); - if (airplay) - max_volume = cfg_getint(airplay, "max_volume"); - - if ((max_volume < 1) || (max_volume > AIRPLAY_CONFIG_MAX_VOLUME)) + if (airplay_volume <= -30.0) { - DPRINTF(E_LOG, L_AIRPLAY, "Config has bad max_volume (%d) for device '%s', using default instead\n", max_volume, rd->name); - max_volume = AIRPLAY_CONFIG_MAX_VOLUME; + return 0; // -144.0 is muted } + max_volume = volume_max_get(rd->name); + +/* + This is an attempt at scaling the input volume that didn't really work for all + speakers (e.g. my Sony), but I'm leaving it here in case it should be a config + option some time + + // If the input volume is -25 and we are playing at -20, then we only want the + // resulting volume to be -25 if there is no volume scaling. If the scaling is + // set to say 20% then we want the resulting volume to be -21, i.e. 20% of the + // change. Expressed as an equation: + // a_r = a_0 + m/M * (a_i - a_0) - where a_0 is the current airplay volume, a_i is the input and a_r is the scaled result + // + // Since current volume (rd->volume) is measured on the 0-100 scale, and the + // result of this func should also be on that scale, we have the following two + // relationships (the first is also found in _from_pct() above): + // a_0 = -30 + m/M * 30/100 * v_0 - where v_0 is rd->volume + // v_r = M/m * 100 * (1 + a_r / 30) - converts a_r to v_r which is [0-100] + // + // Solving these three equations gives this: + volume_base = 100.0 * (1.0 + airplay_volume / 30.0); + volume = (float)rd->volume * (1.0 - (float)max_volume/AIRPLAY_CONFIG_MAX_VOLUME) + volume_base; + +*/ // RAOP volume: -144.0 is off, -30.0 - 0 scaled by max_volume maps to 0 - 100 - if (airplay_volume > -30.0 && airplay_volume <= 0.0) - return (int)(100.0 * (airplay_volume / 30.0 + 1.0) * AIRPLAY_CONFIG_MAX_VOLUME / (float)max_volume); - else - return 0; + volume = (100.0 * (airplay_volume / 30.0 + 1.0) * AIRPLAY_CONFIG_MAX_VOLUME / (float)max_volume); + return MAX(0, MIN(100, (int)volume)); } /* Volume in [0 - 100] */ @@ -2359,13 +2380,18 @@ static int payload_make_set_volume(struct evrtsp_request *req, struct airplay_session *rs, void *arg) { float raop_volume; + char volstr[32]; int ret; raop_volume = airplay_volume_from_pct(rs->volume, rs->devname); /* Don't let locales get in the way here */ /* We use -%d and -(int)raop_volume so -0.3 won't become 0.3 */ - ret = evbuffer_add_printf(req->output_buffer, "volume: -%d.%06d\r\n", -(int)raop_volume, -(int)(1000000.0 * (raop_volume - (int)raop_volume))); + snprintf(volstr, sizeof(volstr), "-%d.%06d", -(int)raop_volume, -(int)(1000000.0 * (raop_volume - (int)raop_volume))); + + DPRINTF(E_DBG, L_AIRPLAY, "Sending volume %s to '%s'\n", volstr, rs->devname); + + ret = evbuffer_add_printf(req->output_buffer, "volume: %s\r\n", volstr); if (ret < 0) { DPRINTF(E_LOG, L_AIRPLAY, "Out of memory for SET_PARAMETER payload (volume)\n"); @@ -3408,7 +3434,7 @@ static struct airplay_seq_definition airplay_seq_definition[] = // The size of the second array dimension MUST at least be the size of largest // sequence + 1, because then we can count on a zero terminator when iterating -static struct airplay_seq_request airplay_seq_request[][7] = +static struct airplay_seq_request airplay_seq_request[][7] = { { { AIRPLAY_SEQ_START, "GET /info", EVRTSP_REQ_GET, NULL, response_handler_info_start, NULL, "/info", false }, @@ -3420,8 +3446,9 @@ static struct airplay_seq_request airplay_seq_request[][7] = { AIRPLAY_SEQ_START_PLAYBACK, "SETUP (session)", EVRTSP_REQ_SETUP, payload_make_setup_session, response_handler_setup_session, "application/x-apple-binary-plist", NULL, false }, { AIRPLAY_SEQ_START_PLAYBACK, "SETPEERS", EVRTSP_REQ_SETPEERS, payload_make_setpeers, NULL, "/peer-list-changed", NULL, false }, { AIRPLAY_SEQ_START_PLAYBACK, "SETUP (stream)", EVRTSP_REQ_SETUP, payload_make_setup_stream, response_handler_setup_stream, "application/x-apple-binary-plist", NULL, false }, - { AIRPLAY_SEQ_START_PLAYBACK, "SET_PARAMETER (volume)", EVRTSP_REQ_SET_PARAMETER, payload_make_set_volume, response_handler_volume_start, "text/parameters", NULL, true }, { AIRPLAY_SEQ_START_PLAYBACK, "RECORD", EVRTSP_REQ_RECORD, payload_make_record, response_handler_record, NULL, NULL, false }, + // Some devices (e.g. Sonos Symfonisk) don't register the volume if it isn't last + { AIRPLAY_SEQ_START_PLAYBACK, "SET_PARAMETER (volume)", EVRTSP_REQ_SET_PARAMETER, payload_make_set_volume, response_handler_volume_start, "text/parameters", NULL, true }, }, { { AIRPLAY_SEQ_PROBE, "GET /info (probe)", EVRTSP_REQ_GET, NULL, response_handler_info_probe, NULL, "/info", false }, diff --git a/src/outputs/raop.c b/src/outputs/raop.c index 41cbf527..e76b947b 100644 --- a/src/outputs/raop.c +++ b/src/outputs/raop.c @@ -2495,10 +2495,10 @@ raop_volume_from_pct(int volume, struct output_device *device) float raop_volume; /* RAOP volume - * -144.0 is off - * 0 - 100 maps to -30.0 - 0 + * -144.0 is off (not really used since we have no concept of muted/off) + * 0 - 100 maps to -30.0 - 0 (if no max_volume set) */ - if (volume > 0 && volume <= 100) + if (volume >= 0 && volume <= 100) raop_volume = -30.0 + ((float)device->max_volume * (float)volume * 30.0) / (100.0 * RAOP_CONFIG_MAX_VOLUME); else raop_volume = -144.0; @@ -2507,24 +2507,27 @@ raop_volume_from_pct(int volume, struct output_device *device) } static int -raop_volume_to_pct(struct output_device *device, const char *volume) +raop_volume_to_pct(struct output_device *device, const char *volstr) { float raop_volume; + float volume; - raop_volume = atof(volume); + raop_volume = atof(volstr); - // Basic sanity check - if (raop_volume == 0.0 && volume[0] != '0') + if ((raop_volume == 0.0 && volstr[0] != '0') || raop_volume > 0.0) { - DPRINTF(E_LOG, L_RAOP, "RAOP device volume is invalid: '%s'\n", volume); + DPRINTF(E_LOG, L_RAOP, "RAOP device volume is invalid: '%s'\n", volstr); return -1; } + if (raop_volume <= -30.0) + { + return 0; + } + // RAOP volume: -144.0 is off, -30.0 - 0 scaled by max_volume maps to 0 - 100 - if (raop_volume > -30.0 && raop_volume <= 0.0) - return (int)(100.0 * (raop_volume / 30.0 + 1.0) * RAOP_CONFIG_MAX_VOLUME / (float)device->max_volume); - else - return 0; + volume = (100.0 * (raop_volume / 30.0 + 1.0) * RAOP_CONFIG_MAX_VOLUME / (float)device->max_volume); + return MAX(0, MIN(100, (int)volume)); } static int diff --git a/src/player.c b/src/player.c index 7e066394..8be8f6e1 100644 --- a/src/player.c +++ b/src/player.c @@ -2938,9 +2938,8 @@ volume_setabs_speaker(void *arg, int *retval) return COMMAND_END; } -// Just updates internal volume params (does not make actual requests to the speaker) static enum command_state -volume_update_speaker(void *arg, int *retval) +volume_setraw_speaker(void *arg, int *retval) { struct speaker_attr_param *vol_param = arg; struct output_device *device; @@ -2964,7 +2963,11 @@ volume_update_speaker(void *arg, int *retval) outputs_device_volume_register(device, volume, -1); - *retval = 0; + *retval = outputs_device_volume_set(device, device_volume_cb); + + if (*retval > 0) + return COMMAND_PENDING; // async + return COMMAND_END; } @@ -3453,7 +3456,7 @@ player_volume_setabs_speaker(uint64_t id, int vol) } int -player_volume_update_speaker(uint64_t id, const char *volstr) +player_volume_setraw_speaker(uint64_t id, const char *volstr) { struct speaker_attr_param vol_param; int ret; @@ -3461,7 +3464,7 @@ player_volume_update_speaker(uint64_t id, const char *volstr) vol_param.spk_id = id; vol_param.volstr = volstr; - ret = commands_exec_sync(cmdbase, volume_update_speaker, volume_generic_bh, &vol_param); + ret = commands_exec_sync(cmdbase, volume_setraw_speaker, volume_generic_bh, &vol_param); return ret; } diff --git a/src/player.h b/src/player.h index 30a90a5e..974794f4 100644 --- a/src/player.h +++ b/src/player.h @@ -154,7 +154,7 @@ int player_volume_setabs_speaker(uint64_t id, int vol); int -player_volume_update_speaker(uint64_t id, const char *volstr); +player_volume_setraw_speaker(uint64_t id, const char *volstr); int player_repeat_set(enum repeat_mode mode);