Created attachment 67623 [details] pulse-tunnel-auto.patch This is a bit of a niche feature, but I figured we wrote the code so we might as well share. :) The attached patch makes sure that module-tunnel-* can use the environment and X11 properties to figure out how to reach the PulseAudio daemon, the same way a client would. We use this in our Linux Remote Desktop product ThinLinc, where the client launches its own PulseAudio daemon (so we can be sure of what goes over the network), but we want it to talk to the local PulseAudio daemon to actually output sound.
Created attachment 67765 [details] [review] pulse-servers.patch Second patch that also makes sure the modules can handle server specifications with multiple entries.
Thanks for the patches! I have trouble applying them, though. What version are they generated against? Also, would formatting them with "git format-patch" be possible?
Unfortunately I have yet to convince the powers at be that we should convert from subversion to git. :) The patches are against a git snapshot taken Sept 20th. Unfortunately I can't get the exact commit id. Given the current log it should have been 2c8aa18b1d though.
If there's anything we can do to make it easier to integrate this work, please let us know. Would a git pull request help?
A patch generated by "git format-patch" would be preferable, otherwise please give instructions for how to apply the patches.
Created attachment 82334 [details] [review] 0001-module-tunnel-automatically-find-the-PulseAudio-serv.patch git-format-patch against master branch.
The tunnel module should get rewritten so that it uses libpulse, and this bug should get resolved as a side effect. There is no 100% guarantee that the rewrite project will finish, so if you want this patch applied anyway, I'm not against that. Some review comments follow. In pa__init(), the server variable is never freed, even if it should be: #ifdef HAVE_X11 if ((server == NULL) && (xcb != NULL)) { char t[1024]; if (pa_x11_get_prop(xcb, 0, "PULSE_SERVER", t, sizeof(t))) server = pa_xstrdup(t); } #endif It doesn't seem like a good idea to connect to a local server by default. If the user loads module-tunnel-sink without any arguments, I think it should fail, because tunneling to a local daemon doesn't usually make sense. I think it would be better if connecting to a local server would require an explicit indication that the user really wants to do that.
(In reply to comment #7) > The tunnel module should get rewritten so that it uses libpulse, and this > bug should get resolved as a side effect. This was a bit unclear. When I said "should get rewritten", I meant that there's an ongoing effort to do that.
(In reply to comment #7) > The tunnel module should get rewritten so that it uses libpulse, and this > bug should get resolved as a side effect. There is no 100% guarantee that > the rewrite project will finish, so if you want this patch applied anyway, > I'm not against that. Some review comments follow. I'd appreciate that. The rewrite is very interesting to us, but it has a bit more to go before it can replace module-tunnel. So we'll probably be doing relevant fixes and improvements to module-tunnel until then. > > In pa__init(), the server variable is never freed, even if it should be: > Will have a look at that. Thanks. > It doesn't seem like a good idea to connect to a local server by default. If > the user loads module-tunnel-sink without any arguments, I think it should > fail, because tunneling to a local daemon doesn't usually make sense. I > think it would be better if connecting to a local server would require an > explicit indication that the user really wants to do that. I can sympathise with that. Would something like an "auto" argument suffice?
(In reply to comment #9) > (In reply to comment #7) > > It doesn't seem like a good idea to connect to a local server by default. If > > the user loads module-tunnel-sink without any arguments, I think it should > > fail, because tunneling to a local daemon doesn't usually make sense. I > > think it would be better if connecting to a local server would require an > > explicit indication that the user really wants to do that. > > I can sympathise with that. Would something like an "auto" argument suffice? Yes.
Created attachment 82484 [details] [review] 0001-module-tunnel-automatically-find-the-PulseAudio-serv.patch Updated patch. The automatic magic now requires an "auto=yes" argument. Without this it will behave like before, with one difference: the server name is interpreted as a list rather than a single server.
Thanks, applied: http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=23c39bb54041a8774b5184802fc7764376bf0565
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.