Skip to content

Commit 55b7cc2

Browse files
committed
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.
1 parent b95fdec commit 55b7cc2

2 files changed

Lines changed: 79 additions & 15 deletions

File tree

internal/airplay/mirror.go

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -642,17 +642,27 @@ func (c *AirPlayClient) setupMirrorSession(ctx context.Context, cfg StreamConfig
642642
}
643643
}
644644

645-
// Set volume to 0 dB (full scale). Positive dB values are invalid here and
646-
// current receivers may interpret them as zero gain.
647-
volumeBody := audioVolumeBody(false)
648-
_, _, err = c.rtspRequest("SET_PARAMETER", audioURI, "text/parameters", volumeBody, nil)
649-
if err != nil {
650-
dbg("[SETUP] SET_PARAMETER volume failed (non-fatal): %v", err)
645+
// Only set the receiver's volume when this session actually carries audio.
646+
// `volume: 0.000000` is 0 dB — full scale in AirPlay, not silence — so a
647+
// video-only session would force the receiver to maximum on every connect.
648+
// Sending -144 instead would be equally destructive of the user's setting;
649+
// a session that transmits no audio has no use for the receiver's audio
650+
// state and should leave its volume untouched.
651+
if cfg.NoAudio {
652+
dbg("[SETUP] no-audio session: skipping SET_PARAMETER volume")
651653
} else {
652-
dbg("[SETUP] SET_PARAMETER volume=0 sent")
654+
// Positive dB values are invalid here and current receivers may
655+
// interpret them as zero gain. Real senders send the sender's own
656+
// slider value; 0 dB is this sender's fixed choice.
657+
volumeBody := audioVolumeBody(false)
658+
if _, _, err := c.rtspRequest("SET_PARAMETER", audioURI, "text/parameters", volumeBody, nil); err != nil {
659+
dbg("[SETUP] SET_PARAMETER volume failed (non-fatal): %v", err)
660+
} else {
661+
dbg("[SETUP] SET_PARAMETER volume=0 sent")
662+
}
663+
// Send volume twice (pcap shows real senders do this)
664+
_, _, _ = c.rtspRequest("SET_PARAMETER", audioURI, "text/parameters", volumeBody, nil)
653665
}
654-
// Send volume twice (pcap shows real senders do this)
655-
_, _, _ = c.rtspRequest("SET_PARAMETER", audioURI, "text/parameters", volumeBody, nil)
656666

657667
if timingProtocol == timingProtocolPTP {
658668
// 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 {
17411751
if s == nil || s.client == nil || s.sessionURI == "" {
17421752
return fmt.Errorf("audio control unavailable")
17431753
}
1754+
// A session started with audio disabled must not write the receiver's
1755+
// volume either: unmuting sends 0 dB — full scale — which would discard
1756+
// whatever the user had set, exactly as the setup path once did. The
1757+
// session negotiates an audio stream even in this mode, so HasAudio() is
1758+
// not sufficient to tell the two apart.
1759+
if s.noAudio {
1760+
return fmt.Errorf("audio control unavailable: session was started with audio disabled")
1761+
}
17441762

17451763
if _, _, err := s.client.rtspRequest("SET_PARAMETER", s.sessionURI, "text/parameters", audioVolumeBody(muted), nil); err != nil {
17461764
return fmt.Errorf("set audio muted=%t: %w", muted, err)

internal/airplay/mirror_setup_test.go

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -201,12 +201,52 @@ func TestSetupMirrorNoAudioStillNegotiatesAudioSession(t *testing.T) {
201201
{name: "skip record", skipRecord: true},
202202
} {
203203
t.Run(test.name, func(t *testing.T) {
204-
testSetupMirrorNoAudioStillNegotiatesAudioSession(t, test.skipRecord)
204+
testSetupMirrorAudioSessionNegotiation(t, audioSessionCase{skipRecord: test.skipRecord, noAudio: true})
205205
})
206206
}
207207
}
208208

209-
func testSetupMirrorNoAudioStillNegotiatesAudioSession(t *testing.T, skipRecord bool) {
209+
// A session that DOES carry audio must still set the receiver's volume, so the
210+
// -no-audio guard narrows that behaviour rather than removing it.
211+
func TestSetupMirrorWithAudioSetsReceiverVolume(t *testing.T) {
212+
for _, test := range []struct {
213+
name string
214+
skipRecord bool
215+
}{
216+
{name: "record", skipRecord: false},
217+
{name: "skip record", skipRecord: true},
218+
} {
219+
t.Run(test.name, func(t *testing.T) {
220+
testSetupMirrorAudioSessionNegotiation(t, audioSessionCase{skipRecord: test.skipRecord, noAudio: false})
221+
})
222+
}
223+
}
224+
225+
// A no-audio session must not be able to write the receiver's volume through
226+
// the daemon's mute control either: unmuting sends 0 dB, which is full scale.
227+
func TestSetAudioMutedRefusesWhenSessionHasNoAudio(t *testing.T) {
228+
for _, muted := range []bool{true, false} {
229+
session := &MirrorSession{client: &AirPlayClient{}, sessionURI: "rtsp://example/session", noAudio: true}
230+
err := session.SetAudioMuted(muted)
231+
if err == nil {
232+
t.Fatalf("SetAudioMuted(%v) = nil, want a refusal for a no-audio session", muted)
233+
}
234+
// Assert the REASON, not merely that something failed: without the
235+
// guard this call still errors (no connection), so an error alone
236+
// would pass on the unfixed code.
237+
if !strings.Contains(err.Error(), "audio disabled") {
238+
t.Fatalf("SetAudioMuted(%v) error = %q, want a refusal naming the disabled audio", muted, err)
239+
}
240+
}
241+
}
242+
243+
type audioSessionCase struct {
244+
skipRecord bool
245+
noAudio bool
246+
}
247+
248+
func testSetupMirrorAudioSessionNegotiation(t *testing.T, test audioSessionCase) {
249+
skipRecord, noAudio := test.skipRecord, test.noAudio
210250
eventListener, err := net.Listen("tcp", "127.0.0.1:0")
211251
if err != nil {
212252
t.Fatalf("listen event channel: %v", err)
@@ -448,12 +488,12 @@ func testSetupMirrorNoAudioStillNegotiatesAudioSession(t *testing.T, skipRecord
448488
}
449489
defer client.Close()
450490

451-
session, err := client.SetupMirror(ctx, StreamConfig{FPS: 30, NoAudio: true})
491+
session, err := client.SetupMirror(ctx, StreamConfig{FPS: 30, NoAudio: noAudio})
452492
if err != nil {
453-
t.Fatalf("SetupMirror(no audio): %v", err)
493+
t.Fatalf("SetupMirror(noAudio=%v): %v", noAudio, err)
454494
}
455495
if !session.HasAudio() {
456-
t.Fatal("expected no-audio session setup to keep the negotiated audio stream state")
496+
t.Fatalf("noAudio=%v: expected the negotiated audio stream state to survive setup", noAudio)
457497
}
458498
if !skipRecord && session.timestampBias != 250*time.Millisecond {
459499
t.Fatalf("session timestamp bias = %v, want RECORD Audio-Latency of 250ms", session.timestampBias)
@@ -488,7 +528,13 @@ func testSetupMirrorNoAudioStillNegotiatesAudioSession(t *testing.T, skipRecord
488528
recordIndex = len(wantMethods)
489529
wantMethods = append(wantMethods, "RECORD")
490530
}
491-
wantMethods = append(wantMethods, "SET_PARAMETER", "SET_PARAMETER", "POST", "TEARDOWN")
531+
// The two volume SET_PARAMETERs are sent only when the session carries
532+
// audio: `volume: 0.000000` is 0 dB — full scale — so a video-only
533+
// session would force the receiver to maximum.
534+
if !noAudio {
535+
wantMethods = append(wantMethods, "SET_PARAMETER", "SET_PARAMETER")
536+
}
537+
wantMethods = append(wantMethods, "POST", "TEARDOWN")
492538
got := make([]rtspTestRequest, 0, len(wantMethods))
493539
for range wantMethods {
494540
select {

0 commit comments

Comments
 (0)