Re: [libdc] Big patch committed
Capture and control API for IIDC compliant cameras
Brought to you by:
ddouxchamps,
gordp
From: David M. <dcm@MIT.EDU> - 2007-11-27 15:43:56
|
On Tue, 2007-11-27 at 21:19 +0900, Damien Douxchamps wrote: > 1) the member "unit_directory" of dc1394_camera_t is gone. Any reasons > for that? It's not used a lot (just a register read/write function). But > since the unit directory contains important information it may be useful > to keep this in the camera struct for direct-access-debugging. > Yeah, I renamed that one to unit_dependent_directory because I figured the latter was more useful. But since you use the old one, I can add it back and populate both. > 2) register.c/h: some functions need to be renamed as dc1394_xx (I'll do > that soon) > Yes, thanks. > 3) note to self: the flags member of dc1394_camera_t should be > initialised to zero when a new camera instance is created. (I'll do that > too) > The camera is created with calloc() so you don't have to do this, everything will automatically be set to zero. Also, what is the flags field for? Is that something that should be moved to the private structure or do you want that public? > 4) still regarding dc1394_camera_t, the member "memory_channel_num" is > gone too. Is there another way to get that information? IOW, do we need > to write a function that gets this info? If we don't have the channel > number we can't use memory channels at all. BTW this channel number is > not volatile; it remains the same. It's renamed to max_mem_channel. I figured the new name is more descriptive of what it actually is. Is that okay with you? > 5) yet some other members of dc1394_camera_t removed: port and node. > This was planned for hotplug etc... But right now the result is that the > broadcast function can't be used. Shall we write some access functions > to port and node? Yes, we can write some access functions. But we will have to add them to the platform API as well, since getting those quantities is platform specific. (Writing the functions for linux is easy, but they will be much harder to write for Juju and Mac OS since those platforms don't name their "ports" with integers and they don't use the node id for addressing). Also, the broadcast issue is a larger problem. The old method of changing the value of node was suitable for linux, but it's not a cross platform solution, since juju and Mac OS don't address cameras by node id. I'll have to do some more investigation on how to make that work, but we'll try to get that working before rc8. It will probably need a platform-specific API function. > > Two other random thoughts: > > I'm also wondering if we should keep the dc1394_camera_id_t since it's > not used much anymore. The function to compare cameras based on that > struct could be removed too. OR: We put the struct back in > dc1394_camera_t, use dc1394_camera_id_t as input for > dc1394_camera_new_unit and still allow people to create a camera with > only the GUID (dc1394_camera_new). > I think we should keep it, but perhaps rename it to something more general like "dc1394camera_info_t". We can put some additional useful information in there like vendor and model, just for informational purposes of the application. > Another thing to do will be to move the new API higher in control.h and > stop referring it as new since it's the only one now ;) > Yes, agreed. -David |