[RFC PATCH v1] libcamera: pipeline: uvcvideo: Fix `ExposureTimeMode` control

[Kieran Bingham]

Quoting Barnabás Pőcze (2025-01-29 12:13:12)
> Hi
> 
> 
> 2025. január 29., szerda 10:22 keltezéssel, Jacopo Mondi <jacopo.mondi at ideasonboard.com> írta:
> 
> > Hi Barnabás
> > 
> > On Tue, Jan 28, 2025 at 12:14:01PM +0000, Barnabás Pőcze wrote:
> > > `ControlInfo(Span<const int32_t>{...})` calls the incorrect constructor
> > > of `ControlInfo`. The intended constructor to be called is
> > > `ControlInfo(Span<const ControlValue>, ...)` however that is not called
> > > because a span of `const int32_t` is passed. Instead, the constructor
> > > `ControlInfo(const ControlValue &min, const ControlValue &max, ...)`
> > > will be called.
> > >
> > > To fix this, simply pass a span of `ControlValue` instead.
> > >
> > > Furthermore, the mapping in `UVCCameraData::processControl()` is also not
> > > entirely correct because the control value is retrieved as a `bool`
> > > instead of - the correct type - `int32_t`. Additionally, the available
> > > modes are not taken into account.
> > >
> > > To fix this, stores the available modes for `V4L2_CID_EXPOSURE_AUTO`
> > > and select the appropriate mode based on the mapping established
> > > in the comment.
> > >
> > > Fixes: bad8d591f8acfa ("libcamera: uvcvideo: Register ExposureTimeMode control")
> > > Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>
> > > ---
> > >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 81 +++++++++++++-------
> > >  1 file changed, 53 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > index dedcac89b..7821cceb0 100644
> > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > @@ -6,6 +6,7 @@
> > >   */
> > >
> > >  #include <algorithm>
> > > +#include <bitset>
> > >  #include <cmath>
> > >  #include <fstream>
> > >  #include <map>
> > > @@ -58,6 +59,13 @@ public:
> > >     Stream stream_;
> > >     std::map<PixelFormat, std::vector<SizeRange>> formats_;
> > >
> > > +   std::bitset<std::max({
> > > +           V4L2_EXPOSURE_AUTO,
> > > +           V4L2_EXPOSURE_MANUAL,
> > > +           V4L2_EXPOSURE_APERTURE_PRIORITY,
> > > +           V4L2_EXPOSURE_SHUTTER_PRIORITY,
> > > +   }) + 1> availableExposureModes_;
> > > +
> > 
> > I presume a bitset is used for efficiency ?
> 
> I suppose you could say that. I thought an `std::set` was a bit of an overkill.

It looks like GCC-9 has some issues around here:

https://gitlab.freedesktop.org/camera/libcamera/-/jobs/70104933

--
Kieran