[libcamera-devel] [PATCH] libcamera: controls: Remove merge assertion
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri May 7 18:11:50 CEST 2021
Hi Kieran,
Thank you for the patch.
On Fri, May 07, 2021 at 01:44:44PM +0100, Kieran Bingham wrote:
> The ControlList merge operation is protected with an ASSERT to guarantee
> that the two lists are compatible.
>
> Unfortunately this assertion fails when we run IPAs in an isolated case
> as while the lists are compatible, the isolated IPA has a unique
> instance of the id map. This breaks the pointer comparison, and the
> assertion fails with a false positive.
Paul, is this caused by the deserializer using a deserialized idmap
instead of a cached pointer to an idmap previously serialized ?
> Remove the assertion, leaving only a todo in it's place as this breaks
> active users of the library.
>
> Bugzilla: https://bugs.libcamera.org/show_bug.cgi?id=31
How about "Bug:" to not make it depend on any particular tool ?
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> src/libcamera/controls.cpp | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index b763148d4391..5aef4e7145bd 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -890,7 +890,17 @@ ControlList::ControlList(const ControlInfoMap &infoMap, ControlValidator *valida
> */
> void ControlList::merge(const ControlList &source)
> {
> - ASSERT(idmap_ == source.idmap_);
> + /**
> + * \todo: ASSERT that the current and source ControlList are derived
> + * from a compatible ControlIdMap, to prevent undefined behaviour due to
> + * id collisions.
> + *
> + * This can not currently be a direct pointer comparison due to the
> + * duplication of the ControlIdMaps in the isolated IPA use cases.
> + * Furthermore, manually checking each entry of the id map is identical
> + * is expensive.
> + * See https://bugs.libcamera.org/show_bug.cgi?id=31 for further details
> + */
>
> for (const auto &ctrl : source) {
> if (contains(ctrl.first)) {
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list