On Fri, 2013-02-22 at 12:03 +0100, 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).
>
> 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.
I'm not totally confident about the form factor information reliability
either. Options (1) and (3) are both acceptable to me. Regarding (1), in
case of e.g. microphone, the port name/description for the "illogical"
port should not be "microphone output", as pointed out by David.
Instead, something generic should be used, like "bluetooth output".
--
Tanu
