[pulseaudio-discuss] [PATCH v3 1/2] loopback: Enable routing on loopback streams

[Tanu Kaskinen]

On Tue, 2012-04-03 at 17:18 +0200, Frédéric Dalleau wrote:
> At module-loopback load, if no sink is given, the default sink is used. If the
> stream has a media.role property, the property cannot be used because a the
> source or sink is forced to default. Both module-intended-roles and
> module-device-manager are affected. The same apply to sources.
> 
> With this patch, if sink or source is missing, routing modules can be used.
> ---
>  src/modules/module-loopback.c |   39 +++++++++++++++++++++++++++------------
>  1 files changed, 27 insertions(+), 12 deletions(-)
> 
> diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c
> index 0d65682..1fdf273 100644
> --- a/src/modules/module-loopback.c
> +++ b/src/modules/module-loopback.c
> @@ -645,10 +645,10 @@ static pa_bool_t sink_input_may_move_to_cb(pa_sink_input *i, pa_sink *dest) {
>  int pa__init(pa_module *m) {
>      pa_modargs *ma = NULL;
>      struct userdata *u;
> -    pa_sink *sink;
> +    pa_sink *sink = NULL;
>      pa_sink_input_new_data sink_input_data;
>      pa_bool_t sink_dont_move;
> -    pa_source *source;
> +    pa_source *source = NULL;
>      pa_source_output_new_data source_output_data;
>      pa_bool_t source_dont_move;
>      uint32_t latency_msec;
> @@ -666,12 +666,14 @@ int pa__init(pa_module *m) {
>          goto fail;
>      }
>  
> -    if (!(source = pa_namereg_get(m->core, pa_modargs_get_value(ma, "source", NULL), PA_NAMEREG_SOURCE))) {
> +    n = pa_modargs_get_value(ma, "source", NULL);
> +    if (n && !(source = pa_namereg_get(m->core, n, PA_NAMEREG_SOURCE))) {
>          pa_log("No such source.");
>          goto fail;
>      }
>  
> -    if (!(sink = pa_namereg_get(m->core, pa_modargs_get_value(ma, "sink", NULL), PA_NAMEREG_SINK))) {
> +    n = pa_modargs_get_value(ma, "sink", NULL);
> +    if (n && !(sink = pa_namereg_get(m->core, n, PA_NAMEREG_SINK))) {
>          pa_log("No such sink.");
>          goto fail;
>      }
> @@ -681,8 +683,18 @@ int pa__init(pa_module *m) {
>          goto fail;
>      }
>  
> -    ss = sink->sample_spec;
> -    map = sink->channel_map;
> +    if (!sink && !source)
> +        sink = pa_namereg_get_default_sink(m->core);

This still overrides the routing modules, right?

I guess this is done, because otherwise you don't know how to initialize
ss and map? I think a better solution would be to specify the
PA_SINK_INPUT_FIX_FORMAT flag and its friends when calling
pa_sink_input_new() when neither sink or source has been given. After
the pa_sink_input_new() call, ss and map can be copied from the sink
input, which has got them from the sink to which it was routed.

> +
> +    if (sink) {
> +        ss = sink->sample_spec;
> +        map = sink->channel_map;
> +    } else if (source) {
> +        ss = source->sample_spec;
> +        map = source->channel_map;
> +    } else
> +        goto fail;
> +
>      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");
>          goto fail;
> @@ -713,7 +725,9 @@ int pa__init(pa_module *m) {
>      pa_sink_input_new_data_init(&sink_input_data);
>      sink_input_data.driver = __FILE__;
>      sink_input_data.module = m;
> -    pa_sink_input_new_data_set_sink(&sink_input_data, sink, FALSE);
> +
> +    if(sink)

Cosmetic: Missing space after "if".

> +        pa_sink_input_new_data_set_sink(&sink_input_data, sink, FALSE);
>  
>      if (pa_modargs_get_proplist(ma, "sink_input_properties", sink_input_data.proplist, PA_UPDATE_REPLACE) < 0) {
>          pa_log("Failed to parse the sink_input_properties value.");
> @@ -723,12 +737,12 @@ int pa__init(pa_module *m) {
>  
>      if (!pa_proplist_contains(sink_input_data.proplist, PA_PROP_MEDIA_NAME))
>          pa_proplist_setf(sink_input_data.proplist, PA_PROP_MEDIA_NAME, "Loopback from %s",
> -                         pa_strnull(pa_proplist_gets(source->proplist, PA_PROP_DEVICE_DESCRIPTION)));
> +                         source ? pa_strnull(pa_proplist_gets(source->proplist, PA_PROP_DEVICE_DESCRIPTION)) : "unspecified source");
>  
>      if (!pa_proplist_contains(sink_input_data.proplist, PA_PROP_MEDIA_ROLE))
>          pa_proplist_sets(sink_input_data.proplist, PA_PROP_MEDIA_ROLE, "abstract");
>  
> -    if (!pa_proplist_contains(sink_input_data.proplist, PA_PROP_MEDIA_ICON_NAME)
> +    if (source && !pa_proplist_contains(sink_input_data.proplist, PA_PROP_MEDIA_ICON_NAME)
>              && (n = pa_proplist_gets(source->proplist, PA_PROP_DEVICE_ICON_NAME)))
>          pa_proplist_sets(sink_input_data.proplist, PA_PROP_MEDIA_ICON_NAME, n);

Now if sink or source is not set, the device description and icon name
properties are left at a suboptimal state. Instead of checking for the
sink and source pointers, maybe this proplist handling could be moved
after pa_sink_input_new() and pa_source_output_new() have been called?
At that time the sink and source would be available.

-- 
Tanu