On Thursday 25 September 2014 13:27:04 David Henningsson wrote:
> On 2014-09-25 02:25, Pali Roh?r wrote:
> > With this patch it is possible to configure different volume
> > settings for a2dp and hsp profiles. It is usefull for
> > bluetooth headsets which do not have normalized a2dp and
> > hsp volume levels.
> >
> > Module module-device-restore uses port name as identifier,
> > so if different profiles have different names
> > module-device-restore can store volume settings for each
> > profile.
>
> Hi Pali and thanks for the patch,
>
> I understand the problem here, but this is the wrong way to
> fix it.
>
> First, ports is what unity- and gnome-control-center thinks
> are most closely to physical devices. If you have two
> different ports, they will appear like two different devices.
>
Bluetooth A2DP and HSP are different streams, so they are
basically different devices. They are using different codec and
have different quality. So I think that "port" name for these
bluetooth transport should be different.
In future I'm planning to add support for bidirectional a2dp
streaming support (with two bluetooth transports) so here I need
two ports for it. I did not see any bluetooth headset with a2dp
microphone, but on linux bluez systems it is possible and it is
*only* one way how to get high quality sink+source streaming
between two machine (via bluetooth).
And some another rare headsets support using both a2dp and hsp
transports at same time. Pulseaudio does not support this
configuration too, but maybe in future (if I have one) I will
want to see support in PA too. And for this configuration is more
ports support needed.
In above both bluetooth possible configurations are bluetooth
transports in use and thus should be visible as *different* sound
sink/sources (as this is for sending different streams via
bluetooth). So I think that using more ports (with different)
names is good idea and if gnome-control-center shows different
ports as different cards, I think this is correct.
> Second, I believe this problem could potentially exist with
> other (non-bluez) devices too. Imagine the same "Line Out"
> that is present in both stereo and 5.1 profiles. If you
> switch to 5.1 mode, set all your volumes right, and then
> switch to stereo and change the stereo controls, the other
> four channels' settings are lost.
>
> Maybe it's module-device-restore that needs fixing instead,
> but it would be good to have more people's opinion on that
> first.
>
> > Before this patch all bluetooth sinks and sources had same
> > identifier (port name) so module-device-restore was not
> > able to distinguish between profiles. Now every port name
> > has suffix with profile name.
> >
> > Note that similar patch is needed also for bluez5, but I'm
> > not using bluez5 so I cannot write or test it.
> >
> > Signed-off-by: Pali Roh?r <pali.rohar at gmail.com>
> > ---
> >
> > src/modules/bluetooth/module-bluez4-device.c | 203
> > +++++++++++++++++++------- 1 file changed, 150
> > insertions(+), 53 deletions(-)
> >
> > diff --git a/src/modules/bluetooth/module-bluez4-device.c
> > b/src/modules/bluetooth/module-bluez4-device.c index
> > c70e4a6..e1368f0 100644
> > --- a/src/modules/bluetooth/module-bluez4-device.c
> > +++ b/src/modules/bluetooth/module-bluez4-device.c
> > @@ -152,8 +152,12 @@ struct userdata {
> >
> > pa_bluez4_discovery *discovery;
> > bool auto_connect;
> >
> > - char *output_port_name;
> > - char *input_port_name;
> > + char *output_a2dp_port_name;
> > + char *output_hsp_port_name;
> > + char *output_hfgw_port_name;
> > + char *input_a2dp_port_name;
> > + char *input_hsp_port_name;
> > + char *input_hfgw_port_name;
> >
> > pa_card *card;
> > pa_sink *sink;
> >
> > @@ -1218,38 +1222,26 @@ static pa_direction_t
> > get_profile_direction(pa_bluez4_profile_t p) {
> >
> > }
> >
> > /* Run from main thread */
> >
> > -static pa_available_t get_port_availability(struct userdata
> > *u, pa_direction_t direction) { - pa_available_t result
> > = PA_AVAILABLE_NO;
> > - unsigned i;
> > +static pa_available_t get_port_availability(struct userdata
> > *u, pa_direction_t direction, pa_bluez4_profile_t p) { +
> > pa_bluez4_transport *transport;
> >
> > pa_assert(u);
> > pa_assert(u->device);
> >
> > - for (i = 0; i < PA_BLUEZ4_PROFILE_COUNT; i++) {
> > - pa_bluez4_transport *transport;
> > -
> > - if (!(get_profile_direction(i) & direction))
> > - continue;
> > -
> > - if (!(transport = u->device->transports[i]))
> > - continue;
> > -
> > - switch(transport->state) {
> > - case PA_BLUEZ4_TRANSPORT_STATE_DISCONNECTED:
> > - continue;
> > -
> > - case PA_BLUEZ4_TRANSPORT_STATE_IDLE:
> > - if (result == PA_AVAILABLE_NO)
> > - result = PA_AVAILABLE_UNKNOWN;
> > -
> > - break;
> > + if (!(get_profile_direction(p) & direction))
> > + return PA_AVAILABLE_NO;
> >
> > - case PA_BLUEZ4_TRANSPORT_STATE_PLAYING:
> > - return PA_AVAILABLE_YES;
> > - }
> > - }
> > + if (!(transport = u->device->transports[p]))
> > + return PA_AVAILABLE_NO;
> >
> > - return result;
> > + switch(transport->state) {
> > + case PA_BLUEZ4_TRANSPORT_STATE_IDLE:
> > + case PA_BLUEZ4_TRANSPORT_STATE_PLAYING:
> > + return PA_AVAILABLE_YES;
> > + case PA_BLUEZ4_TRANSPORT_STATE_DISCONNECTED:
> > + default:
> > + return PA_AVAILABLE_NO;
> > + }
> >
> > }
> >
> > /* Run from main thread */
> >
> > @@ -1274,11 +1266,23 @@ static void
> > handle_transport_state_change(struct userdata *u, struct
> > pa_bluez4_t
> >
> > pa_card_profile_set_available(cp,
> > transport_state_to_availability(state));
> >
> > /* Update port availability */
> >
> > - pa_assert_se(port = pa_hashmap_get(u->card->ports,
> > u->output_port_name)); -
> > pa_device_port_set_available(port, get_port_availability(u,
> > PA_DIRECTION_OUTPUT)); + pa_assert_se(port =
> > pa_hashmap_get(u->card->ports, u->output_a2dp_port_name));
> > + pa_device_port_set_available(port,
> > get_port_availability(u, PA_DIRECTION_OUTPUT,
> > PA_BLUEZ4_PROFILE_A2DP));
> >
> > - pa_assert_se(port = pa_hashmap_get(u->card->ports,
> > u->input_port_name)); -
> > pa_device_port_set_available(port, get_port_availability(u,
> > PA_DIRECTION_INPUT)); + pa_assert_se(port =
> > pa_hashmap_get(u->card->ports, u->output_hsp_port_name)); +
> > pa_device_port_set_available(port,
> > get_port_availability(u, PA_DIRECTION_OUTPUT,
> > PA_BLUEZ4_PROFILE_HSP)); +
> > + pa_assert_se(port = pa_hashmap_get(u->card->ports,
> > u->output_hfgw_port_name)); +
> > pa_device_port_set_available(port, get_port_availability(u,
> > PA_DIRECTION_OUTPUT, PA_BLUEZ4_PROFILE_HFGW)); +
> > + pa_assert_se(port = pa_hashmap_get(u->card->ports,
> > u->input_a2dp_port_name)); +
> > pa_device_port_set_available(port, get_port_availability(u,
> > PA_DIRECTION_INPUT, PA_BLUEZ4_PROFILE_A2DP_SOURCE)); +
> > + pa_assert_se(port = pa_hashmap_get(u->card->ports,
> > u->input_hsp_port_name)); +
> > pa_device_port_set_available(port, get_port_availability(u,
> > PA_DIRECTION_INPUT, PA_BLUEZ4_PROFILE_HSP)); +
> > + pa_assert_se(port = pa_hashmap_get(u->card->ports,
> > u->input_hfgw_port_name)); +
> > pa_device_port_set_available(port, get_port_availability(u,
> > PA_DIRECTION_INPUT, PA_BLUEZ4_PROFILE_HFGW));
> >
> > /* Acquire or release transport as needed */
> > acquire = (state == PA_BLUEZ4_TRANSPORT_STATE_PLAYING
> > && u->profile == profile);
> >
> > @@ -1515,21 +1519,60 @@ static pa_hook_result_t
> > transport_speaker_gain_changed_cb(pa_bluez4_discovery *y
> >
> > return PA_HOOK_OK;
> >
> > }
> >
> > +static const char *get_output_port_name(struct userdata *u)
> > { + switch (u->profile) {
> > + case PA_BLUEZ4_PROFILE_A2DP:
> > + return u->output_a2dp_port_name;
> > + case PA_BLUEZ4_PROFILE_HSP:
> > + return u->output_hsp_port_name;
> > + case PA_BLUEZ4_PROFILE_HFGW:
> > + return u->output_hfgw_port_name;
> > + case PA_BLUEZ4_PROFILE_A2DP_SOURCE:
> > + case PA_BLUEZ4_PROFILE_OFF:
> > + default:
> > + return NULL;
> > + }
> > +}
> > +
> > +static const char *get_input_port_name(struct userdata *u)
> > { + switch (u->profile) {
> > + case PA_BLUEZ4_PROFILE_A2DP_SOURCE:
> > + return u->input_a2dp_port_name;
> > + case PA_BLUEZ4_PROFILE_HSP:
> > + return u->input_hsp_port_name;
> > + case PA_BLUEZ4_PROFILE_HFGW:
> > + return u->input_hfgw_port_name;
> > + case PA_BLUEZ4_PROFILE_A2DP:
> > + case PA_BLUEZ4_PROFILE_OFF:
> > + default:
> > + return NULL;
> > + }
> > +}
> > +
> >
> > static void connect_ports(struct userdata *u, void
> > *sink_or_source_new_data, pa_direction_t direction) {
> >
> > + const char *port_name;
> >
> > pa_device_port *port;
> >
> > if (direction == PA_DIRECTION_OUTPUT) {
> >
> > pa_sink_new_data *sink_new_data =
> > sink_or_source_new_data;
> >
> > + port_name = get_output_port_name(u);
> >
> > - pa_assert_se(port = pa_hashmap_get(u->card->ports,
> > u->output_port_name)); + pa_assert(port_name);
> > + pa_assert_se(port = pa_hashmap_get(u->card->ports,
> > port_name));
> >
> > pa_assert_se(pa_hashmap_put(sink_new_data->ports,
> > port->name, port) >= 0);
> > pa_device_port_ref(port);
> >
> > + pa_sink_new_data_set_port(sink_new_data,
> > port_name); + sink_new_data->save_port = true;
> >
> > } else {
> >
> > pa_source_new_data *source_new_data =
> > sink_or_source_new_data;
> >
> > + port_name = get_input_port_name(u);
> >
> > - pa_assert_se(port = pa_hashmap_get(u->card->ports,
> > u->input_port_name)); + pa_assert(port_name);
> > + pa_assert_se(port = pa_hashmap_get(u->card->ports,
> > port_name));
> >
> > pa_assert_se(pa_hashmap_put(source_new_data->ports
> > , port->name, port) >= 0);
> > pa_device_port_ref(port);
> >
> > + pa_source_new_data_set_port(source_new_data,
> > port_name); + source_new_data->save_port = true;
> >
> > }
> >
> > }
> >
> > @@ -2141,23 +2184,63 @@ static void create_card_ports(struct
> > userdata *u, pa_hashmap *ports) {
> >
> > if (!input_description)
> >
> > input_description = _("Bluetooth Input");
> >
> > - u->output_port_name = pa_sprintf_malloc("%s-output",
> > name_prefix); - u->input_port_name =
> > pa_sprintf_malloc("%s-input", name_prefix); +
> > u->output_a2dp_port_name =
> > pa_sprintf_malloc("%s-output-a2dp", name_prefix); +
> > u->output_hsp_port_name =
> > pa_sprintf_malloc("%s-output-hsp", name_prefix); +
> > u->output_hfgw_port_name =
> > pa_sprintf_malloc("%s-output-hfgw", name_prefix); +
> > u->input_a2dp_port_name =
> > pa_sprintf_malloc("%s-input-a2dp", name_prefix); +
> > u->input_hsp_port_name = pa_sprintf_malloc("%s-input-hsp",
> > name_prefix); + u->input_hfgw_port_name =
> > pa_sprintf_malloc("%s-input-hfgw", name_prefix);
> >
> > pa_device_port_new_data_init(&port_data);
> >
> > - pa_device_port_new_data_set_name(&port_data,
> > u->output_port_name); +
> > pa_device_port_new_data_set_name(&port_data,
> > u->output_a2dp_port_name);
> >
> > pa_device_port_new_data_set_description(&port_data,
> > output_description);
> > pa_device_port_new_data_set_direction(&port_data,
> > PA_DIRECTION_OUTPUT);
> >
> > - pa_device_port_new_data_set_available(&port_data,
> > get_port_availability(u, PA_DIRECTION_OUTPUT)); +
> > pa_device_port_new_data_set_available(&port_data,
> > get_port_availability(u, PA_DIRECTION_OUTPUT,
> > PA_BLUEZ4_PROFILE_A2DP)); + pa_assert_se(port =
> > pa_device_port_new(u->core, &port_data, 0)); +
> > pa_assert_se(pa_hashmap_put(ports, port->name, port) >= 0);
> > + pa_device_port_new_data_done(&port_data);
> > +
> > + pa_device_port_new_data_init(&port_data);
> > + pa_device_port_new_data_set_name(&port_data,
> > u->output_hsp_port_name); +
> > pa_device_port_new_data_set_description(&port_data,
> > output_description); +
> > pa_device_port_new_data_set_direction(&port_data,
> > PA_DIRECTION_OUTPUT); +
> > pa_device_port_new_data_set_available(&port_data,
> > get_port_availability(u, PA_DIRECTION_OUTPUT,
> > PA_BLUEZ4_PROFILE_HSP)); + pa_assert_se(port =
> > pa_device_port_new(u->core, &port_data, 0)); +
> > pa_assert_se(pa_hashmap_put(ports, port->name, port) >= 0);
> > + pa_device_port_new_data_done(&port_data);
> > +
> > + pa_device_port_new_data_init(&port_data);
> > + pa_device_port_new_data_set_name(&port_data,
> > u->output_hfgw_port_name); +
> > pa_device_port_new_data_set_description(&port_data,
> > output_description); +
> > pa_device_port_new_data_set_direction(&port_data,
> > PA_DIRECTION_OUTPUT); +
> > pa_device_port_new_data_set_available(&port_data,
> > get_port_availability(u, PA_DIRECTION_OUTPUT,
> > PA_BLUEZ4_PROFILE_HFGW)); + pa_assert_se(port =
> > pa_device_port_new(u->core, &port_data, 0)); +
> > pa_assert_se(pa_hashmap_put(ports, port->name, port) >= 0);
> > + pa_device_port_new_data_done(&port_data);
> > +
> > + pa_device_port_new_data_init(&port_data);
> > + pa_device_port_new_data_set_name(&port_data,
> > u->input_a2dp_port_name); +
> > pa_device_port_new_data_set_description(&port_data,
> > input_description); +
> > pa_device_port_new_data_set_direction(&port_data,
> > PA_DIRECTION_INPUT); +
> > pa_device_port_new_data_set_available(&port_data,
> > get_port_availability(u, PA_DIRECTION_INPUT,
> > PA_BLUEZ4_PROFILE_A2DP_SOURCE)); + pa_assert_se(port =
> > pa_device_port_new(u->core, &port_data, 0)); +
> > pa_assert_se(pa_hashmap_put(ports, port->name, port) >= 0);
> > + pa_device_port_new_data_done(&port_data);
> > +
> > + pa_device_port_new_data_init(&port_data);
> > + pa_device_port_new_data_set_name(&port_data,
> > u->input_hsp_port_name); +
> > pa_device_port_new_data_set_description(&port_data,
> > input_description); +
> > pa_device_port_new_data_set_direction(&port_data,
> > PA_DIRECTION_INPUT); +
> > pa_device_port_new_data_set_available(&port_data,
> > get_port_availability(u, PA_DIRECTION_INPUT,
> > PA_BLUEZ4_PROFILE_HSP));
> >
> > pa_assert_se(port = pa_device_port_new(u->core,
> > &port_data, 0)); pa_assert_se(pa_hashmap_put(ports,
> > port->name, port) >= 0);
> > pa_device_port_new_data_done(&port_data);
> >
> > pa_device_port_new_data_init(&port_data);
> >
> > - pa_device_port_new_data_set_name(&port_data,
> > u->input_port_name); +
> > pa_device_port_new_data_set_name(&port_data,
> > u->input_hfgw_port_name);
> >
> > pa_device_port_new_data_set_description(&port_data,
> > input_description);
> > pa_device_port_new_data_set_direction(&port_data,
> > PA_DIRECTION_INPUT);
> >
> > - pa_device_port_new_data_set_available(&port_data,
> > get_port_availability(u, PA_DIRECTION_INPUT)); +
> > pa_device_port_new_data_set_available(&port_data,
> > get_port_availability(u, PA_DIRECTION_INPUT,
> > PA_BLUEZ4_PROFILE_HFGW));
> >
> > pa_assert_se(port = pa_device_port_new(u->core,
> > &port_data, 0)); pa_assert_se(pa_hashmap_put(ports,
> > port->name, port) >= 0);
> > pa_device_port_new_data_done(&port_data);
> >
> > @@ -2165,14 +2248,24 @@ static void create_card_ports(struct
> > userdata *u, pa_hashmap *ports) {
> >
> > /* Run from main thread */
> > static pa_card_profile *create_card_profile(struct
> > userdata *u, const char *uuid, pa_hashmap *ports) {
> >
> > - pa_device_port *input_port, *output_port;
> > + pa_device_port *input_a2dp_port, *output_a2dp_port;
> > + pa_device_port *input_hsp_port, *output_hsp_port;
> > + pa_device_port *input_hfgw_port, *output_hfgw_port;
> >
> > pa_card_profile *p = NULL;
> > pa_bluez4_profile_t *d;
> >
> > - pa_assert(u->input_port_name);
> > - pa_assert(u->output_port_name);
> > - pa_assert_se(input_port = pa_hashmap_get(ports,
> > u->input_port_name)); - pa_assert_se(output_port =
> > pa_hashmap_get(ports, u->output_port_name)); +
> > pa_assert(u->input_a2dp_port_name);
> > + pa_assert(u->input_hsp_port_name);
> > + pa_assert(u->input_hfgw_port_name);
> > + pa_assert(u->output_a2dp_port_name);
> > + pa_assert(u->output_hsp_port_name);
> > + pa_assert(u->output_hfgw_port_name);
> > + pa_assert_se(input_a2dp_port = pa_hashmap_get(ports,
> > u->input_a2dp_port_name)); + pa_assert_se(input_hsp_port
> > = pa_hashmap_get(ports, u->input_hsp_port_name)); +
> > pa_assert_se(input_hfgw_port = pa_hashmap_get(ports,
> > u->input_hfgw_port_name)); +
> > pa_assert_se(output_a2dp_port = pa_hashmap_get(ports,
> > u->output_a2dp_port_name)); +
> > pa_assert_se(output_hsp_port = pa_hashmap_get(ports,
> > u->output_hsp_port_name)); +
> > pa_assert_se(output_hfgw_port = pa_hashmap_get(ports,
> > u->output_hfgw_port_name));
> >
> > if (pa_streq(uuid, A2DP_SINK_UUID)) {
> >
> > p = pa_card_profile_new("a2dp", _("High Fidelity
> > Playback (A2DP)"), sizeof(pa_bluez4_profile_t));
> >
> > @@ -2181,7 +2274,7 @@ static pa_card_profile
> > *create_card_profile(struct userdata *u, const char *uuid
> >
> > p->n_sources = 0;
> > p->max_sink_channels = 2;
> > p->max_source_channels = 0;
> >
> > - pa_hashmap_put(output_port->profiles, p->name, p);
> > + pa_hashmap_put(output_a2dp_port->profiles, p->name,
> > p);
> >
> > d = PA_CARD_PROFILE_DATA(p);
> > *d = PA_BLUEZ4_PROFILE_A2DP;
> >
> > @@ -2192,7 +2285,7 @@ static pa_card_profile
> > *create_card_profile(struct userdata *u, const char *uuid
> >
> > p->n_sources = 1;
> > p->max_sink_channels = 0;
> > p->max_source_channels = 2;
> >
> > - pa_hashmap_put(input_port->profiles, p->name, p);
> > + pa_hashmap_put(input_a2dp_port->profiles, p->name,
> > p);
> >
> > d = PA_CARD_PROFILE_DATA(p);
> > *d = PA_BLUEZ4_PROFILE_A2DP_SOURCE;
> >
> > @@ -2203,8 +2296,8 @@ static pa_card_profile
> > *create_card_profile(struct userdata *u, const char *uuid
> >
> > p->n_sources = 1;
> > p->max_sink_channels = 1;
> > p->max_source_channels = 1;
> >
> > - pa_hashmap_put(input_port->profiles, p->name, p);
> > - pa_hashmap_put(output_port->profiles, p->name, p);
> > + pa_hashmap_put(input_hsp_port->profiles, p->name,
> > p); + pa_hashmap_put(output_hsp_port->profiles,
> > p->name, p);
> >
> > d = PA_CARD_PROFILE_DATA(p);
> > *d = PA_BLUEZ4_PROFILE_HSP;
> >
> > @@ -2215,8 +2308,8 @@ static pa_card_profile
> > *create_card_profile(struct userdata *u, const char *uuid
> >
> > p->n_sources = 1;
> > p->max_sink_channels = 1;
> > p->max_source_channels = 1;
> >
> > - pa_hashmap_put(input_port->profiles, p->name, p);
> > - pa_hashmap_put(output_port->profiles, p->name, p);
> > + pa_hashmap_put(input_hfgw_port->profiles, p->name,
> > p); + pa_hashmap_put(output_hfgw_port->profiles,
> > p->name, p);
> >
> > d = PA_CARD_PROFILE_DATA(p);
> > *d = PA_BLUEZ4_PROFILE_HFGW;
> >
> > @@ -2613,8 +2706,12 @@ void pa__done(pa_module *m) {
> >
> > if (u->modargs)
> >
> > pa_modargs_free(u->modargs);
> >
> > - pa_xfree(u->output_port_name);
> > - pa_xfree(u->input_port_name);
> > + pa_xfree(u->output_a2dp_port_name);
> > + pa_xfree(u->output_hsp_port_name);
> > + pa_xfree(u->output_hfgw_port_name);
> > + pa_xfree(u->input_a2dp_port_name);
> > + pa_xfree(u->input_hsp_port_name);
> > + pa_xfree(u->input_hfgw_port_name);
> >
> > pa_xfree(u->address);
> > pa_xfree(u->path);
--
Pali Roh?r
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/pulseaudio-discuss/attachments/20140925/8c1cfbd5/attachment.sig>
