[libcamera-devel,v6,6/9] libcamera: Add IPADataSerializer

10720 diff mbox series

Add an IPADataSerializer which implments de/serialization of built-in
(PODs, vector, map, string) and libcamera data structures. This is
intended to be used by the proxy and the proxy worker in the IPC layer.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

---
Changes in v6:
- remove (de)serializers for IPASettings, CameraSensorInfo, IPAStream,
  and IPABuffer as they are now defined in core.mojom and generated
- ControlList (de)serializer no longer needs a ControlInfoMap
- ControlList (de)serializer now checks the ControlSerializer if the
  ControlList's ControlInfoMap is cached
- add some todos for optimization and hardening

Changes in v5:
- fix style of: {{}, {}} -> { {}, {} }
- add documentation on serialization formats (not in doxygen, though)
- compress readPOD
  - use memcpy
  - use ASSERT
- remove ifdef DOXYGEN guard from base IPADataSerializer
- remove integer overflow risk when calculating vector offsets
- use const iterators and const references
- remove const ControlList and const ControlInfoMap specializations

Changes in v4:
- rename readUInt/appendUInt to readPOD/appendPOD
  - put them in anonymous namespace
  - and add length check
    - fatal if failure, because it means not enough data to deserialize,
      which is technically a segfault
  - use the new readPOD/appendPOD with length protections
  - expand on their docs correspondingly
