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