On Tue, Nov 03, 2009 at 11:02:58PM +0100, Lennart Poettering wrote:
> On Tue, 03.11.09 14:40, Daniel Mack (daniel at caiaq.de) wrote:
>
> > +typedef struct ca_device ca_device;
> > +
> > +struct ca_device {
> > + AudioDeviceID id;
> > + unsigned int module_index;
> > + PA_LLIST_FIELDS(ca_device);
> > +};
>
> In PA we generally don't prefix static variables or file-local
> structs, since they are unlikely to cause namespace collisions and
> this is a nice way to determine easily if something is exported or not
> exported API. But that's just nitpicking.
Which prefix do you mean? I just didn't want to call it 'device', and
'ca_device' was short enough for me to be 'local' :)
> > +struct userdata {
> > + PA_LLIST_HEAD(ca_device, devices);
> > +};
> > +
> > +static int ca_device_added(struct pa_module *m, AudioDeviceID id) {
> > + char args[32];
> > + pa_module *mod;
> > + struct userdata *u = m->userdata;
> > + struct ca_device *dev;
> > +
> > + pa_assert(u);
> > +
> > + pa_snprintf(args, sizeof(args), "device_id=%d", id);
>
> Using pa_malloc_sprintf() is recommended. I am actually trying to get
> rid of the use of pa_snprintf() in PA entirely and am replacing it
> with the malloc version wherever I encounter it and it makes sense
> (i.e. outside of RT threads).
Ok, will fix.
> > + if (!mod) {
> > + pa_log_info("Failed to load module %s with arguments '%s'", DEVICE_MODULE_NAME, args);
> > + return -1;
> > + }
> > +
> > + dev = pa_xnew0(ca_device, 1);
> > + pa_assert(dev);
>
> No need for this assert. The "x" in "pa_xnew0" should make clear that
> this is an aborting allocation anyway.
Ok.
> > + if (AudioHardwareAddPropertyListener(kAudioHardwarePropertyDevices, property_listener_proc, m)) {
> > + pa_log("AudioHardwareAddPropertyListener() failed.");
> > + goto fail;
> > + }
>
> Hmm, how are these event callbacks called? From a background thread?
> Or is this hooked up to the event loop in some way? Or is this only
> one-time during initialization?
They're called from a background thread, indeed. CoreAudio has no clue
about PA's mainloop implementation, so it can't hook up in there.
So yes, we might need some locking here. I just wasn't sure how PA likes
that (iow, which resources I need to lock and how), so I ignored it for
now. Any hint?
Thanks,
Daniel
