[pulseaudio-discuss] [PATCHv2 0/4] modules: add module-remap-source

[Stefan Huber]

Hi Tanu,

thank you very much for your comments! I have adapted the patch
according to your suggestions. In addition I added a three additional
patches for other modules that address your comments.

Best,
Stefan

On Tue 05.03.13  16:49, Tanu Kaskinen wrote:
> On Fri, 2013-02-22 at 18:26 +0100, Stefan Huber wrote:
> > +/* Called from output thread context */
> > +static int source_output_process_msg_cb(pa_msgobject *obj, int code, void *data, int64_t offset, pa_memchunk *chunk) {
> > +    return pa_source_output_process_msg(obj, code, data, offset, chunk);
> > +}
> 
> This function is redundant. When a new source output is created, the
> default message handler will be pa_source_output_process_msg().

module-virtual-source contains the same callback. Patch 3 removes it,
too.

> > +/* Called from main thread */
> > +static bool source_output_may_move_to_cb(pa_source_output *o, pa_source *dest) {
> > +    struct userdata *u;
> > +
> > +    pa_source_output_assert_ref(o);
> > +    pa_assert_ctl_context();
> > +    pa_assert_se(u = o->userdata);
> > +
> > +    return u->source != dest;
> > +}
> 
> This function is redundant, because the core already does loop
> detection. u->source != dest isn't sufficient for detecting loops
> anyway.
> 
> There are probably several modules that have a similar may_move_to_cb
> function. All those could be removed.

Patch 2 removes it from module-loopback and module-virtual-source.
Probably, module-echo-cancel should be adapted as well?

> > +    u = pa_xnew0(struct userdata, 1);
> > +    if (!u) {
> > +        pa_log("Failed to alloc userdata.");
> > +        goto fail;
> > +    }
> 
> pa_xnew0() never fails, so this is a redundant check.

Thanks, fixed by Patch 1 for module-remap-source and by Patch 3 for
module-virtual-source.

> > +    /* FIXME
> > +       source_output_data.flags = PA_SOURCE_OUTPUT_DONT_INHIBIT_AUTO_SUSPEND; */
> 
> What's the story behind this FIXME? Why would you want to set that flag?
> Note that when the remap source is suspended, the source output is
> automatically corked, which in turn allows the master source to be
> suspended too.

module-echo-cancel and module-virtual-source contain these lines as
well. Patch 4 removes them.


Stefan Huber (3):
  modules: add module-remap-source
  modules: Remove obsolete source_output_may_move_to_cb()
  module-virtual-source: remove redundant checks and callbacks
  modules: remove obsolete comment on
    PA_SOURCE_OUTPUT_DONT_INHIBIT_AUTO_SUSPEND

 src/Makefile.am                              |    6 +
 src/modules/echo-cancel/module-echo-cancel.c |    2 -
 src/modules/module-loopback.c                |   12 -
 src/modules/module-remap-source.c            |  435 ++++++++++++++++++++++++++
 src/modules/module-virtual-source.c          |   30 --
 5 files changed, 441 insertions(+), 44 deletions(-)
 create mode 100644 src/modules/module-remap-source.c

-- 
1.7.9.5