- change snake_case to camelCase
- add optimizations to the hand-written de/serializers
- reserve vector length where trivially possible
- remove unnecessary IPADataSerializer<type>:: explicit calls (if
  they're calling a specialization from the same specialization)

Changes in v3:
- reimplement append/readUInt with memcpy (intead of bit shifting)
- change DECLARE_INTEGRAL_SERIALIZER with DECLARE_POD_SERIALIZER
  - use this for int64_t, bool, float, and double
- fix comment style

Changes in v2:
- added serializers for all integer types, bool, and string
- use string serializer for IPASettings serializer
- add documentation
- add serializer for const ControlList
---
 .../libcamera/internal/ipa_data_serializer.h  | 701 ++++++++++++++++++
 src/libcamera/ipa_data_serializer.cpp         | 185 +++++
 src/libcamera/meson.build                     |   1 +
 3 files changed, 887 insertions(+)
 create mode 100644 include/libcamera/internal/ipa_data_serializer.h
 create mode 100644 src/libcamera/ipa_data_serializer.cpp

Hi Paul,

Thanks for your work.

On 2020-12-24 17:15:31 +0900, Paul Elder wrote:
> Add an IPADataSerializer which implments de/serialization of built-in
> (PODs, vector, map, string) and libcamera data structures. This is
> intended to be used by the proxy and the proxy worker in the IPC layer.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

I have read it all and looks really nice, I have not compared each 
libcamera type ser/des matches the class implementations in master as of 
today. It's nice work.

Acked-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> 
> ---
> Changes in v6:
> - remove (de)serializers for IPASettings, CameraSensorInfo, IPAStream,
>   and IPABuffer as they are now defined in core.mojom and generated
> - ControlList (de)serializer no longer needs a ControlInfoMap
> - ControlList (de)serializer now checks the ControlSerializer if the
>   ControlList's ControlInfoMap is cached
> - add some todos for optimization and hardening
> 
> Changes in v5:
> - fix style of: {{}, {}} -> { {}, {} }
> - add documentation on serialization formats (not in doxygen, though)
> - compress readPOD
>   - use memcpy
>   - use ASSERT
> - remove ifdef DOXYGEN guard from base IPADataSerializer
> - remove integer overflow risk when calculating vector offsets
> - use const iterators and const references
> - remove const ControlList and const ControlInfoMap specializations
> 
> Changes in v4:
> - rename readUInt/appendUInt to readPOD/appendPOD
>   - put them in anonymous namespace
>   - and add length check
>     - fatal if failure, because it means not enough data to deserialize,
>       which is technically a segfault
>   - use the new readPOD/appendPOD with length protections
>   - expand on their docs correspondingly
> - change snake_case to camelCase
> - add optimizations to the hand-written de/serializers
> - reserve vector length where trivially possible
> - remove unnecessary IPADataSerializer<type>:: explicit calls (if
>   they're calling a specialization from the same specialization)
> 
> Changes in v3:
> - reimplement append/readUInt with memcpy (intead of bit shifting)
> - change DECLARE_INTEGRAL_SERIALIZER with DECLARE_POD_SERIALIZER
>   - use this for int64_t, bool, float, and double
> - fix comment style
> 
> Changes in v2:
> - added serializers for all integer types, bool, and string
> - use string serializer for IPASettings serializer
> - add documentation
> - add serializer for const ControlList
> ---
>  .../libcamera/internal/ipa_data_serializer.h  | 701 ++++++++++++++++++
>  src/libcamera/ipa_data_serializer.cpp         | 185 +++++
>  src/libcamera/meson.build                     |   1 +
>  3 files changed, 887 insertions(+)
>  create mode 100644 include/libcamera/internal/ipa_data_serializer.h
>  create mode 100644 src/libcamera/ipa_data_serializer.cpp
> 
> diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h
> new file mode 100644
> index 00000000..5468a0b7
> --- /dev/null
> +++ b/include/libcamera/internal/ipa_data_serializer.h
> @@ -0,0 +1,701 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * ipa_data_serializer.h - Image Processing Algorithm data serializer
> + */
> +#ifndef __LIBCAMERA_INTERNAL_IPA_DATA_SERIALIZER_H__
> +#define __LIBCAMERA_INTERNAL_IPA_DATA_SERIALIZER_H__
> +
> +#include <deque>
> +#include <iostream>
> +#include <string.h>
> +#include <tuple>
> +#include <type_traits>
> +#include <vector>
> +
> +#include <libcamera/buffer.h>
> +#include <libcamera/control_ids.h>
> +#include <libcamera/geometry.h>
> +#include <libcamera/ipa/ipa_interface.h>
> +
> +#include "libcamera/internal/byte_stream_buffer.h"
> +#include "libcamera/internal/camera_sensor.h"
> +#include "libcamera/internal/control_serializer.h"
> +#include "libcamera/internal/log.h"
> +
> +namespace libcamera {
> +
> +LOG_DECLARE_CATEGORY(IPADataSerializer)
> +
> +namespace {
> +
> +template<typename T>
> +void appendPOD(std::vector<uint8_t> &vec,
> +	       typename std::enable_if<std::is_arithmetic_v<T>, T>::type val)
> +{
> +	size_t byteWidth = sizeof(val);
> +	vec.resize(vec.size() + byteWidth);
> +	memcpy(&*(vec.end() - byteWidth), &val, byteWidth);
> +}
> +
> +template<typename T>
> +typename std::enable_if<std::is_arithmetic_v<T>, T>::type
> +readPOD(std::vector<uint8_t>::const_iterator it, size_t pos,
> +	std::vector<uint8_t>::const_iterator end)
> +{
> +	ASSERT(pos + it < end);
> +
> +	T ret = 0;
> +	memcpy(&ret, &(*(it + pos)), sizeof(ret));
> +
> +	return ret;
> +}
> +
> +template<typename T>
> +typename std::enable_if<std::is_arithmetic_v<T>, T>::type
> +readPOD(std::vector<uint8_t> &vec, size_t pos)
> +{
> +	return readPOD<T>(vec.cbegin(), pos, vec.end());
> +}
> +
> +} /* namespace */
> +
> +template<typename T>
> +class IPADataSerializer
> +{
> +public:
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +	serialize(const T &data, ControlSerializer *cs);
> +
> +	static T deserialize(const std::vector<uint8_t> &data,
> +			     ControlSerializer *cs);
> +	static T deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +			     std::vector<uint8_t>::const_iterator dataEnd,
> +			     ControlSerializer *cs);
> +
> +	static T deserialize(const std::vector<uint8_t> &data,
> +			     const std::vector<int32_t> &fds,
> +			     ControlSerializer *cs);
> +	static T deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +			     std::vector<uint8_t>::const_iterator dataEnd,
> +			     std::vector<int32_t>::const_iterator fdsBegin,
> +			     std::vector<int32_t>::const_iterator fdsEnd,
> +			     ControlSerializer *cs);
> +};
> +
> +#ifndef __DOXYGEN__
> +
> +/*
> + * Serialization format for vector of type V:
> + *
> + * 4 bytes - uint32_t Length of vector, in number of elements
> + *
> + * For every element in the vector:
> + *
> + * 4 bytes - uint32_t Size of element, in bytes
> + * 4 bytes - uint32_t Size of fds for the element, in number of fds
> + * X bytes - Serialized element
> + */
> +template<typename V>
> +class IPADataSerializer<std::vector<V>>
> +{
> +public:
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +	serialize(const std::vector<V> &data, ControlSerializer *cs = nullptr)
> +	{
> +		std::vector<uint8_t> dataVec;
> +		std::vector<int32_t> fdsVec;
> +
> +		/* Serialize the length. */
> +		uint32_t vecLen = data.size();
> +		appendPOD<uint32_t>(dataVec, vecLen);
> +
> +		/* Serialize the members. */
> +		for (auto const &it : data) {
> +			std::vector<uint8_t> dvec;
> +			std::vector<int32_t> fvec;
> +
> +			std::tie(dvec, fvec) =
> +				IPADataSerializer<V>::serialize(it, cs);
> +
> +			appendPOD<uint32_t>(dataVec, dvec.size());
> +			appendPOD<uint32_t>(dataVec, fvec.size());
> +
> +			dataVec.insert(dataVec.end(), dvec.begin(), dvec.end());
> +			fdsVec.insert(fdsVec.end(), fvec.begin(), fvec.end());
> +		}
> +
> +		return { dataVec, fdsVec };
> +	}
> +
> +	static std::vector<V> deserialize(std::vector<uint8_t> &data, ControlSerializer *cs = nullptr)
> +	{
> +		return deserialize(data.cbegin(), data.end(), cs);
> +	}
> +
> +	static std::vector<V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +					  std::vector<uint8_t>::const_iterator dataEnd,
> +					  ControlSerializer *cs = nullptr)
> +	{
> +		std::vector<int32_t> fds;
> +		return deserialize(dataBegin, dataEnd, fds.cbegin(), fds.end(), cs);
> +	}
> +
> +	static std::vector<V> deserialize(std::vector<uint8_t> &data, std::vector<int32_t> &fds,
> +					  ControlSerializer *cs = nullptr)
> +	{
> +		return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs);
> +	}
> +
> +	static std::vector<V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +					  std::vector<uint8_t>::const_iterator dataEnd,
> +					  std::vector<int32_t>::const_iterator fdsBegin,
> +					  [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> +					  ControlSerializer *cs = nullptr)
> +	{
> +		uint32_t vecLen = readPOD<uint32_t>(dataBegin, 0, dataEnd);
> +		std::vector<V> ret(vecLen);
> +
> +		std::vector<uint8_t>::const_iterator dataIter = dataBegin + 4;
> +		std::vector<int32_t>::const_iterator fdIter = fdsBegin;
> +		for (uint32_t i = 0; i < vecLen; i++) {
> +			uint32_t sizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd);
> +			uint32_t sizeofFds  = readPOD<uint32_t>(dataIter, 4, dataEnd);
> +			dataIter += 8;
> +
> +			ret[i] = IPADataSerializer<V>::deserialize(dataIter,
> +								   dataIter + sizeofData,
> +								   fdIter,
> +								   fdIter + sizeofFds,
> +								   cs);
> +
> +			dataIter += sizeofData;
> +			fdIter += sizeofFds;
> +		}
> +
> +		return ret;
> +	}
> +};
> +
> +/*
> + * Serialization format for map of key type K and value type V:
> + *
> + * 4 bytes - uint32_t Length of map, in number of pairs
> + *
> + * For every pair in the map:
> + *
> + * 4 bytes - uint32_t Size of key, in bytes
> + * 4 bytes - uint32_t Size of fds for the key, in number of fds
> + * X bytes - Serialized key
> + * 4 bytes - uint32_t Size of value, in bytes
> + * 4 bytes - uint32_t Size of fds for the value, in number of fds
> + * X bytes - Serialized value
> + */
> +template<typename K, typename V>
> +class IPADataSerializer<std::map<K, V>>
> +{
> +public:
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +	serialize(const std::map<K, V> &data, ControlSerializer *cs = nullptr)
> +	{
> +		std::vector<uint8_t> dataVec;
> +		std::vector<int32_t> fdsVec;
> +
> +		/* Serialize the length. */
> +		uint32_t mapLen = data.size();
> +		appendPOD<uint32_t>(dataVec, mapLen);
> +
> +		/* Serialize the members. */
> +		for (auto const &it : data) {
> +			std::vector<uint8_t> dvec;
> +			std::vector<int32_t> fvec;
> +
> +			std::tie(dvec, fvec) =
> +				IPADataSerializer<K>::serialize(it.first, cs);
> +
> +			appendPOD<uint32_t>(dataVec, dvec.size());
> +			appendPOD<uint32_t>(dataVec, fvec.size());
> +
> +			dataVec.insert(dataVec.end(), dvec.begin(), dvec.end());
> +			fdsVec.insert(fdsVec.end(), fvec.begin(), fvec.end());
> +
> +			std::tie(dvec, fvec) =
> +				IPADataSerializer<V>::serialize(it.second, cs);
> +
> +			appendPOD<uint32_t>(dataVec, dvec.size());
> +			appendPOD<uint32_t>(dataVec, fvec.size());
> +
> +			dataVec.insert(dataVec.end(), dvec.begin(), dvec.end());
> +			fdsVec.insert(fdsVec.end(), fvec.begin(), fvec.end());
> +		}
> +
> +		return { dataVec, fdsVec };
> +	}
> +
> +	static std::map<K, V> deserialize(std::vector<uint8_t> &data, ControlSerializer *cs = nullptr)
> +	{
> +		return deserialize(data.cbegin(), data.end(), cs);
> +	}
> +
> +	static std::map<K, V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +					  std::vector<uint8_t>::const_iterator dataEnd,
> +					  ControlSerializer *cs = nullptr)
> +	{
> +		std::vector<int32_t> fds;
> +		return deserialize(dataBegin, dataEnd, fds.cbegin(), fds.end(), cs);
> +	}
> +
> +	static std::map<K, V> deserialize(std::vector<uint8_t> &data, std::vector<int32_t> &fds,
> +					  ControlSerializer *cs = nullptr)
> +	{
> +		return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs);
> +	}
> +
> +	static std::map<K, V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +					  std::vector<uint8_t>::const_iterator dataEnd,
> +					  std::vector<int32_t>::const_iterator fdsBegin,
> +					  [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> +					  ControlSerializer *cs = nullptr)
> +	{
> +		std::map<K, V> ret;
> +
> +		uint32_t mapLen = readPOD<uint32_t>(dataBegin, 0, dataEnd);
> +
> +		std::vector<uint8_t>::const_iterator dataIter = dataBegin + 4;
> +		std::vector<int32_t>::const_iterator fdIter = fdsBegin;
> +		for (uint32_t i = 0; i < mapLen; i++) {
> +			uint32_t sizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd);
> +			uint32_t sizeofFds  = readPOD<uint32_t>(dataIter, 4, dataEnd);
> +			dataIter += 8;
> +
> +			K key = IPADataSerializer<K>::deserialize(dataIter,
> +								  dataIter + sizeofData,
> +								  fdIter,
> +								  fdIter + sizeofFds,
> +								  cs);
> +
> +			dataIter += sizeofData;
> +			fdIter += sizeofFds;
> +			sizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd);
> +			sizeofFds  = readPOD<uint32_t>(dataIter, 4, dataEnd);
> +			dataIter += 8;
> +
> +			const V value = IPADataSerializer<V>::deserialize(dataIter,
> +									  dataIter + sizeofData,
> +									  fdIter,
> +									  fdIter + sizeofFds,
> +									  cs);
> +			ret.insert({key, value});
> +
> +			dataIter += sizeofData;
> +			fdIter += sizeofFds;
> +		}
> +
> +		return ret;
> +	}
> +};
> +
> +#define DECLARE_POD_SERIALIZER(type)					\
> +template<>								\
> +class IPADataSerializer<type>						\
> +{									\
> +public:									\
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>	\
> +	serialize(const type data,					\
> +		  [[maybe_unused]] ControlSerializer *cs = nullptr)	\
> +	{								\
> +		std::vector<uint8_t> dataVec;				\
> +		dataVec.reserve(sizeof(type));				\
> +		appendPOD<type>(dataVec, data);			\
> +									\
> +		return { dataVec, {} };					\
> +	}								\
> +									\
> +	static type deserialize(std::vector<uint8_t> &data,		\
> +				[[maybe_unused]] ControlSerializer *cs = nullptr)\
> +	{								\
> +		return deserialize(data.cbegin(), data.end());		\
> +	}								\
> +									\
> +	static type deserialize(std::vector<uint8_t>::const_iterator dataBegin,\
> +				[[maybe_unused]] std::vector<uint8_t>::const_iterator dataEnd,\
> +				[[maybe_unused]] ControlSerializer *cs = nullptr)\
> +	{								\
> +		return readPOD<type>(dataBegin, 0, dataEnd);		\
> +	}								\
> +									\
> +	static type deserialize(std::vector<uint8_t> &data,		\
> +				[[maybe_unused]] std::vector<int32_t> &fds,\
> +				[[maybe_unused]] ControlSerializer *cs = nullptr)\
> +	{								\
> +		return deserialize(data.cbegin(), data.end());		\
> +	}								\
> +									\
> +	static type deserialize(std::vector<uint8_t>::const_iterator dataBegin,\
> +				std::vector<uint8_t>::const_iterator dataEnd,\
> +				[[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin,\
> +				[[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,\
> +				[[maybe_unused]] ControlSerializer *cs = nullptr)\
> +	{								\
> +		return deserialize(dataBegin, dataEnd);			\
> +	}								\
> +};
> +
> +DECLARE_POD_SERIALIZER(bool)
> +DECLARE_POD_SERIALIZER(uint8_t)
> +DECLARE_POD_SERIALIZER(uint16_t)
> +DECLARE_POD_SERIALIZER(uint32_t)
> +DECLARE_POD_SERIALIZER(uint64_t)
> +DECLARE_POD_SERIALIZER(int8_t)
> +DECLARE_POD_SERIALIZER(int16_t)
> +DECLARE_POD_SERIALIZER(int32_t)
> +DECLARE_POD_SERIALIZER(int64_t)
> +DECLARE_POD_SERIALIZER(float)
> +DECLARE_POD_SERIALIZER(double)
> +
> +/*
> + * Strings are serialized simply by converting by {string.cbegin(), string.end()}.
> + * The size of the string is recorded by the container (struct, vector, map, or
> + * function parameter serdes).
> + */
> +template<>
> +class IPADataSerializer<std::string>
> +{
> +public:
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +	serialize(const std::string &data, [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		return { { data.cbegin(), data.end() }, {} };
> +	}
> +
> +	static std::string deserialize(std::vector<uint8_t> &data,
> +				       [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		return { data.cbegin(), data.cend() };
> +	}
> +
> +	static std::string deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +				       std::vector<uint8_t>::const_iterator dataEnd,
> +				       [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		return { dataBegin, dataEnd };
> +	}
> +
> +	static std::string deserialize(std::vector<uint8_t> &data,
> +				       [[maybe_unused]] std::vector<int32_t> &fds,
> +				       [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		return { data.cbegin(), data.cend() };
> +	}
> +
> +	static std::string deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +				       std::vector<uint8_t>::const_iterator dataEnd,
> +				       [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin,
> +				       [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> +				       [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		return { dataBegin, dataEnd };
> +	}
> +};
> +
> +/*
> + * FileDescriptors are serialized into a single byte that tells if the
> + * FileDescriptor is valid or not. If it is valid, then for serialization
> + * the fd will be written to the fd vector, or for deserialization the
> + * fd vector const_iterator will be valid.
> + *
> + * This validity is necessary so that we don't send -1 fd over sendmsg(). It
> + * also allows us to simply send the entire fd vector into the deserializer
> + * and it will be recursively consumed as necessary.
> + */
> +template<>
> +class IPADataSerializer<FileDescriptor>
> +{
> +public:
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +	serialize(const FileDescriptor &data, [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		std::vector<uint8_t> dataVec = { data.isValid() };
> +		std::vector<int32_t> fdVec;
> +		if (data.isValid())
> +			fdVec.push_back(data.fd());
> +
> +		return { dataVec, fdVec };
> +	}
> +
> +	static FileDescriptor deserialize(std::vector<uint8_t> &data, std::vector<int32_t> &fds,
> +					  [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end());
> +	}
> +
> +	static FileDescriptor deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +					  std::vector<uint8_t>::const_iterator dataEnd,
> +					  std::vector<int32_t>::const_iterator fdsBegin,
> +					  std::vector<int32_t>::const_iterator fdsEnd,
> +					  [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		ASSERT(std::distance(dataBegin, dataEnd) >= 1);
> +
> +		bool valid = !!(*dataBegin);
> +
> +		ASSERT(!(valid && std::distance(fdsBegin, fdsEnd) < 1));
> +
> +		return valid ? FileDescriptor(*fdsBegin) : FileDescriptor();
> +	}
> +};
> +
> +/*
> + * ControlList is serialized as:
> + *
> + * 4 bytes - uint32_t Size of serialized ControlInfoMap, in bytes
> + * 4 bytes - uint32_t Size of serialized ControlList, in bytes
> + * X bytes - Serialized ControlInfoMap (using ControlSerializer)
> + * X bytes - Serialized ControlList (using ControlSerializer)
> + *
> + * If data.infoMap() is nullptr, then the default controls::controls will
> + * be used. The serialized ControlInfoMap will have zero length.
> + */
> +template<>
> +class IPADataSerializer<ControlList>
> +{
> +public:
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +	serialize(const ControlList &data, ControlSerializer *cs)
> +	{
> +		if (!cs)
> +			LOG(IPADataSerializer, Fatal)
> +				<< "ControlSerializer not provided for serialization of ControlList";
> +
> +		size_t size;
> +		std::vector<uint8_t> infoData;
> +		int ret;
> +
> +		if (data.infoMap() && !cs->isCached(data.infoMap())) {
> +			size = cs->binarySize(*data.infoMap());
> +			infoData.resize(size);
> +			ByteStreamBuffer buffer(infoData.data(), infoData.size());
> +			ret = cs->serialize(*data.infoMap(), buffer);
> +
> +			if (ret < 0 || buffer.overflow()) {
> +				LOG(IPADataSerializer, Error) << "Failed to serialize ControlList's ControlInfoMap";
> +				return { {}, {} };
> +			}
> +		}
> +
> +		size = cs->binarySize(data);
> +		std::vector<uint8_t> listData(size);
> +		ByteStreamBuffer buffer(listData.data(), listData.size());
> +		ret = cs->serialize(data, buffer);
> +
> +		if (ret < 0 || buffer.overflow()) {
> +			LOG(IPADataSerializer, Error) << "Failed to serialize ControlList";
> +			return { {}, {} };
> +		}
> +
> +		std::vector<uint8_t> dataVec;
> +		dataVec.reserve(8 + infoData.size() + listData.size());
> +		appendPOD<uint32_t>(dataVec, infoData.size());
> +		appendPOD<uint32_t>(dataVec, listData.size());
> +		dataVec.insert(dataVec.end(), infoData.begin(), infoData.end());
> +		dataVec.insert(dataVec.end(), listData.begin(), listData.end());
> +
> +		return { dataVec, {} };
> +	}
> +
> +	static ControlList deserialize(std::vector<uint8_t> &data, ControlSerializer *cs)
> +	{
> +		return deserialize(data.cbegin(), data.end(), cs);
> +	}
> +
> +	static ControlList deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +				       std::vector<uint8_t>::const_iterator dataEnd,
> +				       ControlSerializer *cs)
> +	{
> +		if (!cs)
> +			LOG(IPADataSerializer, Fatal)
> +				<< "ControlSerializer not provided for deserialization of ControlList";
> +
> +		if (!std::distance(dataBegin, dataEnd))
> +			return {};
> +
> +		uint32_t infoDataSize = readPOD<uint32_t>(dataBegin, 0, dataEnd);
> +		uint32_t listDataSize = readPOD<uint32_t>(dataBegin, 4, dataEnd);
> +
> +		std::vector<uint8_t>::const_iterator it = dataBegin + 8;
> +
> +		std::vector<uint8_t> infoData(it, it + infoDataSize);
> +		std::vector<uint8_t> listData(it + infoDataSize, it + infoDataSize + listDataSize);
> +
> +		if (infoDataSize > 0) {
> +			ByteStreamBuffer buffer(const_cast<const uint8_t *>(infoData.data()), infoData.size());
> +			ControlInfoMap map = cs->deserialize<ControlInfoMap>(buffer);
> +			/* It's fine if map is empty. */
> +			if (buffer.overflow()) {
> +				LOG(IPADataSerializer, Error)
> +					<< "Failed to deserialize ControlLists's ControlInfoMap: buffer overflow";
> +				return ControlList();
> +			}
> +		}
> +
> +		ByteStreamBuffer buffer(const_cast<const uint8_t *>(listData.data()), listData.size());
> +		ControlList list = cs->deserialize<ControlList>(buffer);
> +		if (buffer.overflow())
> +			LOG(IPADataSerializer, Error) << "Failed to deserialize ControlList: buffer overflow";
> +		if (list.empty())
> +			LOG(IPADataSerializer, Error) << "Failed to deserialize ControlList: empty list";
> +
> +		return list;
> +	}
> +
> +	static ControlList deserialize(std::vector<uint8_t> &data,
> +				       [[maybe_unused]] std::vector<int32_t> &fds,
> +				       ControlSerializer *cs)
> +	{
> +		return deserialize(data.cbegin(), data.end(), cs);
> +	}
> +
> +	static ControlList deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +				       std::vector<uint8_t>::const_iterator dataEnd,
> +				       [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin,
> +				       [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> +				       ControlSerializer *cs)
> +	{
> +		return deserialize(dataBegin, dataEnd, cs);
> +	}
> +};
> +
> +/*
> + * const ControlInfoMap is serialized as:
> + *
> + * 4 bytes - uint32_t Size of serialized ControlInfoMap, in bytes
> + * X bytes - Serialized ControlInfoMap (using ControlSerializer)
> + */
> +template<>
> +class IPADataSerializer<ControlInfoMap>
> +{
> +public:
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +	serialize(const ControlInfoMap &map, ControlSerializer *cs)
> +	{
> +		if (!cs)
> +			LOG(IPADataSerializer, Fatal)
> +				<< "ControlSerializer not provided for serialization of ControlInfoMap";
> +
> +		size_t size = cs->binarySize(map);
> +		std::vector<uint8_t> infoData(size);
> +		ByteStreamBuffer buffer(infoData.data(), infoData.size());
> +		int ret = cs->serialize(map, buffer);
> +
> +		if (ret < 0 || buffer.overflow()) {
> +			LOG(IPADataSerializer, Error) << "Failed to serialize ControlInfoMap";
> +			return { {}, {} };
> +		}
> +
> +		std::vector<uint8_t> dataVec;
> +		appendPOD<uint32_t>(dataVec, infoData.size());
> +		dataVec.insert(dataVec.end(), infoData.begin(), infoData.end());
> +
> +		return { dataVec, {} };
> +	}
> +
> +	static ControlInfoMap deserialize(std::vector<uint8_t> &data,
> +					  ControlSerializer *cs)
> +	{
> +		return deserialize(data.cbegin(), data.end(), cs);
> +	}
> +
> +	static ControlInfoMap deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +					  [[maybe_unused]] std::vector<uint8_t>::const_iterator dataEnd,
> +					  ControlSerializer *cs)
> +	{
> +		if (!cs)
> +			LOG(IPADataSerializer, Fatal)
> +				<< "ControlSerializer not provided for deserialization of ControlInfoMap";
> +
> +		uint32_t infoDataSize = readPOD<uint32_t>(dataBegin, 0, dataEnd);
> +
> +		std::vector<uint8_t>::const_iterator it = dataBegin + 4;
> +
> +		std::vector<uint8_t> infoData(it, it + infoDataSize);
> +
> +		ByteStreamBuffer buffer(const_cast<const uint8_t *>(infoData.data()), infoData.size());
> +		ControlInfoMap map = cs->deserialize<ControlInfoMap>(buffer);
> +
> +		return map;
> +	}
> +
> +	static ControlInfoMap deserialize(std::vector<uint8_t> &data,
> +					  [[maybe_unused]] std::vector<int32_t> &fds,
> +					  ControlSerializer *cs)
> +	{
> +		return deserialize(data.cbegin(), data.end(), cs);
> +	}
> +
> +	static ControlInfoMap deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +					  std::vector<uint8_t>::const_iterator dataEnd,
> +					  [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin,
> +					  [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> +					  ControlSerializer *cs)
> +	{
> +		return deserialize(dataBegin, dataEnd, cs);
> +	}
> +};
> +
> +/*
> + * FrameBuffer::Plane is serialized as:
> + *
> + * 1 byte  - FileDescriptor
> + * 4 bytes - uint32_t Length
> + */
> +template<>
> +class IPADataSerializer<FrameBuffer::Plane>
> +{
> +public:
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +	serialize(const FrameBuffer::Plane &data, [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		std::vector<uint8_t> dataVec;
> +		std::vector<int32_t> fdsVec;
> +
> +		std::vector<uint8_t> fdBuf;
> +		std::vector<int32_t> fdFds;
> +		std::tie(fdBuf, fdFds) =
> +			IPADataSerializer<FileDescriptor>::serialize(data.fd);
> +		dataVec.insert(dataVec.end(), fdBuf.begin(), fdBuf.end());
> +		fdsVec.insert(fdsVec.end(), fdFds.begin(), fdFds.end());
> +
> +		appendPOD<uint32_t>(dataVec, data.length);
> +
> +		return { dataVec, fdsVec };
> +	}
> +
> +	static FrameBuffer::Plane deserialize(std::vector<uint8_t> &data,
> +					      std::vector<int32_t> &fds,
> +					      ControlSerializer *cs = nullptr)
> +	{
> +		return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs);
> +	}
> +
> +	static FrameBuffer::Plane deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +					      [[maybe_unused]] std::vector<uint8_t>::const_iterator dataEnd,
> +					      std::vector<int32_t>::const_iterator fdsBegin,
> +					      [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> +					      [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		FrameBuffer::Plane ret;
> +
> +		ret.fd = IPADataSerializer<FileDescriptor>::deserialize(dataBegin, dataBegin + 1,
> +									fdsBegin, fdsBegin + 1);
> +		ret.length = readPOD<uint32_t>(dataBegin, 1, dataEnd);
> +
> +		return ret;
> +	}
> +};
> +
> +#endif /* __DOXYGEN__ */
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_INTERNAL_IPA_DATA_SERIALIZER_H__ */
> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp
> new file mode 100644
> index 00000000..34287d6a
> --- /dev/null
> +++ b/src/libcamera/ipa_data_serializer.cpp
> @@ -0,0 +1,185 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * ipa_data_serializer.cpp - Image Processing Algorithm data serializer
> + */
> +
> +#include "libcamera/internal/ipa_data_serializer.h"
> +
> +#include "libcamera/internal/log.h"
> +
> +/**
> + * \file ipa_data_serializer.h
> + * \brief IPA Data Serializer
> + */
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(IPADataSerializer)
> +
> +/**
> + * \class IPADataSerializer
> + * \brief IPA Data Serializer
> + *
> + * Static template class that provides functions for serializing and
> + * deserializing IPA data.
> + *
> + * \todo Switch to Span instead of byte and fd vector
> + *
> + * \todo Harden the vector and map deserializer
> + *
> + * \todo For FileDescriptors, instead of storing a validity flag, store an
> + * index into the fd array. This will allow us to use views instead of copying.
> + */
> +
> +namespace {
> +
> +/**
> + * \fn template<typename T> void appendPOD(std::vector<uint8_t> &vec, T val)
> + * \brief Append POD to end of byte vector, in little-endian order
> + * \tparam T Type of POD to append
> + * \param[in] vec Byte vector to append to
> + * \param[in] val Value to append
> + *
> + * This function is meant to be used by the IPA data serializer, and the
> + * generated IPA proxies.
> + */
> +
> +/**
> + * \fn template<typename T> T readPOD(std::vector<uint8_t>::iterator it, size_t pos,
> + * 				      std::vector<uint8_t>::iterator end)
> + * \brief Read POD from byte vector, in little-endian order
> + * \tparam T Type of POD to read
> + * \param[in] it Iterator of byte vector to read from
> + * \param[in] pos Index in byte vector to read from
> + * \param[in] end Iterator marking end of byte vector
> + *
> + * This function is meant to be used by the IPA data serializer, and the
> + * generated IPA proxies.
> + *
> + * If the \a pos plus the byte-width of the desired POD is past \a end, it is
> + * a fata error will occur, as it means there is insufficient data for
> + * deserialization, which should never happen.
> + *
> + * \return The POD read from \a it at index \a pos
> + */
> +
> +/**
> + * \fn template<typename T> T readPOD(std::vector<uint8_t> &vec, size_t pos)
> + * \brief Read POD from byte vector, in little-endian order
> + * \tparam T Type of POD to read
> + * \param[in] vec Byte vector to read from
> + * \param[in] pos Index in vec to start reading from
> + *
> + * This function is meant to be used by the IPA data serializer, and the
> + * generated IPA proxies.
> + *
> + * If the \a pos plus the byte-width of the desired POD is past the end of
> + * \a vec, a fatal error will occur, as it means there is insufficient data
> + * for deserialization, which should never happen.
> + *
> + * \return The POD read from \a vec at index \a pos
> + */
> +
> +} /* namespace */
> +
> +/**
> + * \fn template<typename T> IPADataSerializer<T>::serialize(
> + * 	T data,
> + * 	ControlSerializer *cs = nullptr)
> + * \brief Serialize an object into byte vector and fd vector
> + * \tparam T Type of object to serialize
> + * \param[in] data Object to serialize
> + * \param[in] cs ControlSerializer
> + *
> + * \a cs is only necessary if the object type \a T or its members contain
> + * ControlList or ControlInfoMap.
> + *
> + * \return Tuple of byte vector and fd vector, that is the serialized form
> + * of \a data
> + */
> +
> +/**
> + * \fn template<typename T> IPADataSerializer<T>::deserialize(
> + * 	const std::vector<uint8_t> &data,
> + * 	ControlSerializer *cs = nullptr)
> + * \brief Deserialize byte vector into an object
> + * \tparam T Type of object to deserialize to
> + * \param[in] data Byte vector to deserialize from
> + * \param[in] cs ControlSerializer
> + *
> + * This version of deserialize() can be used if the object type \a T and its
> + * members don't have any FileDescriptor.
> + *
> + * \a cs is only necessary if the object type \a T or its members contain
> + * ControlList or ControlInfoMap.
> + *
> + * \return The deserialized object
> + */
> +
> +/**
> + * \fn template<typename T> IPADataSerializer<T>::deserialize(
> + * 	std::vector<uint8_t>::const_iterator dataBegin,
> + * 	std::vector<uint8_t>::const_iterator dataEnd,
> + * 	ControlSerializer *cs = nullptr)
> + * \brief Deserialize byte vector into an object
> + * \tparam T Type of object to deserialize to
> + * \param[in] dataBegin Begin iterator of byte vector to deserialize from
> + * \param[in] dataEnd End iterator of byte vector to deserialize from
> + * \param[in] cs ControlSerializer
> + *
> + * This version of deserialize() can be used if the object type \a T and its
> + * members don't have any FileDescriptor.
> + *
> + * \a cs is only necessary if the object type \a T or its members contain
> + * ControlList or ControlInfoMap.
> + *
> + * \return The deserialized object
> + */
> +
> +/**
> + * \fn template<typename T> IPADataSerializer<T>::deserialize(
> + * 	const std::vector<uint8_t> &data,
> + * 	const std::vector<int32_t> &fds,
> + * 	ControlSerializer *cs = nullptr)
> + * \brief Deserialize byte vector and fd vector into an object
> + * \tparam T Type of object to deserialize to
> + * \param[in] data Byte vector to deserialize from
> + * \param[in] fds Fd vector to deserialize from
> + * \param[in] cs ControlSerializer
> + *
> + * This version of deserialize() (or the iterator version) must be used if
> + * the object type \a T or its members contain FileDescriptor.
> + *
> + * \a cs is only necessary if the object type \a T or its members contain
> + * ControlList or ControlInfoMap.
> + *
> + * \return The deserialized object
> + */
> +
> +/**
> + * \fn template<typename T> IPADataSerializer::deserialize(
> + * 	std::vector<uint8_t>::const_iterator dataBegin,
> + * 	std::vector<uint8_t>::const_iterator dataEnd,
> + * 	std::vector<int32_t>::const_iterator fdsBegin,
> + * 	std::vector<int32_t>::const_iterator fdsEnd,
> + * 	ControlSerializer *cs = nullptr)
> + * \brief Deserialize byte vector and fd vector into an object
> + * \tparam T Type of object to deserialize to
> + * \param[in] dataBegin Begin iterator of byte vector to deserialize from
> + * \param[in] dataEnd End iterator of byte vector to deserialize from
> + * \param[in] fdsBegin Begin iterator of fd vector to deserialize from
> + * \param[in] fdsEnd End iterator of fd vector to deserialize from
> + * \param[in] cs ControlSerializer
> + *
> + * This version of deserialize() (or the vector version) must be used if
> + * the object type \a T or its members contain FileDescriptor.
> + *
> + * \a cs is only necessary if the object type \a T or its members contain
> + * ControlList or ControlInfoMap.
> + *
> + * \return The deserialized object
> + */
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 387d5d88..1291f1b3 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -25,6 +25,7 @@ libcamera_sources = files([
>      'geometry.cpp',
>      'ipa_context_wrapper.cpp',
>      'ipa_controls.cpp',
> +    'ipa_data_serializer.cpp',
>      'ipa_interface.cpp',
>      'ipa_manager.cpp',
>      'ipa_module.cpp',
> -- 
> 2.27.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Paul,

Thank you for the patch.

On Thu, Dec 24, 2020 at 05:15:31PM +0900, Paul Elder wrote:
> Add an IPADataSerializer which implments de/serialization of built-in

s/implments/implements/

> (PODs, vector, map, string) and libcamera data structures. This is
> intended to be used by the proxy and the proxy worker in the IPC layer.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> ---
> Changes in v6:
> - remove (de)serializers for IPASettings, CameraSensorInfo, IPAStream,
>   and IPABuffer as they are now defined in core.mojom and generated
> - ControlList (de)serializer no longer needs a ControlInfoMap
> - ControlList (de)serializer now checks the ControlSerializer if the
>   ControlList's ControlInfoMap is cached
> - add some todos for optimization and hardening
> 
> Changes in v5:
> - fix style of: {{}, {}} -> { {}, {} }
> - add documentation on serialization formats (not in doxygen, though)
> - compress readPOD
>   - use memcpy
>   - use ASSERT
> - remove ifdef DOXYGEN guard from base IPADataSerializer
> - remove integer overflow risk when calculating vector offsets
> - use const iterators and const references
> - remove const ControlList and const ControlInfoMap specializations
> 
> Changes in v4:
> - rename readUInt/appendUInt to readPOD/appendPOD
>   - put them in anonymous namespace
>   - and add length check
>     - fatal if failure, because it means not enough data to deserialize,
>       which is technically a segfault
>   - use the new readPOD/appendPOD with length protections
>   - expand on their docs correspondingly
> - change snake_case to camelCase
> - add optimizations to the hand-written de/serializers
> - reserve vector length where trivially possible
> - remove unnecessary IPADataSerializer<type>:: explicit calls (if
>   they're calling a specialization from the same specialization)
> 
> Changes in v3:
> - reimplement append/readUInt with memcpy (intead of bit shifting)
> - change DECLARE_INTEGRAL_SERIALIZER with DECLARE_POD_SERIALIZER
>   - use this for int64_t, bool, float, and double
> - fix comment style
> 
> Changes in v2:
> - added serializers for all integer types, bool, and string
> - use string serializer for IPASettings serializer
> - add documentation
> - add serializer for const ControlList
> ---
>  .../libcamera/internal/ipa_data_serializer.h  | 701 ++++++++++++++++++
>  src/libcamera/ipa_data_serializer.cpp         | 185 +++++
>  src/libcamera/meson.build                     |   1 +
>  3 files changed, 887 insertions(+)
>  create mode 100644 include/libcamera/internal/ipa_data_serializer.h
>  create mode 100644 src/libcamera/ipa_data_serializer.cpp
> 
> diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h
> new file mode 100644
> index 00000000..5468a0b7
> --- /dev/null
> +++ b/include/libcamera/internal/ipa_data_serializer.h
> @@ -0,0 +1,701 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * ipa_data_serializer.h - Image Processing Algorithm data serializer
> + */
> +#ifndef __LIBCAMERA_INTERNAL_IPA_DATA_SERIALIZER_H__
> +#define __LIBCAMERA_INTERNAL_IPA_DATA_SERIALIZER_H__
> +
> +#include <deque>
> +#include <iostream>
> +#include <string.h>
> +#include <tuple>
> +#include <type_traits>
> +#include <vector>
> +
> +#include <libcamera/buffer.h>
> +#include <libcamera/control_ids.h>
> +#include <libcamera/geometry.h>
> +#include <libcamera/ipa/ipa_interface.h>
> +
> +#include "libcamera/internal/byte_stream_buffer.h"
> +#include "libcamera/internal/camera_sensor.h"
> +#include "libcamera/internal/control_serializer.h"
> +#include "libcamera/internal/log.h"
> +
> +namespace libcamera {
> +
> +LOG_DECLARE_CATEGORY(IPADataSerializer)
> +
> +namespace {
> +
> +template<typename T>
> +void appendPOD(std::vector<uint8_t> &vec,
> +	       typename std::enable_if<std::is_arithmetic_v<T>, T>::type val)

How about

template<typename T,
	 typename std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>
void appendPOD(std::vector<uint8_t> &vec, T val)

The effect should be the same, but the signature of the function is
easier to read.

> +{
> +	size_t byteWidth = sizeof(val);

	constexpr size_t byteWidth = sizeof(val);

> +	vec.resize(vec.size() + byteWidth);
> +	memcpy(&*(vec.end() - byteWidth), &val, byteWidth);
> +}
> +
> +template<typename T>
> +typename std::enable_if<std::is_arithmetic_v<T>, T>::type
> +readPOD(std::vector<uint8_t>::const_iterator it, size_t pos,
> +	std::vector<uint8_t>::const_iterator end)

Same here,

template<typename T,
	 typename std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>
T readPOD(std::vector<uint8_t>::const_iterator it, size_t pos,
	  std::vector<uint8_t>::const_iterator end)

and for the next function too.

> +{
> +	ASSERT(pos + it < end);
> +
> +	T ret = 0;
> +	memcpy(&ret, &(*(it + pos)), sizeof(ret));
> +
> +	return ret;
> +}
> +
> +template<typename T>
> +typename std::enable_if<std::is_arithmetic_v<T>, T>::type
> +readPOD(std::vector<uint8_t> &vec, size_t pos)
> +{
> +	return readPOD<T>(vec.cbegin(), pos, vec.end());
> +}
> +
> +} /* namespace */
> +
> +template<typename T>
> +class IPADataSerializer
> +{
> +public:
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +	serialize(const T &data, ControlSerializer *cs);
> +
> +	static T deserialize(const std::vector<uint8_t> &data,
> +			     ControlSerializer *cs);
> +	static T deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +			     std::vector<uint8_t>::const_iterator dataEnd,
> +			     ControlSerializer *cs);
> +
> +	static T deserialize(const std::vector<uint8_t> &data,
> +			     const std::vector<int32_t> &fds,
> +			     ControlSerializer *cs);
> +	static T deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +			     std::vector<uint8_t>::const_iterator dataEnd,
> +			     std::vector<int32_t>::const_iterator fdsBegin,
> +			     std::vector<int32_t>::const_iterator fdsEnd,
> +			     ControlSerializer *cs);
> +};
> +
> +#ifndef __DOXYGEN__
> +
> +/*
> + * Serialization format for vector of type V:
> + *
> + * 4 bytes - uint32_t Length of vector, in number of elements
> + *
> + * For every element in the vector:
> + *
> + * 4 bytes - uint32_t Size of element, in bytes
> + * 4 bytes - uint32_t Size of fds for the element, in number of fds

 * 4 bytes - uint32_t Number of fds for the element

> + * X bytes - Serialized element
> + */
> +template<typename V>
> +class IPADataSerializer<std::vector<V>>
> +{
> +public:
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +	serialize(const std::vector<V> &data, ControlSerializer *cs = nullptr)
> +	{
> +		std::vector<uint8_t> dataVec;
> +		std::vector<int32_t> fdsVec;
> +
> +		/* Serialize the length. */
> +		uint32_t vecLen = data.size();
> +		appendPOD<uint32_t>(dataVec, vecLen);
> +
> +		/* Serialize the members. */
> +		for (auto const &it : data) {
> +			std::vector<uint8_t> dvec;
> +			std::vector<int32_t> fvec;
> +
> +			std::tie(dvec, fvec) =
> +				IPADataSerializer<V>::serialize(it, cs);
> +
> +			appendPOD<uint32_t>(dataVec, dvec.size());
> +			appendPOD<uint32_t>(dataVec, fvec.size());
> +
> +			dataVec.insert(dataVec.end(), dvec.begin(), dvec.end());
> +			fdsVec.insert(fdsVec.end(), fvec.begin(), fvec.end());
> +		}
> +
> +		return { dataVec, fdsVec };
> +	}
> +
> +	static std::vector<V> deserialize(std::vector<uint8_t> &data, ControlSerializer *cs = nullptr)
> +	{
> +		return deserialize(data.cbegin(), data.end(), cs);
> +	}
> +
> +	static std::vector<V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +					  std::vector<uint8_t>::const_iterator dataEnd,
> +					  ControlSerializer *cs = nullptr)
> +	{
> +		std::vector<int32_t> fds;
> +		return deserialize(dataBegin, dataEnd, fds.cbegin(), fds.end(), cs);
> +	}
> +
> +	static std::vector<V> deserialize(std::vector<uint8_t> &data, std::vector<int32_t> &fds,
> +					  ControlSerializer *cs = nullptr)
> +	{
> +		return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs);
> +	}
> +
> +	static std::vector<V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +					  std::vector<uint8_t>::const_iterator dataEnd,
> +					  std::vector<int32_t>::const_iterator fdsBegin,
> +					  [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> +					  ControlSerializer *cs = nullptr)
> +	{
> +		uint32_t vecLen = readPOD<uint32_t>(dataBegin, 0, dataEnd);
> +		std::vector<V> ret(vecLen);
> +
> +		std::vector<uint8_t>::const_iterator dataIter = dataBegin + 4;
> +		std::vector<int32_t>::const_iterator fdIter = fdsBegin;
> +		for (uint32_t i = 0; i < vecLen; i++) {
> +			uint32_t sizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd);
> +			uint32_t sizeofFds  = readPOD<uint32_t>(dataIter, 4, dataEnd);
> +			dataIter += 8;
> +
> +			ret[i] = IPADataSerializer<V>::deserialize(dataIter,
> +								   dataIter + sizeofData,
> +								   fdIter,
> +								   fdIter + sizeofFds,
> +								   cs);
> +
> +			dataIter += sizeofData;
> +			fdIter += sizeofFds;
> +		}
> +
> +		return ret;
> +	}
> +};
> +
> +/*
> + * Serialization format for map of key type K and value type V:
> + *
> + * 4 bytes - uint32_t Length of map, in number of pairs
> + *
> + * For every pair in the map:
> + *
> + * 4 bytes - uint32_t Size of key, in bytes
> + * 4 bytes - uint32_t Size of fds for the key, in number of fds

Same as above.

> + * X bytes - Serialized key
> + * 4 bytes - uint32_t Size of value, in bytes
> + * 4 bytes - uint32_t Size of fds for the value, in number of fds

And here too.

> + * X bytes - Serialized value
> + */
> +template<typename K, typename V>
> +class IPADataSerializer<std::map<K, V>>
> +{
> +public:
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +	serialize(const std::map<K, V> &data, ControlSerializer *cs = nullptr)
> +	{
> +		std::vector<uint8_t> dataVec;
> +		std::vector<int32_t> fdsVec;
> +
> +		/* Serialize the length. */
> +		uint32_t mapLen = data.size();
> +		appendPOD<uint32_t>(dataVec, mapLen);
> +
> +		/* Serialize the members. */
> +		for (auto const &it : data) {
> +			std::vector<uint8_t> dvec;
> +			std::vector<int32_t> fvec;
> +
> +			std::tie(dvec, fvec) =
> +				IPADataSerializer<K>::serialize(it.first, cs);
> +
> +			appendPOD<uint32_t>(dataVec, dvec.size());
> +			appendPOD<uint32_t>(dataVec, fvec.size());
> +
> +			dataVec.insert(dataVec.end(), dvec.begin(), dvec.end());
> +			fdsVec.insert(fdsVec.end(), fvec.begin(), fvec.end());
> +
> +			std::tie(dvec, fvec) =
> +				IPADataSerializer<V>::serialize(it.second, cs);
> +
> +			appendPOD<uint32_t>(dataVec, dvec.size());
> +			appendPOD<uint32_t>(dataVec, fvec.size());
> +
> +			dataVec.insert(dataVec.end(), dvec.begin(), dvec.end());
> +			fdsVec.insert(fdsVec.end(), fvec.begin(), fvec.end());
> +		}
> +
> +		return { dataVec, fdsVec };
> +	}
> +
> +	static std::map<K, V> deserialize(std::vector<uint8_t> &data, ControlSerializer *cs = nullptr)
> +	{
> +		return deserialize(data.cbegin(), data.end(), cs);
> +	}
> +
> +	static std::map<K, V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +					  std::vector<uint8_t>::const_iterator dataEnd,
> +					  ControlSerializer *cs = nullptr)
> +	{
> +		std::vector<int32_t> fds;
> +		return deserialize(dataBegin, dataEnd, fds.cbegin(), fds.end(), cs);
> +	}
> +
> +	static std::map<K, V> deserialize(std::vector<uint8_t> &data, std::vector<int32_t> &fds,
> +					  ControlSerializer *cs = nullptr)
> +	{
> +		return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs);
> +	}
> +
> +	static std::map<K, V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +					  std::vector<uint8_t>::const_iterator dataEnd,
> +					  std::vector<int32_t>::const_iterator fdsBegin,
> +					  [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> +					  ControlSerializer *cs = nullptr)
> +	{
> +		std::map<K, V> ret;
> +
> +		uint32_t mapLen = readPOD<uint32_t>(dataBegin, 0, dataEnd);
> +
> +		std::vector<uint8_t>::const_iterator dataIter = dataBegin + 4;
> +		std::vector<int32_t>::const_iterator fdIter = fdsBegin;
> +		for (uint32_t i = 0; i < mapLen; i++) {
> +			uint32_t sizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd);
> +			uint32_t sizeofFds  = readPOD<uint32_t>(dataIter, 4, dataEnd);
> +			dataIter += 8;
> +
> +			K key = IPADataSerializer<K>::deserialize(dataIter,
> +								  dataIter + sizeofData,
> +								  fdIter,
> +								  fdIter + sizeofFds,
> +								  cs);
> +
> +			dataIter += sizeofData;
> +			fdIter += sizeofFds;
> +			sizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd);
> +			sizeofFds  = readPOD<uint32_t>(dataIter, 4, dataEnd);
> +			dataIter += 8;
> +
> +			const V value = IPADataSerializer<V>::deserialize(dataIter,
> +									  dataIter + sizeofData,
> +									  fdIter,
> +									  fdIter + sizeofFds,
> +									  cs);
> +			ret.insert({key, value});
> +
> +			dataIter += sizeofData;
> +			fdIter += sizeofFds;
> +		}
> +
> +		return ret;
> +	}
> +};
> +
> +#define DECLARE_POD_SERIALIZER(type)					\
> +template<>								\
> +class IPADataSerializer<type>						\
> +{									\
> +public:									\
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>	\
> +	serialize(const type data,					\
> +		  [[maybe_unused]] ControlSerializer *cs = nullptr)	\
> +	{								\
> +		std::vector<uint8_t> dataVec;				\
> +		dataVec.reserve(sizeof(type));				\
> +		appendPOD<type>(dataVec, data);			\
> +									\
> +		return { dataVec, {} };					\
> +	}								\
> +									\
> +	static type deserialize(std::vector<uint8_t> &data,		\
> +				[[maybe_unused]] ControlSerializer *cs = nullptr)\
> +	{								\
> +		return deserialize(data.cbegin(), data.end());		\
> +	}								\
> +									\
> +	static type deserialize(std::vector<uint8_t>::const_iterator dataBegin,\
> +				[[maybe_unused]] std::vector<uint8_t>::const_iterator dataEnd,\
> +				[[maybe_unused]] ControlSerializer *cs = nullptr)\
> +	{								\
> +		return readPOD<type>(dataBegin, 0, dataEnd);		\
> +	}								\
> +									\
> +	static type deserialize(std::vector<uint8_t> &data,		\
> +				[[maybe_unused]] std::vector<int32_t> &fds,\
> +				[[maybe_unused]] ControlSerializer *cs = nullptr)\
> +	{								\
> +		return deserialize(data.cbegin(), data.end());		\
> +	}								\
> +									\
> +	static type deserialize(std::vector<uint8_t>::const_iterator dataBegin,\
> +				std::vector<uint8_t>::const_iterator dataEnd,\
> +				[[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin,\
> +				[[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,\
> +				[[maybe_unused]] ControlSerializer *cs = nullptr)\
> +	{								\
> +		return deserialize(dataBegin, dataEnd);			\
> +	}								\
> +};
> +
> +DECLARE_POD_SERIALIZER(bool)
> +DECLARE_POD_SERIALIZER(uint8_t)
> +DECLARE_POD_SERIALIZER(uint16_t)
> +DECLARE_POD_SERIALIZER(uint32_t)
> +DECLARE_POD_SERIALIZER(uint64_t)
> +DECLARE_POD_SERIALIZER(int8_t)
> +DECLARE_POD_SERIALIZER(int16_t)
> +DECLARE_POD_SERIALIZER(int32_t)
> +DECLARE_POD_SERIALIZER(int64_t)
> +DECLARE_POD_SERIALIZER(float)
> +DECLARE_POD_SERIALIZER(double)
> +
> +/*
> + * Strings are serialized simply by converting by {string.cbegin(), string.end()}.
> + * The size of the string is recorded by the container (struct, vector, map, or
> + * function parameter serdes).
> + */
> +template<>
> +class IPADataSerializer<std::string>
> +{
> +public:
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +	serialize(const std::string &data, [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		return { { data.cbegin(), data.end() }, {} };
> +	}
> +
> +	static std::string deserialize(std::vector<uint8_t> &data,
> +				       [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		return { data.cbegin(), data.cend() };
> +	}
> +
> +	static std::string deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +				       std::vector<uint8_t>::const_iterator dataEnd,
> +				       [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		return { dataBegin, dataEnd };
> +	}
> +
> +	static std::string deserialize(std::vector<uint8_t> &data,
> +				       [[maybe_unused]] std::vector<int32_t> &fds,
> +				       [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		return { data.cbegin(), data.cend() };
> +	}
> +
> +	static std::string deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +				       std::vector<uint8_t>::const_iterator dataEnd,
> +				       [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin,
> +				       [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> +				       [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		return { dataBegin, dataEnd };
> +	}
> +};
> +
> +/*
> + * FileDescriptors are serialized into a single byte that tells if the
> + * FileDescriptor is valid or not. If it is valid, then for serialization
> + * the fd will be written to the fd vector, or for deserialization the
> + * fd vector const_iterator will be valid.
> + *
> + * This validity is necessary so that we don't send -1 fd over sendmsg(). It
> + * also allows us to simply send the entire fd vector into the deserializer
> + * and it will be recursively consumed as necessary.
> + */
> +template<>
> +class IPADataSerializer<FileDescriptor>
> +{
> +public:
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +	serialize(const FileDescriptor &data, [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		std::vector<uint8_t> dataVec = { data.isValid() };
> +		std::vector<int32_t> fdVec;
> +		if (data.isValid())
> +			fdVec.push_back(data.fd());
> +
> +		return { dataVec, fdVec };
> +	}
> +
> +	static FileDescriptor deserialize(std::vector<uint8_t> &data, std::vector<int32_t> &fds,
> +					  [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end());
> +	}
> +
> +	static FileDescriptor deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +					  std::vector<uint8_t>::const_iterator dataEnd,
> +					  std::vector<int32_t>::const_iterator fdsBegin,
> +					  std::vector<int32_t>::const_iterator fdsEnd,
> +					  [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		ASSERT(std::distance(dataBegin, dataEnd) >= 1);
> +
> +		bool valid = !!(*dataBegin);
> +
> +		ASSERT(!(valid && std::distance(fdsBegin, fdsEnd) < 1));
> +
> +		return valid ? FileDescriptor(*fdsBegin) : FileDescriptor();
> +	}
> +};
> +
> +/*
> + * ControlList is serialized as:
> + *
> + * 4 bytes - uint32_t Size of serialized ControlInfoMap, in bytes
> + * 4 bytes - uint32_t Size of serialized ControlList, in bytes
> + * X bytes - Serialized ControlInfoMap (using ControlSerializer)
> + * X bytes - Serialized ControlList (using ControlSerializer)
> + *
> + * If data.infoMap() is nullptr, then the default controls::controls will
> + * be used. The serialized ControlInfoMap will have zero length.
> + */
> +template<>
> +class IPADataSerializer<ControlList>
> +{
> +public:
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +	serialize(const ControlList &data, ControlSerializer *cs)
> +	{
> +		if (!cs)
> +			LOG(IPADataSerializer, Fatal)
> +				<< "ControlSerializer not provided for serialization of ControlList";
> +
> +		size_t size;
> +		std::vector<uint8_t> infoData;
> +		int ret;
> +

This opportunistic serialization of the control info map still makes me
worry about the implementation being fragile, but maybe I just worry too
much. A todo comment to mention this should be revisited could be nice,
but I'm not sure what a good rationale would be ("This worries Laurent"
isn't a good rationale :-)).

> +		if (data.infoMap() && !cs->isCached(data.infoMap())) {
> +			size = cs->binarySize(*data.infoMap());
> +			infoData.resize(size);
> +			ByteStreamBuffer buffer(infoData.data(), infoData.size());
> +			ret = cs->serialize(*data.infoMap(), buffer);
> +
> +			if (ret < 0 || buffer.overflow()) {
> +				LOG(IPADataSerializer, Error) << "Failed to serialize ControlList's ControlInfoMap";
> +				return { {}, {} };
> +			}
> +		}
> +
> +		size = cs->binarySize(data);
> +		std::vector<uint8_t> listData(size);
> +		ByteStreamBuffer buffer(listData.data(), listData.size());
> +		ret = cs->serialize(data, buffer);
> +
> +		if (ret < 0 || buffer.overflow()) {
> +			LOG(IPADataSerializer, Error) << "Failed to serialize ControlList";
> +			return { {}, {} };
> +		}
> +
> +		std::vector<uint8_t> dataVec;
> +		dataVec.reserve(8 + infoData.size() + listData.size());
> +		appendPOD<uint32_t>(dataVec, infoData.size());
> +		appendPOD<uint32_t>(dataVec, listData.size());
> +		dataVec.insert(dataVec.end(), infoData.begin(), infoData.end());
> +		dataVec.insert(dataVec.end(), listData.begin(), listData.end());
> +
> +		return { dataVec, {} };
> +	}
> +
> +	static ControlList deserialize(std::vector<uint8_t> &data, ControlSerializer *cs)
> +	{
> +		return deserialize(data.cbegin(), data.end(), cs);
> +	}
> +
> +	static ControlList deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +				       std::vector<uint8_t>::const_iterator dataEnd,
> +				       ControlSerializer *cs)
> +	{
> +		if (!cs)
> +			LOG(IPADataSerializer, Fatal)
> +				<< "ControlSerializer not provided for deserialization of ControlList";
> +
> +		if (!std::distance(dataBegin, dataEnd))

		if (std::distance(dataBegin, dataEnd) < 8)

> +			return {};
> +
> +		uint32_t infoDataSize = readPOD<uint32_t>(dataBegin, 0, dataEnd);
> +		uint32_t listDataSize = readPOD<uint32_t>(dataBegin, 4, dataEnd);
> +
> +		std::vector<uint8_t>::const_iterator it = dataBegin + 8;

		if (infoDataSize + listDataSize < infoDataSize ||
		    std::distance(it, dataEnd) < infoDataSize + listDataSize)
			return {};

> +
> +		std::vector<uint8_t> infoData(it, it + infoDataSize);
> +		std::vector<uint8_t> listData(it + infoDataSize, it + infoDataSize + listDataSize);
> +
> +		if (infoDataSize > 0) {
> +			ByteStreamBuffer buffer(const_cast<const uint8_t *>(infoData.data()), infoData.size());

Do you need the const cast ? Passing a non-const pointer to a function
expecting a const shouldn't be an issue.

			ByteStreamBuffer buffer(&*it, infoDataSize);

and you can drop infoData.


> +			ControlInfoMap map = cs->deserialize<ControlInfoMap>(buffer);
> +			/* It's fine if map is empty. */
> +			if (buffer.overflow()) {
> +				LOG(IPADataSerializer, Error)
> +					<< "Failed to deserialize ControlLists's ControlInfoMap: buffer overflow";
> +				return ControlList();
> +			}
> +		}
> +
> +		ByteStreamBuffer buffer(const_cast<const uint8_t *>(listData.data()), listData.size());

Same here,

		it += infoDataSize;
		ByteStreamBuffer buffer(&*it, listDataSize);

and you can drop listData.

> +		ControlList list = cs->deserialize<ControlList>(buffer);
> +		if (buffer.overflow())
> +			LOG(IPADataSerializer, Error) << "Failed to deserialize ControlList: buffer overflow";
> +		if (list.empty())
> +			LOG(IPADataSerializer, Error) << "Failed to deserialize ControlList: empty list";
> +
> +		return list;
> +	}
> +
> +	static ControlList deserialize(std::vector<uint8_t> &data,
> +				       [[maybe_unused]] std::vector<int32_t> &fds,
> +				       ControlSerializer *cs)
> +	{
> +		return deserialize(data.cbegin(), data.end(), cs);
> +	}
> +
> +	static ControlList deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +				       std::vector<uint8_t>::const_iterator dataEnd,
> +				       [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin,
> +				       [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> +				       ControlSerializer *cs)
> +	{
> +		return deserialize(dataBegin, dataEnd, cs);
> +	}
> +};
> +
> +/*
> + * const ControlInfoMap is serialized as:
> + *
> + * 4 bytes - uint32_t Size of serialized ControlInfoMap, in bytes
> + * X bytes - Serialized ControlInfoMap (using ControlSerializer)
> + */
> +template<>
> +class IPADataSerializer<ControlInfoMap>
> +{
> +public:
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +	serialize(const ControlInfoMap &map, ControlSerializer *cs)
> +	{
> +		if (!cs)
> +			LOG(IPADataSerializer, Fatal)
> +				<< "ControlSerializer not provided for serialization of ControlInfoMap";
> +
> +		size_t size = cs->binarySize(map);
> +		std::vector<uint8_t> infoData(size);
> +		ByteStreamBuffer buffer(infoData.data(), infoData.size());
> +		int ret = cs->serialize(map, buffer);
> +
> +		if (ret < 0 || buffer.overflow()) {
> +			LOG(IPADataSerializer, Error) << "Failed to serialize ControlInfoMap";
> +			return { {}, {} };
> +		}
> +
> +		std::vector<uint8_t> dataVec;
> +		appendPOD<uint32_t>(dataVec, infoData.size());
> +		dataVec.insert(dataVec.end(), infoData.begin(), infoData.end());
> +
> +		return { dataVec, {} };
> +	}
> +
> +	static ControlInfoMap deserialize(std::vector<uint8_t> &data,
> +					  ControlSerializer *cs)
> +	{
> +		return deserialize(data.cbegin(), data.end(), cs);
> +	}
> +
> +	static ControlInfoMap deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +					  [[maybe_unused]] std::vector<uint8_t>::const_iterator dataEnd,
> +					  ControlSerializer *cs)
> +	{
> +		if (!cs)
> +			LOG(IPADataSerializer, Fatal)
> +				<< "ControlSerializer not provided for deserialization of ControlInfoMap";
> +

Missing overflows check.

> +		uint32_t infoDataSize = readPOD<uint32_t>(dataBegin, 0, dataEnd);
> +
> +		std::vector<uint8_t>::const_iterator it = dataBegin + 4;
> +
> +		std::vector<uint8_t> infoData(it, it + infoDataSize);
> +
> +		ByteStreamBuffer buffer(const_cast<const uint8_t *>(infoData.data()), infoData.size());

Same comments as above.

> +		ControlInfoMap map = cs->deserialize<ControlInfoMap>(buffer);
> +
> +		return map;
> +	}
> +
> +	static ControlInfoMap deserialize(std::vector<uint8_t> &data,
> +					  [[maybe_unused]] std::vector<int32_t> &fds,
> +					  ControlSerializer *cs)
> +	{
> +		return deserialize(data.cbegin(), data.end(), cs);
> +	}
> +
> +	static ControlInfoMap deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +					  std::vector<uint8_t>::const_iterator dataEnd,
> +					  [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin,
> +					  [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> +					  ControlSerializer *cs)
> +	{
> +		return deserialize(dataBegin, dataEnd, cs);
> +	}
> +};
> +
> +/*
> + * FrameBuffer::Plane is serialized as:
> + *
> + * 1 byte  - FileDescriptor
> + * 4 bytes - uint32_t Length
> + */
> +template<>
> +class IPADataSerializer<FrameBuffer::Plane>
> +{
> +public:
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +	serialize(const FrameBuffer::Plane &data, [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		std::vector<uint8_t> dataVec;
> +		std::vector<int32_t> fdsVec;
> +
> +		std::vector<uint8_t> fdBuf;
> +		std::vector<int32_t> fdFds;
> +		std::tie(fdBuf, fdFds) =
> +			IPADataSerializer<FileDescriptor>::serialize(data.fd);
> +		dataVec.insert(dataVec.end(), fdBuf.begin(), fdBuf.end());
> +		fdsVec.insert(fdsVec.end(), fdFds.begin(), fdFds.end());
> +
> +		appendPOD<uint32_t>(dataVec, data.length);
> +
> +		return { dataVec, fdsVec };
> +	}
> +
> +	static FrameBuffer::Plane deserialize(std::vector<uint8_t> &data,
> +					      std::vector<int32_t> &fds,
> +					      ControlSerializer *cs = nullptr)
> +	{
> +		return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs);
> +	}
> +
> +	static FrameBuffer::Plane deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +					      [[maybe_unused]] std::vector<uint8_t>::const_iterator dataEnd,

dataEnd is used.

> +					      std::vector<int32_t>::const_iterator fdsBegin,
> +					      [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
> +					      [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		FrameBuffer::Plane ret;
> +
> +		ret.fd = IPADataSerializer<FileDescriptor>::deserialize(dataBegin, dataBegin + 1,
> +									fdsBegin, fdsBegin + 1);
> +		ret.length = readPOD<uint32_t>(dataBegin, 1, dataEnd);
> +
> +		return ret;
> +	}
> +};
> +
> +#endif /* __DOXYGEN__ */
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_INTERNAL_IPA_DATA_SERIALIZER_H__ */
> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp
> new file mode 100644
> index 00000000..34287d6a
> --- /dev/null
> +++ b/src/libcamera/ipa_data_serializer.cpp
> @@ -0,0 +1,185 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * ipa_data_serializer.cpp - Image Processing Algorithm data serializer
> + */
> +
> +#include "libcamera/internal/ipa_data_serializer.h"
> +
> +#include "libcamera/internal/log.h"
> +
> +/**
> + * \file ipa_data_serializer.h
> + * \brief IPA Data Serializer
> + */
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(IPADataSerializer)
> +
> +/**
> + * \class IPADataSerializer
> + * \brief IPA Data Serializer
> + *
> + * Static template class that provides functions for serializing and
> + * deserializing IPA data.
> + *
> + * \todo Switch to Span instead of byte and fd vector
> + *
> + * \todo Harden the vector and map deserializer
> + *
> + * \todo For FileDescriptors, instead of storing a validity flag, store an
> + * index into the fd array. This will allow us to use views instead of copying.
> + */
> +
> +namespace {
> +
> +/**
> + * \fn template<typename T> void appendPOD(std::vector<uint8_t> &vec, T val)
> + * \brief Append POD to end of byte vector, in little-endian order
> + * \tparam T Type of POD to append
> + * \param[in] vec Byte vector to append to
> + * \param[in] val Value to append
> + *
> + * This function is meant to be used by the IPA data serializer, and the
> + * generated IPA proxies.
> + */
> +
> +/**
> + * \fn template<typename T> T readPOD(std::vector<uint8_t>::iterator it, size_t pos,
> + * 				      std::vector<uint8_t>::iterator end)
> + * \brief Read POD from byte vector, in little-endian order
> + * \tparam T Type of POD to read
> + * \param[in] it Iterator of byte vector to read from
> + * \param[in] pos Index in byte vector to read from
> + * \param[in] end Iterator marking end of byte vector
> + *
> + * This function is meant to be used by the IPA data serializer, and the
> + * generated IPA proxies.
> + *
> + * If the \a pos plus the byte-width of the desired POD is past \a end, it is
> + * a fata error will occur, as it means there is insufficient data for
> + * deserialization, which should never happen.
> + *
> + * \return The POD read from \a it at index \a pos
> + */
> +
> +/**
> + * \fn template<typename T> T readPOD(std::vector<uint8_t> &vec, size_t pos)
> + * \brief Read POD from byte vector, in little-endian order
> + * \tparam T Type of POD to read
> + * \param[in] vec Byte vector to read from
> + * \param[in] pos Index in vec to start reading from
> + *
> + * This function is meant to be used by the IPA data serializer, and the
> + * generated IPA proxies.
> + *
> + * If the \a pos plus the byte-width of the desired POD is past the end of
> + * \a vec, a fatal error will occur, as it means there is insufficient data
> + * for deserialization, which should never happen.
> + *
> + * \return The POD read from \a vec at index \a pos
> + */
> +
> +} /* namespace */
> +
> +/**
> + * \fn template<typename T> IPADataSerializer<T>::serialize(
> + * 	T data,
> + * 	ControlSerializer *cs = nullptr)
> + * \brief Serialize an object into byte vector and fd vector
> + * \tparam T Type of object to serialize
> + * \param[in] data Object to serialize
> + * \param[in] cs ControlSerializer
> + *
> + * \a cs is only necessary if the object type \a T or its members contain
> + * ControlList or ControlInfoMap.
> + *
> + * \return Tuple of byte vector and fd vector, that is the serialized form
> + * of \a data
> + */
> +
> +/**
> + * \fn template<typename T> IPADataSerializer<T>::deserialize(
> + * 	const std::vector<uint8_t> &data,
> + * 	ControlSerializer *cs = nullptr)
> + * \brief Deserialize byte vector into an object
> + * \tparam T Type of object to deserialize to
> + * \param[in] data Byte vector to deserialize from
> + * \param[in] cs ControlSerializer
> + *
> + * This version of deserialize() can be used if the object type \a T and its
> + * members don't have any FileDescriptor.
> + *
> + * \a cs is only necessary if the object type \a T or its members contain
> + * ControlList or ControlInfoMap.
> + *
> + * \return The deserialized object
> + */
> +
> +/**
> + * \fn template<typename T> IPADataSerializer<T>::deserialize(
> + * 	std::vector<uint8_t>::const_iterator dataBegin,
> + * 	std::vector<uint8_t>::const_iterator dataEnd,
> + * 	ControlSerializer *cs = nullptr)
> + * \brief Deserialize byte vector into an object
> + * \tparam T Type of object to deserialize to
> + * \param[in] dataBegin Begin iterator of byte vector to deserialize from
> + * \param[in] dataEnd End iterator of byte vector to deserialize from
> + * \param[in] cs ControlSerializer
> + *
> + * This version of deserialize() can be used if the object type \a T and its
> + * members don't have any FileDescriptor.
> + *
> + * \a cs is only necessary if the object type \a T or its members contain
> + * ControlList or ControlInfoMap.
> + *
> + * \return The deserialized object
> + */
> +
> +/**
> + * \fn template<typename T> IPADataSerializer<T>::deserialize(
> + * 	const std::vector<uint8_t> &data,
> + * 	const std::vector<int32_t> &fds,
> + * 	ControlSerializer *cs = nullptr)
> + * \brief Deserialize byte vector and fd vector into an object
> + * \tparam T Type of object to deserialize to
> + * \param[in] data Byte vector to deserialize from
> + * \param[in] fds Fd vector to deserialize from
> + * \param[in] cs ControlSerializer
> + *
> + * This version of deserialize() (or the iterator version) must be used if
> + * the object type \a T or its members contain FileDescriptor.
> + *
> + * \a cs is only necessary if the object type \a T or its members contain
> + * ControlList or ControlInfoMap.
> + *
> + * \return The deserialized object
> + */
> +
> +/**
> + * \fn template<typename T> IPADataSerializer::deserialize(
> + * 	std::vector<uint8_t>::const_iterator dataBegin,
> + * 	std::vector<uint8_t>::const_iterator dataEnd,
> + * 	std::vector<int32_t>::const_iterator fdsBegin,
> + * 	std::vector<int32_t>::const_iterator fdsEnd,
> + * 	ControlSerializer *cs = nullptr)
> + * \brief Deserialize byte vector and fd vector into an object
> + * \tparam T Type of object to deserialize to
> + * \param[in] dataBegin Begin iterator of byte vector to deserialize from
> + * \param[in] dataEnd End iterator of byte vector to deserialize from
> + * \param[in] fdsBegin Begin iterator of fd vector to deserialize from
> + * \param[in] fdsEnd End iterator of fd vector to deserialize from
> + * \param[in] cs ControlSerializer
> + *
> + * This version of deserialize() (or the vector version) must be used if
> + * the object type \a T or its members contain FileDescriptor.
> + *
> + * \a cs is only necessary if the object type \a T or its members contain
> + * ControlList or ControlInfoMap.
> + *
> + * \return The deserialized object
> + */

How about squashing this patch in ?

diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h
index bde5eaf7e42c..7f29205235e8 100644
--- a/include/libcamera/internal/ipa_data_serializer.h
+++ b/include/libcamera/internal/ipa_data_serializer.h
@@ -66,22 +66,22 @@ class IPADataSerializer
 {
 public:
 	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
-	serialize(const T &data, ControlSerializer *cs);
+	serialize(const T &data, ControlSerializer *cs = nullptr);

 	static T deserialize(const std::vector<uint8_t> &data,
-			     ControlSerializer *cs);
+			     ControlSerializer *cs = nullptr);
 	static T deserialize(std::vector<uint8_t>::const_iterator dataBegin,
 			     std::vector<uint8_t>::const_iterator dataEnd,
-			     ControlSerializer *cs);
+			     ControlSerializer *cs = nullptr);

 	static T deserialize(const std::vector<uint8_t> &data,
 			     const std::vector<int32_t> &fds,
-			     ControlSerializer *cs);
+			     ControlSerializer *cs = nullptr);
 	static T deserialize(std::vector<uint8_t>::const_iterator dataBegin,
 			     std::vector<uint8_t>::const_iterator dataEnd,
 			     std::vector<int32_t>::const_iterator fdsBegin,
 			     std::vector<int32_t>::const_iterator fdsEnd,
-			     ControlSerializer *cs);
+			     ControlSerializer *cs = nullptr);
 };

 #ifndef __DOXYGEN__
@@ -296,64 +296,6 @@ public:
 	}
 };

-#define DECLARE_POD_SERIALIZER(type)					\
-template<>								\
-class IPADataSerializer<type>						\
-{									\
-public:									\
-	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>	\
-	serialize(const type data,					\
-		  [[maybe_unused]] ControlSerializer *cs = nullptr)	\
-	{								\
-		std::vector<uint8_t> dataVec;				\
-		dataVec.reserve(sizeof(type));				\
-		appendPOD<type>(dataVec, data);			\
-									\
-		return { dataVec, {} };					\
-	}								\
-									\
-	static type deserialize(std::vector<uint8_t> &data,		\
-				[[maybe_unused]] ControlSerializer *cs = nullptr)\
-	{								\
-		return deserialize(data.cbegin(), data.end());		\
-	}								\
-									\
-	static type deserialize(std::vector<uint8_t>::const_iterator dataBegin,\
-				[[maybe_unused]] std::vector<uint8_t>::const_iterator dataEnd,\
-				[[maybe_unused]] ControlSerializer *cs = nullptr)\
-	{								\
-		return readPOD<type>(dataBegin, 0, dataEnd);		\
-	}								\
-									\
-	static type deserialize(std::vector<uint8_t> &data,		\
-				[[maybe_unused]] std::vector<int32_t> &fds,\
-				[[maybe_unused]] ControlSerializer *cs = nullptr)\
-	{								\
-		return deserialize(data.cbegin(), data.end());		\
-	}								\
-									\
-	static type deserialize(std::vector<uint8_t>::const_iterator dataBegin,\
-				std::vector<uint8_t>::const_iterator dataEnd,\
-				[[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin,\
-				[[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,\
-				[[maybe_unused]] ControlSerializer *cs = nullptr)\
-	{								\
-		return deserialize(dataBegin, dataEnd);			\
-	}								\
-};
-
-DECLARE_POD_SERIALIZER(bool)
-DECLARE_POD_SERIALIZER(uint8_t)
-DECLARE_POD_SERIALIZER(uint16_t)
-DECLARE_POD_SERIALIZER(uint32_t)
-DECLARE_POD_SERIALIZER(uint64_t)
-DECLARE_POD_SERIALIZER(int8_t)
-DECLARE_POD_SERIALIZER(int16_t)
-DECLARE_POD_SERIALIZER(int32_t)
-DECLARE_POD_SERIALIZER(int64_t)
-DECLARE_POD_SERIALIZER(float)
-DECLARE_POD_SERIALIZER(double)
-
 /*
  * Strings are serialized simply by converting by {string.cbegin(), string.end()}.
  * The size of the string is recorded by the container (struct, vector, map, or
diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp
index 34287d6a2601..6d98a3acfbc6 100644
--- a/src/libcamera/ipa_data_serializer.cpp
+++ b/src/libcamera/ipa_data_serializer.cpp
@@ -182,4 +182,62 @@ namespace {
  * \return The deserialized object
  */

+#define DECLARE_POD_SERIALIZER(type)					\
+template<>								\
+class IPADataSerializer<type>						\
+{									\
+public:									\
+	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>	\
+	serialize(const type data,					\
+		  [[maybe_unused]] ControlSerializer *cs = nullptr)	\
+	{								\
+		std::vector<uint8_t> dataVec;				\
+		dataVec.reserve(sizeof(type));				\
+		appendPOD<type>(dataVec, data);			\
+									\
+		return { dataVec, {} };					\
+	}								\
+									\
+	static type deserialize(std::vector<uint8_t> &data,		\
+				[[maybe_unused]] ControlSerializer *cs = nullptr)\
+	{								\
+		return deserialize(data.cbegin(), data.end());		\
+	}								\
+									\
+	static type deserialize(std::vector<uint8_t>::const_iterator dataBegin,\
+				[[maybe_unused]] std::vector<uint8_t>::const_iterator dataEnd,\
+				[[maybe_unused]] ControlSerializer *cs = nullptr)\
+	{								\
+		return readPOD<type>(dataBegin, 0, dataEnd);		\
+	}								\
+									\
+	static type deserialize(std::vector<uint8_t> &data,		\
+				[[maybe_unused]] std::vector<int32_t> &fds,\
+				[[maybe_unused]] ControlSerializer *cs = nullptr)\
+	{								\
+		return deserialize(data.cbegin(), data.end());		\
+	}								\
+									\
+	static type deserialize(std::vector<uint8_t>::const_iterator dataBegin,\
+				std::vector<uint8_t>::const_iterator dataEnd,\
+				[[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin,\
+				[[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,\
+				[[maybe_unused]] ControlSerializer *cs = nullptr)\
+	{								\
+		return deserialize(dataBegin, dataEnd);			\
+	}								\
+};
+
+DECLARE_POD_SERIALIZER(bool)
+DECLARE_POD_SERIALIZER(uint8_t)
+DECLARE_POD_SERIALIZER(uint16_t)
+DECLARE_POD_SERIALIZER(uint32_t)
+DECLARE_POD_SERIALIZER(uint64_t)
+DECLARE_POD_SERIALIZER(int8_t)
+DECLARE_POD_SERIALIZER(int16_t)
+DECLARE_POD_SERIALIZER(int32_t)
+DECLARE_POD_SERIALIZER(int64_t)
+DECLARE_POD_SERIALIZER(float)
+DECLARE_POD_SERIALIZER(double)
+
 } /* namespace libcamera */


Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 387d5d88..1291f1b3 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -25,6 +25,7 @@ libcamera_sources = files([
>      'geometry.cpp',
>      'ipa_context_wrapper.cpp',
>      'ipa_controls.cpp',
> +    'ipa_data_serializer.cpp',
>      'ipa_interface.cpp',
>      'ipa_manager.cpp',
>      'ipa_module.cpp',