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
