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
