Re: [libcamera-devel] Using the Selection API with image sensors for arbitrary cropping

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Laurent

Sorry for my late reply here - I've been off sick for the last week.

On Tue, 12 Apr 2022 at 10:20, Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>
> 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.

That seems like a longer term proposition.
It sounds like converting to use the selection API largely isn't
possible at present, so I'll keep these as downstream patches, and
allow S_FMT to update the crop.

Thanks
  Dave

> > 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



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux