Skip to content

Commit 9efc33d

Browse files
Add vol_f_buffer functionality, imporving relativistic volume changes
1 parent 3788d90 commit 9efc33d

File tree

13 files changed

+178
-93
lines changed

13 files changed

+178
-93
lines changed

CHANGELOG.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,18 @@
33
# Future Release
44
* System
55
* Update our spotify provider `go-librespot` to `0.7.1`
6+
* Upgraded volume calculations to preserve relative positions when hitting the min or max setting via source volume bar
67
* Web App
78
* Changed caching rules to ensure that users don't get stuck with old versions of the webapp post update
89

910

1011
## 0.4.10
11-
1212
* Web App
1313
* Fixed internet radio search functionality
1414
* System
1515
* Changed apt source from `http://raspbian.raspberrypi.org/raspbian/` to `http://archive.raspberrypi.org/raspbian/`
1616

17-
## 0.4.9
18-
17+
# 0.4.9
1918
* System
2019
* Update our spotify provider `go-librespot` to `0.5.2` to accomodate spotify's API update
2120

amplipi/app.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -966,7 +966,7 @@ async def get_response(self, path, scope):
966966

967967

968968
# Website
969-
app.mount('/', CachelessFiles(directory=WEB_DIR, html=True), name='web')
969+
app.mount('/', StaticFiles(directory=WEB_DIR, html=True), name='web')
970970

971971

972972
def create_app(mock_ctrl=None, mock_streams=None, config_file=None, delay_saves=None, settings: models.AppSettings = models.AppSettings()) -> FastAPI:

amplipi/ctrl.py

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -847,14 +847,20 @@ def set_mute():
847847

848848
def set_vol():
849849
""" Update the zone's volume. Could be triggered by a change in
850-
vol, vol_f, vol_min, or vol_max.
850+
vol, vol_f, vol_f_delta, vol_min, or vol_max.
851851
"""
852852
# Field precedence: vol (db) > vol_delta > vol (float)
853-
# NOTE: checks happen in reverse precedence to cover default case of unchanged volume
853+
# vol (db) is first in precedence yet last in the stack to cover the default case of no volume change
854854
if update.vol_delta_f is not None and update.vol is None:
855-
applied_delta = utils.clamp((vol_delta_f + zone.vol_f), 0, 1)
856-
vol_db = utils.vol_float_to_db(applied_delta, zone.vol_min, zone.vol_max)
857-
vol_f_new = applied_delta
855+
true_vol_f = zone.vol_f + zone.vol_f_overflow
856+
expected_vol_total = update.vol_delta_f + true_vol_f
857+
vol_f_new = utils.clamp(expected_vol_total, models.MIN_VOL_F, models.MAX_VOL_F)
858+
859+
vol_db = utils.vol_float_to_db(vol_f_new, zone.vol_min, zone.vol_max)
860+
zone.vol_f_overflow = 0 if models.MIN_VOL_F < expected_vol_total and expected_vol_total < models.MAX_VOL_F \
861+
else utils.clamp((expected_vol_total - vol_f_new), models.MIN_VOL_F_OVERFLOW, models.MAX_VOL_F_OVERFLOW)
862+
# Clamp the remaining delta to be between -1 and 1
863+
858864
elif update.vol_f is not None and update.vol is None:
859865
clamp_vol_f = utils.clamp(vol_f, 0, 1)
860866
vol_db = utils.vol_float_to_db(clamp_vol_f, zone.vol_min, zone.vol_max)
@@ -866,9 +872,14 @@ def set_vol():
866872
if self._rt.update_zone_vol(idx, vol_db):
867873
zone.vol = vol_db
868874
zone.vol_f = vol_f_new
875+
869876
else:
870877
raise Exception('unable to update zone volume')
871878

879+
# Reset the overflow when vol_f goes in bounds, there is no longer an overflow
880+
# Avoids reporting spurious volume oscillations
881+
zone.vol_f_overflow = 0 if vol_f_new != models.MIN_VOL_F and vol_f_new != models.MAX_VOL_F else zone.vol_f_overflow
882+
872883
# To avoid potential unwanted loud output:
873884
# If muting, mute before setting volumes
874885
# If un-muting, set desired volume first

