Hello Marius, First of all I'd like to apologise profusely for not getting back to you until now. Two months is a ridiculous delay and I'm sorry about that (especially as you mention that you are new to open source - it's not the best first impression to receive!) Secondly I'd like to thank you very much indeed for your contribution. Sadly, (and this is one of the reasons for the delay), there was a pending patch that updated the LADSPA module to give it multichannel support. I'm not really an expert in the land of LADSPA, which is why it was sitting in my moderation queue for so long and also why I'm only just now getting around to contacting you! As you are clearly interested in this work and obviously have knowledge of LADSPA and (now!) some knowledge of PA, I'd very much appreciate it if you could review the code now in current git master related to ladsap. Just to be clear, I've not used any of the code you attached to your mail, so it would mean starting with a clean slate for the review. I'm am sure that there is still room for improvement in the committed code, so any patches that you are able to make to it, I promise I'll review and push in a timely manner (or give you really good excuses if not :p) For clarity the main patch is this one: http://git.0pointer.de/?p=pulseaudio.git;a=commitdiff;h=6bd32e (although the commit date is from October, I only "pushed" it into our git master a week or so ago). If you do create further patches, I'd encourage you to clone the git master repository and make your changes. Then commit the results locally and supply the patches to us via the git format-patch command. This allows us to review the changes easily (as reading a list of the differences is much easier than reading the whole file) and also allows us to properly attribute the work to yourself in our changelog. I'd also very much encourage you to come on to #pulseaudio on IRC as a lot of the devs hang out there and we're a friendly and approachable bunch (usually!). It's not always super busy, so just hang out and talk occasionally and you'll soon see folks chat about stuff (I'm coling) I've not got much to say about your other comments but with regards to what you said about volume controls and why they are separate rather than just sharing one: this has been recently addressed by Tanu (and Marc-Andr?!) with a feature known as "Volume Sharing" :) Again, apologises for the lack of response until now and I hope you'll still be keen to contribute - it would be very much appreciated. Cheers Col 'Twas brillig, and Marius Bj?rnstad at 30/12/10 20:51 did gyre and gimble: > Dear PulseAudio community members, > > I am new to open source, but I wanted to extend the functionality of > module-ladspa-sink. I have created a new version of > module-ladspa-sink.c, based on the current version in git. I don't know > if it will be useful to include it in PulseAudio, so I will present it > here. I will summarise the changes, then provide some details, then list > some problems. I don't understand a lot of PA, but I tried to change > only the things I understood. > > Summary: > The motivation was to get bs2b ( http://bs2b.sourceforge.net/ ) working > in PulseAudio. The original module has a one-to-one mapping between > instances of the LADSPA plugin and channels. The new version works in > two modes: by default it has the same behaviour as the original module. > Alternatively, the option "cpmap" can be used to specify a mapping > between the channels of the stream in :Pulse, and input/output ports of > the plugin. In this mode, only one instance of the LADSPA plugin will be > created. > > Details: > > *** Parsing of the cpmap option *** > The cpmap option accepts a comma-separated list of port-channel > mappings. Each mapping contains values separated by colons (:). There > are three alternative formats that can be used: > > 1) inputport:outputport:channel > > 2) suffix:channel > uses Input suffix and Output suffix as ports, e.g. > "Input left" and "Output left" > > 3) :channel > Uses Input and Output as ports (used for the default value > of cpmap) > > The input and output port names are matched case sensitively. The > channel names are those used with pa_channel_map. > > The mappings are separated by commas , . It is not foreseen that port > names contain colons or commas (changing the separator, or even escaping > it, could be considered). > > Channels that do not have a listed mapping are passed through the module > unchanged (except for volume, see Problems). > > The keyword "all" can be used in place of a channel, and this invokes > the behaviour of the original module. The default value for cpmap is > thus ":all". "all" can not be mixed with other channels. > > > *** Other changes *** > The new module uses one buffer per channel, where the previous module > used a single buffer. In many places, the number of channels and the > number of plugin instances is disambiguated. > > > > Problems: > I found no additional problems with the new code as compared with the > original. It is only tested on a stereo stream (I don't have a surround > sound system, and I don't have a LADSPA plugin that supports surround). > > 1) When playing back multiple streams of audio, from different sources > (media players, etc), I get some error messages in syslog when the > module is used. In debug mode, I get these messages (in normal mode just > the "ratelimit" ones): > W: ratelimit.c: 721 events suppressed > D: memblock.c: Pool full > D: memblock.c: Pool full > D: memblock.c: Pool full > D: memblock.c: Pool full > D: memblock.c: Pool full > D: memblock.c: Pool full > D: memblock.c: Pool full > D: memblock.c: Pool full > D: memblock.c: Pool full > D: memblock.c: Pool full > D: memblock.c: Pool full > W: ratelimit.c: 775 events suppressed > D: memblock.c: Pool full > D: memblock.c: Pool full > D: memblock.c: Pool full > D: memblock.c: Pool full > D: memblock.c: Pool full > D: memblock.c: Pool full > D: memblock.c: Pool full > D: memblock.c: Pool full > D: memblock.c: Pool full > D: memblock.c: Pool full > D: memblock.c: Pool full > > This happens with the original version of the module as well as with the > new one. > > 2) An additional software volume is associated with the sink, where one > would expect that it would control the volume of the master sink directly. > > > Any comments or questions are welcome. > > Thanks for your attention, > Marius Bj?rnstad > > > > > > _______________________________________________ > pulseaudio-discuss mailing list > pulseaudio-discuss at mail.0pointer.de > https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited [http://www.tribalogic.net/] Open Source: Mageia Contributor [http://www.mageia.org/] PulseAudio Hacker [http://www.pulseaudio.org/] Trac Hacker [http://trac.edgewall.org/]
