feat(player): leave the volume to the system where the system owns it

On Android the hardware keys are the volume control and the framework
mixes our stream against the device level, so a second control inside
the app moves something the user already moved.  Where that is true the
player's level sits at maximum, SetVolume / ChangeVolume / MuteToggle
are refused, and nothing persists a level nobody chose: restore
remembers the stored value instead of applying it, and saveState writes
that same value back rather than recording the synthetic maximum.

Mute is in that list because it is a level of zero by another name --
and because with no control rendered it would be the one state on such
a platform the user could not get out of.

The predicate is named after the capability rather than the platform,
because that is what makes it testable.  Only platformOwnsVolume is
behind a build tag, in two files that declare nothing else; everything
else is decided against Player.systemVolume, a field a test sets either
way.  That is mediacontrols' split, with androidpayload.go's reasoning
for keeping the contract out of a tagged file, and the tagged pair is
covered by a source sweep since no tier here compiles both halves.

SetDuck is deliberately untouched: it applies its attenuation by
re-applying the *user's* level through setVolumeLocked, so pinning that
level to maximum leaves the offset arithmetic exactly as it was.  It is
the only thing that may still move the output on such a platform, and
TestSystemVolumeStillDucks is that property rather than a comment.

backend/player/player.go

@@ -71,6 +71,19 @@ type Player struct {
 	// not something the user chose.
 	duckAmount float64
 
+	// systemVolume is what SystemOwnsVolume answers: the platform's own
+	// control is the only one, so ours neither acts nor persists.  It is
+	// a field rather than the build constant read directly so that a
+	// test can exercise both sides on any machine.  See systemvolume.go.
+	systemVolume bool
+
+	// storedVolume and storedMuted hold the persisted level as it was
+	// found at restore, for a platform whose volume we do not own: the
+	// maximum we then run at is not a level the user chose, so saveState
+	// writes back what it read rather than overwriting it.
+	storedVolume UserVolume
+	storedMuted  bool
+
 	// trackLengthMs holds the authoritative track duration in
 	// milliseconds, sourced from the database (which uses the
 	// custom header parser).  The go-mp3 decoder's Len() can be
@@ -156,6 +169,8 @@ func NewPlayer(logger *slog.Logger, db *database.DB) *Player {
 		logger:       logger,
 		db:           db,
 		state:        Stopped,
+		systemVolume: platformOwnsVolume,
+		storedVolume: DefaultUserVol,
 		baseStreamer: generators.Silence(-1),
 		format: beep.Format{
 			SampleRate: speakerSampleRate,
@@ -875,6 +890,10 @@ func (p *Player) SetVolume(desiredVolume UserVolume) {
 	p.mu.Lock()
 	defer p.mu.Unlock()
 
+	if p.systemVolume {
+		return
+	}
+
 	p.setVolumeLocked(desiredVolume)
 	p.emitVolumeChanged()
 	p.saveState()
@@ -923,6 +942,10 @@ func (p *Player) ChangeVolume(deltaVolume int) error {
 	p.mu.Lock()
 	defer p.mu.Unlock()
 
+	if p.systemVolume {
+		return nil
+	}
+
 	p.setVolumeLocked(p.getUserVolume() + UserVolume(deltaVolume))
 	p.emitVolumeChanged()
 	p.saveState()
@@ -953,6 +976,14 @@ func (p *Player) MuteToggle() error {
 		return errNoAudioFileLoaded
 	}
 
+	// Mute is a level of zero by another name, so it goes with the rest
+	// of the volume where the system owns it -- and it would be the one
+	// state on such a platform the user could not get out of, since with
+	// no control rendered there is nothing left to un-mute with.
+	if p.systemVolume {
+		return nil
+	}
+
 	speaker.Lock()
 	p.volume.Silent = !p.volume.Silent
 	speaker.Unlock()
@@ -1403,7 +1434,15 @@ func (p *Player) saveState() {
 	volume := int64(DefaultUserVol)
 	muted := false
 
-	if p.volume != nil {
+	switch {
+	case p.systemVolume:
+		// The maximum this platform runs at is not a level anybody
+		// chose, so it is not one to remember.  Writing back what
+		// restore found keeps the row a description of the user's
+		// setting without needing a second query that omits the column.
+		volume = int64(p.storedVolume)
+		muted = p.storedMuted
+	case p.volume != nil:
 		volume = int64(p.getUserVolume())
 		muted = p.volume.Silent
 	}
@@ -1487,11 +1526,20 @@ func (p *Player) restoreStateLocked() {
 		}
 	}
 
-	vol := clampVolume(UserVolume(state.Volume))
-	p.setVolumeLocked(vol)
+	if p.systemVolume {
+		// Remembered, not applied: the device's keys are the volume
+		// control here, so the player runs wide open and hands the
+		// stored level back untouched at the next save.
+		p.storedVolume = clampVolume(UserVolume(state.Volume))
+		p.storedMuted = state.Muted
+		p.setVolumeLocked(MaxUserVol)
+	} else {
+		vol := clampVolume(UserVolume(state.Volume))
+		p.setVolumeLocked(vol)
 
-	if state.Muted {
-		p.volume.Silent = true
+		if state.Muted {
+			p.volume.Silent = true
+		}
 	}
 
 	// Restore last track if the file still exists.
@@ -1531,8 +1579,9 @@ func (p *Player) restoreStateLocked() {
 	}
 
 	p.logger.Info("Player state restored",
-		"volume", vol,
-		"muted", state.Muted,
+		"volume", p.getUserVolume(),
+		"muted", p.volume.Silent,
+		"systemVolume", p.systemVolume,
 		"trackPath", state.LastTrackPath,
 		"positionSeconds", state.LastPositionSeconds,
 	)

backend/player/systemvolume.go

@@ -0,0 +1,42 @@
+package player
+
+// Who owns the volume, and what follows when it is not us.
+//
+// On Android the hardware keys *are* the volume control and the
+// framework mixes our stream against the device level, so a second
+// control inside the app is a slider that moves something the user
+// already moved (#64).  Where that is true the player's own level sits
+// at maximum, nothing changes it, and nothing persists it.
+//
+// **The predicate is named after the capability, not the platform.**
+// The frontend asks "is there a volume for me to control", which is a
+// question about this build; asking "is this a phone" instead would
+// key the answer to a viewport, and an Android tablet at 600px or more
+// would then draw the bottom bar's slider over a level pinned at
+// maximum -- a control that cannot act, which is the thing
+// `library-status-indicator` already settled is worse than none.
+//
+// **Only `platformOwnsVolume` is behind a build tag**, in two files
+// that declare nothing else.  A tagged file is compiled by nothing
+// `make lint` or `make test` runs and is untestable off a phone, which
+// is the reasoning `mediacontrols/androidpayload.go` states for
+// keeping its contract out of one -- so everything decidable here is
+// decided against `Player.systemVolume`, a field a test sets either
+// way, and the tag decides only what that field starts as.
+//
+// The one thing this must not disturb is ducking.  `SetDuck` applies
+// its attenuation by re-applying the *user's* level through
+// `setVolumeLocked`, so pinning that level to maximum leaves the
+// offset arithmetic exactly as it was: an OS asking us to get out of
+// the way of a navigation prompt is not the user setting a volume, and
+// it is the only thing that may move the output on such a platform.
+
+// SystemOwnsVolume reports whether the platform's own control is the
+// only volume control there is, so this app neither offers one nor
+// remembers a level.
+//
+// It is bound: the frontend renders no `<volume-control>` when it is
+// true, at any width.
+func (p *Player) SystemOwnsVolume() bool {
+	return p.systemVolume
+}

backend/player/systemvolume_android.go

@@ -0,0 +1,11 @@
+//go:build android
+
+package player
+
+// platformOwnsVolume is true on Android: volume is the device's, set
+// with the hardware keys, and `mediacontrols`' Android handler
+// implements no volume callback for the same reason.
+//
+// See systemvolume.go for why this constant is the whole of what a
+// build tag decides here.
+const platformOwnsVolume = true

backend/player/systemvolume_other.go

@@ -0,0 +1,10 @@
+//go:build !android
+
+package player
+
+// platformOwnsVolume is false everywhere but Android: a desktop mixer
+// is per-application, so our level is the one the user reaches for.
+//
+// See systemvolume.go for why this constant is the whole of what a
+// build tag decides here.
+const platformOwnsVolume = false

backend/player/systemvolume_test.go

@@ -0,0 +1,215 @@
+package player
+
+import (
+	"log/slog"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/gopxl/beep/v2/effects"
+
+	"yellowjacket/backend/database"
+)
+
+// pinnedPlayer is a player on a platform whose volume belongs to the
+// device.  The field is set rather than the build constant read,
+// because the constant is true on exactly one platform and no tier
+// here runs on it -- see systemvolume.go.
+func pinnedPlayer(t *testing.T, db *database.DB) *Player {
+	t.Helper()
+
+	p := NewPlayer(slog.Default(), db)
+	p.systemVolume = true
+	p.volume = &effects.Volume{Base: 2}
+	p.setVolumeLocked(MaxUserVol)
+
+	return p
+}
+
+// TestSystemVolumeRefusesEveryWayToChangeTheLevel is the first half of
+// #64: where the device owns the volume, ours sits at maximum and none
+// of the three routes to a level moves it.  Mute is in that list
+// because it is a level of zero by another name, and because with no
+// control rendered it is the one state on such a platform there would
+// be nothing to get out of.
+func TestSystemVolumeRefusesEveryWayToChangeTheLevel(t *testing.T) {
+	t.Parallel()
+
+	p := pinnedPlayer(t, nil)
+
+	if !p.SystemOwnsVolume() {
+		t.Fatal("SystemOwnsVolume() = false on a pinned player")
+	}
+
+	if got := p.getUserVolume(); got != MaxUserVol {
+		t.Errorf("starting volume = %d, want %d", got, MaxUserVol)
+	}
+
+	p.SetVolume(20)
+
+	if got := p.getUserVolume(); got != MaxUserVol {
+		t.Errorf("volume after SetVolume(20) = %d, want %d", got, MaxUserVol)
+	}
+
+	if err := p.ChangeVolume(-30); err != nil {
+		t.Fatalf("ChangeVolume: %v", err)
+	}
+
+	if got := p.getUserVolume(); got != MaxUserVol {
+		t.Errorf("volume after ChangeVolume(-30) = %d, want %d", got, MaxUserVol)
+	}
+
+	if err := p.MuteToggle(); err != nil {
+		t.Fatalf("MuteToggle: %v", err)
+	}
+
+	if p.volume.Silent {
+		t.Error("MuteToggle silenced a player whose volume the system owns")
+	}
+}
+
+// TestAnUnpinnedPlayerStillChangesItsVolume is the other side of the
+// same switch.  Without it the test above passes on a player that
+// refuses everything, which is what a mis-wired field would produce.
+func TestAnUnpinnedPlayerStillChangesItsVolume(t *testing.T) {
+	t.Parallel()
+
+	p := NewPlayer(slog.Default(), nil)
+	p.volume = &effects.Volume{Base: 2}
+	p.setVolumeLocked(MaxUserVol)
+
+	if p.SystemOwnsVolume() {
+		t.Fatal("SystemOwnsVolume() = true off Android")
+	}
+
+	p.SetVolume(20)
+
+	if got := p.getUserVolume(); got != 20 {
+		t.Errorf("volume after SetVolume(20) = %d, want 20", got)
+	}
+
+	if err := p.MuteToggle(); err != nil {
+		t.Fatalf("MuteToggle: %v", err)
+	}
+
+	if !p.volume.Silent {
+		t.Error("MuteToggle did not silence an ordinary player")
+	}
+}
+
+// TestSystemVolumeStillDucks is the issue's second Finding, made a
+// test: pinning the user's level must leave the OS's attenuation
+// working, because a duck is not a volume the user chose and is the
+// only thing that may move the output on such a platform.
+func TestSystemVolumeStillDucks(t *testing.T) {
+	t.Parallel()
+
+	p := pinnedPlayer(t, nil)
+	open := p.volume.Volume
+
+	p.SetDuck(true)
+
+	if p.volume.Volume >= open {
+		t.Errorf(
+			"ducked output = %v, want less than %v", p.volume.Volume, open,
+		)
+	}
+
+	if got := p.getUserVolume(); got != MaxUserVol {
+		t.Errorf("user volume while ducked = %d, want %d", got, MaxUserVol)
+	}
+
+	// A refused SetVolume must not disturb the offset either: it
+	// returns before setVolumeLocked, which is what re-applies it.
+	ducked := p.volume.Volume
+
+	p.SetVolume(10)
+
+	if p.volume.Volume != ducked {
+		t.Errorf(
+			"output after a refused SetVolume = %v, want %v",
+			p.volume.Volume, ducked,
+		)
+	}
+
+	p.SetDuck(false)
+
+	if p.volume.Volume != open {
+		t.Errorf("output after unduck = %v, want %v", p.volume.Volume, open)
+	}
+}
+
+// TestSystemVolumeWritesBackTheLevelItFound is the rest of the
+// Direction: "make sure nothing writes a persisted volume from that
+// platform".  The maximum the player runs at is synthetic, so saving
+// must not record it over whatever the row already said.
+func TestSystemVolumeWritesBackTheLevelItFound(t *testing.T) {
+	t.Parallel()
+
+	db := database.NewTestDB(t)
+
+	// A level set by some earlier, unpinned session.
+	writer := NewPlayer(slog.Default(), db)
+	writer.volume = &effects.Volume{Base: 2}
+	writer.setVolumeLocked(30)
+	writer.SaveState()
+
+	p := pinnedPlayer(t, db)
+	p.RestoreState()
+
+	if got := p.getUserVolume(); got != MaxUserVol {
+		t.Errorf("restored volume = %d, want %d (the level is pinned)", got, MaxUserVol)
+	}
+
+	if p.volume.Silent {
+		t.Error("restore muted a player whose volume the system owns")
+	}
+
+	p.SaveState()
+
+	state, err := db.Queries.GetPlayerState(db.Ctx)
+	if err != nil {
+		t.Fatalf("GetPlayerState: %v", err)
+	}
+
+	if state.Volume != 30 {
+		t.Errorf("persisted volume = %d, want 30 (untouched)", state.Volume)
+	}
+}
+
+// TestPlatformVolumeOwnershipIsDeclaredOncePerPlatform sweeps the
+// source, because the pair of tagged files is the one thing here no
+// tier compiles both halves of: `make lint` and `make test` build the
+// `!android` side only, so a deleted or edited android file fails
+// nothing until somebody has a phone in their hand.
+func TestPlatformVolumeOwnershipIsDeclaredOncePerPlatform(t *testing.T) {
+	t.Parallel()
+
+	want := map[string]string{
+		"systemvolume_other.go":   "const platformOwnsVolume = false",
+		"systemvolume_android.go": "const platformOwnsVolume = true",
+	}
+
+	tags := map[string]string{
+		"systemvolume_other.go":   "//go:build !android",
+		"systemvolume_android.go": "//go:build android",
+	}
+
+	for name, decl := range want {
+		src, err := os.ReadFile(filepath.Join(".", name))
+		if err != nil {
+			t.Errorf("%s: %v", name, err)
+
+			continue
+		}
+
+		if !strings.Contains(string(src), decl) {
+			t.Errorf("%s does not declare %q", name, decl)
+		}
+
+		if !strings.Contains(string(src), tags[name]) {
+			t.Errorf("%s does not carry %q", name, tags[name])
+		}
+	}
+}

frontend/bindings/yellowjacket/backend/player/player.ts

@@ -142,6 +142,18 @@ export function SetVolume(desiredVolume: $models.UserVolume): $CancellablePromis
     return $Call.ByID(1375836663, desiredVolume);
 }
 
+/**
+ * SystemOwnsVolume reports whether the platform's own control is the
+ * only volume control there is, so this app neither offers one nor
+ * remembers a level.
+ * 
+ * It is bound: the frontend renders no `<volume-control>` when it is
+ * true, at any width.
+ */
+export function SystemOwnsVolume(): $CancellablePromise<boolean> {
+    return $Call.ByID(1027623185);
+}
+
 /**
  * TrackLengthInSeconds returns the duration of the current track.
  */