On Sat, 2017-07-15 at 11:28 +0530, Arun Raghavan wrote:
>
> On Thu, 13 Jul 2017, at 12:59 AM, Tanu Kaskinen wrote:
> > If the description is not updated when moving, the old automatically
> > generated description will refer to the old master sink after the move,
> > which is not nice.
> > ---
> > src/modules/module-equalizer-sink.c | 26 ++++++++++++++++++++++----
> > 1 file changed, 22 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/modules/module-equalizer-sink.c
> > b/src/modules/module-equalizer-sink.c
> > index 280ca25fb..489062fc0 100644
> > --- a/src/modules/module-equalizer-sink.c
> > +++ b/src/modules/module-equalizer-sink.c
> > @@ -127,6 +127,8 @@ struct userdata {
> >
> > pa_database *database;
> > char **base_profiles;
> > +
> > + bool automatic_description;
> > };
> >
> > static const char* const valid_modargs[] = {
> > @@ -1080,6 +1082,17 @@ static void sink_input_moving_cb(pa_sink_input *i,
> > pa_sink *dest) {
> > if (dest) {
> > pa_sink_set_asyncmsgq(u->sink, dest->asyncmsgq);
> > pa_sink_update_flags(u->sink,
> > PA_SINK_LATENCY|PA_SINK_DYNAMIC_LATENCY, dest->flags);
> > +
> > + if (u->automatic_description) {
> > + const char *master_description;
> > + char *new_description;
> > +
> > + master_description = pa_proplist_gets(dest->proplist,
> > PA_PROP_DEVICE_DESCRIPTION);
> > + new_description = pa_sprintf_malloc(_("FFT based equalizer
> > on %s"),
> > + master_description ?
> > master_description : dest->name);
> > + pa_sink_set_description(u->sink, new_description);
> > + pa_xfree(new_description);
> > + }
> > } else
> > pa_sink_set_asyncmsgq(u->sink, NULL);
> > }
> > @@ -1089,7 +1102,6 @@ int pa__init(pa_module*m) {
> > pa_sample_spec ss;
> > pa_channel_map map;
> > pa_modargs *ma;
> > - const char *z;
> > pa_sink *master;
> > pa_sink_input_new_data sink_input_data;
> > pa_sink_new_data sink_data;
> > @@ -1185,9 +1197,6 @@ int pa__init(pa_module*m) {
> > pa_sink_new_data_set_sample_spec(&sink_data, &ss);
> > pa_sink_new_data_set_channel_map(&sink_data, &map);
> >
> > - z = pa_proplist_gets(master->proplist, PA_PROP_DEVICE_DESCRIPTION);
> > - pa_proplist_setf(sink_data.proplist, PA_PROP_DEVICE_DESCRIPTION,
> > "FFT based equalizer on %s", z ? z : master->name);
> > -
> > pa_proplist_sets(sink_data.proplist, PA_PROP_DEVICE_MASTER_DEVICE,
> > master->name);
> > pa_proplist_sets(sink_data.proplist, PA_PROP_DEVICE_CLASS,
> > "filter");
> >
> > @@ -1197,6 +1206,15 @@ int pa__init(pa_module*m) {
> > goto fail;
> > }
> >
> > + if (!pa_proplist_contains(sink_data.proplist,
> > PA_PROP_DEVICE_DESCRIPTION)) {
> > + const char *master_description;
> > +
> > + master_description = pa_proplist_gets(master->proplist,
> > PA_PROP_DEVICE_DESCRIPTION);
> > + pa_proplist_setf(sink_data.proplist, PA_PROP_DEVICE_DESCRIPTION,
> > + _("FFT based equalizer on %s"),
> > master_description ? master_description : master->name);
> > + u->automatic_description = true;
> > + }
> > +
>
> Looks fine, modulo a minor style comment that you can implement if you
> feel like -- it's nicer to factor the string construction to a single
> function rather than repeat between the two locations.
>
> We likely need to do this across filters, so maybe we should be
> factoring out other such code (such as autoload handling) and have them
> reusable across modules.
I'll apply this as is, but I'll later look into simplifying the
description updating for all filter devices.
--
Tanu
https://www.patreon.com/tanuk
