[pulseaudio-discuss] [RESEND] [PATCH v3 1/2] bluetooth: Add support for automatic switch between hsp and a2dp profiles

[Pali Rohár]

On Tuesday 02 February 2016 14:24:07 Arun Raghavan wrote:
> > > > @@ -35,16 +35,18 @@
> > > >  
> > > >  #include "module-bluetooth-policy-symdef.h"
> > > >  
> > > > -PA_MODULE_AUTHOR("Frédéric Dalleau");
> > > > -PA_MODULE_DESCRIPTION("When a bluetooth sink or source is added,
> > > > load module-loopback");
> > > > +PA_MODULE_AUTHOR("Frédéric Dalleau, Pali Rohár");
> > > > +PA_MODULE_DESCRIPTION("Automatically switch between bluetooth
> > > > hsp
> > > > and a2dp profiles and automatically load module-loopback when a
> > > > bluetooth sink (ag) or source (ag, a2dp_source) is added");
> > > 
> > > I'd just rewrite this to be "Policy module to make using bluetooth
> > > devices out-of-the-box easier".
> > 
> > Personally I'm fine with both descriptions so its up to other
> > pulseaudio
> > reviewers/maintainers.
> 
> I prefer the shorter name in the description, so let's go with that.

Ok.

> > > >  PA_MODULE_VERSION(PACKAGE_VERSION);
> > > >  PA_MODULE_LOAD_ONCE(true);
> > > >  PA_MODULE_USAGE(
> > > > +        "switch=<Switch between hsp and a2dp profile?> "
> > > 
> > > Maybe call this "autoswitch".
> > 
> > "autoswitch" or "auto_switch"?
> 
> I'm not a huge fan of the underscores in our modarg names, but I'm fine
> with either.

Ok.

> > > > +    /* Flag this card for revert */
> > > > +    pa_proplist_sets(new_data->proplist, "bluez-revert",
> > > > "true");
> > > > +    return PA_HOOK_OK;
> > > 
> > > What is the reasoning behind having this property?
> > 
> > For each card I need to know if card needs to be "reverted" in
> > future.
> 
> Is this for a case where a device is manually kept in the hsp profile
> and you don't want to touch that?

Yes. When user chose manually to use hsp profile.

> > > In general, I prefer the module doing its book-keeping locally
> > > rather
> > > than (ab)using the proplist for state.
> > 
> > Ok, what to use then? How to store for each private boolean flag
> > needed
> > just by this module?
> 
> Usually you'd do something like have a hashmap in the module's
> userdata, and then you could add your pa_card to that hashmap if it
> needs a revert (and remove when you're done with the revert). The
> userdata is available in all the hook callbacks.

But then I need to deal with removing card (e.g. somebody unplug usb
bluetooth adapter when voice call via hsp profile is active)... And
maybe other scenarios?

-- 
Pali Rohár
pali.rohar at gmail.com