| Message ID | 20260324123214.1762198-2-barnabas.pocze@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Quoting Barnabás Pőcze (2026-03-24 12:32:14) > As per the kernel documentation: > > If the error is associated with a particular control, then ``error_idx`` > is set to the index of that control. If the error is not related to a > specific control, or the validation step failed , then ``error_idx`` is > set to ``count``. > > Thus a value of 0 means that the first control could not be processed > properly. So fix the conditions. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/libcamera/v4l2_device.cpp | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > index 53bd7865a..67bbaa037 100644 > --- a/src/libcamera/v4l2_device.cpp > +++ b/src/libcamera/v4l2_device.cpp > @@ -260,7 +260,7 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids, const V4L2Request > unsigned int errorIdx = v4l2ExtCtrls.error_idx; > > /* Generic validation error. */ > - if (errorIdx == 0 || errorIdx >= v4l2Ctrls.size()) { > + if (errorIdx >= v4l2Ctrls.size()) { > LOG(V4L2, Error) << "Unable to read controls: " > << strerror(-ret); Should we at least print the control name/identifier that failed here? > return {}; > @@ -405,7 +405,7 @@ int V4L2Device::setControls(ControlList *ctrls, const V4L2Request *request) > unsigned int errorIdx = v4l2ExtCtrls.error_idx; > > /* Generic validation error. */ > - if (errorIdx == 0 || errorIdx >= v4l2Ctrls.size()) { > + if (errorIdx >= v4l2Ctrls.size()) { > LOG(V4L2, Error) << "Unable to set controls: " > << strerror(-ret); > return -EINVAL; > -- > 2.53.0 >
2026. 03. 24. 13:57 keltezéssel, Kieran Bingham írta: > Quoting Barnabás Pőcze (2026-03-24 12:32:14) >> As per the kernel documentation: >> >> If the error is associated with a particular control, then ``error_idx`` >> is set to the index of that control. If the error is not related to a >> specific control, or the validation step failed , then ``error_idx`` is >> set to ``count``. >> >> Thus a value of 0 means that the first control could not be processed >> properly. So fix the conditions. >> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> src/libcamera/v4l2_device.cpp | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp >> index 53bd7865a..67bbaa037 100644 >> --- a/src/libcamera/v4l2_device.cpp >> +++ b/src/libcamera/v4l2_device.cpp >> @@ -260,7 +260,7 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids, const V4L2Request >> unsigned int errorIdx = v4l2ExtCtrls.error_idx; >> >> /* Generic validation error. */ >> - if (errorIdx == 0 || errorIdx >= v4l2Ctrls.size()) { >> + if (errorIdx >= v4l2Ctrls.size()) { >> LOG(V4L2, Error) << "Unable to read controls: " >> << strerror(-ret); > > Should we at least print the control name/identifier that failed here? It is printed below that condition, both in `{get,set}Controls()`. > >> return {}; >> @@ -405,7 +405,7 @@ int V4L2Device::setControls(ControlList *ctrls, const V4L2Request *request) >> unsigned int errorIdx = v4l2ExtCtrls.error_idx; >> >> /* Generic validation error. */ >> - if (errorIdx == 0 || errorIdx >= v4l2Ctrls.size()) { >> + if (errorIdx >= v4l2Ctrls.size()) { >> LOG(V4L2, Error) << "Unable to set controls: " >> << strerror(-ret); >> return -EINVAL; >> -- >> 2.53.0 >>
Quoting Barnabás Pőcze (2026-03-24 12:59:56) > 2026. 03. 24. 13:57 keltezéssel, Kieran Bingham írta: > > Quoting Barnabás Pőcze (2026-03-24 12:32:14) > >> As per the kernel documentation: > >> > >> If the error is associated with a particular control, then ``error_idx`` > >> is set to the index of that control. If the error is not related to a > >> specific control, or the validation step failed , then ``error_idx`` is > >> set to ``count``. > >> > >> Thus a value of 0 means that the first control could not be processed > >> properly. So fix the conditions. > >> > >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > >> --- > >> src/libcamera/v4l2_device.cpp | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > >> index 53bd7865a..67bbaa037 100644 > >> --- a/src/libcamera/v4l2_device.cpp > >> +++ b/src/libcamera/v4l2_device.cpp > >> @@ -260,7 +260,7 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids, const V4L2Request > >> unsigned int errorIdx = v4l2ExtCtrls.error_idx; > >> > >> /* Generic validation error. */ > >> - if (errorIdx == 0 || errorIdx >= v4l2Ctrls.size()) { > >> + if (errorIdx >= v4l2Ctrls.size()) { > >> LOG(V4L2, Error) << "Unable to read controls: " > >> << strerror(-ret); > > > > Should we at least print the control name/identifier that failed here? > > It is printed below that condition, both in `{get,set}Controls()`. Aha great, well then: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > >> return {}; > >> @@ -405,7 +405,7 @@ int V4L2Device::setControls(ControlList *ctrls, const V4L2Request *request) > >> unsigned int errorIdx = v4l2ExtCtrls.error_idx; > >> > >> /* Generic validation error. */ > >> - if (errorIdx == 0 || errorIdx >= v4l2Ctrls.size()) { > >> + if (errorIdx >= v4l2Ctrls.size()) { > >> LOG(V4L2, Error) << "Unable to set controls: " > >> << strerror(-ret); > >> return -EINVAL; > >> -- > >> 2.53.0 > >> >
diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 53bd7865a..67bbaa037 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -260,7 +260,7 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids, const V4L2Request unsigned int errorIdx = v4l2ExtCtrls.error_idx; /* Generic validation error. */ - if (errorIdx == 0 || errorIdx >= v4l2Ctrls.size()) { + if (errorIdx >= v4l2Ctrls.size()) { LOG(V4L2, Error) << "Unable to read controls: " << strerror(-ret); return {}; @@ -405,7 +405,7 @@ int V4L2Device::setControls(ControlList *ctrls, const V4L2Request *request) unsigned int errorIdx = v4l2ExtCtrls.error_idx; /* Generic validation error. */ - if (errorIdx == 0 || errorIdx >= v4l2Ctrls.size()) { + if (errorIdx >= v4l2Ctrls.size()) { LOG(V4L2, Error) << "Unable to set controls: " << strerror(-ret); return -EINVAL;
As per the kernel documentation: If the error is associated with a particular control, then ``error_idx`` is set to the index of that control. If the error is not related to a specific control, or the validation step failed , then ``error_idx`` is set to ``count``. Thus a value of 0 means that the first control could not be processed properly. So fix the conditions. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/libcamera/v4l2_device.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)