Re: your mail

[Jacopo Mondi <jacopo@jmondi.org>]

Hi Laurent,

On Sun, Aug 09, 2020 at 06:53:11PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Wed, Aug 05, 2020 at 12:57:17PM +0200, Jacopo Mondi wrote:
> > Subject: [PATCH 0/4] media: docs: Document pixel array properties
> >
> > Hans' patch "[PATCH] imx219: selection compliance fixes" sparkled a discussion
> > on how the V4L2 selection targets have to be used in order to access an
> > image sensor pixel array properties.
> >
> > The discussion shown how much under-specified that part was, so this is
> > an attempt to provide a bit documentation for this.
> >
> > My feeling is that we're hijacking the existing targets for this use case
> > and we should probably define new ones, considering how few users we have in
> > mainline of them at the moment.
>
> Do you mean you think we should create new targets instead of reusing
> the existing ones as you do in this series ? I don't see anything really
> wrong in reusing the existing targets, provided we document them
> properly, and it's not too much of a hack (that is, the documented
> purpose reasonably matches the target name), but maybe I'm missing the
> issue.

Yes, that's what I meant.

To me, if you have something and you need to document it with "if
applied to X it means Y, if applied to Z it means W etcetc" feels like
we're abusing it. Having sensor's specific targets would make clear
what a target represents without the ambiguities of defining the
symbol semantic depending on the device it applies to (which means
userspace would need to know what kind of device it's dealing with
precisely).

That said, my preference would be using a different API, but see my
reply to your other email for that.

Thanks
  j

>
> > On top Hans' patch with reworded commit message and minor updates.
> >
> > For reference, we're using the V4L2 selection targets in libcamera to retrieve
> > the sensor pixel array properties to be reported to applications for
> > calibration purposes. The currently defined pixel properties for libcamera
> > are available here:
> > https://git.linuxtv.org/libcamera.git/tree/src/libcamera/property_ids.yaml#n390
> >
> > Thanks
> >    j
> >
> > Hans Verkuil (1):
> >   media: i2c: imx219: Selection compliance fixes
> >
> > Jacopo Mondi (3):
> >   media: docs: Describe pixel array properties
> >   media: docs: Describe targets for sensor properties
> >   media: docs: USe SUBDEV_G_SELECTION for sensor properties
> >
> >  .../userspace-api/media/v4l/dev-subdev.rst    | 85 +++++++++++++++++++
> >  .../media/v4l/v4l2-selection-targets.rst      | 49 +++++++++++
> >  .../media/v4l/vidioc-subdev-g-selection.rst   |  4 +
> >  drivers/media/i2c/imx219.c                    | 17 ++--
> >  4 files changed, 147 insertions(+), 8 deletions(-)
>
> --
> Regards,
>
> Laurent Pinchart