diff --git a/src/protocol/command/playback_stream.rs b/src/protocol/command/playback_stream.rs index 7e76271..b8fa06e 100644 --- a/src/protocol/command/playback_stream.rs +++ b/src/protocol/command/playback_stream.rs @@ -13,6 +13,8 @@ pub struct PlaybackStreamParams { pub sample_spec: SampleSpec, /// Channel map for the stream. + /// + /// Number of channels should match `sample_spec.channels`. pub channel_map: ChannelMap, /// Index of the sink to connect to. @@ -28,6 +30,8 @@ pub struct PlaybackStreamParams { pub sync_id: u32, /// Volume of the stream. + /// + /// Number of channels should match `sample_spec.channels`. pub cvolume: Option, /// Additional properties for the stream. @@ -144,7 +148,10 @@ impl TagStructWrite for PlaybackStreamParams { ts.write_u32(self.buffer_attr.pre_buffering)?; ts.write_u32(self.buffer_attr.minimum_request_length)?; ts.write_u32(self.sync_id)?; - ts.write(self.cvolume.unwrap_or_default())?; + ts.write( + self.cvolume + .unwrap_or_else(|| ChannelVolume::muted(self.sample_spec.channels.into())), + )?; ts.write_bool(self.flags.no_remap_channels)?; ts.write_bool(self.flags.no_remix_channels)?; ts.write_bool(self.flags.fix_format)?; @@ -367,4 +374,61 @@ mod integration_tests { Ok(()) } + + /// Tests that PlaybackStreamParams maintains consistent + /// channel counts across its implicitly set fields. + #[test] + fn create_playback_stream_channel_count_invariants() -> anyhow::Result<()> { + let (mut sock, protocol_version) = connect_and_init()?; + + // Arbitrarily chosen number of channels that should be kept in sync + // across fields (chosen to test beyond the usual 1 or 2). + const CHANNEL_COUNT: u8 = 3; + + // Explicitly set case (for reference). + { + write_command_message( + sock.get_mut(), + 0, + &Command::CreatePlaybackStream(PlaybackStreamParams { + sample_spec: SampleSpec { + format: SampleFormat::S16Le, + channels: CHANNEL_COUNT, + ..Default::default() + }, + sync_id: 0, + channel_map: ChannelMap::with_n_channels(CHANNEL_COUNT), + cvolume: Some(ChannelVolume::norm(CHANNEL_COUNT)), + ..Default::default() + }), + protocol_version, + )?; + + let _ = read_reply_message::(&mut sock, protocol_version)?; + } + + // Implicitly set case (on fields that allow it). + { + write_command_message( + sock.get_mut(), + 1, + &Command::CreatePlaybackStream(PlaybackStreamParams { + sample_spec: SampleSpec { + format: SampleFormat::S16Le, + channels: CHANNEL_COUNT, + ..Default::default() + }, + sync_id: 1, + channel_map: ChannelMap::with_n_channels(CHANNEL_COUNT), + cvolume: None, + ..Default::default() + }), + protocol_version, + )?; + + let _ = read_reply_message::(&mut sock, protocol_version)?; + } + + Ok(()) + } } diff --git a/src/protocol/command/record_stream.rs b/src/protocol/command/record_stream.rs index d96f603..847bc50 100644 --- a/src/protocol/command/record_stream.rs +++ b/src/protocol/command/record_stream.rs @@ -13,6 +13,8 @@ pub struct RecordStreamParams { pub sample_spec: SampleSpec, /// Channel map for the stream. + /// + /// Number of channels should match `sample_spec.channels`. pub channel_map: ChannelMap, /// Index of the source to connect to. @@ -32,6 +34,8 @@ pub struct RecordStreamParams { pub direct_on_input_index: Option, /// Volume of the stream. + /// + /// Number of channels should match `sample_spec.channels`. pub cvolume: Option, /// Additional properties for the stream. @@ -163,7 +167,10 @@ impl TagStructWrite for RecordStreamParams { ts.write(format)?; } - ts.write(self.cvolume.unwrap_or_default())?; + ts.write( + self.cvolume + .unwrap_or_else(|| ChannelVolume::muted(self.sample_spec.channels.into())), + )?; ts.write_bool(self.flags.start_muted.unwrap_or_default())?; ts.write_bool(self.cvolume.is_some())?; ts.write_bool(self.flags.start_muted.is_some())?; @@ -301,3 +308,98 @@ mod tests { test_serde(&reply) } } + +#[cfg(test)] +#[cfg(feature = "_integration-tests")] +mod integration_tests { + use super::*; + use crate::integration_test_util::*; + use crate::protocol::*; + + #[test] + fn create_playback_stream() -> anyhow::Result<()> { + let (mut sock, protocol_version) = connect_and_init()?; + + write_command_message( + sock.get_mut(), + 0, + &Command::CreatePlaybackStream(PlaybackStreamParams { + sample_spec: SampleSpec { + format: SampleFormat::S16Le, + sample_rate: 44100, + channels: 2, + }, + channel_map: ChannelMap::stereo(), + cvolume: Some(ChannelVolume::norm(2)), + flags: StreamFlags { + start_corked: true, + start_muted: Some(true), + ..Default::default() + }, + sink_index: None, + sink_name: Some(CString::new("@DEFAULT_SINK@")?), + ..Default::default() + }), + protocol_version, + )?; + + let _ = read_reply_message::(&mut sock, protocol_version)?; + + Ok(()) + } + + /// Tests that RecordStreamParams maintains consistent + /// channel counts across its implicitly set fields. + #[test] + fn create_record_stream_channel_count_invariants() -> anyhow::Result<()> { + let (mut sock, protocol_version) = connect_and_init()?; + + // Arbitrarily chosen number of channels that should be kept in sync + // across fields (chosen to test beyond the usual 1 or 2). + const CHANNEL_COUNT: u8 = 3; + + // Explicitly set case (for reference). + { + write_command_message( + sock.get_mut(), + 0, + &Command::CreateRecordStream(RecordStreamParams { + sample_spec: SampleSpec { + format: SampleFormat::S16Le, + channels: CHANNEL_COUNT, + ..Default::default() + }, + channel_map: ChannelMap::with_n_channels(CHANNEL_COUNT), + cvolume: Some(ChannelVolume::norm(CHANNEL_COUNT)), + ..Default::default() + }), + protocol_version, + )?; + + let _ = read_reply_message::(&mut sock, protocol_version)?; + } + + // Implicitly set case (on fields that allow it). + { + write_command_message( + sock.get_mut(), + 1, + &Command::CreateRecordStream(RecordStreamParams { + sample_spec: SampleSpec { + format: SampleFormat::S16Le, + channels: CHANNEL_COUNT, + ..Default::default() + }, + channel_map: ChannelMap::with_n_channels(CHANNEL_COUNT), + cvolume: None, + ..Default::default() + }), + protocol_version, + )?; + + let _ = read_reply_message::(&mut sock, protocol_version)?; + } + + Ok(()) + } +} diff --git a/src/protocol/serde/channel_map.rs b/src/protocol/serde/channel_map.rs index cac6109..d2e4538 100644 --- a/src/protocol/serde/channel_map.rs +++ b/src/protocol/serde/channel_map.rs @@ -129,6 +129,21 @@ impl ChannelMap { map } + /// Creates a channel map with N (arbitrarily chosen) channels. + /// + /// Intended for testing cases where only the number of channels matters. + #[cfg(test)] + #[cfg(feature = "_integration-tests")] + pub(crate) fn with_n_channels(channels: u8) -> Self { + let mut map = ChannelMap::empty(); + for pos in + (1..=channels).map(|idx| ChannelPosition::from_u8(idx).expect("known channel index")) + { + map.push(pos); + } + map + } + /// Tries to append another `ChannelPosition` to the end of this map. /// /// Panics if the map already has MAX_CHANNELS channels. diff --git a/src/protocol/serde/volume.rs b/src/protocol/serde/volume.rs index f4d0f21..bcf08dc 100644 --- a/src/protocol/serde/volume.rs +++ b/src/protocol/serde/volume.rs @@ -136,17 +136,17 @@ impl ChannelVolume { } /// Create a `ChannelVolume` with N channels, all muted. - pub fn muted(channels: usize) -> ChannelVolume { + pub fn muted(channels: u8) -> ChannelVolume { Self { - channels: channels as u8, + channels, volumes: [Volume::MUTED; MAX_CHANNELS as usize], } } /// Create a `ChannelVolume` with N channels, all at full volume. - pub fn norm(channels: usize) -> ChannelVolume { + pub fn norm(channels: u8) -> ChannelVolume { Self { - channels: channels as u8, + channels, volumes: [Volume::NORM; MAX_CHANNELS as usize], } }