[libcamera-devel] [PATCH 2/3] libcamera: controls: Specify manual gain units and change exposure units

[Laurent Pinchart]

Hello,

On Mon, Feb 17, 2020 at 04:21:32PM +0000, Kieran Bingham wrote:
> On 17/02/2020 14:26, Naushir Patuck wrote:
> > Use micro-seconds for ManualExposure. This is changed from milli-
> > seconds. The latter would not allow very low exposure times.
> > 
> > Use double for ManualGain to allow fractional gain adjustments.
> > 
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> >  src/libcamera/control_ids.yaml | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > index 4befec7..33062d6 100644
> > --- a/src/libcamera/control_ids.yaml
> > +++ b/src/libcamera/control_ids.yaml
> > @@ -44,10 +44,10 @@ controls:
> >  
> >    - ManualExposure:
> >        type: int32_t
> > -      description: Specify a fixed exposure time in milli-seconds
> > +      description: Specify a fixed exposure time in micro-seconds
> 
> At the moment, the only user of this control
> (PipelineHandlerUVC::processControls()) sets this directly on the V4L2
> control.
> 
> As we don't yet have anything much actually testing this, it's fairly
> arbitrary - so I'm certainly not against changing the units.
> 
> But we may need to consider how to map this directly at the platform.
> 
> That said - that's a job for the pipeline handlers ...
> 
> But perhaps we need to divide the value by 1000 in the current code at
> pipeline/uvcvideo.cpp to maintain consistency.

UVC expresses the exposure time in units of 100µs (I mentioned 100ns
earlier on IRC, that was a mistake, it's the unit for UVC frame
intervals, not exposure time), so both would be wrong :-) We sure need
to fix the UVC pipeline handler, but it doesn't need to be part of this
patch.

> >    - ManualGain:
> > -      type: int32_t
> > -      description: Specify a fixed gain parameter
> > +      type: double
> > +      description: Specify a fixed gain parameter.
> 
> Same comment applies here that if this is changed, the usage in
> pipeline/uvcvideo.cpp needs to be updated at the same time, so I think
> we'll need to split this patch out to change the units and fix the
> (current) implementation for each of the controls one control at a time.

UVC doesn't specify units for gain. One option would be to map the
default value to 1.0 and consider the gain range to be linear.

> But Otherwise, I agree on the unit changes.

-- 
Regards,

Laurent Pinchart