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

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

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?

Thanks and best regards

David

On Thu, 5 Aug 2021 at 19:05, Sakari Ailus <sakari.ailus@iki.fi> wrote:
>
> On Thu, Aug 05, 2021 at 06:49:33PM +0300, Laurent Pinchart wrote:
> > Hi Sakari,
> >
> > 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).
>
> --
> Sakari Ailus