Hi Dave, (CC'ing Sakari) Sorry for the late reply. This is the kind of topic where I always hope someone will beat me to it and I won't have to reply :-) On Tue, Apr 05, 2022 at 12:37:56PM +0100, Dave Stevenson via libcamera-devel wrote: > On Thu, 17 Mar 2022 at 17:47, Dave Stevenson 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. Good description of a complex problem. I'm afraid I have no ready-made answer that will solve all issues. If we were to forget about history and API compatibility, I'm sure we could design a decent API to expose the capabilities of camera sensors to userspace. Unfortunately, we can't, but none of the options I can think of will preserve the API. I'm tempted to design something decent going forward, and move older drivers to the new API over time as needed, with backward-compatibility handled as a module option or even by forking and duplicating drivers. It's messy, but keeping the status quo is even worse, we have lots of drivers that expose the same features in many different ways. It's becoming impossible to handle in any decent way in userspace. > 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 -- Regards, Laurent Pinchart
