[pulseaudio-discuss] [PATCH 2/3] device-port: Add mechanism to free implementation data

[Tanu Kaskinen]

On Fri, 2013-09-20 at 18:08 +0530, Arun Raghavan wrote:
> On Fri, 2013-09-20 at 15:33 +0300, Tanu Kaskinen wrote:
> > On Fri, 2013-09-20 at 17:56 +0530, Arun Raghavan wrote:
> > > On Fri, 2013-09-20 at 15:21 +0300, Tanu Kaskinen wrote:
> > > > On Fri, 2013-09-20 at 17:27 +0530, Arun Raghavan wrote:
> > > > > This will be needed if the implementation data stores pointers to
> > > > > additional data that needs to be freed as well.
> > > > > ---
> > > > >  src/pulsecore/device-port.c | 3 +++
> > > > >  src/pulsecore/device-port.h | 3 +++
> > > > >  2 files changed, 6 insertions(+)
> > > > > 
> > > > > diff --git a/src/pulsecore/device-port.c b/src/pulsecore/device-port.c
> > > > > index 0b65d5c..ac2c95e 100644
> > > > > --- a/src/pulsecore/device-port.c
> > > > > +++ b/src/pulsecore/device-port.c
> > > > > @@ -94,6 +94,9 @@ static void device_port_free(pa_object *o) {
> > > > >      pa_assert(p);
> > > > >      pa_assert(pa_device_port_refcnt(p) == 0);
> > > > >  
> > > > > +    if (p->impl_free)
> > > > > +        p->impl_free(p);
> > > > > +
> > > > >      if (p->proplist)
> > > > >          pa_proplist_free(p->proplist);
> > > > >  
> > > > > diff --git a/src/pulsecore/device-port.h b/src/pulsecore/device-port.h
> > > > > index b10d554..b5e80a5 100644
> > > > > --- a/src/pulsecore/device-port.h
> > > > > +++ b/src/pulsecore/device-port.h
> > > > > @@ -54,6 +54,9 @@ struct pa_device_port {
> > > > >      pa_direction_t direction;
> > > > >      int64_t latency_offset;
> > > > >  
> > > > > +    /* Free the extra implementation specific data. Called before other members are freed. */
> > > > > +    void (*impl_free)(pa_device_port *port);
> > > > > +
> > > > >      /* .. followed by some implementation specific data */
> > > > >  };
> > > > >  
> > > > 
> > > > It's sad that this is needed, but ack. (I have probably said this
> > > > earlier: I'd like ports to not require refcounting, in which case they
> > > > would probably be always owned by cards.)
> > > 
> > > In which case the card would have to have a way to check what kind of
> > > implementation-specific data is there and call the appropriate free
> > > function. Doable, but not the prettiest, imo.
> > 
> > If you mean that pulsecore/card.c would know what kind of data the alsa
> > card is storing in the port, I don't see how that would be doable. But
> > that wasn't my idea anyway. If cards always owned the ports,
> > pa_device_port could have a userdata pointer set by the alsa card, and
> > the alsa card could free the extra port data at the same time when it
> > frees the pa_card object.
> 
> I actually meant alsa card, not pa_card. In this case, alsa card would
> basically need to check if it's in UCM mode or not and then call
> pa_alsa_ucm_port_free() if required. Given all the other UCM-specific
> checks, maybe it's not that big a deal to do it this way if we do remove
> the port refcounts.

What's your plan about this? The original patch hasn't been applied; are
you going to change it or did you just forget to push it?

-- 
Tanu