[pulseaudio-discuss] [PATCH] rtp-send: Add "allow_suspend_on_idle" module argument

[Alexander E. Patrakov]

22.02.2014 16:21, Tanu Kaskinen wrote:
> Some people want module-rtp-send to send silence when the sink that is
> monitored goes idle, and some people want module-rtp-send to pause the
> RTP stream to avoid unnecessary bandwidth consumption.
> ---
>   src/modules/rtp/module-rtp-send.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/src/modules/rtp/module-rtp-send.c b/src/modules/rtp/module-rtp-send.c
> index 581e6be..0d82c51 100644
> --- a/src/modules/rtp/module-rtp-send.c
> +++ b/src/modules/rtp/module-rtp-send.c
> @@ -68,7 +68,8 @@ PA_MODULE_USAGE(
>           "port=<port number> "
>           "mtu=<maximum transfer unit> "
>           "loop=<loopback to local host?> "
> -        "ttl=<ttl value>"
> +        "ttl=<ttl value> "
> +        "allow_suspend_on_idle=<boolean>"
>   );
>
>   #define DEFAULT_PORT 46000
> @@ -92,6 +93,7 @@ static const char* const valid_modargs[] = {
>       "mtu" ,
>       "loop",
>       "ttl",
> +    "allow_suspend_on_idle",
>       NULL
>   };
>
> @@ -188,6 +190,7 @@ int pa__init(pa_module*m) {
>       socklen_t k;
>       char hn[128], *n;
>       bool loop = false;
> +    bool allow_suspend_on_idle;
>       pa_source_output_new_data data;
>
>       pa_assert(m);
> @@ -207,6 +210,15 @@ int pa__init(pa_module*m) {
>           goto fail;
>       }
>
> +    /* By default, allow monitor sources to suspend on idle to avoid consuming
> +     * bandwidth just for sending silence. */
> +    allow_suspend_on_idle = !!s->monitor_of;
> +
> +    if (pa_modargs_get_value_boolean(ma, "allow_suspend_on_idle", &allow_suspend_on_idle)) {
> +        pa_log("Failed to parse \"allow_suspend_on_idle\" parameter.");
> +        goto fail;
> +    }
> +
>       ss = s->sample_spec;
>       pa_rtp_sample_spec_fixup(&ss);
>       cm = s->channel_map;
> @@ -378,6 +390,9 @@ int pa__init(pa_module*m) {
>       pa_source_output_new_data_set_sample_spec(&data, &ss);
>       pa_source_output_new_data_set_channel_map(&data, &cm);
>
> +    if (allow_suspend_on_idle)
> +        data.flags = PA_SOURCE_OUTPUT_DONT_INHIBIT_AUTO_SUSPEND;
> +
>       pa_source_output_new(&o, m->core, &data);
>       pa_source_output_new_data_done(&data);
>
>

I have looked at the patch, but have not tested it. For me, it looks 
good, except one minor thing.

The default policy whether to allow suspending on idle depends on the 
module argument (which is OK), and, if it is not given, on the initial 
source. The module creates a source output for its operation, but that 
source output, by default, can be moved. It did not matter before, 
because the policy was the same anyway. Now it matters, so please think 
about it. Maybe PA_SOURCE_OUTPUT_DONT_MOVE is needed, too? I am not 
sure, so please use your own judgement.

Reviewed-by: Alexander E. Patrakov <patrakov at gmail.com>

-- 
Alexander E. Patrakov