Hi Sakari, On 4/12/23 17:12, Sakari Ailus wrote: > Hi Dave, Michael, > > On Wed, Apr 12, 2023 at 02:55:56PM +0100, Dave Stevenson wrote: >>>> If the ranges aren't updated, where should that out-of-range lens >>>> movement leave the lens? >>> >>> This is up to the hardware controller, but I would guess it typically >>> stops one step before disaster. Wherever that may be, the error >>> condition and the current position can be read out via this new STATUS >>> control. >>> >>> Does this sound good so far? >> >> Sounds reasonable, but I'm not the gatekeeper (that would be Sakari or >> Laurent), and I'm just expressing my views based on the lenses I've >> encountered. >> All of my lenses have a single drive for focus, a single drive for >> zoom, and where there are multiple elements they are all connected >> mechanically. Your setup sounds far more complex and is likely to need >> a more extensive driver, but it'd be nice to not unnecessarily >> overcomplicate the interface. > > Could we also have a driver that uses these new controls? If you are referring to the driver for our custom lens controller, then I have to say that it is under development and simply not ready for release yet. Also, the decision has not yet been made whether or not this will be an open-source driver. A different approach could be the adaptation of the vimc-lens driver, which currently only supports FOCUS_ABSOLUTE. But this would raise several implementation questions and at least for me this would be a nontrivial task. Is it required to have a driver for this interface (in the sense that the patches cannot be accepted otherwise)? > The controls themselves appear reasonable to me as well. I guess there are > changes to be made based on the discussion? I'd summarize that whether or not the status controls are compound controls of the type V4L2_CTRL_TYPE_LENS_STATUS is the open question. As a potential follow-up question I recently asked myself if the struct v4l2_ctrl_lens_status should contain trailing reserved bytes for future extension (no idea, though, what this could be). Alternatively, we could come up with "V4L2_CID_FOCUS_CURRENT (integer)" for the current position and "V4L2_CID_FOCUS_STATUS (bitmask)" (and add further controls when they are needed. Here, we lose atomicity but maybe this can be ignored. One could assume that all relevant controls are read out with a single ioctl which provides at least some level of atomicity. Any comments and/or recommendations to this open question would be much appreciated. Other review comments will be incorporated in the next iteration of this series as well, but they are quite straightforward. Best regards, Michael
