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

[David Henningsson]

On 02/27/2013 09:04 AM, Mikel Astiz wrote:
> 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.

Ok. To give you some background to the question; when we first 
implemented port availability, there was the question of whether to 
completely hide or just "gray out" the unavailable items. Graying out 
seemed better to some people (e g, being able to set the volume of the 
speakers before switching to them could have been useful), but the 
reason why "completely hide" was chosen was due to HDMI: most HDMI 
cards, at least two years ago (things have got a bit better since), 
declare three or four ports, but only one (or possibly two) were 
actually physically connected to something.
To show gray items for stuff that the user don't have at all seemed 
wrong, so therefore we hid it instead.

Now; I'm considering making these HDMI outputs separate cards, that 
would be added and removed on the fly as they are 
connected/disconnected. With that, we could "gray out" instead of 
"hiding", but if we get into the same situation for Bluetooth instead, 
then we must continue to hide unavailable ports.

>>> 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
>



-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic