[libcamera-devel] [PATCH v3 02/12] libcamera: camera: Introduce SensorConfiguration

[Sakari Ailus]

On Mon, Nov 11, 2024 at 11:59:50AM +0200, Laurent Pinchart wrote:
> On Mon, Nov 11, 2024 at 09:46:46AM +0000, Sakari Ailus wrote:
> > On Mon, Nov 11, 2024 at 11:27:02AM +0200, Laurent Pinchart wrote:
> > > On Mon, Nov 11, 2024 at 09:04:03AM +0000, Sakari Ailus wrote:
> > > > On Mon, Nov 11, 2024 at 10:57:55AM +0200, Laurent Pinchart wrote:
> > > > > On Mon, Nov 11, 2024 at 08:34:53AM +0000, Sakari Ailus wrote:
> > > > > > On Mon, Nov 11, 2024 at 10:12:30AM +0200, Laurent Pinchart wrote:
> > > > > > > On Mon, Nov 11, 2024 at 08:04:19AM +0000, Sakari Ailus wrote:
> > > > > > > > On Sun, Nov 10, 2024 at 09:05:03PM +0200, Laurent Pinchart wrote:
> > > > > > > > > On Sun, Nov 10, 2024 at 11:25:08AM +0000, Sakari Ailus wrote:
> > > > > > > > > > On Sun, Sep 17, 2023 at 06:45:55PM +0300, Laurent Pinchart wrote:
> > > > > > > > > > > > > > +		unsigned int xOddInc = 1;
> > > > > > > > > > > > > > +		unsigned int xEvenInc = 1;
> > > > > > > > > > > > > > +		unsigned int yOddInc = 1;
> > > > > > > > > > > > > > +		unsigned int yEvenInc = 1;
> > > > > > > > > > > > >
> > > > > > > > > > > > > What's the reason for exposing the odd and even increments separately,
> > > > > > > > > > > > > instead of a skipping factor ?
> > > > > > > > > > > > 
> > > > > > > > > > > > CCS expresses the two skipping factors separately. I agree there are
> > > > > > > > > > > > not many meaningful ways this two parameters can be combined, but if
> > > > > > > > > > > > one knows how a sensor work it might be more natural expressing them
> > > > > > > > > > > > in this way instead of simply saying x_skip = 2 ( == x_odd_inc=3,
> > > > > > > > > > > > x_even_inc=2)
> > > > > > > > > > > 
> > > > > > > > > > > Lots of sensors that support configuring skipping do no expose separate
> > > > > > > > > > > odd and even skipping increments. That's why I think a simpler parameter
> > > > > > > > > > > would be better.
> > > > > > > > > > > 
> > > > > > > > > > > Sakari, do you have an opinion on this ? Are there reasonable use cases
> > > > > > > > > > > for controlling the odd and even increments separately in a way that
> > > > > > > > > > > couldn't be expressed by a single skipping factor (for each direction) ?
> > > > > > > > > > 
> > > > > > > > > > Skipping (sub-sampling) is generally independent horizontally and
> > > > > > > > > > vertically, binning is not. On some sensors reasonable output sizes may be
> > > > > > > > > > impelemented a combination of the two.
> > > > > > > > > 
> > > > > > > > > There's no disagreement that horizontal and vertical factors need to be
> > > > > > > > > expressed independently. The question was about independent even and odd
> > > > > > > > > increments.
> > > > > > > > 
> > > > > > > > That's what I meant.
> > > > > > > > 
> > > > > > > > CCS separates odd and even increments but in practice the odd increment
> > > > > > > > should always be 1. I guess we could add support for these as well.
> > > > > > > 
> > > > > > > I'm not asking to add support for exposing the odd and even increments
> > > > > > > to userspace. Quite the opposite, I was proposing combining them into a
> > > > > > > single factor in the API that libcamera exposes to applications. The
> > > > > > > feedback I would like is whether or not there is a real need for
> > > > > > > applications to configure the increments separately. If so, why ?
> > > > > > 
> > > > > > The limits for horizontal and vertical sub-sampling are not connected in
> > > > > > hardware. Beyond that, as I noted above, some reasonable output sizes are
> > > > > > achieved as combination of sub-sampling and binning on some sensors.
> > > > > > Therefore I don't think you can combine the two, at least without limiting
> > > > > > what can be supported, which is why I think horizontal and vertical
> > > > > > sub-sampling need to remain separate.
> > > > > 
> > > > > ... I don't think we understand each other.
> > > > > 
> > > > > The odd and even increments exist both horizontally and vertically.
> > > > > Quoting myself from a previous e-mail,
> > > > > 
> > > > > > > > > There's no disagreement that horizontal and vertical factors need to be
> > > > > > > > > expressed independently. The question was about independent even and odd
> > > > > > > > > increments.
> > > > 
> > > > Ah, right. As noted above, the even increment is almost always 1. In
> > > > practice this could have other values on monochrome sensors. We could
> > > > expose both (via controls) right from the begininning, I think that makes
> > > > sense.
> > > 
> > > What's the use case ?
> > 
> > To do sub-sampling on monochrome sensors that require even values (and the
> > same values for odd and even increments). There are different options in
> > how this can be implemented, see table 78 (page 145) and the examples on
> > page 148 in CCS 1.1 spec.
> 
> Does this have to be exposed to userspace, or can appropriate even and
> odd increments be computed by the kernel ?

It would seem like that they could, but if the API e.g. CCS provides is
more expressive than what we provide towards the user space, there could be
situations where there are more options with different image quality
related properties. I'm not sure if we'll ever see that, though -- right
now that's not the case at least.

I guess we could do with ratios for now, and if something else is needed,
then add support for that.

-- 
Sakari Ailus