Re: [PATCH v3 1/2] media: v4l2-ctrls: Add V4L2_CID_NOTIFY_GAIN_XXX controls

[David Plowman <david.plowman@raspberrypi.com>]

Hi Laurent

Thanks for the reply. Yes, I agree that a fixed order (B, Gb, Gr, R)
is much easier to use, and I think that works well for the cases I'm
dealing with. I'll update the patch set accordingly and re-post it.

Best regards
David

On Fri, 6 Aug 2021 at 09:38, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi David,
>
> On Fri, Aug 06, 2021 at 09:15:09AM +0100, David Plowman wrote:
> > Hi Sakari, Laurent
> >
> > Thanks for the various comments and discussion. It did prompt me to
> > have some second thoughts about some of the details here, as follows.
> >
> > These controls are aimed specifically at sensors that do some kind of
> > on-board "demosaic / remosaic" process, for instance to produce
> > standard Bayer patterns from non-standard ones. As such the principal
> > requirement is for the sensor to know what "grey" looks like, which we
> > tell it by sending it the red and blue gains from the white balance
> > algorithm. (This obviously enables it to reduce colour aliasing during
> > the processing that it does.)
> >
> > So perhaps the comparison here should be with the existing
> > V4L2_CID_RED/BLUE_BALANCE controls. I'm not sure that it really
> > matters exactly what the colours of the pixels on the sensor really
> > are, it's knowing what "grey" looks like that is important. Any new
> > controls could be quite cumbersome to use if you have to figure out
> > what the underlying pixel arrangement looks like, it certainly feels
> > much easier to refer simply to red/blue pixels, leaving the driver to
> > deal with its own internal idiosyncrasies.
> >
> > Having said that, the particular sensor I have exposes a gain for each
> > of the four Bayer channels, even though I find myself ignoring the
> > green ones!!
> >
> > Anyway, I certainly feel I could go either way on this one - do you
> > have any more thoughts on the matter?
>
> With an array control, we would have to decide (and document) which
> components are stored in the array, and in which order. For Bayer
> sensors, that would be B, Gb, Gr and R. As for the order, there are
> three options:
>
> - fixed order (e.g. always B, Gb, Gr, R)
> - matching the sensor's CFA native order (always the same for a given
>   sensor, but varying between different sensors)
> - matching the currently configured format (the bayer pattern can change
>   when moving the crop rectangle by one pixels or when mirroring the
>   sensor readout)
>
> The last two options seem quite impractical to me. The first option, if
> we only consider Bayer sensors, is equivalent to the four controls your
> propose in the sense that the semantics is defined in the control
> specification and identical for all sensors, but with the advantage of
> bundling all four values together. It will also allow expanding this to
> other patterns if the need arise, which I think would be useful.
>
> If we were to redesign the red/blue gains, I'd go for a single array
> control today, with one value per CFA component.
>
> > On Thu, 5 Aug 2021 at 19:05, Sakari Ailus wrote:
> > > On Thu, Aug 05, 2021 at 06:49:33PM +0300, Laurent Pinchart wrote:
> > > > On Thu, Aug 05, 2021 at 06:40:42PM +0300, Sakari Ailus wrote:
> > > > > On Thu, Aug 05, 2021 at 06:22:32PM +0300, Laurent Pinchart wrote:
> > > > > > On Thu, Jul 22, 2021 at 01:12:48PM +0100, David Plowman wrote:
> > > > > > > We add new controls, one for each of the four usual Bayer channels:
> > > > > > >
> > > > > > > V4L2_CID_NOTIFY_GAIN_RED
> > > > > > > V4L2_CID_NOTIFY_GAIN_GREENR
> > > > > > > V4L2_CID_NOTIFY_GAIN_BLUE
> > > > > > > V4L2_CID_NOTIFY_GAIN_GREENB
> > > > > >
> > > > > > This will effectively limit the API to Bayer patterns. I wonder if we
> > > > > > should instead implement it as a single array control, with one element
> > > > > > per CFA component.
> > > > >
> > > > > There are other raw patterns, too. Supporting them would likely require one
> > > > > or a few more controls.
> > > > >
> > > > > That said, as the values change often it's more efficient to use a single
> > > > > control. But each colour combination (not each pattern) would require its
> > > > > own control in this case, eventually requiring more controls.
> > > >
> > > > I'm not sure to follow you. My idea is to define a single control, with
> > > > a number of elements that depends on the pattern being used, and the
> > > > order specified in the native sensor pattern. I don't think each colour
> > > > combination would then require its own control.
> > >
> > > Ah, I guess that would work, too. Then we'll need to define what kind of
> > > pixel orders are supported for that single control (and for which formats).
>
> --
> Regards,
>
> Laurent Pinchart