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