[pulseaudio-discuss] [PATCH v0] bluetooth: Merge headset ports into one
Mikel Astiz
mikel.astiz.oss at gmail.com
Fri Nov 23 05:13:18 PST 2012
Hi David,
On Fri, Nov 23, 2012 at 2:00 PM, David Henningsson
<david.henningsson at canonical.com> wrote:
> On 11/23/2012 01:41 PM, Mikel Astiz wrote:
>>
>> From: Mikel Astiz <mikel.astiz at bmw-carit.de>
>>
>> Merge the former "hsp-output" and "a2dp-output" ports into one single
>> port, in order to fix the regression of having several independent
>> entries in the UI.
>> ---
>> I did a fairly limited amount of testing so please review carefully.
>> src/modules/bluetooth/module-bluetooth-device.c | 72
>> ++++++++++++++++++-------
>> 1 file changed, 52 insertions(+), 20 deletions(-)
>>
>> diff --git a/src/modules/bluetooth/module-bluetooth-device.c
>> b/src/modules/bluetooth/module-bluetooth-device.c
>> index dd1bb86..506a479 100644
>> --- a/src/modules/bluetooth/module-bluetooth-device.c
>> +++ b/src/modules/bluetooth/module-bluetooth-device.c
>> @@ -1281,6 +1281,15 @@ static pa_port_available_t
>> audio_state_to_availability(pa_bt_audio_state_t state
>> return PA_PORT_AVAILABLE_UNKNOWN;
>> }
>>
>> +static pa_port_available_t
>> audio_state_to_availability_merged(pa_bt_audio_state_t state1,
>> pa_bt_audio_state_t state2) {
>> + if (state1 < PA_BT_AUDIO_STATE_CONNECTED && state2 <
>> PA_BT_AUDIO_STATE_CONNECTED)
>> + return PA_PORT_AVAILABLE_NO;
>> + else if (state1 >= PA_BT_AUDIO_STATE_PLAYING || state2 >=
>> PA_BT_AUDIO_STATE_PLAYING)
>> + return PA_PORT_AVAILABLE_YES;
>> + else
>> + return PA_PORT_AVAILABLE_UNKNOWN;
>> +}
>
>
> We might need talk about this too. If something is PA_PORT_AVAILABLE_NO, it
> completely disappears from the GUI, so it can't be activated. Just like it
> does not make sense to switch to a pair of headphones which are not
> connected.
>
> Is this expected?
I think yes. If no audio profile is connected, there is no way you can
use the headset from PA. In fact, you don't even know if the device is
physically there.
You need to connect it first using BlueZ, so I think it makes sense
that is disappears from the UI.
>> +
>> /* Run from main thread */
>> static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m,
>> void *userdata) {
>> DBusError err;
>> @@ -1356,9 +1365,14 @@ static DBusHandlerResult filter_cb(DBusConnection
>> *bus, DBusMessage *m, void *us
>>
>> if (state != PA_BT_AUDIO_STATE_INVALID &&
>> pa_hashmap_get(u->card->profiles, "hsp")) {
>> pa_device_port *port;
>> - pa_port_available_t available =
>> audio_state_to_availability(state);
>> + pa_port_available_t available;
>>
>> - pa_assert_se(port = pa_hashmap_get(u->card->ports,
>> "hsp-output"));
>> + if (pa_hashmap_get(u->card->profiles, "a2dp") == NULL)
>> + available = audio_state_to_availability(state);
>> + else
>> + available = audio_state_to_availability_merged(state,
>> u->device->audio_sink_state);
>> +
>> + pa_assert_se(port = pa_hashmap_get(u->card->ports,
>> "bluetooth-output"));
>> pa_device_port_set_available(port, available);
>>
>> pa_assert_se(port = pa_hashmap_get(u->card->ports,
>> "hsp-input"));
>> @@ -1385,9 +1399,14 @@ static DBusHandlerResult filter_cb(DBusConnection
>> *bus, DBusMessage *m, void *us
>>
>> if (state != PA_BT_AUDIO_STATE_INVALID &&
>> pa_hashmap_get(u->card->profiles, "a2dp")) {
>> pa_device_port *port;
>> - pa_port_available_t available =
>> audio_state_to_availability(state);
>> + pa_port_available_t available;
>> +
>> + if (pa_hashmap_get(u->card->profiles, "hsp") == NULL)
>> + available = audio_state_to_availability(state);
>> + else
>> + available = audio_state_to_availability_merged(state,
>> u->device->headset_state);
>>
>> - pa_assert_se(port = pa_hashmap_get(u->card->ports,
>> "a2dp-output"));
>> + pa_assert_se(port = pa_hashmap_get(u->card->ports,
>> "bluetooth-output"));
>> pa_device_port_set_available(port, available);
>>
>> acquire = (available == PA_PORT_AVAILABLE_YES && u->profile
>> == PROFILE_A2DP);
>> @@ -1614,7 +1633,7 @@ static void connect_ports(struct userdata *u, void
>> *sink_or_source_new_data, pa_
>>
>> switch (u->profile) {
>> case PROFILE_A2DP:
>> - pa_assert_se(port = pa_hashmap_get(u->card->ports,
>> "a2dp-output"));
>> + pa_assert_se(port = pa_hashmap_get(u->card->ports,
>> "bluetooth-output"));
>> pa_assert_se(pa_hashmap_put(data.sink_new_data->ports,
>> port->name, port) >= 0);
>> pa_device_port_ref(port);
>> break;
>> @@ -1627,7 +1646,7 @@ static void connect_ports(struct userdata *u, void
>> *sink_or_source_new_data, pa_
>>
>> case PROFILE_HSP:
>> if (direction == PA_DIRECTION_OUTPUT) {
>> - pa_assert_se(port = pa_hashmap_get(u->card->ports,
>> "hsp-output"));
>> + pa_assert_se(port = pa_hashmap_get(u->card->ports,
>> "bluetooth-output"));
>> pa_assert_se(pa_hashmap_put(data.sink_new_data->ports,
>> port->name, port) >= 0);
>> } else {
>> pa_assert_se(port = pa_hashmap_get(u->card->ports,
>> "hsp-input"));
>> @@ -2247,13 +2266,20 @@ static void create_ports_for_profile(struct
>> userdata *u, pa_hashmap *ports, pa_c
>>
>> switch (*d) {
>> case PROFILE_A2DP:
>> - pa_assert_se(port = pa_device_port_new(u->core,
>> "a2dp-output", _("Bluetooth High Quality (A2DP)"), 0));
>> - pa_assert_se(pa_hashmap_put(ports, port->name, port) >= 0);
>> - port->is_output = 1;
>> - port->is_input = 0;
>> - port->priority = profile->priority * 100;
>> - port->available =
>> audio_state_to_availability(device->audio_sink_state);
>> - pa_hashmap_put(port->profiles, profile->name, profile);
>> + if ((port = pa_hashmap_get(ports, "bluetooth-output")) !=
>> NULL) {
>> + port->priority = PA_MAX(port->priority, profile->priority
>> * 100);
>> + port->available =
>> audio_state_to_availability_merged(device->headset_state,
>> device->audio_sink_state);
>> + pa_hashmap_put(port->profiles, profile->name, profile);
>> + } else {
>> + pa_assert_se(port = pa_device_port_new(u->core,
>> "bluetooth-output", _("Bluetooth Output"), 0));
>
>
> As tanuk commented on IRC: "Headset Output" would be even better, but that
> requires detecting the form-factor.
I'm fine either way but this could create some trouble if we wanted to
merge "a2dp-input" with "hsp-input", or even
"hfgw-input"/"hfgw-output".
>
> I think we do detect the form factor: at least a headset icon shows up in
> the GUI.
>
> A list of currently used names are in src/modules/alsa/alsa-mixer.c,
> function path_verify.
> I believe it would make sense to make these generic, possibly amend, and use
> for bluetooth as well?
>
>
>
>> + pa_assert_se(pa_hashmap_put(ports, port->name, port) >=
>> 0);
>> + port->is_output = 1;
>> + port->is_input = 0;
>> + port->priority = profile->priority * 100;
>> + port->available =
>> audio_state_to_availability(device->audio_sink_state);
>> + pa_hashmap_put(port->profiles, profile->name, profile);
>> + }
>> +
>> break;
>>
>> case PROFILE_A2DP_SOURCE:
>> @@ -2267,13 +2293,19 @@ static void create_ports_for_profile(struct
>> userdata *u, pa_hashmap *ports, pa_c
>> break;
>>
>> case PROFILE_HSP:
>> - pa_assert_se(port = pa_device_port_new(u->core, "hsp-output",
>> _("Bluetooth Telephony (HSP/HFP)"), 0));
>> - pa_assert_se(pa_hashmap_put(ports, port->name, port) >= 0);
>> - port->is_output = 1;
>> - port->is_input = 0;
>> - port->priority = profile->priority * 100;
>> - port->available =
>> audio_state_to_availability(device->headset_state);
>> - pa_hashmap_put(port->profiles, profile->name, profile);
>> + if ((port = pa_hashmap_get(ports, "bluetooth-output")) !=
>> NULL) {
>> + port->priority = PA_MAX(port->priority, profile->priority
>> * 100);
>> + port->available =
>> audio_state_to_availability_merged(device->headset_state,
>> device->audio_sink_state);
>> + pa_hashmap_put(port->profiles, profile->name, profile);
>> + } else {
>> + pa_assert_se(port = pa_device_port_new(u->core,
>> "bluetooth-output", _("Bluetooth Output"), 0));
>> + pa_assert_se(pa_hashmap_put(ports, port->name, port) >=
>> 0);
>> + port->is_output = 1;
>> + port->is_input = 0;
>> + port->priority = profile->priority * 100;
>> + port->available =
>> audio_state_to_availability(device->headset_state);
>> + pa_hashmap_put(port->profiles, profile->name, profile);
>> + }
>>
>> pa_assert_se(port = pa_device_port_new(u->core, "hsp-input",
>> _("Bluetooth Telephony (HSP/HFP)"), 0));
>> pa_assert_se(pa_hashmap_put(ports, port->name, port) >= 0);
>>
Cheers,
Mikel
More information about the pulseaudio-discuss
mailing list