On Thu, 17 Mar 2022 at 17:47, Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> wrote: > > Hi All > > I'm trying to tally the selection API documentation for image sensors > with implementing it in practice, specifically over arbitrary cropping > on the sensor. > > I've had a downstream PR for IMX219 that adds support for the > selection API to allow arbitrary cropping and selection of binning > mode [1]. > > The docs for "Writing camera sensor drivers" [2] lists the two options > as either freely configurable via multiple subdevices, or register > based. It doesn't apparently cover just cropping (there is no scaler > on IMX219), but there is the IMX274 driver that implements setting the > sensor cropping via the selection API [3]. > > The current IMX219 register-based modes are > - 3280x2464 up to 15fps > - 1920x1080 up to 30fps as a crop of the 3280x2464 mode > - 1640x1232 up to 40fps, 2x2 binned > - 640x480 up to 200fps, "special" 2x2 binning and cropped. > > The main issue is that implementing the selection API reduces the > number of modes that can be selected directly via set_fmt to the base > 3280x2464 and 1640x1232. Surely that constitutes a regression as use > cases that did work now don't, and therefore it is not acceptable. > 3280x2464 can't run at 30fps, therefore we can't easily get a 1080p30 > source without additional knowledge of modes and crop settings. > > So how should the selection API be implemented without introducing regressions? > Is it permitted to enumerate the extra modes as before and have them > update the crop rectangle? The docs [4] say not: > "Drivers shall set the active crop rectangle to the default when the > driver is first loaded, but not later." > which leaves a bit of a quandry. > > If we do drop the existing modes it just pushes the problem of which > modes to select onto the client. Most likely you end up with an > extended sensor specific helper in libcamera with a list of modes and > the framerates that each can achieve, pretty much identical to the > list of modes in the kernel at present. > Any other clients are forced to jump through similar hoops (unlikely > to happen), or we rename it to Video 4 Libcamera 2 ;-) > > Making that shift also means that selecting the special binning mode > has to be done via some other heuristics. AIUI it's optimised for high > frame rates so that's possible (but not nice). > > Guidance sought please. A gentle ping on this. At present I'm going to ignore the spec and allow setting the format to update the crop. That way we retain the modes and avoid regression, but allow those who wish to set an arbitrary selection to do so. Dave > Thanks > Dave > > [1] https://github.com/raspberrypi/linux/pull/4935 > [2] https://www.kernel.org/doc/html/latest/driver-api/media/camera-sensor.html#frame-size > [3] https://github.com/torvalds/linux/blob/master/drivers/media/i2c/imx274.c > [4] https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/selection-api-configuration.html#configuration-of-video-capture
