On Tue, Jun 05, 2012 at 01:36:58PM +0200, David Henningsson wrote:
> On 06/04/2012 07:35 PM, poljar wrote:
> >The bluetooth device should have ports so we can attach a latency to
> >the ports.
>
> I think it is good that we create ports for more types of devices,
> but I don't see how that correlates to different latencies...?
>
This is part of my GSOC project [1].
> >Every profile (a2dp, hsp...) has his own set of ports depending on the
> >number of sinks and sources it provides.
> >---
> > src/modules/bluetooth/module-bluetooth-device.c | 124 +++++++++++++++++++++++
> > 1 file changed, 124 insertions(+)
> >
> >diff --git a/src/modules/bluetooth/module-bluetooth-device.c b/src/modules/bluetooth/module-bluetooth-device.c
> >index 68c4efc..4901ef9 100644
> >--- a/src/modules/bluetooth/module-bluetooth-device.c
> >+++ b/src/modules/bluetooth/module-bluetooth-device.c
> >@@ -2073,6 +2073,61 @@ static pa_hook_result_t source_state_changed_cb(pa_core *c, pa_source *s, struct
> > return PA_HOOK_OK;
> > }
> >
> >+static void connect_ports(struct userdata *u, void *sink_or_source_new_data, pa_direction_t direction) {
> >+ union {
> >+ pa_sink_new_data *sink_new_data;
> >+ pa_source_new_data *source_new_data;
> >+ } data;
>
> Using unions this way is error-prone IMO - just use two different
> pointers "sink_new_data" and "source_new_data", no point in
> encapsulating them into "data".
>
This was a suggestion by Tanu, I don't know how and why this should be
error-prone.
> >+ pa_device_port *port;
> >+
> >+ if (direction == PA_DIRECTION_OUTPUT) {
> >+ data.sink_new_data = sink_or_source_new_data;
> >+ data.sink_new_data->ports = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
> >+ } else {
> >+ data.source_new_data = sink_or_source_new_data;
> >+ data.source_new_data->ports = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
> >+ }
> >+
> >+ switch (u->profile) {
> >+ case PROFILE_A2DP:
> >+ pa_assert_se(port = pa_hashmap_get(u->card->ports, "a2dp-output"));
> >+ pa_assert_se(pa_hashmap_put(data.sink_new_data->ports, port->name, port)>= 0);
> >+ pa_device_port_ref(port);
> >+ break;
>
> This might be something I'm not seeing here, but you're ref-ing the
> port here, but where is it unref-ed so it can be freed properly?
>
The unref-ing should be done when the pa_device_port_hashmap_free() is
called. If I'm not mistaken that is done in sink.c automatically when we
create and free a sink.
Thanks for the review.
[1] http://www.google-melange.com/gsoc/project/google/gsoc2012/poljar/13001
