[pulseaudio-discuss] [PATCH 0/2] module-tunnel-source broken - Something went wrong at protocol version 22 :-(

[Arun Raghavan]

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