Hi Jacopo, On Wed, Dec 04, 2024 at 02:37:21PM +0100, Jacopo Mondi wrote: > > > > +Common raw camera sensor model > > > > +------------------------------ > > > > + > > > > +The common raw camera sensor model defines a set of enumeration and > > > > +configuration interfaces (formats, selections etc.) that cover the vast majority > > > > +of funcitionality of raw camera sensors. Not all of the interfaces are > > > > > > s/funcitionality/functionalities > > > > I'd say singular is right in this case. Maybe Kieran or Dave have an > > opinion as well? :-) > > > Then > s/funcitionality/functionality > > :) Ah, indeed. ;-) ... > > > > +targets are omitted, the further selection rectangle or format is instead > > > > +related to the previous implemented selection rectangle. For instance, if the > > > > +sensor supports binning but not analogue crop, then the binning configuration > > > > +(``V4L2_SEL_TGT_COMPOSE`` selection target) is done in relation to the visible > > > > +pixel area (``V4L2_SEL_TGT_CROP_DEFAULT`` selection target). > > > > > > Let alone the fact I would have used, say, digital crop as an example > > > of an optional feature, I think allowing to read all the possible > > > targets would save userspace keeping track of what target the > > > rectangle they want to configure refers to. > > > > This is how the selection API works. If we want to deviate from that, it's > > another thing, but currently if a driver doesn't support configuring > > a selection, it won't support that target either. > > > > If we required all selection rectangles to be supported even if they > > wouldn't be configurable, then it'd be up to user to try to change them to > > see if they can be modified. I'm not sure if that would be an improvement > > as a whole. > > > > Yeah, probably a set-then-verify-if-changed is more cumbersome than > detecting an -EINVAL on set_selection. > > I'm fine with this as long as all the mentioned targets are readable. Currently it's how this is documented in V4L2 sub-device UAPI documentation: if you have nothing to configure, then there's no selection target either. > > > This makes me think it is intentional not to document digital > > > scaling/post-scaler crop in this version ? > > > > Not in this patch, no. :-) > > > > I'm fine merging this to the 5th patch if there's an agreement they should > > be merged together (probably?). > > > > I would probably merge the two, yes Ack. Let's wait for the discussion to conclude. -- Kind regards, Sakari Ailus
