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

[Arun Raghavan]

On Tue, 2012-02-07 at 16:20 +0530, Arun Raghavan wrote:
> 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.

Pushed with massive commit latency now. :p

So I guess the official line is tunnel sources are broken in 1.x and
supported again in 2.x. At least until someone complains loudly enough
about the breakage.

-- Arun