amplipi/defaults.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,17 +41,17 @@
4141
],
4242
"zones": [ # this is an array of zones, array length depends on # of boxes connected
4343
{"id": 0, "name": "Zone 1", "source_id": 0, "mute": True, "disabled": False,
44-
"vol_f": models.MIN_VOL_F, "vol_min": models.MIN_VOL_DB, "vol_max": models.MAX_VOL_DB},
44+
"vol_f": models.MIN_VOL_F, "vol_f_overflow": 0.0, "vol_min": models.MIN_VOL_DB, "vol_max": models.MAX_VOL_DB},
4545
{"id": 1, "name": "Zone 2", "source_id": 0, "mute": True, "disabled": False,
46-
"vol_f": models.MIN_VOL_F, "vol_min": models.MIN_VOL_DB, "vol_max": models.MAX_VOL_DB},
46+
"vol_f": models.MIN_VOL_F, "vol_f_overflow": 0.0, "vol_min": models.MIN_VOL_DB, "vol_max": models.MAX_VOL_DB},
4747
{"id": 2, "name": "Zone 3", "source_id": 0, "mute": True, "disabled": False,
48-
"vol_f": models.MIN_VOL_F, "vol_min": models.MIN_VOL_DB, "vol_max": models.MAX_VOL_DB},
48+
"vol_f": models.MIN_VOL_F, "vol_f_overflow": 0.0, "vol_min": models.MIN_VOL_DB, "vol_max": models.MAX_VOL_DB},
4949
{"id": 3, "name": "Zone 4", "source_id": 0, "mute": True, "disabled": False,
50-
"vol_f": models.MIN_VOL_F, "vol_min": models.MIN_VOL_DB, "vol_max": models.MAX_VOL_DB},
50+
"vol_f": models.MIN_VOL_F, "vol_f_overflow": 0.0, "vol_min": models.MIN_VOL_DB, "vol_max": models.MAX_VOL_DB},
5151
{"id": 4, "name": "Zone 5", "source_id": 0, "mute": True, "disabled": False,
52-
"vol_f": models.MIN_VOL_F, "vol_min": models.MIN_VOL_DB, "vol_max": models.MAX_VOL_DB},
52+
"vol_f": models.MIN_VOL_F, "vol_f_overflow": 0.0, "vol_min": models.MIN_VOL_DB, "vol_max": models.MAX_VOL_DB},
5353
{"id": 5, "name": "Zone 6", "source_id": 0, "mute": True, "disabled": False,
54-
"vol_f": models.MIN_VOL_F, "vol_min": models.MIN_VOL_DB, "vol_max": models.MAX_VOL_DB},
54+
"vol_f": models.MIN_VOL_F, "vol_f_overflow": 0.0, "vol_min": models.MIN_VOL_DB, "vol_max": models.MAX_VOL_DB},
5555
],
5656
"groups": [
5757
],

amplipi/models.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@
3838
MAX_VOL_F = 1.0
3939
""" Max volume for slider bar. Will be mapped to dB. """
4040

41+
MIN_VOL_F_OVERFLOW = MIN_VOL_F - MAX_VOL_F
42+
"""Min overflow for volume sliders, set to be the full range of vol_f below zero"""
43+
44+
MAX_VOL_F_OVERFLOW = MAX_VOL_F - MIN_VOL_F
45+
"""Max overflow for volume sliders, set to be the full range of vol_f above zero"""
46+
4147
MIN_VOL_DB = -80
4248
""" Min volume in dB. -80 is special and is actually -90 dB (mute). """
4349

