| Message ID | 20250721115029.1561179-1-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabás, Thank you for the patch! On Mon, 2025-07-21 at 13:50 +0200, Barnabás Pőcze wrote: > Accept empty `ControlList`s without an info map. Serialization > supports `ControlList`s without an associated `ControlInfoMap` > object. However, for deserialization, a "v4l2" control list > without an info map resulted in a fatal error. > > After 30114cadd8cb65 ("ipa: rpi: Defer initialising AF LensPosition > ControlInfo and value") > the `lensControls` member of `ipa::RPi::ConfigResult` is left empty > and without an info map in every case, not just when a lens is not > present. > This causes the above assertion to trigger every single time. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/libcamera/control_serializer.cpp | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/control_serializer.cpp > b/src/libcamera/control_serializer.cpp > index 17834648c..1534fc5c8 100644 > --- a/src/libcamera/control_serializer.cpp > +++ b/src/libcamera/control_serializer.cpp > @@ -601,8 +601,11 @@ ControlList > ControlSerializer::deserialize<ControlList>(ByteStreamBuffer &buffer > > case IPA_CONTROL_ID_MAP_V4L2: > default: > - LOG(Serializer, Fatal) > - << "A list of V4L2 controls requires > an ControlInfoMap"; > + if (hdr->entries > 0) { > + LOG(Serializer, Fatal) > + << "A list of V4L2 controls > requires an ControlInfoMap"; > + } > + Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com> > return {}; > } > }
Quoting Barnabás Pőcze (2025-07-21 12:50:29) > Accept empty `ControlList`s without an info map. Serialization > supports `ControlList`s without an associated `ControlInfoMap` > object. However, for deserialization, a "v4l2" control list > without an info map resulted in a fatal error. > > After 30114cadd8cb65 ("ipa: rpi: Defer initialising AF LensPosition ControlInfo and value") > the `lensControls` member of `ipa::RPi::ConfigResult` is left empty > and without an info map in every case, not just when a lens is not present. > This causes the above assertion to trigger every single time. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/libcamera/control_serializer.cpp | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp > index 17834648c..1534fc5c8 100644 > --- a/src/libcamera/control_serializer.cpp > +++ b/src/libcamera/control_serializer.cpp > @@ -601,8 +601,11 @@ ControlList ControlSerializer::deserialize<ControlList>(ByteStreamBuffer &buffer > > case IPA_CONTROL_ID_MAP_V4L2: > default: > - LOG(Serializer, Fatal) > - << "A list of V4L2 controls requires an ControlInfoMap"; > + if (hdr->entries > 0) { > + LOG(Serializer, Fatal) > + << "A list of V4L2 controls requires an ControlInfoMap"; > + } > + I think this sounds ok ... I have nothing to argue against it ... Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > return {}; > } > } > -- > 2.50.1 >
Quoting Barnabás Pőcze (2025-07-21 20:50:29) > Accept empty `ControlList`s without an info map. Serialization > supports `ControlList`s without an associated `ControlInfoMap` > object. However, for deserialization, a "v4l2" control list > without an info map resulted in a fatal error. > > After 30114cadd8cb65 ("ipa: rpi: Defer initialising AF LensPosition ControlInfo and value") > the `lensControls` member of `ipa::RPi::ConfigResult` is left empty > and without an info map in every case, not just when a lens is not present. > This causes the above assertion to trigger every single time. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/libcamera/control_serializer.cpp | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp > index 17834648c..1534fc5c8 100644 > --- a/src/libcamera/control_serializer.cpp > +++ b/src/libcamera/control_serializer.cpp > @@ -601,8 +601,11 @@ ControlList ControlSerializer::deserialize<ControlList>(ByteStreamBuffer &buffer > > case IPA_CONTROL_ID_MAP_V4L2: > default: > - LOG(Serializer, Fatal) > - << "A list of V4L2 controls requires an ControlInfoMap"; > + if (hdr->entries > 0) { > + LOG(Serializer, Fatal) > + << "A list of V4L2 controls requires an ControlInfoMap"; > + } > + > return {}; > } > } > -- > 2.50.1 >
On Mon, Jul 21, 2025 at 08:09:01PM +0100, Kieran Bingham wrote: > Quoting Barnabás Pőcze (2025-07-21 12:50:29) > > Accept empty `ControlList`s without an info map. Serialization > > supports `ControlList`s without an associated `ControlInfoMap` > > object. However, for deserialization, a "v4l2" control list > > without an info map resulted in a fatal error. > > > > After 30114cadd8cb65 ("ipa: rpi: Defer initialising AF LensPosition ControlInfo and value") > > the `lensControls` member of `ipa::RPi::ConfigResult` is left empty > > and without an info map in every case, not just when a lens is not present. > > This causes the above assertion to trigger every single time. > > > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > --- > > src/libcamera/control_serializer.cpp | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp > > index 17834648c..1534fc5c8 100644 > > --- a/src/libcamera/control_serializer.cpp > > +++ b/src/libcamera/control_serializer.cpp > > @@ -601,8 +601,11 @@ ControlList ControlSerializer::deserialize<ControlList>(ByteStreamBuffer &buffer > > > > case IPA_CONTROL_ID_MAP_V4L2: > > default: > > - LOG(Serializer, Fatal) > > - << "A list of V4L2 controls requires an ControlInfoMap"; > > + if (hdr->entries > 0) { > > + LOG(Serializer, Fatal) > > + << "A list of V4L2 controls requires an ControlInfoMap"; While at it, s/ an / a / > > + } > > + > > I think this sounds ok ... I have nothing to argue against it ... No need for curly braces though. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > return {}; > > } > > }
Hi 2025. 07. 24. 14:22 keltezéssel, Laurent Pinchart írta: > On Mon, Jul 21, 2025 at 08:09:01PM +0100, Kieran Bingham wrote: >> Quoting Barnabás Pőcze (2025-07-21 12:50:29) >>> Accept empty `ControlList`s without an info map. Serialization >>> supports `ControlList`s without an associated `ControlInfoMap` >>> object. However, for deserialization, a "v4l2" control list >>> without an info map resulted in a fatal error. >>> >>> After 30114cadd8cb65 ("ipa: rpi: Defer initialising AF LensPosition ControlInfo and value") >>> the `lensControls` member of `ipa::RPi::ConfigResult` is left empty >>> and without an info map in every case, not just when a lens is not present. >>> This causes the above assertion to trigger every single time. >>> >>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >>> --- >>> src/libcamera/control_serializer.cpp | 7 +++++-- >>> 1 file changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp >>> index 17834648c..1534fc5c8 100644 >>> --- a/src/libcamera/control_serializer.cpp >>> +++ b/src/libcamera/control_serializer.cpp >>> @@ -601,8 +601,11 @@ ControlList ControlSerializer::deserialize<ControlList>(ByteStreamBuffer &buffer >>> >>> case IPA_CONTROL_ID_MAP_V4L2: >>> default: >>> - LOG(Serializer, Fatal) >>> - << "A list of V4L2 controls requires an ControlInfoMap"; >>> + if (hdr->entries > 0) { >>> + LOG(Serializer, Fatal) >>> + << "A list of V4L2 controls requires an ControlInfoMap"; > > While at it, s/ an / a / > >>> + } >>> + >> >> I think this sounds ok ... I have nothing to argue against it ... > > No need for curly braces though. Hmmm, true... I just like to use them if the statements is more than 1 line. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> >>> return {}; >>> } >>> } >
diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp index 17834648c..1534fc5c8 100644 --- a/src/libcamera/control_serializer.cpp +++ b/src/libcamera/control_serializer.cpp @@ -601,8 +601,11 @@ ControlList ControlSerializer::deserialize<ControlList>(ByteStreamBuffer &buffer case IPA_CONTROL_ID_MAP_V4L2: default: - LOG(Serializer, Fatal) - << "A list of V4L2 controls requires an ControlInfoMap"; + if (hdr->entries > 0) { + LOG(Serializer, Fatal) + << "A list of V4L2 controls requires an ControlInfoMap"; + } + return {}; } }
Accept empty `ControlList`s without an info map. Serialization supports `ControlList`s without an associated `ControlInfoMap` object. However, for deserialization, a "v4l2" control list without an info map resulted in a fatal error. After 30114cadd8cb65 ("ipa: rpi: Defer initialising AF LensPosition ControlInfo and value") the `lensControls` member of `ipa::RPi::ConfigResult` is left empty and without an info map in every case, not just when a lens is not present. This causes the above assertion to trigger every single time. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/libcamera/control_serializer.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)