[pulseaudio-discuss] [PATCH 3/4] bluetooth: Use form factor to set port name and description

[Mikel Astiz]

Hi David,

On Fri, Feb 22, 2013 at 12:48 PM, David Henningsson
<david.henningsson at canonical.com> wrote:
> On 02/22/2013 12:03 PM, Mikel Astiz wrote:
>>
>> Hi David,
>>
>> On Thu, Feb 21, 2013 at 8:49 AM, David Henningsson
>> <david.henningsson at canonical.com> wrote:
>>>
>>> On 02/19/2013 10:33 AM, Mikel Astiz wrote:
>>>>
>>>>
>>>> +        case PA_BT_FORM_FACTOR_MICROPHONE:
>>>> +            name_prefix = "microphone";
>>>> +            output_description = _("Microphone Output");
>>>> +            input_description = _("Microphone Input");
>>>> +            break;
>>>
>>>
>>>
>>> This doesn't look quite right. "Microphone Output" will make our dear
>>> translators wonder if you can suddenly put a microphone to your ear and
>>> listen to music through it :-)
>>>
>>> If you detect a microphone, there should be only one port created, in the
>>> input direction, and the description should be just "Microphone" (not
>>> "Microphone Input"). Feel free to make the name input-microphone,
>>> bluetooth-input-microphone, or whatever makes the most sense, as long as
>>> the
>>> description is kept to the bare essentials.
>>>
>>> In the case of a dual-direction device such as a headset, there should be
>>> two ports (one in each direction), both of them with the description
>>> "Headset".
>>
>>
>> Agreed.
>>
>>>> +    pa_assert_se(port = pa_device_port_new(u->core,
>>>> u->output_port_name,
>>>> output_description, 0));
>>>>        pa_assert_se(pa_hashmap_put(ports, port->name, port) >= 0);
>>>>        port->is_output = 1;
>>>>        port->is_input = 0;
>>>>        port->available = get_port_availability(u, PA_DIRECTION_OUTPUT);
>>>>
>>>> -    pa_assert_se(port = pa_device_port_new(u->core, "bluetooth-input",
>>>> _("Bluetooth Input"), 0));
>>>> +    pa_assert_se(port = pa_device_port_new(u->core, u->input_port_name,
>>>> input_description, 0));
>>>>        pa_assert_se(pa_hashmap_put(ports, port->name, port) >= 0);
>>>>        port->is_output = 0;
>>>>        port->is_input = 1;
>>>
>>>
>>>
>>> This looks like both ports are always created, which is wrong.
>>
>>
>> This one is more tricky, as already discussed with Tanu.
>>
>> I can think of three possible approaches:
>> (1) Create both ports always, but set the availability flag as needed
>> (as proposed here).
>> (2) Create ports based on the form factor (device class).
>> (3) Create ports based on the UUIDs (the actual Bluetooth profiles
>> supported by the device).
>>
>> Approaches (2) and (3) sounds desirable but I have some concerns about
>> them.
>>
>> Regarding (2), the problem is I don't have a 100% confidence on the
>> reliability of this information, I fear this could create regressions.
>> We would have to make sure all devices (or most of them) out there
>> report a proper form factor (device class), which is consistent with
>> the I/O capabilities. Perhaps I'm being too conservative here, but
>> I've seen headsets being paired and not classified as headsets (this
>> seems to be a bug in BlueZ, but still relevant enough IMO).
>
>
> Would it be possible to use what we know about the I/O capabilities at this
> point, so that if the form factor is inconsistent with the I/O capabilities,
> we fallback to something generic?

In this context, I/O capabilities are provided by the UUIDs, which
represent the remotely supported services. The problem with them is
that they can change dynamically, and that's why approach (3) requires
a more complex core, supporting dynamic port creation. In practice,
this is mostly observed during device pairing, where the UUIDs are
discovered and announced incrementally. If we followed your proposal
to just consider "what we know about the I/O capabilities at this
point", chances are that the port name would change the next time the
device is connected (when all UUIDs would be known from the
beginning).

In addition, some UUIDs don't specify the actual I/O capabilities. For
example, I assume that a Bluetooth microphone would support HSP/HFP,
and we would therefore have both a sink and a source.

Considering all this, I don't think we can do much unless we assume
the form factor (device class) is reliable.

>
>
>> Regarding (3), I would require supporting dynamic port creation in the
>> core, since the UUIDs can be announced at any point in time. As I
>> already pointed out and proposed in patch 4/4, I'd rather remove this
>> recently added functionality from the core.
>>
>> So my personal favorite is (1) and keep it as simple as possible,
>> assuming the port available = NO already provides enough information.
>> Approach (2) is also nice but I haven't tested enough devices to have
>> a feeling about how reliable this could be.

Cheers,
Mikel