@@ -111,6 +117,8 @@ class fields_w_default(SimpleNamespace):
111117
Volume = Field(default=MIN_VOL_DB, ge=MIN_VOL_DB, le=MAX_VOL_DB, description='Output volume in dB')
112118
VolumeF = Field(default=MIN_VOL_F, ge=MIN_VOL_F, le=MAX_VOL_F,
113119
description='Output volume as a floating-point scalar from 0.0 to 1.0 representing MIN_VOL_DB to MAX_VOL_DB')
120+
VolumeFOverflow = Field(default=0.0, ge=MIN_VOL_F_OVERFLOW, le=MAX_VOL_F_OVERFLOW,
121+
description='Output volume as a floating-point scalar that has a range equal to MIN_VOL_F - MAX_VOL_F in both directions from zero, and is used to keep track of the relative distance between two or more zone volumes when they would otherwise have to exceed their VOL_F range')
114122
VolumeMin = Field(default=MIN_VOL_DB, ge=MIN_VOL_DB, le=MAX_VOL_DB,
115123
description='Min output volume in dB')
116124
VolumeMax = Field(default=MAX_VOL_DB, ge=MIN_VOL_DB, le=MAX_VOL_DB,
@@ -321,6 +329,7 @@ class Zone(Base):
321329
mute: bool = fields_w_default.Mute
322330
vol: int = fields_w_default.Volume
323331
vol_f: float = fields_w_default.VolumeF
332+
vol_f_overflow: float = fields_w_default.VolumeFOverflow
324333
vol_min: int = fields_w_default.VolumeMin
325334
vol_max: int = fields_w_default.VolumeMax
326335
disabled: bool = fields_w_default.Disabled

tests/test_rest.py

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -605,20 +605,44 @@ def test_patch_zones_vol_delta(client):
605605
# check that each update worked as expected
606606
for z in jrv['zones']:
607607
if z['id'] in range(6):
608-
assert z['vol_f'] - (zones[z['id']]['vol_f'] + 0.1) < 0.0001
608+
assert z['vol_f'] - (zones[z['id']]['vol_f'] + 0.1) < 0.0001 and z["vol_f_overflow"] == 0
609609

610-
# test oversized deltas
611-
rv = client.patch('/api/zones', json={'zones': [z['id'] for z in zones], 'update': {'vol_delta_f': -10.0}})
610+
# test overflowing deltas
611+
rv = client.patch('/api/zones', json={'zones': [z['id'] for z in zones], 'update': {'vol_delta_f': -1.0}})
612612
assert rv.status_code == HTTPStatus.OK
613613
jrv = rv.json()
614614
assert len(jrv['zones']) >= 6
615615
# check that each update worked as expected
616616
for z in jrv['zones']:
617617
if z['id'] in range(6):
618618
assert z['vol_f'] == amplipi.models.MIN_VOL_F
619+
assert z["vol_f_overflow"] == zones[z['id']]['vol_f'] + 0.1 - 1
620+
621+
# test oversized overflowing deltas
622+
rv = client.patch('/api/zones', json={'zones': [z['id'] for z in zones], 'update': {'vol_delta_f': 10.0}})
623+
assert rv.status_code == HTTPStatus.OK
624+
jrv = rv.json()
625+
assert len(jrv['zones']) >= 6
626+
# check that each update worked as expected
627+
for z in jrv['zones']:
628+
if z['id'] in range(6):
629+
assert z['vol_f'] == amplipi.models.MAX_VOL_F
630+
assert z["vol_f_overflow"] == amplipi.models.MAX_VOL_F_OVERFLOW
631+
632+
# test overflow reset
633+
mid_vol_f = (amplipi.models.MIN_VOL_F + amplipi.models.MAX_VOL_F) / 2
634+
rv = client.patch('/api/zones', json={'zones': [z['id'] for z in zones], 'update': {'vol_f': mid_vol_f}})
635+
assert rv.status_code == HTTPStatus.OK
636+
jrv = rv.json()
637+
assert len(jrv['zones']) >= 6
638+
# check that each update worked as expected
639+
for z in jrv['zones']:
640+
if z['id'] in range(6):
641+
assert z['vol_f'] == mid_vol_f
642+
assert z["vol_f_overflow"] == 0
619643

620644
# test precedence
621-
rv = client.patch('/api/zones', json={'zones': [z['id'] for z in zones], 'update': {'vol_delta_f': 10.0, "vol": amplipi.models.MIN_VOL_DB}})
645+
rv = client.patch('/api/zones', json={'zones': [z['id'] for z in zones], 'update': {'vol_delta_f': 1.0, "vol": amplipi.models.MIN_VOL_DB}})
622646
assert rv.status_code == HTTPStatus.OK
623647
jrv = rv.json()
624648
assert len(jrv['zones']) >= 6

web/src/App.jsx

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,12 @@ export const useStatusStore = create((set, get) => ({
5050
applyPlayerVol(vol, zones, sourceId, (zone_id, new_vol) => {
5151
for (const i in s.status.zones) {
5252
if (s.status.zones[i].id === zone_id) {
53-
s.status.zones[i].vol_f = new_vol;
53+
// Calculate vol_f and vol_f_overflow to match expected future API polled state
54+
let combined_vol = new_vol + s.status.zones[i].vol_f_overflow;
55+
let new_vol_f = Math.min(Math.max(combined_vol, 0), 1);
56+
57+
s.status.zones[i].vol_f = new_vol_f;
58+
s.status.zones[i].vol_f_overflow = combined_vol - new_vol_f;
5459
}
5560
}
5661
});
@@ -61,11 +66,17 @@ export const useStatusStore = create((set, get) => ({
6166
setZonesMute: (mute, zones, source_id) => {
6267
set(
6368
produce((s) => {
64-
for (const i of getSourceZones(source_id, zones)) {
65-
for (const j of s.status.zones) {
66-
if (j.id === i.id) {
67-
j.mute = mute;
68-
}
69+
const affectedZones = getSourceZones(source_id, zones).map(z => z.id);
70+
for (const j of s.status.zones) {
71+
if (affectedZones.includes(j.id)) {
72+
j.mute = mute;
73+
}
74+
}
75+
76+
// Mute groups if they are now completely muted
77+
for (const g of s.status.groups) {
78+
if (g.zones.every(zid => affectedZones.includes(zid))) {
79+
g.mute = mute;
6980
}
7081
}
7182
})
@@ -164,6 +175,8 @@ export const useStatusStore = create((set, get) => ({
164175
const g = s.status.groups.filter((g) => g.id === groupId)[0];
165176
for (const i of g.zones) {
166177
s.skipUpdate = true;
178+
// vol_f_overflow is set to 0 whenever vol_f is between 0 and 1, groups authoritatively set the volume so we reflect that here too
179+
s.status.zones[i].vol_f_overflow = 0;
167180
s.status.zones[i].vol_f = new_vol;
168181
}
169182

@@ -198,7 +211,8 @@ export const useStatusStore = create((set, get) => ({
198211
const updateGroupVols = (s) => {
199212
s.status.groups.forEach((g) => {
200213
if (g.zones.length > 1) {
201-
const vols = g.zones.map((id) => s.status.zones[id].vol_f);
214+
// Combine vol_f with vol_f_overflow to ensure the group volume slider moves at the same relative speed even when a zone overflows
215+
const vols = g.zones.map((id) => s.status.zones[id].vol_f + s.status.zones[id].vol_f_overflow);
202216
let calculated_vol = Math.min(...vols) * 0.5 + Math.max(...vols) * 0.5;
203217
g.vol_f = calculated_vol;
204218
} else if (g.zones.length == 1) {
@@ -226,14 +240,14 @@ Page.propTypes = {
226240

227241
const App = ({ selectedPage }) => {
228242
return (
229-
<div className="app">
243+
<div className="app">
230244
<DisconnectedIcon />
231245
<div className="background-gradient"></div> {/* Used to make sure the background doesn't stretch or stop prematurely on scrollable pages */}
232-
<div className="app-body">
233-
<Page selectedPage={selectedPage} />
234-
</div>
235-
<MenuBar pageNumber={selectedPage} />
246+
<div className="app-body">
247+
<Page selectedPage={selectedPage} />
236248
</div>
249+
<MenuBar pageNumber={selectedPage} />
250+
</div>
237251
);
238252
};
239253
App.propTypes = {

web/src/components/CardVolumeSlider/CardVolumeSlider.jsx

Lines changed: 42 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const getPlayerVol = (sourceId, zones) => {
1111
let n = 0;
1212
for (const i of getSourceZones(sourceId, zones)) {
1313
n += 1;
14-
vol += i.vol_f;
14+
vol += i.vol_f + i.vol_f_overflow; // Add buffer to retain proper relative space when doing an action that would un-overload the slider
1515
}
1616

1717
const avg = vol / n;
@@ -27,22 +27,23 @@ export const applyPlayerVol = (vol, zones, sourceId, apply) => {
2727
let delta = vol - getPlayerVol(sourceId, zones);
2828

2929
for (let i of getSourceZones(sourceId, zones)) {
30-
let set_pt = Math.max(0, Math.min(1, i.vol_f + delta));
31-
apply(i.id, set_pt);
30+
apply(i.id, i.vol_f + delta);
3231
}
3332
};
3433

35-
// cumulativeDelta reflects the amount of movement that the
36-
let cumulativeDelta = 0;
34+
// cumulativeDelta reflects the amount of movement that the volume bar has had that has gone unreflected in the backend
35+
let cumulativeDelta = 0.0;
36+
let previous_delta = null;
3737
let sendingPacketCount = 0;
3838

3939
// main volume slider on player and volume slider on player card
4040
const CardVolumeSlider = ({ sourceId }) => {
4141
const zones = useStatusStore((s) => s.status.zones);
4242
const setZonesVol = useStatusStore((s) => s.setZonesVol);
4343
const setZonesMute = useStatusStore((s) => s.setZonesMute);
44+
const setSystemState = useStatusStore((s) => s.setSystemState);
4445

45-
// needed to ensure that polling doesn't cause the delta volume to be made inacurrate during volume slider interactions
46+
// needed to ensure that polling doesn't cause the delta volume to be made inaccurate during volume slider interactions
4647
const skipNextUpdate = useStatusStore((s) => s.skipNextUpdate);
4748

4849
const value = getPlayerVol(sourceId, zones);
@@ -52,32 +53,42 @@ const CardVolumeSlider = ({ sourceId }) => {
5253
setZonesMute(false, zones, sourceId);
5354
};
5455

55-
function setPlayerVol(vol, val) {
56-
cumulativeDelta += vol - val;
57-
58-
if(sendingPacketCount <= 0){
56+
function setPlayerVol(force = false) {
57+
if(sendingPacketCount <= 0 || force){ // Sometimes slide and release interactions can send the same request twice, this check helps prevent that from doubling the intended change
5958
sendingPacketCount += 1;
6059

6160
const delta = cumulativeDelta;
62-
63-
fetch("/api/zones", {
64-
method: "PATCH",
65-
headers: {
66-
"Content-type": "application/json",
67-
},
68-
body: JSON.stringify({
69-
zones: getSourceZones(sourceId, zones).map((z) => z.id),
70-
update: { vol_delta_f: cumulativeDelta, mute: false },
71-
}),
72-
}).then(() => {
73-
// NOTE: This used to just set cumulativeDelta to 0
74-
// that would skip all accumulated delta from fetch start to backend response time
75-
// causing jittering issues
76-
cumulativeDelta -= delta;
61+
if(previous_delta !== delta){
62+
previous_delta = delta;
63+
64+
fetch("/api/zones", {
65+
method: "PATCH",
66+
headers: {
67+
"Content-type": "application/json",
68+
},
69+
body: JSON.stringify({
70+
zones: getSourceZones(sourceId, zones).map((z) => z.id),
71+
update: { vol_delta_f: delta, mute: false },
72+
}),
73+
}).then(() => {
74+
// NOTE: This used to just set cumulativeDelta to 0
75+
// that would skip all accumulated delta from fetch start to backend response time
76+
// causing jittering issues
77+
if(force){
78+
cumulativeDelta = 0.0; // If you force push a change, do not store any unrecognized changes
79+
} else {
80+
cumulativeDelta -= delta; // If this is a normal change, there is likely a change coming up and setting to zero would lose part of that upcoming delta
81+
}
82+
sendingPacketCount -= 1;
83+
// In many similar requests we instantly consume the response by doing something like this:
84+
// if(res.ok){res.json().then(s=> setSystemState(s));}
85+
// This cannot be done here due to how rapid fire the requests can be when sliding the slider rather than just tapping it
86+
});
87+
} else {
7788
sendingPacketCount -= 1;
78-
});
89+
}
7990
}
80-
};
91+
}
8192

8293
const mute = getSourceZones(sourceId, zones)
8394
.map((z) => z.mute)
@@ -95,6 +106,8 @@ const CardVolumeSlider = ({ sourceId }) => {
95106
zones: getSourceZones(sourceId, zones).map((z) => z.id),
96107
update: { mute: mute },
97108
}),
109+
}).then(res => {
110+
if(res.ok){res.json().then(s => setSystemState(s));}
98111
});
99112
};
100113

@@ -107,9 +120,9 @@ const CardVolumeSlider = ({ sourceId }) => {
107120
setVol={(val, force) => {
108121
// Cannot use value directly as that changes during the request when setValue() is called
109122
// Cannot call setValue() as a .then() after the request as that causes the ui to feel unresponsive and choppy
110-
let current_val = value;
111-
setPlayerVol(val, current_val);
123+
cumulativeDelta += Math.round((val - value) * 1000) / 1000;
112124
setValue(val);
125+
setPlayerVol(force);
113126
skipNextUpdate();
114127
}}
115128
disabled={getSourceZones(sourceId, zones) == 0}

0 commit comments

Comments
 (0)