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

[Niels Ole Salscheider]

Hello,

> One question about the processing: does it add any latency?

That depends on the impulse response. In general, the current output sample is 
a weighted sum of the current input sample and some preceding.
Provided that the first values of the impulse response are not equal (or 
approximately) zero there is no added latency.

> The hrir data and the input buffer don't really need to be stored inside
> memchunks. They could be just arrays of floats.

I fixed this for the input buffer but "pa_resampler_run" stores the hrir data 
inside a memchunk.

> I guess master_channels is unneeded now that it's hardcoded to be 2. And
> this piece of code can be simplified to be just two assignments to dst.

Fixed. Is it ok to assume that the first channel is left and the second is 
right or do I need to get this information from the sink input sample spec?

> Pulseaudio's convention is one more indentation level for the cases
> inside the switch.

> You need to check that hrir_file isn't NULL.

> The hrir_temp_chunk memblock needs to be unreffed also in the fail
> section if it's non-NULL to avoid memory leaks. For that reason the
> memblock pointer also has to be set to NULL here and in the beginning of
> the function.

All fixed. I have attached an updated patch.

> Wouldn't it be better to default to the hrir file channel map? In that
> case the file reading needs to be done before initializing the sink
> channel map.

> Shouldn't the default hrir channel map be hrir_temp_map, i.e. the
> channel map of the file? And aren't hrir_ss and hrir_map redundant
> anyway - shouldn't the hrir sample spec and channel map be the same as
> what the sink has?

That is what I would like to do, too. But the channel map that is returned by 
"pa_sound_file_load" is wrong. I am not even sure if there is a way to store 
the channel map in the wav header.
Because of that I provided a way to specify the channel map of the hrir wav. 
Is there a better solution?

Regards,

Ole