[pulseaudio-discuss] [PATCH] add module-virtual-surround-sink

Niels Ole Salscheider niels_ole at salscheider-online.de
Fri Feb 17 06:56:55 PST 2012


Hello,

> First complaint: doesn't your git nag about trailing whitespace?

It seems that this was not enabled in my git config (although I cannot remember 
disabling it). I have fixed the mentioned lines.

> current_latency is in module-virtual-sink only for instructive purposes.
> If it's not needed in the filter, it should be removed from the code.

> I think a switch would be nicer than if here.

Both fixed.

> I'd put this variable inside the innermost if, because it's not used
> elsewhere in this function.

I think that is not allowed in C89/C90...

[...]
> My preferences for selecting the channel map and sample spec for the
> sink input and sink are the following:
> 
> Sink input:
> Channel map: Always stereo.
> Sample format: Always float32ne.
> Sample rate: Always match the filter sink.
> 
> Sink:
> Channel map: Default to the impulse response file channel map. Allow the
> user to override that default with the "channels" and/or "channel_map"
> module arguments. pa_modargs_get_sample_spec_and_channel_map() will take
> care of initializing the channel map appropriately.
> Sample format: Always float32ne.
> Sample rate: I'm not quite sure about this. I guess defaulting to the
> master sink rate would be best. The other alternative would be
> defaulting to the impulse response file's rate. The user should be able
> to override the default with the "rate" module argument.

That sounds sensible. I think at some point I got confused by sink_input being 
the input for the master sink that accepts the output signal I compute.

I hope I got it right now. I have attached a new version of my patch.

Regards,

Ole


More information about the pulseaudio-discuss mailing list