| Message ID | 20230710132240.7864-3-jacopo.mondi@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Documentation: v4l: more camera sensor doc | expand |
Hi Jacopo, On Mon, Jul 10, 2023 at 03:22:40PM +0200, Jacopo Mondi wrote: > Document the suggested way to exposure controls for exposure and gain > for camera sensor drivers. > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > --- > .../driver-api/media/camera-sensor.rst | 27 +++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst > index cd915ca119ea..67fe77b1edb9 100644 > --- a/Documentation/driver-api/media/camera-sensor.rst > +++ b/Documentation/driver-api/media/camera-sensor.rst > @@ -189,3 +189,30 @@ the ``V4L2_CID_VFLIP`` and ``V4L2_CID_HFLIP`` controls with the > a flip can potentially change the output buffer content layout. Flips should > also be taken into account when enumerating and handling media bus formats > on the camera sensor source pads. > + > +Exposure and Gain Control > +------------------------- > + > +Camera sensor drivers that allow applications to control the image exposure > +and gain should do so by exposing dedicated controls to applications. > + > +Exposure time is controlled by registering the ``V4L2_CID_EXPOSURE`` control. > +The control definition does not specify a unit to allow maximum flexibility > +for multiple device types, but when used for camera sensor drivers it should be > +expressed in unit of lines whenever possible. This part of the documentation applies to both raw and SoC cameras. Should the exposure unit be something more user-friendly for SoC cameras? We have two exposure controls now, V4L2_CID_EXPOSURE and V4L2_CID_EXPOSURE_ABSOLUTE. The former doesn't specity a unit whereas the latter suggests the unit of 100 µs. As exposure is specific to cameras, I think at least a part of this should make it to the controls documentation. The UVC, for instance, uses EXPOSURE_ABSOLUTE. Could we document V4L2_CID_EXPOSURE is in lines (if possible)? > + > +To convert lines into units of time, the total line length (visible and > +not visible pixels) has to be divided by the pixel rate:: > + > + line duration = total line length / pixel rate > + = (image width + horizontal blanking) / pixel rate > + > +Camera sensor driver should try whenever possible to distinguish between the > +analogue and digital gain control functions. Analogue gain is a multiplication > +factor applied to all color channels on the pixel array before they get > +converted into the digital domain. It should be made controllable by The analogue gain may not be linear. This depends on the sensor. I'd thus drop the wording related to multiplication factor. > +registering the ``V4L2_CID_ANALOGUE_GAIN`` control, expressed as a device > +specific gain code. Digital gain control is optional and should be exposed to > +applications by registering ``V4L2_CID_DIGITAL_GAIN``. Camera sensor drivers are > +discouraged from using ``V4L2_CID_GAIN`` as it doesn't allow differentiation of > +analogue vs digital gain.
Hi Sakari On Mon, Jul 17, 2023 at 10:24:21AM +0000, Sakari Ailus wrote: > Hi Jacopo, > > On Mon, Jul 10, 2023 at 03:22:40PM +0200, Jacopo Mondi wrote: > > Document the suggested way to exposure controls for exposure and gain > > for camera sensor drivers. > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > --- > > .../driver-api/media/camera-sensor.rst | 27 +++++++++++++++++++ > > 1 file changed, 27 insertions(+) > > > > diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst > > index cd915ca119ea..67fe77b1edb9 100644 > > --- a/Documentation/driver-api/media/camera-sensor.rst > > +++ b/Documentation/driver-api/media/camera-sensor.rst > > @@ -189,3 +189,30 @@ the ``V4L2_CID_VFLIP`` and ``V4L2_CID_HFLIP`` controls with the > > a flip can potentially change the output buffer content layout. Flips should > > also be taken into account when enumerating and handling media bus formats > > on the camera sensor source pads. > > + > > +Exposure and Gain Control > > +------------------------- > > + > > +Camera sensor drivers that allow applications to control the image exposure > > +and gain should do so by exposing dedicated controls to applications. > > + > > +Exposure time is controlled by registering the ``V4L2_CID_EXPOSURE`` control. > > +The control definition does not specify a unit to allow maximum flexibility > > +for multiple device types, but when used for camera sensor drivers it should be > > +expressed in unit of lines whenever possible. > > This part of the documentation applies to both raw and SoC cameras. > > Should the exposure unit be something more user-friendly for SoC cameras? SoC cameras == YUV/RGB sensors ? Are you thinking about using the actual exposure time for YUV/RGB sensors ? > > We have two exposure controls now, V4L2_CID_EXPOSURE and > V4L2_CID_EXPOSURE_ABSOLUTE. The former doesn't specity a unit whereas the Apparently only 2 drivers in mainline register V4L2_CID_EXPOSURE_ABSOLUTE > latter suggests the unit of 100 µs. > > As exposure is specific to cameras, I think at least a part of this should > make it to the controls documentation. The UVC, for instance, uses > EXPOSURE_ABSOLUTE. > > Could we document V4L2_CID_EXPOSURE is in lines (if possible)? I would indeed be happy with something like "The suggested unit for the control is lines" > > > + > > +To convert lines into units of time, the total line length (visible and > > +not visible pixels) has to be divided by the pixel rate:: > > + > > + line duration = total line length / pixel rate > > + = (image width + horizontal blanking) / pixel rate > > + > > +Camera sensor driver should try whenever possible to distinguish between the > > +analogue and digital gain control functions. Analogue gain is a multiplication > > +factor applied to all color channels on the pixel array before they get > > +converted into the digital domain. It should be made controllable by > > The analogue gain may not be linear. This depends on the sensor. I'd thus > drop the wording related to multiplication factor. > I might have missed why the gain being linear or not has implications on the fact it acts as a multiplication factor for the color channels... > > +registering the ``V4L2_CID_ANALOGUE_GAIN`` control, expressed as a device > > +specific gain code. Digital gain control is optional and should be exposed to > > +applications by registering ``V4L2_CID_DIGITAL_GAIN``. Camera sensor drivers are > > +discouraged from using ``V4L2_CID_GAIN`` as it doesn't allow differentiation of > > +analogue vs digital gain. > > -- > Kind regards, > > Sakari Ailus
Hi Jacopo, Just found this old e-mail I apparently forgot to reply... On Thu, Jul 27, 2023 at 11:42:45AM +0200, Jacopo Mondi wrote: > Hi Sakari > > On Mon, Jul 17, 2023 at 10:24:21AM +0000, Sakari Ailus wrote: > > Hi Jacopo, > > > > On Mon, Jul 10, 2023 at 03:22:40PM +0200, Jacopo Mondi wrote: > > > Document the suggested way to exposure controls for exposure and gain > > > for camera sensor drivers. > > > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > --- > > > .../driver-api/media/camera-sensor.rst | 27 +++++++++++++++++++ > > > 1 file changed, 27 insertions(+) > > > > > > diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst > > > index cd915ca119ea..67fe77b1edb9 100644 > > > --- a/Documentation/driver-api/media/camera-sensor.rst > > > +++ b/Documentation/driver-api/media/camera-sensor.rst > > > @@ -189,3 +189,30 @@ the ``V4L2_CID_VFLIP`` and ``V4L2_CID_HFLIP`` controls with the > > > a flip can potentially change the output buffer content layout. Flips should > > > also be taken into account when enumerating and handling media bus formats > > > on the camera sensor source pads. > > > + > > > +Exposure and Gain Control > > > +------------------------- > > > + > > > +Camera sensor drivers that allow applications to control the image exposure > > > +and gain should do so by exposing dedicated controls to applications. > > > + > > > +Exposure time is controlled by registering the ``V4L2_CID_EXPOSURE`` control. > > > +The control definition does not specify a unit to allow maximum flexibility > > > +for multiple device types, but when used for camera sensor drivers it should be > > > +expressed in unit of lines whenever possible. > > > > This part of the documentation applies to both raw and SoC cameras. > > > > Should the exposure unit be something more user-friendly for SoC cameras? > > SoC cameras == YUV/RGB sensors ? > > Are you thinking about using the actual exposure time for YUV/RGB > sensors ? Some devices support both but there are devices that don't natively support it, including UVC and Alvium. I wonder whether we should suggest using the control method that best works with device-native units? I.e. if the device natively uses frame length and line length, then use blankings + the pixel clock, otherwise [gs]_frame_interval? > > > > > We have two exposure controls now, V4L2_CID_EXPOSURE and > > V4L2_CID_EXPOSURE_ABSOLUTE. The former doesn't specity a unit whereas the > > Apparently only 2 drivers in mainline register V4L2_CID_EXPOSURE_ABSOLUTE It's not very popular, no. :-) 100 µs is also a long time, I would expect to have issues with that large granularity. > > > latter suggests the unit of 100 µs. > > > > As exposure is specific to cameras, I think at least a part of this should > > make it to the controls documentation. The UVC, for instance, uses > > EXPOSURE_ABSOLUTE. > > > > Could we document V4L2_CID_EXPOSURE is in lines (if possible)? > > I would indeed be happy with something like "The suggested unit for > the control is lines" Should there be another control for exposure in (µ)s then? > > > > > > + > > > +To convert lines into units of time, the total line length (visible and > > > +not visible pixels) has to be divided by the pixel rate:: > > > + > > > + line duration = total line length / pixel rate > > > + = (image width + horizontal blanking) / pixel rate > > > + > > > +Camera sensor driver should try whenever possible to distinguish between the > > > +analogue and digital gain control functions. Analogue gain is a multiplication > > > +factor applied to all color channels on the pixel array before they get > > > +converted into the digital domain. It should be made controllable by > > > > The analogue gain may not be linear. This depends on the sensor. I'd thus > > drop the wording related to multiplication factor. > > > > I might have missed why the gain being linear or not has implications > on the fact it acts as a multiplication factor for the color > channels... I must have read this as the analogue gain being the control value. Could you still add that the analogue gain factor may have a non-linear relation to the control value? > > > > +registering the ``V4L2_CID_ANALOGUE_GAIN`` control, expressed as a device > > > +specific gain code. Digital gain control is optional and should be exposed to > > > +applications by registering ``V4L2_CID_DIGITAL_GAIN``. Camera sensor drivers are > > > +discouraged from using ``V4L2_CID_GAIN`` as it doesn't allow differentiation of > > > +analogue vs digital gain. > >
Hi Sakari On Mon, Nov 20, 2023 at 08:25:45AM +0000, Sakari Ailus wrote: > Hi Jacopo, > > Just found this old e-mail I apparently forgot to reply... > This really fell into the cracks for me as well > On Thu, Jul 27, 2023 at 11:42:45AM +0200, Jacopo Mondi wrote: > > Hi Sakari > > > > On Mon, Jul 17, 2023 at 10:24:21AM +0000, Sakari Ailus wrote: > > > Hi Jacopo, > > > > > > On Mon, Jul 10, 2023 at 03:22:40PM +0200, Jacopo Mondi wrote: > > > > Document the suggested way to exposure controls for exposure and gain > > > > for camera sensor drivers. > > > > > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > > --- > > > > .../driver-api/media/camera-sensor.rst | 27 +++++++++++++++++++ > > > > 1 file changed, 27 insertions(+) > > > > > > > > diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst > > > > index cd915ca119ea..67fe77b1edb9 100644 > > > > --- a/Documentation/driver-api/media/camera-sensor.rst > > > > +++ b/Documentation/driver-api/media/camera-sensor.rst > > > > @@ -189,3 +189,30 @@ the ``V4L2_CID_VFLIP`` and ``V4L2_CID_HFLIP`` controls with the > > > > a flip can potentially change the output buffer content layout. Flips should > > > > also be taken into account when enumerating and handling media bus formats > > > > on the camera sensor source pads. > > > > + > > > > +Exposure and Gain Control > > > > +------------------------- > > > > + > > > > +Camera sensor drivers that allow applications to control the image exposure > > > > +and gain should do so by exposing dedicated controls to applications. > > > > + > > > > +Exposure time is controlled by registering the ``V4L2_CID_EXPOSURE`` control. > > > > +The control definition does not specify a unit to allow maximum flexibility > > > > +for multiple device types, but when used for camera sensor drivers it should be > > > > +expressed in unit of lines whenever possible. > > > > > > This part of the documentation applies to both raw and SoC cameras. > > > > > > Should the exposure unit be something more user-friendly for SoC cameras? > > > > SoC cameras == YUV/RGB sensors ? > > > > Are you thinking about using the actual exposure time for YUV/RGB > > sensors ? > > Some devices support both but there are devices that don't natively support > it, including UVC and Alvium. > > I wonder whether we should suggest using the control method that best works > with device-native units? I.e. if the device natively uses frame length and > line length, then use blankings + the pixel clock, otherwise > [gs]_frame_interval? > Are we mixing two things here ? The above documentation block is about the suggested unit for the exposure control, while [gs]_frame_interval vs {blankings + pixel_rate} is to control the frame duration ? > > > > > > > > We have two exposure controls now, V4L2_CID_EXPOSURE and > > > V4L2_CID_EXPOSURE_ABSOLUTE. The former doesn't specity a unit whereas the > > > > Apparently only 2 drivers in mainline register V4L2_CID_EXPOSURE_ABSOLUTE > > It's not very popular, no. :-) 100 µs is also a long time, I would expect > to have issues with that large granularity. > is 100 micro-seconds a too large granularity when it comes to exposure time ?? > > > > > latter suggests the unit of 100 µs. > > > > > > As exposure is specific to cameras, I think at least a part of this should > > > make it to the controls documentation. The UVC, for instance, uses > > > EXPOSURE_ABSOLUTE. > > > > > > Could we document V4L2_CID_EXPOSURE is in lines (if possible)? > > > > I would indeed be happy with something like "The suggested unit for > > the control is lines" > > Should there be another control for exposure in (µ)s then? > Isn't it V4L2_CID_EXPOSURE_ABSOLUTE ? > > > > > > > > > + > > > > +To convert lines into units of time, the total line length (visible and > > > > +not visible pixels) has to be divided by the pixel rate:: > > > > + > > > > + line duration = total line length / pixel rate > > > > + = (image width + horizontal blanking) / pixel rate > > > > + > > > > +Camera sensor driver should try whenever possible to distinguish between the > > > > +analogue and digital gain control functions. Analogue gain is a multiplication > > > > +factor applied to all color channels on the pixel array before they get > > > > +converted into the digital domain. It should be made controllable by > > > > > > The analogue gain may not be linear. This depends on the sensor. I'd thus > > > drop the wording related to multiplication factor. > > > > > > > I might have missed why the gain being linear or not has implications > > on the fact it acts as a multiplication factor for the color > > channels... > > I must have read this as the analogue gain being the control value. Could > you still add that the analogue gain factor may have a non-linear relation > to the control value? > Sure! Thanks for digging this one out! > > > > > > +registering the ``V4L2_CID_ANALOGUE_GAIN`` control, expressed as a device > > > > +specific gain code. Digital gain control is optional and should be exposed to > > > > +applications by registering ``V4L2_CID_DIGITAL_GAIN``. Camera sensor drivers are > > > > +discouraged from using ``V4L2_CID_GAIN`` as it doesn't allow differentiation of > > > > +analogue vs digital gain. > > > > > -- > Regards, > > Sakari Ailus
Hi Jacopo, On Mon, Nov 20, 2023 at 05:49:30PM +0100, Jacopo Mondi wrote: > Hi Sakari > > On Mon, Nov 20, 2023 at 08:25:45AM +0000, Sakari Ailus wrote: > > Hi Jacopo, > > > > Just found this old e-mail I apparently forgot to reply... > > > > This really fell into the cracks for me as well > > > On Thu, Jul 27, 2023 at 11:42:45AM +0200, Jacopo Mondi wrote: > > > Hi Sakari > > > > > > On Mon, Jul 17, 2023 at 10:24:21AM +0000, Sakari Ailus wrote: > > > > Hi Jacopo, > > > > > > > > On Mon, Jul 10, 2023 at 03:22:40PM +0200, Jacopo Mondi wrote: > > > > > Document the suggested way to exposure controls for exposure and gain > > > > > for camera sensor drivers. > > > > > > > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > > > --- > > > > > .../driver-api/media/camera-sensor.rst | 27 +++++++++++++++++++ > > > > > 1 file changed, 27 insertions(+) > > > > > > > > > > diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst > > > > > index cd915ca119ea..67fe77b1edb9 100644 > > > > > --- a/Documentation/driver-api/media/camera-sensor.rst > > > > > +++ b/Documentation/driver-api/media/camera-sensor.rst > > > > > @@ -189,3 +189,30 @@ the ``V4L2_CID_VFLIP`` and ``V4L2_CID_HFLIP`` controls with the > > > > > a flip can potentially change the output buffer content layout. Flips should > > > > > also be taken into account when enumerating and handling media bus formats > > > > > on the camera sensor source pads. > > > > > + > > > > > +Exposure and Gain Control > > > > > +------------------------- > > > > > + > > > > > +Camera sensor drivers that allow applications to control the image exposure > > > > > +and gain should do so by exposing dedicated controls to applications. > > > > > + > > > > > +Exposure time is controlled by registering the ``V4L2_CID_EXPOSURE`` control. > > > > > +The control definition does not specify a unit to allow maximum flexibility > > > > > +for multiple device types, but when used for camera sensor drivers it should be > > > > > +expressed in unit of lines whenever possible. > > > > > > > > This part of the documentation applies to both raw and SoC cameras. > > > > > > > > Should the exposure unit be something more user-friendly for SoC cameras? > > > > > > SoC cameras == YUV/RGB sensors ? > > > > > > Are you thinking about using the actual exposure time for YUV/RGB > > > sensors ? > > > > Some devices support both but there are devices that don't natively support > > it, including UVC and Alvium. > > > > I wonder whether we should suggest using the control method that best works > > with device-native units? I.e. if the device natively uses frame length and > > line length, then use blankings + the pixel clock, otherwise > > [gs]_frame_interval? > > > > Are we mixing two things here ? The above documentation block is about > the suggested unit for the exposure control, while [gs]_frame_interval > vs {blankings + pixel_rate} is to control the frame duration ? Oops. The context was apparently garbled in the meantime. X-) I meant using ISO units (i.e. second) in this case. > > > > > > > > > > > > We have two exposure controls now, V4L2_CID_EXPOSURE and > > > > V4L2_CID_EXPOSURE_ABSOLUTE. The former doesn't specity a unit whereas the > > > > > > Apparently only 2 drivers in mainline register V4L2_CID_EXPOSURE_ABSOLUTE > > > > It's not very popular, no. :-) 100 µs is also a long time, I would expect > > to have issues with that large granularity. > > > > is 100 micro-seconds a too large granularity when it comes to exposure > time ?? It's a pretty long time in bright lighting conditions. The problem is not a value as such, but the granularity: a change of one has a major relative effect on the exposure time. In practice this value is translated to some number of lines, and the granularity shouldn't be worse than that. I'd therefore make this µs instead. > > > > > > > > latter suggests the unit of 100 µs. > > > > > > > > As exposure is specific to cameras, I think at least a part of this should > > > > make it to the controls documentation. The UVC, for instance, uses > > > > EXPOSURE_ABSOLUTE. > > > > > > > > Could we document V4L2_CID_EXPOSURE is in lines (if possible)? > > > > > > I would indeed be happy with something like "The suggested unit for > > > the control is lines" > > > > Should there be another control for exposure in (µ)s then? > > > > Isn't it V4L2_CID_EXPOSURE_ABSOLUTE ? I guess we could use that but the control name makes no sense, there is also some amount of former use. I'd create a new one instead. At this point it shouldn't really matter for the user space. The existing drivers will continue to also use whatever they're using now (I guess?). > > > > > > > > > > > > > + > > > > > +To convert lines into units of time, the total line length (visible and > > > > > +not visible pixels) has to be divided by the pixel rate:: > > > > > + > > > > > + line duration = total line length / pixel rate > > > > > + = (image width + horizontal blanking) / pixel rate > > > > > + > > > > > +Camera sensor driver should try whenever possible to distinguish between the > > > > > +analogue and digital gain control functions. Analogue gain is a multiplication > > > > > +factor applied to all color channels on the pixel array before they get > > > > > +converted into the digital domain. It should be made controllable by > > > > > > > > The analogue gain may not be linear. This depends on the sensor. I'd thus > > > > drop the wording related to multiplication factor. > > > > > > > > > > I might have missed why the gain being linear or not has implications > > > on the fact it acts as a multiplication factor for the color > > > channels... > > > > I must have read this as the analogue gain being the control value. Could > > you still add that the analogue gain factor may have a non-linear relation > > to the control value? > > > > Sure! > > Thanks for digging this one out! You're welcome! :-)
Hi Sakari On Mon, Nov 20, 2023 at 06:19:46PM +0000, Sakari Ailus wrote: > Hi Jacopo, > > On Mon, Nov 20, 2023 at 05:49:30PM +0100, Jacopo Mondi wrote: > > Hi Sakari > > > > On Mon, Nov 20, 2023 at 08:25:45AM +0000, Sakari Ailus wrote: > > > Hi Jacopo, > > > > > > Just found this old e-mail I apparently forgot to reply... > > > > > > > This really fell into the cracks for me as well > > > > > On Thu, Jul 27, 2023 at 11:42:45AM +0200, Jacopo Mondi wrote: > > > > Hi Sakari > > > > > > > > On Mon, Jul 17, 2023 at 10:24:21AM +0000, Sakari Ailus wrote: > > > > > Hi Jacopo, > > > > > > > > > > On Mon, Jul 10, 2023 at 03:22:40PM +0200, Jacopo Mondi wrote: > > > > > > Document the suggested way to exposure controls for exposure and gain > > > > > > for camera sensor drivers. > > > > > > > > > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > > > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > > > > --- > > > > > > .../driver-api/media/camera-sensor.rst | 27 +++++++++++++++++++ > > > > > > 1 file changed, 27 insertions(+) > > > > > > > > > > > > diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst > > > > > > index cd915ca119ea..67fe77b1edb9 100644 > > > > > > --- a/Documentation/driver-api/media/camera-sensor.rst > > > > > > +++ b/Documentation/driver-api/media/camera-sensor.rst > > > > > > @@ -189,3 +189,30 @@ the ``V4L2_CID_VFLIP`` and ``V4L2_CID_HFLIP`` controls with the > > > > > > a flip can potentially change the output buffer content layout. Flips should > > > > > > also be taken into account when enumerating and handling media bus formats > > > > > > on the camera sensor source pads. > > > > > > + > > > > > > +Exposure and Gain Control > > > > > > +------------------------- > > > > > > + > > > > > > +Camera sensor drivers that allow applications to control the image exposure > > > > > > +and gain should do so by exposing dedicated controls to applications. > > > > > > + > > > > > > +Exposure time is controlled by registering the ``V4L2_CID_EXPOSURE`` control. > > > > > > +The control definition does not specify a unit to allow maximum flexibility > > > > > > +for multiple device types, but when used for camera sensor drivers it should be > > > > > > +expressed in unit of lines whenever possible. > > > > > > > > > > This part of the documentation applies to both raw and SoC cameras. > > > > > > > > > > Should the exposure unit be something more user-friendly for SoC cameras? > > > > > > > > SoC cameras == YUV/RGB sensors ? > > > > > > > > Are you thinking about using the actual exposure time for YUV/RGB > > > > sensors ? > > > > > > Some devices support both but there are devices that don't natively support > > > it, including UVC and Alvium. > > > > > > I wonder whether we should suggest using the control method that best works > > > with device-native units? I.e. if the device natively uses frame length and > > > line length, then use blankings + the pixel clock, otherwise > > > [gs]_frame_interval? > > > > > > > Are we mixing two things here ? The above documentation block is about > > the suggested unit for the exposure control, while [gs]_frame_interval > > vs {blankings + pixel_rate} is to control the frame duration ? > > Oops. The context was apparently garbled in the meantime. X-) > > I meant using ISO units (i.e. second) in this case. > > > > > > > > > > > > > > > > > We have two exposure controls now, V4L2_CID_EXPOSURE and > > > > > V4L2_CID_EXPOSURE_ABSOLUTE. The former doesn't specity a unit whereas the > > > > > > > > Apparently only 2 drivers in mainline register V4L2_CID_EXPOSURE_ABSOLUTE > > > > > > It's not very popular, no. :-) 100 µs is also a long time, I would expect > > > to have issues with that large granularity. > > > > > > > is 100 micro-seconds a too large granularity when it comes to exposure > > time ?? > > It's a pretty long time in bright lighting conditions. The problem is not a > value as such, but the granularity: a change of one has a major relative > effect on the exposure time. > > In practice this value is translated to some number of lines, and the > granularity shouldn't be worse than that. I'd therefore make this µs > instead. > Yeah, you're right, the two controls' granularity should be similar. I actually wonder if that's enough. C-PHY has a total bandwidth of 5.8Gbps if I'm not mistaken, and assuming 12bpp and a line lenght of 4000 pixels (arbitrary pick) the line time is 8usec. I wonder if we shouldn't go for nanoseconds and be done with that. > > > > > > > > > > > latter suggests the unit of 100 µs. > > > > > > > > > > As exposure is specific to cameras, I think at least a part of this should > > > > > make it to the controls documentation. The UVC, for instance, uses > > > > > EXPOSURE_ABSOLUTE. > > > > > > > > > > Could we document V4L2_CID_EXPOSURE is in lines (if possible)? > > > > > > > > I would indeed be happy with something like "The suggested unit for > > > > the control is lines" > > > > > > Should there be another control for exposure in (µ)s then? > > > > > > > Isn't it V4L2_CID_EXPOSURE_ABSOLUTE ? > > I guess we could use that but the control name makes no sense, there is > also some amount of former use. I'd create a new one instead. At this point > it shouldn't really matter for the user space. > > The existing drivers will continue to also use whatever they're using now > (I guess?). > if we introduce a new control V4L2_CID_EXPOSURE_USEC (name to be V4L2_CID_EXPOSURE in lines V4L2_CID_EXPOSURE_ABSOLUTE in 100usec V4L2_CID_EXPOSURE_USEC in 1usec/nanosecs isn't it very confusing ? Ideally there should have been V4L2_CID_EXPOSURE_LINES and V4L2_CID_EXPOSURE_USEC from the start, but how to get there without breaking existing users or duplicating controls is not easy... > > > > > > > > > > > > > > > > > + > > > > > > +To convert lines into units of time, the total line length (visible and > > > > > > +not visible pixels) has to be divided by the pixel rate:: > > > > > > + > > > > > > + line duration = total line length / pixel rate > > > > > > + = (image width + horizontal blanking) / pixel rate > > > > > > + > > > > > > +Camera sensor driver should try whenever possible to distinguish between the > > > > > > +analogue and digital gain control functions. Analogue gain is a multiplication > > > > > > +factor applied to all color channels on the pixel array before they get > > > > > > +converted into the digital domain. It should be made controllable by > > > > > > > > > > The analogue gain may not be linear. This depends on the sensor. I'd thus > > > > > drop the wording related to multiplication factor. > > > > > > > > > > > > > I might have missed why the gain being linear or not has implications > > > > on the fact it acts as a multiplication factor for the color > > > > channels... > > > > > > I must have read this as the analogue gain being the control value. Could > > > you still add that the analogue gain factor may have a non-linear relation > > > to the control value? > > > > > > > Sure! > > > > Thanks for digging this one out! > > You're welcome! :-) > > -- > Regards, > > Sakari Ailus
Hi Jacopo, On Mon, Nov 20, 2023 at 07:47:11PM +0100, Jacopo Mondi wrote: > Hi Sakari > > On Mon, Nov 20, 2023 at 06:19:46PM +0000, Sakari Ailus wrote: > > Hi Jacopo, > > > > On Mon, Nov 20, 2023 at 05:49:30PM +0100, Jacopo Mondi wrote: > > > Hi Sakari > > > > > > On Mon, Nov 20, 2023 at 08:25:45AM +0000, Sakari Ailus wrote: > > > > Hi Jacopo, > > > > > > > > Just found this old e-mail I apparently forgot to reply... > > > > > > > > > > This really fell into the cracks for me as well > > > > > > > On Thu, Jul 27, 2023 at 11:42:45AM +0200, Jacopo Mondi wrote: > > > > > Hi Sakari > > > > > > > > > > On Mon, Jul 17, 2023 at 10:24:21AM +0000, Sakari Ailus wrote: > > > > > > Hi Jacopo, > > > > > > > > > > > > On Mon, Jul 10, 2023 at 03:22:40PM +0200, Jacopo Mondi wrote: > > > > > > > Document the suggested way to exposure controls for exposure and gain > > > > > > > for camera sensor drivers. > > > > > > > > > > > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > > > > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > > > > > --- > > > > > > > .../driver-api/media/camera-sensor.rst | 27 +++++++++++++++++++ > > > > > > > 1 file changed, 27 insertions(+) > > > > > > > > > > > > > > diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst > > > > > > > index cd915ca119ea..67fe77b1edb9 100644 > > > > > > > --- a/Documentation/driver-api/media/camera-sensor.rst > > > > > > > +++ b/Documentation/driver-api/media/camera-sensor.rst > > > > > > > @@ -189,3 +189,30 @@ the ``V4L2_CID_VFLIP`` and ``V4L2_CID_HFLIP`` controls with the > > > > > > > a flip can potentially change the output buffer content layout. Flips should > > > > > > > also be taken into account when enumerating and handling media bus formats > > > > > > > on the camera sensor source pads. > > > > > > > + > > > > > > > +Exposure and Gain Control > > > > > > > +------------------------- > > > > > > > + > > > > > > > +Camera sensor drivers that allow applications to control the image exposure > > > > > > > +and gain should do so by exposing dedicated controls to applications. > > > > > > > + > > > > > > > +Exposure time is controlled by registering the ``V4L2_CID_EXPOSURE`` control. > > > > > > > +The control definition does not specify a unit to allow maximum flexibility > > > > > > > +for multiple device types, but when used for camera sensor drivers it should be > > > > > > > +expressed in unit of lines whenever possible. > > > > > > > > > > > > This part of the documentation applies to both raw and SoC cameras. > > > > > > > > > > > > Should the exposure unit be something more user-friendly for SoC cameras? > > > > > > > > > > SoC cameras == YUV/RGB sensors ? > > > > > > > > > > Are you thinking about using the actual exposure time for YUV/RGB > > > > > sensors ? > > > > > > > > Some devices support both but there are devices that don't natively support > > > > it, including UVC and Alvium. > > > > > > > > I wonder whether we should suggest using the control method that best works > > > > with device-native units? I.e. if the device natively uses frame length and > > > > line length, then use blankings + the pixel clock, otherwise > > > > [gs]_frame_interval? > > > > > > > > > > Are we mixing two things here ? The above documentation block is about > > > the suggested unit for the exposure control, while [gs]_frame_interval > > > vs {blankings + pixel_rate} is to control the frame duration ? > > > > Oops. The context was apparently garbled in the meantime. X-) > > > > I meant using ISO units (i.e. second) in this case. > > > > > > > > > > > > > > > > > > > > > > We have two exposure controls now, V4L2_CID_EXPOSURE and > > > > > > V4L2_CID_EXPOSURE_ABSOLUTE. The former doesn't specity a unit whereas the > > > > > > > > > > Apparently only 2 drivers in mainline register V4L2_CID_EXPOSURE_ABSOLUTE > > > > > > > > It's not very popular, no. :-) 100 µs is also a long time, I would expect > > > > to have issues with that large granularity. > > > > > > > > > > is 100 micro-seconds a too large granularity when it comes to exposure > > > time ?? > > > > It's a pretty long time in bright lighting conditions. The problem is not a > > value as such, but the granularity: a change of one has a major relative > > effect on the exposure time. > > > > In practice this value is translated to some number of lines, and the > > granularity shouldn't be worse than that. I'd therefore make this µs > > instead. > > > > Yeah, you're right, the two controls' granularity should be similar. > > I actually wonder if that's enough. C-PHY has a total bandwidth of 5.8Gbps > if I'm not mistaken, and assuming 12bpp and a line lenght of 4000 > pixels (arbitrary pick) the line time is 8usec. I wonder if we > shouldn't go for nanoseconds and be done with that. That limits the maximum time to around 2 s, but I guess one could use 64-bit controls, too. But how about 10 ns? > > > > > > > > > > > > > > > latter suggests the unit of 100 µs. > > > > > > > > > > > > As exposure is specific to cameras, I think at least a part of this should > > > > > > make it to the controls documentation. The UVC, for instance, uses > > > > > > EXPOSURE_ABSOLUTE. > > > > > > > > > > > > Could we document V4L2_CID_EXPOSURE is in lines (if possible)? > > > > > > > > > > I would indeed be happy with something like "The suggested unit for > > > > > the control is lines" > > > > > > > > Should there be another control for exposure in (µ)s then? > > > > > > > > > > Isn't it V4L2_CID_EXPOSURE_ABSOLUTE ? > > > > I guess we could use that but the control name makes no sense, there is > > also some amount of former use. I'd create a new one instead. At this point > > it shouldn't really matter for the user space. > > > > The existing drivers will continue to also use whatever they're using now > > (I guess?). > > > > if we introduce a new control V4L2_CID_EXPOSURE_USEC (name to be > > V4L2_CID_EXPOSURE in lines > V4L2_CID_EXPOSURE_ABSOLUTE in 100usec > V4L2_CID_EXPOSURE_USEC in 1usec/nanosecs > > isn't it very confusing ? > > Ideally there should have been V4L2_CID_EXPOSURE_LINES and > V4L2_CID_EXPOSURE_USEC from the start, but how to get there without > breaking existing users or duplicating controls is not easy... I wonder what Laurent and Hans think about this. I'll form my opinion tomorrow. :-) > > > > > > > > > > > > > > > > > > > > > > + > > > > > > > +To convert lines into units of time, the total line length (visible and > > > > > > > +not visible pixels) has to be divided by the pixel rate:: > > > > > > > + > > > > > > > + line duration = total line length / pixel rate > > > > > > > + = (image width + horizontal blanking) / pixel rate > > > > > > > + > > > > > > > +Camera sensor driver should try whenever possible to distinguish between the > > > > > > > +analogue and digital gain control functions. Analogue gain is a multiplication > > > > > > > +factor applied to all color channels on the pixel array before they get > > > > > > > +converted into the digital domain. It should be made controllable by > > > > > > > > > > > > The analogue gain may not be linear. This depends on the sensor. I'd thus > > > > > > drop the wording related to multiplication factor. > > > > > > > > > > > > > > > > I might have missed why the gain being linear or not has implications > > > > > on the fact it acts as a multiplication factor for the color > > > > > channels... > > > > > > > > I must have read this as the analogue gain being the control value. Could > > > > you still add that the analogue gain factor may have a non-linear relation > > > > to the control value? > > > > > > > > > > Sure! > > > > > > Thanks for digging this one out! > > > > You're welcome! :-) > >
diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst index cd915ca119ea..67fe77b1edb9 100644 --- a/Documentation/driver-api/media/camera-sensor.rst +++ b/Documentation/driver-api/media/camera-sensor.rst @@ -189,3 +189,30 @@ the ``V4L2_CID_VFLIP`` and ``V4L2_CID_HFLIP`` controls with the a flip can potentially change the output buffer content layout. Flips should also be taken into account when enumerating and handling media bus formats on the camera sensor source pads. + +Exposure and Gain Control +------------------------- + +Camera sensor drivers that allow applications to control the image exposure +and gain should do so by exposing dedicated controls to applications. + +Exposure time is controlled by registering the ``V4L2_CID_EXPOSURE`` control. +The control definition does not specify a unit to allow maximum flexibility +for multiple device types, but when used for camera sensor drivers it should be +expressed in unit of lines whenever possible. + +To convert lines into units of time, the total line length (visible and +not visible pixels) has to be divided by the pixel rate:: + + line duration = total line length / pixel rate + = (image width + horizontal blanking) / pixel rate + +Camera sensor driver should try whenever possible to distinguish between the +analogue and digital gain control functions. Analogue gain is a multiplication +factor applied to all color channels on the pixel array before they get +converted into the digital domain. It should be made controllable by +registering the ``V4L2_CID_ANALOGUE_GAIN`` control, expressed as a device +specific gain code. Digital gain control is optional and should be exposed to +applications by registering ``V4L2_CID_DIGITAL_GAIN``. Camera sensor drivers are +discouraged from using ``V4L2_CID_GAIN`` as it doesn't allow differentiation of +analogue vs digital gain.