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

Tanu Kaskinen tanuk at iki.fi
Thu Feb 23 11:24:49 PST 2012


On Fri, 2012-02-17 at 15:56 +0100, Niels Ole Salscheider wrote:
> > 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...

I think C89 allows variable declarations at the beginning of any block.
And even if it doesn't, variables are declared inside if blocks all over
Pulseaudio's code, so not doing it here won't achieve anything in terms
of complying with C89.

> 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.

Thanks!

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

> From 56ac4d4a95971818657202677bf0646b3626a2a4 Mon Sep 17 00:00:00 2001
> From: Niels Ole Salscheider <niels_ole at salscheider-online.de>
> Date: Sun, 8 Jan 2012 21:22:35 +0100
> Subject: [PATCH] add module-virtual-surround-sink
> 
> It provides a virtual surround sound effect
> 
> v2: Normalize hrir to avoid clipping, some cleanups
> v3: use fabs, not abs
> v4: implement changes proposed by Tanu Kaskinen
> ---
>  src/Makefile.am                                    |    7 +
>  ...rtual-sink.c => module-virtual-surround-sink.c} |  344 ++++++++++++++++----
>  2 files changed, 288 insertions(+), 63 deletions(-)
>  copy src/modules/{module-virtual-sink.c => module-virtual-surround-sink.c} (63%)
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 521bf50..8f942f0 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -1009,6 +1009,7 @@ modlibexec_LTLIBRARIES += \
>                 module-loopback.la \
>                 module-virtual-sink.la \
>                 module-virtual-source.la \
> +               module-virtual-surround-sink.la \
>                 module-switch-on-connect.la \
>                 module-filter-apply.la \
>                 module-filter-heuristics.la
> @@ -1309,6 +1310,7 @@ SYMDEF_FILES = \
>                 module-loopback-symdef.h \
>                 module-virtual-sink-symdef.h \
>                 module-virtual-source-symdef.h \
> +               module-virtual-surround-sink-symdef.h \
>                 module-switch-on-connect-symdef.h \
>                 module-filter-apply-symdef.h \
>                 module-filter-heuristics-symdef.h
> @@ -1525,6 +1527,11 @@ module_virtual_source_la_CFLAGS = $(AM_CFLAGS) $(SERVER_CFLAGS)
>  module_virtual_source_la_LDFLAGS = $(MODULE_LDFLAGS)
>  module_virtual_source_la_LIBADD = $(MODULE_LIBADD)
>  
> +module_virtual_surround_sink_la_SOURCES = modules/module-virtual-surround-sink.c
> +module_virtual_surround_sink_la_CFLAGS = $(AM_CFLAGS) $(SERVER_CFLAGS)
> +module_virtual_surround_sink_la_LDFLAGS = $(MODULE_LDFLAGS)
> +module_virtual_surround_sink_la_LIBADD = $(MODULE_LIBADD)
> +
>  # X11
>  
>  module_x11_bell_la_SOURCES = modules/x11/module-x11-bell.c
> diff --git a/src/modules/module-virtual-sink.c b/src/modules/module-virtual-surround-sink.c
> similarity index 63%
> copy from src/modules/module-virtual-sink.c
> copy to src/modules/module-virtual-surround-sink.c
> index cf11ffa..d3dfe6f 100644
> --- a/src/modules/module-virtual-sink.c
> +++ b/src/modules/module-virtual-surround-sink.c
> @@ -3,6 +3,7 @@
>  
>      Copyright 2010 Intel Corporation
>      Contributor: Pierre-Louis Bossart <pierre-louis.bossart at intel.com>
> +    Copyright 2012 Niels Ole Salscheider <niels_ole at salscheider-online.de>
>  
>      PulseAudio is free software; you can redistribute it and/or modify
>      it under the terms of the GNU Lesser General Public License as published
> @@ -37,11 +38,15 @@
>  #include <pulsecore/rtpoll.h>
>  #include <pulsecore/sample-util.h>
>  #include <pulsecore/ltdl-helper.h>
> +#include <pulsecore/sound-file.h>
> +#include <pulsecore/resampler.h>
>  
> -#include "module-virtual-sink-symdef.h"
> +#include <math.h>
>  
> -PA_MODULE_AUTHOR("Pierre-Louis Bossart");
> -PA_MODULE_DESCRIPTION(_("Virtual sink"));
> +#include "module-virtual-surround-sink-symdef.h"
> +
> +PA_MODULE_AUTHOR("Niels Ole Salscheider");
> +PA_MODULE_DESCRIPTION(_("Virtual surround sink"));
>  PA_MODULE_VERSION(PACKAGE_VERSION);
>  PA_MODULE_LOAD_ONCE(FALSE);
>  PA_MODULE_USAGE(
> @@ -54,6 +59,8 @@ PA_MODULE_USAGE(
>            "channel_map=<channel map> "
>            "use_volume_sharing=<yes or no> "
>            "force_flat_volume=<yes or no> "
> +          "hrir=/path/to/left_hrir.wav "
> +          "hrir_channel_map=<channel map> "
>          ));
>  
>  #define MEMBLOCKQ_MAXLENGTH (16*1024*1024)
> @@ -71,6 +78,23 @@ struct userdata {
>  
>      pa_bool_t auto_desc;
>      unsigned channels;
> +    unsigned hrir_channels;
> +    unsigned master_channels;
> +
> +    unsigned fs, sink_fs;
> +
> +    unsigned *mapping_left;
> +    unsigned *mapping_right;
> +
> +    unsigned output_left, output_right;
> +
> +    unsigned hrir_samples;
> +    pa_sample_spec hrir_ss;
> +    pa_channel_map hrir_map;
> +    pa_memchunk hrir_chunk;
> +
> +    pa_memchunk input_buffer_chunk;

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

> +    int input_buffer_offset;
>  };
>  
>  static const char* const valid_modargs[] = {
> @@ -83,6 +107,8 @@ static const char* const valid_modargs[] = {
>      "channel_map",
>      "use_volume_sharing",
>      "force_flat_volume",
> +    "hrir",
> +    "hrir_channel_map",
>      NULL
>  };
>  
> @@ -198,10 +224,14 @@ static void sink_set_mute_cb(pa_sink *s) {
>  static int sink_input_pop_cb(pa_sink_input *i, size_t nbytes, pa_memchunk *chunk) {
>      struct userdata *u;
>      float *src, *dst;
> -    size_t fs;
> -    unsigned n, c;
> +    unsigned n;
>      pa_memchunk tchunk;
> -    pa_usec_t current_latency PA_GCC_UNUSED;
> +
> +    unsigned j, k, l;
> +    float sum_right, sum_left;
> +    float current_sample;
> +    float *hrir_data;
> +    float *input_buffer_data;
>  
>      pa_sink_input_assert_ref(i);
>      pa_assert(chunk);
> @@ -210,60 +240,69 @@ static int sink_input_pop_cb(pa_sink_input *i, size_t nbytes, pa_memchunk *chunk
>      /* Hmm, process any rewind request that might be queued up */
>      pa_sink_process_rewind(u->sink, 0);
>  
> -    /* (1) IF YOU NEED A FIXED BLOCK SIZE USE
> -     * pa_memblockq_peek_fixed_size() HERE INSTEAD. NOTE THAT FILTERS
> -     * WHICH CAN DEAL WITH DYNAMIC BLOCK SIZES ARE HIGHLY
> -     * PREFERRED. */
>      while (pa_memblockq_peek(u->memblockq, &tchunk) < 0) {
>          pa_memchunk nchunk;
>  
> -        pa_sink_render(u->sink, nbytes, &nchunk);
> +        pa_sink_render(u->sink, nbytes * u->sink_fs / u->fs, &nchunk);
>          pa_memblockq_push(u->memblockq, &nchunk);
>          pa_memblock_unref(nchunk.memblock);
>      }
>  
> -    /* (2) IF YOU NEED A FIXED BLOCK SIZE, THIS NEXT LINE IS NOT
> -     * NECESSARY */
> -    tchunk.length = PA_MIN(nbytes, tchunk.length);
> +    tchunk.length = PA_MIN(nbytes * u->sink_fs / u->fs, tchunk.length);
>      pa_assert(tchunk.length > 0);
>  
> -    fs = pa_frame_size(&i->sample_spec);
> -    n = (unsigned) (tchunk.length / fs);
> +    n = (unsigned) (tchunk.length / u->sink_fs);
>  
>      pa_assert(n > 0);
>  
>      chunk->index = 0;
> -    chunk->length = n*fs;
> +    chunk->length = n * u->fs;
>      chunk->memblock = pa_memblock_new(i->sink->core->mempool, chunk->length);
>  
> -    pa_memblockq_drop(u->memblockq, chunk->length);
> +    pa_memblockq_drop(u->memblockq, n * u->sink_fs);
>  
>      src = (float*) ((uint8_t*) pa_memblock_acquire(tchunk.memblock) + tchunk.index);
>      dst = (float*) pa_memblock_acquire(chunk->memblock);
> +    hrir_data = (float*) pa_memblock_acquire(u->hrir_chunk.memblock);
> +    input_buffer_data = (float*) pa_memblock_acquire(u->input_buffer_chunk.memblock);
> +
> +    for (l = 0; l < n; l++) {
> +        memcpy(((char*) input_buffer_data) + u->input_buffer_offset * u->sink_fs, ((char *) src) + l * u->sink_fs, u->sink_fs);
>  
> -    /* (3) PUT YOUR CODE HERE TO DO SOMETHING WITH THE DATA */
> +        sum_right = 0;
> +        sum_left = 0;
>  
> -    /* As an example, copy input to output */
> -    for (c = 0; c < u->channels; c++) {
> -        pa_sample_clamp(PA_SAMPLE_FLOAT32NE,
> -                        dst+c, u->channels * sizeof(float),
> -                        src+c, u->channels * sizeof(float),
> -                        n);
> +        /* fold the input buffer with the impulse response */
> +        for (j = 0; j < u->hrir_samples; j++) {
> +            for (k = 0; k < u->channels; k++) {
> +                current_sample = input_buffer_data[((u->input_buffer_offset + j) % u->hrir_samples) * u->channels + k];
> +
> +                sum_left += current_sample * hrir_data[j * u->hrir_channels + u->mapping_left[k]];
> +                sum_right += current_sample * hrir_data[j * u->hrir_channels + u->mapping_right[k]];
> +            }
> +        }
> +
> +        for (k = 0; k < u->master_channels; k++) {
> +            if (k == u->output_left)
> +                dst[u->master_channels * l + k] = PA_CLAMP_UNLIKELY(sum_left, -1.0f, 1.0f);
> +            else if (k == u->output_right)
> +                dst[u->master_channels * l + k] = PA_CLAMP_UNLIKELY(sum_right, -1.0f, 1.0f);
> +            else
> +                dst[u->master_channels * l + k] = 0;
> +        }

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.

> +
> +        u->input_buffer_offset--;
> +        if (u->input_buffer_offset < 0)
> +            u->input_buffer_offset += u->hrir_samples;
>      }
>  
>      pa_memblock_release(tchunk.memblock);
>      pa_memblock_release(chunk->memblock);
> +    pa_memblock_release(u->hrir_chunk.memblock);
> +    pa_memblock_release(u->input_buffer_chunk.memblock);
>  
>      pa_memblock_unref(tchunk.memblock);
>  
> -    /* (4) IF YOU NEED THE LATENCY FOR SOMETHING ACQUIRE IT LIKE THIS: */
> -    current_latency =
> -        /* Get the latency of the master sink */
> -        pa_sink_get_latency_within_thread(i->sink) +
> -
> -        /* Add the latency internal to our sink input on top */
> -        pa_bytes_to_usec(pa_memblockq_get_length(i->thread_info.render_memblockq), &i->sink->sample_spec);
> -
>      return 0;
>  }
>  
> @@ -271,6 +310,7 @@ static int sink_input_pop_cb(pa_sink_input *i, size_t nbytes, pa_memchunk *chunk
>  static void sink_input_process_rewind_cb(pa_sink_input *i, size_t nbytes) {
>      struct userdata *u;
>      size_t amount = 0;
> +    char *input_buffer;
>  
>      pa_sink_input_assert_ref(i);
>      pa_assert_se(u = i->userdata);
> @@ -278,19 +318,23 @@ static void sink_input_process_rewind_cb(pa_sink_input *i, size_t nbytes) {
>      if (u->sink->thread_info.rewind_nbytes > 0) {
>          size_t max_rewrite;
>  
> -        max_rewrite = nbytes + pa_memblockq_get_length(u->memblockq);
> -        amount = PA_MIN(u->sink->thread_info.rewind_nbytes, max_rewrite);
> +        max_rewrite = nbytes * u->sink_fs / u->fs + pa_memblockq_get_length(u->memblockq);
> +        amount = PA_MIN(u->sink->thread_info.rewind_nbytes * u->sink_fs / u->fs, max_rewrite);
>          u->sink->thread_info.rewind_nbytes = 0;
>  
>          if (amount > 0) {
>              pa_memblockq_seek(u->memblockq, - (int64_t) amount, PA_SEEK_RELATIVE, TRUE);
>  
> -            /* (5) PUT YOUR CODE HERE TO RESET YOUR FILTER  */
> +            /* Reset the input buffer */
> +            input_buffer = (char *) pa_memblock_acquire(u->input_buffer_chunk.memblock);
> +            memset(input_buffer, 0, u->input_buffer_chunk.length);
> +            pa_memblock_release(u->input_buffer_chunk.memblock);
> +            u->input_buffer_offset = 0;
>          }
>      }
>  
>      pa_sink_process_rewind(u->sink, amount);
> -    pa_memblockq_rewind(u->memblockq, nbytes);
> +    pa_memblockq_rewind(u->memblockq, nbytes * u->sink_fs / u->fs);
>  }
>  
>  /* Called from I/O thread context */
> @@ -300,8 +344,8 @@ static void sink_input_update_max_rewind_cb(pa_sink_input *i, size_t nbytes) {
>      pa_sink_input_assert_ref(i);
>      pa_assert_se(u = i->userdata);
>  
> -    pa_memblockq_set_maxrewind(u->memblockq, nbytes);
> -    pa_sink_set_max_rewind_within_thread(u->sink, nbytes);
> +    pa_memblockq_set_maxrewind(u->memblockq, nbytes * u->sink_fs / u->fs);
> +    pa_sink_set_max_rewind_within_thread(u->sink, nbytes * u->sink_fs / u->fs);
>  }
>  
>  /* Called from I/O thread context */
> @@ -311,10 +355,7 @@ static void sink_input_update_max_request_cb(pa_sink_input *i, size_t nbytes) {
>      pa_sink_input_assert_ref(i);
>      pa_assert_se(u = i->userdata);
>  
> -    /* (6) IF YOU NEED A FIXED BLOCK SIZE ROUND nbytes UP TO MULTIPLES
> -     * OF IT HERE. THE PA_ROUND_UP MACRO IS USEFUL FOR THAT. */
> -
> -    pa_sink_set_max_request_within_thread(u->sink, nbytes);
> +    pa_sink_set_max_request_within_thread(u->sink, nbytes * u->sink_fs / u->fs);
>  }
>  
>  /* Called from I/O thread context */
> @@ -334,10 +375,6 @@ static void sink_input_update_sink_fixed_latency_cb(pa_sink_input *i) {
>      pa_sink_input_assert_ref(i);
>      pa_assert_se(u = i->userdata);
>  
> -    /* (7) IF YOU NEED A FIXED BLOCK SIZE ADD THE LATENCY FOR ONE
> -     * BLOCK MINUS ONE SAMPLE HERE. pa_usec_to_bytes_round_up() IS
> -     * USEFUL FOR THAT. */
> -
>      pa_sink_set_fixed_latency_within_thread(u->sink, i->sink->thread_info.fixed_latency);
>  }
>  
> @@ -363,13 +400,8 @@ static void sink_input_attach_cb(pa_sink_input *i) {
>      pa_sink_set_rtpoll(u->sink, i->sink->thread_info.rtpoll);
>      pa_sink_set_latency_range_within_thread(u->sink, i->sink->thread_info.min_latency, i->sink->thread_info.max_latency);
>  
> -    /* (8.1) IF YOU NEED A FIXED BLOCK SIZE ADD THE LATENCY FOR ONE
> -     * BLOCK MINUS ONE SAMPLE HERE. SEE (7) */
>      pa_sink_set_fixed_latency_within_thread(u->sink, i->sink->thread_info.fixed_latency);
>  
> -    /* (8.2) IF YOU NEED A FIXED BLOCK SIZE ROUND
> -     * pa_sink_input_get_max_request(i) UP TO MULTIPLES OF IT
> -     * HERE. SEE (6) */
>      pa_sink_set_max_request_within_thread(u->sink, pa_sink_input_get_max_request(i));
>      pa_sink_set_max_rewind_within_thread(u->sink, pa_sink_input_get_max_rewind(i));
>  
> @@ -471,10 +503,53 @@ static void sink_input_mute_changed_cb(pa_sink_input *i) {
>      pa_sink_mute_changed(u->sink, i->muted);
>  }
>  
> +static pa_channel_position_t mirror_channel(pa_channel_position_t channel) {
> +    switch (channel) {
> +    case PA_CHANNEL_POSITION_FRONT_LEFT:
> +        return PA_CHANNEL_POSITION_FRONT_RIGHT;
> +
> +    case PA_CHANNEL_POSITION_FRONT_RIGHT:
> +        return PA_CHANNEL_POSITION_FRONT_LEFT;
> +
> +    case PA_CHANNEL_POSITION_REAR_LEFT:
> +        return PA_CHANNEL_POSITION_REAR_RIGHT;
> +
> +    case PA_CHANNEL_POSITION_REAR_RIGHT:
> +        return PA_CHANNEL_POSITION_REAR_LEFT;
> +
> +    case PA_CHANNEL_POSITION_SIDE_LEFT:
> +        return PA_CHANNEL_POSITION_SIDE_RIGHT;
> +
> +    case PA_CHANNEL_POSITION_SIDE_RIGHT:
> +        return PA_CHANNEL_POSITION_SIDE_LEFT;
> +
> +    case PA_CHANNEL_POSITION_FRONT_LEFT_OF_CENTER:
> +        return PA_CHANNEL_POSITION_FRONT_RIGHT_OF_CENTER;
> +
> +    case PA_CHANNEL_POSITION_FRONT_RIGHT_OF_CENTER:
> +        return PA_CHANNEL_POSITION_FRONT_LEFT_OF_CENTER;
> +
> +    case PA_CHANNEL_POSITION_TOP_FRONT_LEFT:
> +        return PA_CHANNEL_POSITION_TOP_FRONT_RIGHT;
> +
> +    case PA_CHANNEL_POSITION_TOP_FRONT_RIGHT:
> +        return PA_CHANNEL_POSITION_TOP_FRONT_LEFT;
> +
> +    case PA_CHANNEL_POSITION_TOP_REAR_LEFT:
> +        return PA_CHANNEL_POSITION_TOP_REAR_RIGHT;
> +
> +    case PA_CHANNEL_POSITION_TOP_REAR_RIGHT:
> +        return PA_CHANNEL_POSITION_TOP_REAR_LEFT;
> +
> +    default:
> +        return channel;
> +    }

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

> +}
> +
>  int pa__init(pa_module*m) {
>      struct userdata *u;
> -    pa_sample_spec ss;
> -    pa_channel_map map;
> +    pa_sample_spec ss, sink_input_ss;
> +    pa_channel_map map, sink_input_map;
>      pa_modargs *ma;
>      pa_sink *master=NULL;
>      pa_sink_input_new_data sink_input_data;
> @@ -483,6 +558,17 @@ int pa__init(pa_module*m) {
>      pa_bool_t force_flat_volume = FALSE;
>      pa_memchunk silence;
>  
> +    const char *hrir_file;
> +    char *input_buffer;
> +    unsigned i, j, found_channel;
> +    float hrir_sum, hrir_max;
> +    float *hrir_data;
> +
> +    pa_sample_spec hrir_temp_ss;
> +    pa_channel_map hrir_temp_map;
> +    pa_memchunk hrir_temp_chunk;
> +    pa_resampler *resampler;
> +
>      pa_assert(m);
>  
>      if (!(ma = pa_modargs_new(m->argument, valid_modargs))) {
> @@ -497,13 +583,15 @@ int pa__init(pa_module*m) {
>  
>      pa_assert(master);
>  
> -    ss = master->sample_spec;
> +    /* sample spec of sink */
> +    ss.rate = master->sample_spec.rate;
>      ss.format = PA_SAMPLE_FLOAT32;
> -    map = master->channel_map;
> -    if (pa_modargs_get_sample_spec_and_channel_map(ma, &ss, &map, PA_CHANNEL_MAP_DEFAULT) < 0) {
> -        pa_log("Invalid sample format specification or channel map");
> +    ss.channels = 6;    /* default to 6 channels */

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.

> +    if (pa_modargs_get_sample_spec(ma, &ss) < 0) {
> +        pa_log("Invalid sample format specification");
>          goto fail;
>      }
> +    ss.format = PA_SAMPLE_FLOAT32;
>  
>      if (pa_modargs_get_value_boolean(ma, "use_volume_sharing", &use_volume_sharing) < 0) {
>          pa_log("use_volume_sharing= expects a boolean argument");
> @@ -524,6 +612,53 @@ int pa__init(pa_module*m) {
>      u->module = m;
>      m->userdata = u;
>      u->channels = ss.channels;
> +    u->master_channels = 2;
> +
> +    /* Initialize hrir and input buffer */
> +    /* this is the hrir file for the left ear! */
> +    hrir_file = pa_modargs_get_value(ma, "hrir", NULL);

You need to check that hrir_file isn't NULL.

> +
> +    if (pa_sound_file_load(master->core->mempool, hrir_file, &hrir_temp_ss, &hrir_temp_map, &hrir_temp_chunk, NULL) < 0) {
> +        pa_log("Cannot load hrir file.");
> +        goto fail;
> +    }
> +
> +    /* sample spec / map of hrir */
> +    u->hrir_ss.format = ss.format;
> +    u->hrir_ss.rate = ss.rate;
> +    u->hrir_ss.channels = hrir_temp_ss.channels;
> +
> +    /* default hrir channel map */
> +    if (u->hrir_ss.channels == 6) {
> +        u->hrir_map.channels = 6;
> +        u->hrir_map.map[0] = PA_CHANNEL_POSITION_FRONT_LEFT;
> +        u->hrir_map.map[1] = PA_CHANNEL_POSITION_FRONT_RIGHT;
> +        u->hrir_map.map[2] = PA_CHANNEL_POSITION_FRONT_CENTER;
> +        u->hrir_map.map[3] = PA_CHANNEL_POSITION_LFE;
> +        u->hrir_map.map[4] = PA_CHANNEL_POSITION_REAR_LEFT;
> +        u->hrir_map.map[5] = PA_CHANNEL_POSITION_REAR_RIGHT;
> +    }

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?

> +
> +    if (pa_modargs_get_channel_map(ma, "hrir_channel_map", &u->hrir_map) < 0) {
> +        pa_log("Invalid hrir channel map");
> +        goto fail;
> +    }
> +
> +    /* map of sink */
> +    map = u->hrir_map;
> +    if (pa_modargs_get_channel_map(ma, NULL, &map) < 0) {
> +        pa_log("Invalid channel map");
> +        goto fail;
> +    }
> +
> +    /* sample spec / map of sink input */
> +    pa_channel_map_init_stereo(&sink_input_map);
> +    sink_input_ss.channels = 2;
> +    sink_input_ss.format = PA_SAMPLE_FLOAT32;
> +    sink_input_ss.rate = ss.rate;
> +
> +    u->sink_fs = pa_frame_size(&ss);
> +    u->fs = pa_frame_size(&sink_input_ss);
>  
>      /* Create sink */
>      pa_sink_new_data_init(&sink_data);
> @@ -583,8 +718,8 @@ int pa__init(pa_module*m) {
>      sink_input_data.origin_sink = u->sink;
>      pa_proplist_setf(sink_input_data.proplist, PA_PROP_MEDIA_NAME, "Virtual Sink Stream from %s", pa_proplist_gets(u->sink->proplist, PA_PROP_DEVICE_DESCRIPTION));
>      pa_proplist_sets(sink_input_data.proplist, PA_PROP_MEDIA_ROLE, "filter");
> -    pa_sink_input_new_data_set_sample_spec(&sink_input_data, &ss);
> -    pa_sink_input_new_data_set_channel_map(&sink_input_data, &map);
> +    pa_sink_input_new_data_set_sample_spec(&sink_input_data, &sink_input_ss);
> +    pa_sink_input_new_data_set_channel_map(&sink_input_data, &sink_input_map);
>  
>      pa_sink_input_new(&u->sink_input, m->core, &sink_input_data);
>      pa_sink_input_new_data_done(&sink_input_data);
> @@ -611,16 +746,88 @@ int pa__init(pa_module*m) {
>      u->sink->input_to_master = u->sink_input;
>  
>      pa_sink_input_get_silence(u->sink_input, &silence);
> -    u->memblockq = pa_memblockq_new("module-virtual-sink memblockq", 0, MEMBLOCKQ_MAXLENGTH, 0, &ss, 1, 1, 0, &silence);
> +    u->memblockq = pa_memblockq_new("module-virtual-surround-sink memblockq", 0, MEMBLOCKQ_MAXLENGTH, 0, &sink_input_ss, 1, 1, 0, &silence);
>      pa_memblock_unref(silence.memblock);
>  
> -    /* (9) INITIALIZE ANYTHING ELSE YOU NEED HERE */
> +    /* resample hrir */
> +    resampler = pa_resampler_new(u->sink->core->mempool, &hrir_temp_ss, &u->hrir_map, &u->hrir_ss, &u->hrir_map,
> +                                 PA_RESAMPLER_SRC_SINC_BEST_QUALITY, PA_RESAMPLER_NO_REMAP);
> +    pa_resampler_run(resampler, &hrir_temp_chunk, &u->hrir_chunk);
> +    pa_resampler_free(resampler);
> +    pa_memblock_unref(hrir_temp_chunk.memblock);

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.

> +
> +    u->hrir_samples =  u->hrir_chunk.length / pa_frame_size(&u->hrir_ss);
> +    u->hrir_channels = u->hrir_ss.channels;
> +
> +    if (u->hrir_map.channels != u->hrir_channels) {
> +        pa_log("number of channels in hrir file does not match number of channels in hrir channel map");
> +        goto fail;
> +    }
> +
> +    if (u->hrir_map.channels < map.channels) {
> +        pa_log("hrir file does not have enough channels!");
> +        goto fail;
> +    }
> +
> +    /* normalize hrir to avoid clipping */
> +    hrir_data = (float*) pa_memblock_acquire(u->hrir_chunk.memblock);
> +    hrir_max = 0;
> +    for (i = 0; i < u->hrir_samples; i++) {
> +        hrir_sum = 0;
> +        for (j = 0; j < u->hrir_channels; j++)
> +            hrir_sum += fabs(hrir_data[i * u->hrir_channels + j]);
> +
> +        if (hrir_sum > hrir_max)
> +            hrir_max = hrir_sum;
> +    }
> +    if (hrir_max > 1) {
> +        for (i = 0; i < u->hrir_samples; i++) {
> +            for (j = 0; j < u->hrir_channels; j++)
> +                hrir_data[i * u->hrir_channels + j] /= hrir_max * 1.2;
> +        }
> +    }
> +    pa_memblock_release(u->hrir_chunk.memblock);
> +
> +    /* create mapping between hrir and input */
> +    u->mapping_left = (unsigned *) pa_xnew0(unsigned, u->channels);
> +    u->mapping_right = (unsigned *) pa_xnew0(unsigned, u->channels);
> +    for (i = 0; i < map.channels; i++) {
> +        found_channel = 0;
> +
> +        for (j = 0; j < u->hrir_map.channels; j++) {
> +            if (u->hrir_map.map[j] == map.map[i]) {
> +                u->mapping_left[i] = j;
> +                found_channel = 1;
> +            }
> +
> +            if (u->hrir_map.map[j] == mirror_channel(map.map[i]))
> +                u->mapping_right[i] = j;
> +        }
> +
> +        if (!found_channel) {
> +            pa_log("Cannot find mapping for channel %s", pa_channel_position_to_string(map.map[i]));
> +            goto fail;
> +        }
> +
> +        if (map.map[i] == PA_CHANNEL_POSITION_FRONT_LEFT)
> +            u->output_left = i;
> +        if (map.map[i] == PA_CHANNEL_POSITION_FRONT_RIGHT)
> +            u->output_right = i;
> +    }
> +
> +    u->input_buffer_chunk.length = u->hrir_chunk.length;
> +    u->input_buffer_chunk.index = 0;
> +    u->input_buffer_chunk.memblock = pa_memblock_new_pool(u->sink->core->mempool, u->input_buffer_chunk.length);
> +
> +    input_buffer = (char *) pa_memblock_acquire(u->input_buffer_chunk.memblock);
> +    memset(input_buffer, 0, u->input_buffer_chunk.length);
> +    pa_memblock_release(u->input_buffer_chunk.memblock);
> +    u->input_buffer_offset = 0;
>  
>      pa_sink_put(u->sink);
>      pa_sink_input_put(u->sink_input);
>  
>      pa_modargs_free(ma);
> -
>      return 0;
>  
>  fail:
> @@ -667,5 +874,16 @@ void pa__done(pa_module*m) {
>      if (u->memblockq)
>          pa_memblockq_free(u->memblockq);
>  
> +    if (u->hrir_chunk.memblock)
> +        pa_memblock_unref(u->hrir_chunk.memblock);
> +
> +    if (u->input_buffer_chunk.memblock)
> +        pa_memblock_unref(u->input_buffer_chunk.memblock);
> +
> +    if (u->mapping_left)
> +        pa_xfree(u->mapping_left);
> +    if (u->mapping_right)
> +        pa_xfree(u->mapping_right);
> +
>      pa_xfree(u);
>  }
> -- 
> 1.7.8.4

-- 
Tanu




More information about the pulseaudio-discuss mailing list