Re: [PATCH v11 39/66] media: Documentation: Add subdev configuration models, raw sensor model

[Jacopo Mondi <jacopo.mondi@ideasonboard.com>]

Hi Sakari,
  I take the occasion as I've noticed you're reviving the series

There was a discussion item we didn't complete last fall, about the
semantic associated to selection rectangles for RAW camera sensors.

On Tue, Oct 07, 2025 at 04:01:11PM +0200, Jacopo Mondi wrote:
> Hi again Sakari,
>    cc Kieran and Stefan
>
> Stefan and Kieran brought to my attention a use case I would like to
> discuss with you
>
> On Fri, Oct 03, 2025 at 03:25:19PM +0200, Jacopo Mondi wrote:
> > Hi Sakari,
> >
> > On Fri, Oct 03, 2025 at 04:15:34PM +0300, Sakari Ailus wrote:
> > > Hi Jacopo,
> >
> > [snip]
> >
> > > > >
> > > >
> > > > A recent discussion on libcamera made me wonder a few things
> > > >
> > > > https://patchwork.libcamera.org/patch/24547/
> > > >
> > > > In the current world (pre-RAW sensor model) the situation can be
> > > > summarized as
> > > >
> > > > TGT_NATIVE_SIZE = full pixel array (readable and non readable)
> > > > TGT_CROP_BOUNDS = readable pixel array (visible and non visibile pixels)
> > >
> > > Crop bounds is generally the same as native size.
> > >
> >
> > I checked two sensors datasheet for this yesterday and both of them
> > had parts of the pixel array that cannot be read out
> >
> > > > TGT_CROP_DEFAULT = visible pixels
> > >
> > > The default could exclude not-so-great pixels, too.
> > >
> >
> > ok, suggested pixel array area used for image capture purposes then
> >
> > > > TGT_CROP = analgoue crop
> > >
> > > This could include digital crop as well.
> > >
> >
> > Yes it might, not all sensor drivers behaves the same indeed
> >
> > > >
> > > > where:
> > > > - visibile = pixels used for image capture purpose
> > > > - non-visible = optically black, dummies etc
> > > >
> > > > With the RAW sensor model:
> > > >
> > > > format(1/0) = readable pixel array (visible and non visible)
> > > > TGT_CROP_DEFAULT(1/0) = visible pixel area
> > > > TGT_CROP(1/0) = analogue crop
> > > > TGT_COMPOSE(1/0) = binning/skipping
> > > >
> > > > Have we lost the ability to report the full pixel array size (readable
> > > > and not readable) ? Is this intentional ? As if pixels cannot be read
> > > > out they basically do no exist, and the information on the actual
> > > > number of pixels (including non readable ones) should be kept
> > > > somewhere else (like the libcamera sensor properties database) ?
> > >
> > > I'd keep this information in the user space if needed. There's little
> > > software could presumably do with this information.
> > >
> >
> > Agreed, there is no value I can think of in having this information in
> > drivers
> >
>
> So, Kieran and Stefan are working with a sensor whose driver was
> initially upstreamed with a wrong "readable pixel array"
> (TGT_CROP_BOUNDS). The developer later realized there was more of the
> pixel array to read and there was a use for the non-image pixels like
> OB ones.
>
> With the current model this is fine (sort of), as all rectangles are
> expressed with the TGT_NATIVE size reference. TGT_BOUNDS might
> increases but TGT_CROP_DEFAULT and TGT_CROP are still valid both in the
> driver but also in userspace, which might have encoded some known
> "tested" configurations.
>
> With the new model we lose the information reported by TGT_NATIVE and
> all rectangles will be expressed with the format on 1/0 as reference.
> If the format changes because we later find out there were portions of
> the pixel array that could have been read out, all other rectangles
> will have to change as well, both in the driver (which is ok-ish) but
> also in userspace, which we have no control on.
>
> Stefan and Kieran could elaborate more on this, but basically, the
> physical array is the only fixed reference we could actually count on.
> Other rectangles, are subject to the driver developer understanding of
> how the device work, which as we know very well, can change over time.
>
> Now, if you agree this is something to be concerned on, I presume the
> fix is quite easy
>
>          format(1/0) = physical pixel array size
>          TGT_CROP_BOUNDS(1/0) = readable pixel array (visible and non visible)
>          TGT_CROP_DEFAULT(1/0) = visible pixel area
>          TGT_CROP(1/0) = analogue crop
>          TGT_COMPOSE(1/0) = binning/skipping
>
> which basically only require re-introducing the use of CROP_BOUNDS in
> the RAW camera model specification.
>
> What do you think ?
>

Sorry for dropping the ball on this, but I think the above still
applies.

Kieran and Stefan provided good points in their replies to this thread
on why we should care about the physical sensor size, which if I'm not
mistaken is now lost with the new proposed model.

Is the above still a valid suggestion in your opinion ?

Thanks
   j

> > > >
> > > > All the discussion about readable/non-readable, visible/non-visibile
> > > > and active and inactive areas make me think we would benefit from
> > > > presenting a small glossary at the beginning of the "Sensor pixel
> > > > array size, cropping and binning" paragraph ?
> > >
> > > The text does not discuss active or inactive areas. I'd add some terms into
> > > the main glossary if needed -- they are used outside this file, too.
> > >
> > > --
> > > Regards,
> > >
> > > Sakari Ailus
>