[libcamera-devel] [PATCH] libcamera: V4L2Device: Fix a type mismatch of ControlValue
Jacopo Mondi
jacopo at jmondi.org
Tue May 25 12:36:26 CEST 2021
Hi Hiro,
On Tue, May 25, 2021 at 02:33:41PM +0900, Hirokazu Honda wrote:
> The commit (34bee5e8) of removing the controls order assumption
We usually spell the commit message out as:
Commit 34bee5e84ecb ("libcamera: V4L2Device: Remove the controls order
assumption in updateControls()") removed the control odering
assumption without changing the updateControls() function accordingly.
> doesn't change the update ControlValue type in
> V4L2Device::updateControls(). However, this is wrong because the
> ControlValue is set to a placeholder ControlValue in
> V4L2Device::getControls(), so the type is ControlTypeNone.
> We should rather look up the type in V4L2Device::controls_ with a
> specified id.
>
> Fixes: 34bee5e8 (libcamera: V4L2Device: Remove the controls order assumption in updateControls())
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> ---
> src/libcamera/v4l2_device.cpp | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 7f7e5b8f..cc4f52bf 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -621,7 +621,11 @@ void V4L2Device::updateControls(ControlList *ctrls,
> const unsigned int id = v4l2Ctrl.id;
>
> ControlValue value = ctrls->get(id);
I've just noticed we do
ControlValue value = ctrls.get()
switch() {
value.set();
}
ctrls.set(id, value);
Can't we just take a reference and modify value ?
(not related to this patch though).
> - switch (value.type()) {
> +
> + auto iter = controls_.find(id);
const auto & ?
> + ASSERT(iter != controls_.end());
> + const ControlType type = iter->first->type();
> + switch (type) {
You could save the local variable if you want to
> case ControlTypeInteger64:
> value.set<int64_t>(v4l2Ctrl.value64);
> break;
> --
> 2.31.1.818.g46aad6cb9e-goog
>
More information about the libcamera-devel
mailing list