Hi Sakari,
(CC'ing Hans for the discussion about the control framework)
On Wed, Oct 12, 2022 at 09:54:54PM +0300, Sakari Ailus wrote:
> On Fri, Oct 07, 2022 at 11:20:13AM +0300, Laurent Pinchart wrote:
> > On Fri, Oct 07, 2022 at 07:28:46AM +0200, Krzysztof Hałasa wrote:
> > > Laurent Pinchart writes:
> > >
> > > > I'm tempted to drop support for the colour gains really, and turn the
> > > > V4L2_CID_GAIN into V4L2_CID_DIGITAL_GAIN. Digital colour gains can still
> > > > be useful on platforms that have no ISP, but I think we need an array of
> > > > gains in that case, not abusing V4L2_CID_RED_BALANCE and
> > > > V4L2_CID_BLUE_BALANCE. Any objection ?
> > >
> > > I'm fine with spliting it into analog/digital as long as there is a way
> > > to set individual R/G/B (digital) gain values.
> >
> > With the controls we have today in V4L2, we could map
> > V4L2_CID_RED_BALANCE and V4L2_CID_BLUE_BALANCE to the red and blue
> > digital gains, with V4L2_CID_DIGITAL_GAIN setting the global digital
> > gain.
> >
> > I'm tempted to bite the bullet and define a new
> > V4L2_CID_DIGITAL_COLOR_GAINS control that would expose an array of
> > gains, but if we extend the API for that, I think we should also include
> > support for HDR at the same time, with at least T1/T2 sets of gains.
> >
> > Sakari, any opinion ?
>
> Would you use multiple controls for that or just a single one?
That's what we need to decide :-)
> The size of a matrix control is not changeable dynamically so I presume the
We now have
* - ``V4L2_CTRL_FLAG_DYNAMIC_ARRAY``
- 0x0800
- This control is a dynamically sized 1-dimensional array. It
behaves the same as a regular array, except that the number
of elements as reported by the ``elems`` field is between 1 and
``dims[0]``. So setting the control with a differently sized
array will change the ``elems`` field when the control is
queried afterwards.
that allows changing a control size dynamically from userspace. It only
works for 1D controls though. The kernel can also change the dimensions
of a control at any time, and that could be done when the HDR control
would be set to dual-gain mode to turn the gain controls into arrays.
> driver would create as large control as needed, and program to hardware as
> much as needed.
That's what we need to decide :-) I'm tempted to go for a single control
that would cover both the multi-channel and multi-gain needs.
V4L2_CID_ANALOG_GAIN control would become an array control of two
elements, one for T1 and one for T2. It may confuse some applications.
For new drivers that could be fine. For existing drivers, an application
that sets the control as if it were a regular V4L2_CID_ANALOG_GAIN of
type V4L2_CTRL_TYPE_INTEGER would break.
This may be fixable in the V4L2 control framework, by only allowing
get/set of a subset of an array control. I'm not sure that't desirable
in the general case though. If the control dimensions were changed by
the driver when turning dual-gain mode on (or off) we would only need (I
think) to update v4l2_s_ctrl() to allow writing array controls when the
array contains a single element, which should be less of a problem.
For the digital gains, V4L2_CID_DIGITAL_GAIN would become a 2D array,
with one dimension covering the colour channels, and the other dimension
covering the T1/T2 gains.
--
Regards,
Laurent Pinchart
