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

[Laurent Pinchart]

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 ?

> > > > > I'd be
> > > > > inclined to support sub-sampling using controls, separately for each
> > > > > direction. 
> > > > 
> > > > Is this still related to the odd and even increments, or a separate
> > > > topic ? If still related, could you please explain more clearly what you
> > > > mean ?
> > > 
> > > What's required for sub-sampling, i.e. odd increments. The even increments
> > > could be added later on.
> > 
> > Aren't both odd and even increments related to subsampling ?
> 
> Yes.
> 
> > > In fact, even increments appear to be useful on monochrome sensors.
> > > 
> > > I'll add this to the next version of the sensor interface patches.

-- 
Regards,

Laurent Pinchart