Hi Jacopo, On Wed, Nov 13, 2024 at 09:20:32AM +0100, Jacopo Mondi wrote: > > > > > + * - 1/0 > > > > > + - Format > > > > > + - X > > > > > + - Image data format. The width and height fields of this format are the > > > > > + same than those for the V4L2_SEL_TGT_CROP_BOUNDS rectangle. The media > > > > > > > > Can sizes be changed at all ? > > > > > > For the internal image pad, I wouldn't think so. I think it should > > > expose the full readable array. Or, reading what you wrote below, do you > > > envision this including non-readable pixels too ? > > > > I don't -- they can't be accessed in any case. Timing information (such as > > the latching points) should probably be conveyed using a different API if > > they're needed. > > > > Ok, what I was suggesting was to explicitly say those sizes are not > modifiable. I'll add that for v2. > > > > > > > Mapping pixel array definitions from datasheets to formats and selection > > > rectangles is hard, as datasheets, when they are available, are often > > > confusing. Datasheets from different vendors, or even for different > > > sensors of the same vendor, often have different models. I think we need > > > to be extra precise here, and possibly work with sensor vendors. Using > > > the CCS model as a base could be useful. > > > > I wouldn't necessarily try to convey what non-visiblepixels might be there, > > apart from in the documentation. These tend to be fairly vendor/sensor > > specific after all. > > > > In the whole document, I would use one of "non-readable" and > "non-visible". I'll check these are the terms used. We won't probably say much -- if anything -- about non-readable pixels as there's (by definition) no way to access them. > > > > > > > > > + bus code of this format reflects the native pixel depth of the sensor. > > > > > + * - 1/0 > > > > > + - V4L2_SEL_TGT_NATIVE_SIZE > > > > > + - X > > > > > + - The full size of the pixel array, including all pixels in the pixel > > > > > + array, even if they cannot be captured. This rectangle is relative to > > > > > + the format on the same (pad, stream). > > > > > > Mix of tabs and spaces. > > > > > > What do you mean by "relative to the format on the same (pad, stream)" ? > > > Does it mean the rectangle is expressed relatively to a rectangle > > > defined as (0,0),(fmt.width x fmt.height) ? That doesn't seem to match > > > > Yes. > > > > > the description of the format and V4L2_SEL_TGT_CROP_BOUNDS, as the > > > format is defined as having the same width and height as > > > V4L2_SEL_TGT_CROP_BOUNDS, and V4L2_SEL_TGT_CROP_BOUNDS is defined as > > > being relative to V4L2_SEL_TGT_NATIVE_SIZE. I assume you mean something > > > else, it should be clarified. > > > > As discussed, I'll drop the NATIVE_SIZE rectangle from the documentation as > > redundant. > > > > Doesn't it help reporting the full pixel array size (readable and non > readable pixels ?). We care about that information in libcamera > (also to comply with Android's requirement to expose the full pixel > array size as a property of the camera) > > I would just drop the "relative" part. NATIVE is a rectangle with > top-left corner in position (0,0). It's the same information (apart from the mbus code) that can be obtained from the format on the internal pad. I'd avoid adding redundant selection targets. -- Kind regards, Sakari Ailus
