Hi Hans,
On Thu, Nov 05, 2020 at 02:03:37PM +0100, Hans Verkuil wrote:
> On 07/10/2020 10:45, Sakari Ailus wrote:
...
> > + .name = "Luminance Shading Correction",
> > + .type = V4L2_CTRL_TYPE_BOOLEAN,
> > + .id = V4L2_CID_CCS_LUMINANCE_SHADING_CORRECTION,
> > + .ops = &ccs_ctrl_ops,
> > + .max = 255,
> > + .step = 1,
> > + .def = 128,
> > + };
> > +
> > + v4l2_ctrl_new_custom(&sensor->pixel_array->ctrl_handler,
> > + &ctrl_cfg, NULL);
> > + }
> > +
> > + if (CCS_LIM(sensor, SHADING_CORRECTION_CAPABILITY) &
> > + (CCS_SHADING_CORRECTION_CAPABILITY_COLOR_SHADING |
> > + CCS_SHADING_CORRECTION_CAPABILITY_LUMINANCE_CORRECTION)) {
> > + u32 val =
> > + ((CCS_LIM(sensor, SHADING_CORRECTION_CAPABILITY) &
> > + CCS_SHADING_CORRECTION_CAPABILITY_COLOR_SHADING) ?
> > + V4L2_CCS_SHADING_CORRECTION_COLOUR : 0) |
> > + ((CCS_LIM(sensor, SHADING_CORRECTION_CAPABILITY) &
> > + CCS_SHADING_CORRECTION_CAPABILITY_LUMINANCE_CORRECTION) ?
> > + V4L2_CCS_SHADING_CORRECTION_LUMINANCE : 0);
> > + const struct v4l2_ctrl_config ctrl_cfg = {
> > + .name = "Shading Correction Capability",
> > + .type = V4L2_CTRL_TYPE_BITMASK,
> > + .id = V4L2_CID_CCS_SHADING_CORRECTION_CAPABILITY,
> > + .ops = &ccs_ctrl_ops,
> > + .max = val,
> > + .def = val,
> > + .flags = V4L2_CTRL_FLAG_READ_ONLY,
> > + };
>
> Is this needed? If e.g. V4L2_CCS_SHADING_CORRECTION_COLOUR is not supported,
> then the V4L2_CID_CCS_SHADING_CORRECTION control is simply not created.
> So calling VIDIOC_QUERYCTRL would simply fail and so indicate that this
> capability is not present.
>
> If it really is needed, then having two bool controls makes more sense
> because a bitmask is less intuitive.
The CCS shading correction support has two capabilities but one bit to
control both. Another (luminance correction) has the correction level
(buggy in this patch, I'll fix for v4), so two controls aren't enough to
tell what is being corrected; is it colour shading or not? Luminance
correction level support is revealed by the luminance correction level
control as well.
So I guess you could also have a boolean control to tell colour correction
is supported.
I guess could also just omit the capability control now and see if someone
needs it. I'd expect this to be rarely needed information.
--
Kind regards,
Sakari Ailus
