fix: set sensible cvolume default

[OpatrilPeter]

Hello,
I've encountered a problem with the current code that caused PulseAudio server 16.1-8-g6f04 (on WSLG) to reject CreatePlaybackStream message with Invalid argument error code.

Examination shown the reason is following code:

CHECK_VALIDITY_GOTO(c->pstream, map.channels == ss.channels && volume.channels == ss.channels, tag, PA_ERR_INVALID, finish);

@ https://github.com/pulseaudio/pulseaudio/blob/86e9c90/src/pulsecore/protocol-native.c#L2026

In short, the server assumes the channel volume has the same amount of channels as the sample spec, but default used when cvolume argument is None always initializes with single channel.

I verified that the fix helped making the audio playback work in Cave Story source port using cpal with your CA support branch with WSL2.

I'd recommend removing the Default implmentation of ChannelVolume altogether as it's error prone due to issue above, but that's out of scope here and would require to rethink defaults for other types too.

[colinmarc]

Thanks for the fix, this looks good to me! How hard would it be to add a test that fails before the fix?

[OpatrilPeter]

Good point. I've added some tests and documentation, let me know if this is what you had in mind or would like to trim it down, for example.
I haven't studied the protocol and so I didn't want to actvely test cases where the server fails, as it may not to.

Also, it seems a bit of a shame that SampleFormat::default() is invalid (rejected by my server, at least). Would love to not have to pick arbitrary value in tests unrelated to it.

[colinmarc]

This looks great :) fwiw I was actually asking, not making a point - I wasn't sure how hard the behavior is to reproduce in a clean environment.

I just triggered CI, I'll be back at my linux workstation in a few days and can do another test then.

[colinmarc]

Let's just inline this in the tests (or add a function inside the #[cfg(test)] block)

[OpatrilPeter]

Since it's already used in two modules and is in theory useful in any kind of test where you'd want arbitrary ChannelMap in a command (as the channel count must presumably align in any usecase), I think it's better left with the class for now. I added the conditionality, there should be no unused warnings now.

[OpatrilPeter]

@colinmarc Let me know if you need any further tweaks, I'd love to help wrapping this up.

[colinmarc]

Merged in 8b8db27. Thanks!!

[colinmarc]

@OpatrilPeter would you mind deleting your fork? I'd like to disassociate this with pulsar-rs (since there is basically zero shared code and the upstream repo is gone anyway) but I can't do that if there are "child forks" apparently.

[OpatrilPeter]

@colinmarc Fork deleted, although it seems there are others out there. For the record, there's the option to 'unlink it from the fork network' that would probably also do the trick, so you may suggest this to people as well.