On Tue, 03.11.09 14:40, Daniel Mack (daniel at caiaq.de) wrote:
> This is for RFC. Don't know if it makes sense to commit it already as
> the module it instantiates is not yet finished. Anyway, let me know what
> you think.
A few comments:
> +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.
> +
> +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).
> + 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.
> + num_devices = size / sizeof(AudioDeviceID);
> + device_id = pa_xnew(AudioDeviceID, num_devices);
> + pa_assert(device_id);
Same here.
> + 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?
Otherwise looks good.
Lennart
--
Lennart Poettering Red Hat, Inc.
lennart [at] poettering [dot] net
http://0pointer.net/lennart/ GnuPG 0x1A015CC4
