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

[Tanu Kaskinen]

On Sat, 2014-02-22 at 18:22 +0600, Alexander E. Patrakov wrote:
> 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.

Good point, thanks. I don't think DONT_MOVE is a good idea, because it
limits what the user can do with the stream, but we can update the
DONT_INHIBIT_AUTO_SUSPEND flag when moving the stream. I'll write
another version where the "allow_suspending_on_idle" argument accepts
three values: "always", "never" or "only_with_monitor_sources".

-- 
Tanu