From 55b7cc277e263846207a384330aa04f3be0a4d4d Mon Sep 17 00:00:00 2001 From: Josiah Bryan Date: Thu, 20 Aug 2026 20:24:24 -0500 Subject: [PATCH] mirror: never write the receiver's volume when audio is disabled `volume: 0.000000` is 0 dB, which in AirPlay is full scale -- maximum, not silence. Two paths sent it for sessions that carry no audio, so `-no-audio` discarded whatever volume the user had set on the receiver. setupMirrorSession sent it unconditionally (twice) during setup, so every connect reset the receiver to maximum. On a TV, reconnecting a dashboard or a mirrored window slammed the set to full volume each time. SetAudioMuted had the same effect one click away. The daemon deliberately routes mute/unmute to the receiver when audio is disabled (daemon.go's `d.cfg.NoAudio || t.session.HasAudio()`), and unmuting sends audioVolumeBody (false) -- the identical full-scale value. A no-audio session still negotiates an audio stream, so HasAudio() reports true and the plasmoid offers a mute toggle for a video-only session; the first press silenced the receiver and the next set it to maximum. Guard both. A session that transmits no audio has no use for the receiver's audio state and should leave its volume exactly as it found it. Skipping is preferable to sending the muted value, since -144 dB would be equally destructive of the user's setting, just in the other direction. The SetAudioMuted guard reads MirrorSession.noAudio, which until now was assigned and never read. HasAudio() cannot substitute for it: it is true in both modes, which is what made this reachable. Echoing the receiver's own reported volume back instead was considered and rejected. On the Roku Streambar Pro tested here, `initialVolume` from /info stayed 0.0 across ten VolumeDown and six VolumeUp presses, so it does not track that receiver's current volume and sending it back would still command full scale. Other receivers may report it faithfully; this was not verified beyond the one device. TESTS TestSetupMirrorNoAudioStillNegotiatesAudioSession asserted the two volume SET_PARAMETERs as part of the expected RTSP sequence for a no-audio session, and was the only test covering that sequence -- so updating it alone would have left no coverage that audio sessions still set the volume. The shared helper is parameterized over noAudio and there are now two named tests, one per direction, plus TestSetAudioMutedRefusesWhenSessionHasNoAudio. Each test was checked against a mutation rather than merely observed green: reverting the setup fix fails the no-audio test, making the skip unconditional fails the with-audio test, inverting the condition fails both, and removing the SetAudioMuted guard fails the mute test. Verified on a Roku Streambar Pro (model 9101R2): -no-audio -> "[SETUP] no-audio session: skipping SET_PARAMETER volume" (sent=0, skipped=1) with audio -> "[SETUP] SET_PARAMETER volume=0 sent" (sent=1, skipped=0) gofmt clean, go vet clean, go test ./... passing. --- internal/airplay/mirror.go | 36 ++++++++++++----- internal/airplay/mirror_setup_test.go | 58 ++++++++++++++++++++++++--- 2 files changed, 79 insertions(+), 15 deletions(-) diff --git a/internal/airplay/mirror.go b/internal/airplay/mirror.go index ff7f83f..4923fe5 100644 --- a/internal/airplay/mirror.go +++ b/internal/airplay/mirror.go @@ -642,17 +642,27 @@ func (c *AirPlayClient) setupMirrorSession(ctx context.Context, cfg StreamConfig } } - // Set volume to 0 dB (full scale). Positive dB values are invalid here and - // current receivers may interpret them as zero gain. - volumeBody := audioVolumeBody(false) - _, _, err = c.rtspRequest("SET_PARAMETER", audioURI, "text/parameters", volumeBody, nil) - if err != nil { - dbg("[SETUP] SET_PARAMETER volume failed (non-fatal): %v", err) + // Only set the receiver's volume when this session actually carries audio. + // `volume: 0.000000` is 0 dB — full scale in AirPlay, not silence — so a + // video-only session would force the receiver to maximum on every connect. + // Sending -144 instead would be equally destructive of the user's setting; + // a session that transmits no audio has no use for the receiver's audio + // state and should leave its volume untouched. + if cfg.NoAudio { + dbg("[SETUP] no-audio session: skipping SET_PARAMETER volume") } else { - dbg("[SETUP] SET_PARAMETER volume=0 sent") + // Positive dB values are invalid here and current receivers may + // interpret them as zero gain. Real senders send the sender's own + // slider value; 0 dB is this sender's fixed choice. + volumeBody := audioVolumeBody(false) + if _, _, err := c.rtspRequest("SET_PARAMETER", audioURI, "text/parameters", volumeBody, nil); err != nil { + dbg("[SETUP] SET_PARAMETER volume failed (non-fatal): %v", err) + } else { + dbg("[SETUP] SET_PARAMETER volume=0 sent") + } + // Send volume twice (pcap shows real senders do this) + _, _, _ = c.rtspRequest("SET_PARAMETER", audioURI, "text/parameters", volumeBody, nil) } - // Send volume twice (pcap shows real senders do this) - _, _, _ = c.rtspRequest("SET_PARAMETER", audioURI, "text/parameters", volumeBody, nil) if timingProtocol == timingProtocolPTP { // PTP uses the receiver's fixed 319/320 ports. The first socket was only @@ -1741,6 +1751,14 @@ func (s *MirrorSession) SetAudioMuted(muted bool) error { if s == nil || s.client == nil || s.sessionURI == "" { return fmt.Errorf("audio control unavailable") } + // A session started with audio disabled must not write the receiver's + // volume either: unmuting sends 0 dB — full scale — which would discard + // whatever the user had set, exactly as the setup path once did. The + // session negotiates an audio stream even in this mode, so HasAudio() is + // not sufficient to tell the two apart. + if s.noAudio { + return fmt.Errorf("audio control unavailable: session was started with audio disabled") + } if _, _, err := s.client.rtspRequest("SET_PARAMETER", s.sessionURI, "text/parameters", audioVolumeBody(muted), nil); err != nil { return fmt.Errorf("set audio muted=%t: %w", muted, err) diff --git a/internal/airplay/mirror_setup_test.go b/internal/airplay/mirror_setup_test.go index a66ac5f..c57825d 100644 --- a/internal/airplay/mirror_setup_test.go +++ b/internal/airplay/mirror_setup_test.go @@ -201,12 +201,52 @@ func TestSetupMirrorNoAudioStillNegotiatesAudioSession(t *testing.T) { {name: "skip record", skipRecord: true}, } { t.Run(test.name, func(t *testing.T) { - testSetupMirrorNoAudioStillNegotiatesAudioSession(t, test.skipRecord) + testSetupMirrorAudioSessionNegotiation(t, audioSessionCase{skipRecord: test.skipRecord, noAudio: true}) }) } } -func testSetupMirrorNoAudioStillNegotiatesAudioSession(t *testing.T, skipRecord bool) { +// A session that DOES carry audio must still set the receiver's volume, so the +// -no-audio guard narrows that behaviour rather than removing it. +func TestSetupMirrorWithAudioSetsReceiverVolume(t *testing.T) { + for _, test := range []struct { + name string + skipRecord bool + }{ + {name: "record", skipRecord: false}, + {name: "skip record", skipRecord: true}, + } { + t.Run(test.name, func(t *testing.T) { + testSetupMirrorAudioSessionNegotiation(t, audioSessionCase{skipRecord: test.skipRecord, noAudio: false}) + }) + } +} + +// A no-audio session must not be able to write the receiver's volume through +// the daemon's mute control either: unmuting sends 0 dB, which is full scale. +func TestSetAudioMutedRefusesWhenSessionHasNoAudio(t *testing.T) { + for _, muted := range []bool{true, false} { + session := &MirrorSession{client: &AirPlayClient{}, sessionURI: "rtsp://example/session", noAudio: true} + err := session.SetAudioMuted(muted) + if err == nil { + t.Fatalf("SetAudioMuted(%v) = nil, want a refusal for a no-audio session", muted) + } + // Assert the REASON, not merely that something failed: without the + // guard this call still errors (no connection), so an error alone + // would pass on the unfixed code. + if !strings.Contains(err.Error(), "audio disabled") { + t.Fatalf("SetAudioMuted(%v) error = %q, want a refusal naming the disabled audio", muted, err) + } + } +} + +type audioSessionCase struct { + skipRecord bool + noAudio bool +} + +func testSetupMirrorAudioSessionNegotiation(t *testing.T, test audioSessionCase) { + skipRecord, noAudio := test.skipRecord, test.noAudio eventListener, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { t.Fatalf("listen event channel: %v", err) @@ -448,12 +488,12 @@ func testSetupMirrorNoAudioStillNegotiatesAudioSession(t *testing.T, skipRecord } defer client.Close() - session, err := client.SetupMirror(ctx, StreamConfig{FPS: 30, NoAudio: true}) + session, err := client.SetupMirror(ctx, StreamConfig{FPS: 30, NoAudio: noAudio}) if err != nil { - t.Fatalf("SetupMirror(no audio): %v", err) + t.Fatalf("SetupMirror(noAudio=%v): %v", noAudio, err) } if !session.HasAudio() { - t.Fatal("expected no-audio session setup to keep the negotiated audio stream state") + t.Fatalf("noAudio=%v: expected the negotiated audio stream state to survive setup", noAudio) } if !skipRecord && session.timestampBias != 250*time.Millisecond { t.Fatalf("session timestamp bias = %v, want RECORD Audio-Latency of 250ms", session.timestampBias) @@ -488,7 +528,13 @@ func testSetupMirrorNoAudioStillNegotiatesAudioSession(t *testing.T, skipRecord recordIndex = len(wantMethods) wantMethods = append(wantMethods, "RECORD") } - wantMethods = append(wantMethods, "SET_PARAMETER", "SET_PARAMETER", "POST", "TEARDOWN") + // The two volume SET_PARAMETERs are sent only when the session carries + // audio: `volume: 0.000000` is 0 dB — full scale — so a video-only + // session would force the receiver to maximum. + if !noAudio { + wantMethods = append(wantMethods, "SET_PARAMETER", "SET_PARAMETER") + } + wantMethods = append(wantMethods, "POST", "TEARDOWN") got := make([]rtspTestRequest, 0, len(wantMethods)) for range wantMethods { select {