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
