On Mon, 2012-01-30 at 16:47 +0100, David Henningsson wrote:
> I'm trying to set up a module-tunnel-source, but it fails with a
> protocol error.
>
> Here's the analysis.
>
> According to spec:
>
> <---
> ## v22, implemented by >= 1.0
>
> New fields PA_COMMAND_CREATE_RECORD_STREAM:
>
> uint8_t n_formats
> format_info format1
> ...
> format_info formatn
> --->
>
>
> 1) According to code in module-tunnel.c, the code for sending n_formats
> is missing.
>
> 2) More interesting is that according to code in protocol-native.c, not
> only does PA_COMMAND_CREATE_RECORD_STREAM assume the above to come in,
> but also these fields:
>
> <---
> if (pa_tagstruct_get_cvolume(t, &volume) < 0 ||
> pa_tagstruct_get_boolean(t, &muted) < 0 ||
> pa_tagstruct_get_boolean(t, &volume_set) < 0 ||
> pa_tagstruct_get_boolean(t, &muted_set) < 0 ||
> pa_tagstruct_get_boolean(t, &relative_volume) < 0 ||
> pa_tagstruct_get_boolean(t, &passthrough) < 0) {
> --->
>
> ...which is consistent with the code in src/pulse/stream.c. These are
> not documented in the PROTOCOL file.
>
> 3) Actually, they remotely correspond to something in the documentation
> for protocol v22 as well:
>
> <---
> Five new fields in reply from PA_COMMAND_GET_SOURCE_OUTPUT_INFO (and
> thus PA_COMMAND_GET_SOURCE_OUTPUT_INFO_LIST)
>
> format_info format
> volume
> bool mute
> bool has_volume
> bool volume_writable
> --->
>
> In the code, the format_info comes last instead of first. :-/
>
> This is a problem both in PulseAudio 1.x and in git master. (And thus in
> Ubuntu 11.10 and Ubuntu 12.04.)
>
> After some thoughts, I think the wisest course of action is to fixup
> module-tunnel to send more fields, and fixup the documentation to match
> the code. I'll send some patches for this. Next question is if we should
> also consider backwards compatibility with 1.x's broken implementation
> of module-tunnel-source...?
Sigh. We really need to automate testing this - it breaks too often. I
don't think we should aim for backward compatibility with the incorrect
implementation. IMO we could either wait for the next release or do a
quick 1.2 and ask people complaining about this to upgrade. Yes, this
sucks, but I'd rather do that than inflict backwards-compatibility
kludge.
If there are no objections, I'll pull the patches.
-- Arun
