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.
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);
>
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
