[libcamera-devel] [PATCH v7 06/10] libcamera: Add IPADataSerializer
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Feb 12 02:50:33 CET 2021
Hi Paul,
Thank you for the patch.
On Thu, Feb 11, 2021 at 04:18:01PM +0900, Paul Elder wrote:
> Add an IPADataSerializer which implements (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 at ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Acked-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
>
> ---
> Changes in v7:
> - move DEFINE_POD_SERIALIZER from header to cpp
> - make readPOD and appendPOD more readable
> - add some overflow checks
> - drop unnecessary const casts
> - add todo for supporting vectors/maps of references
> - remove error message when ControlList is deserialized but it's empty
> - it's fine to send empty ControlLists
>
> 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 | 660 ++++++++++++++++++
> src/libcamera/ipa_data_serializer.cpp | 244 +++++++
> src/libcamera/meson.build | 1 +
> 3 files changed, 905 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..ebc32a9e
> --- /dev/null
> +++ b/include/libcamera/internal/ipa_data_serializer.h
> @@ -0,0 +1,660 @@
> +/* 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,
> + typename std::enable_if<std::is_arithmetic_v<T>> * = nullptr>
Shouldn't this be std::enable_if_t ? Same below.
> +void appendPOD(std::vector<uint8_t> &vec, T val)
> +{
> + constexpr size_t byteWidth = sizeof(val);
> + vec.resize(vec.size() + byteWidth);
> + memcpy(&*(vec.end() - byteWidth), &val, byteWidth);
> +}
> +
> +template<typename T,
> + std::enable_if<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)
> +{
> + ASSERT(pos + it < end);
> +
> + T ret = 0;
> + memcpy(&ret, &(*(it + pos)), sizeof(ret));
> +
> + return ret;
> +}
> +
> +template<typename T,
> + std::enable_if<std::is_arithmetic_v<T>> * = nullptr>
> +T 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 = nullptr);
> +
> + static T deserialize(const std::vector<uint8_t> &data,
> + ControlSerializer *cs = nullptr);
> + static T deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> + std::vector<uint8_t>::const_iterator dataEnd,
> + ControlSerializer *cs = nullptr);
> +
> + static T deserialize(const std::vector<uint8_t> &data,
> + const std::vector<int32_t> &fds,
> + 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 = nullptr);
> +};
> +
> +#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 Number of fds for the element
> + * X bytes - Serialized element
> + *
> + * \todo Support elements that are references
> + */
> +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 Number of fds for the key
> + * X bytes - Serialized key
> + * 4 bytes - uint32_t Size of value, in bytes
> + * 4 bytes - uint32_t Number of fds for the value
> + * X bytes - Serialized value
> + *
> + * \todo Support keys or values that are references
> + */
> +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;
> + }
> +};
> +
All the specializations below should move to the .cpp file. Here one
example for the std::string specialization, the other ones should follow
the same pattern.
diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h
index ebc32a9e298c..cadd237cf7b0 100644
--- a/include/libcamera/internal/ipa_data_serializer.h
+++ b/include/libcamera/internal/ipa_data_serializer.h
@@ -300,51 +300,6 @@ public:
}
};
-/*
- * 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
diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp
index e20682b94e15..61887a7c4a06 100644
--- a/src/libcamera/ipa_data_serializer.cpp
+++ b/src/libcamera/ipa_data_serializer.cpp
@@ -182,6 +182,8 @@ namespace {
* \return The deserialized object
*/
+#ifndef __DOXYGEN__
+
#define DEFINE_POD_SERIALIZER(type) \
\
template<> \
@@ -241,4 +243,56 @@ DEFINE_POD_SERIALIZER(int64_t)
DEFINE_POD_SERIALIZER(float)
DEFINE_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<>
+std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
+IPADataSerializer<std::string>::serialize(const std::string &data,
+ [[maybe_unused]] ControlSerializer *cs)
+{
+ return { { data.cbegin(), data.end() }, {} };
+}
+
+template<>
+std::string
+IPADataSerializer<std::string>::deserialize(const std::vector<uint8_t> &data,
+ [[maybe_unused]] ControlSerializer *cs)
+{
+ return { data.cbegin(), data.cend() };
+}
+
+template<>
+std::string
+IPADataSerializer<std::string>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,
+ std::vector<uint8_t>::const_iterator dataEnd,
+ [[maybe_unused]] ControlSerializer *cs)
+{
+ return { dataBegin, dataEnd };
+}
+
+template<>
+std::string
+IPADataSerializer<std::string>::deserialize(const std::vector<uint8_t> &data,
+ [[maybe_unused]] const std::vector<int32_t> &fds,
+ [[maybe_unused]] ControlSerializer *cs)
+{
+ return { data.cbegin(), data.cend() };
+}
+
+template<>
+std::string
+IPADataSerializer<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)
+{
+ return { dataBegin, dataEnd };
+}
+
+#endif /* __DOXYGEN__ */
+
} /* namespace libcamera */
> +/*
> + * 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;
> +
> + /*
> + * \todo Revisit this opportunistic serialization of the
> + * ControlInfoMap, as it could be fragile
> + */
> + 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) < 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 ||
> + static_cast<uint32_t>(std::abs(std::distance(it, dataEnd))) < infoDataSize + listDataSize)
it can't be larger than dataEnd given the check above, so there's no
need for std::abs(). And if it could be larger, then this would be
wrong, as it would result in the error being silently ignored. Were you
trying to silence a compiler warning ? We want libcamera to build
without warning, but not at the cost of introducing errors :-)
Interestingly, the signed comparison warning is only produced by builds
on 32-bit platforms. The static_cast seems the right solution, but
std::abs isn't needed.
This comment applies to other locations below too.
> + return {};
> +
> + std::vector<uint8_t> infoData(it, it + infoDataSize);
> + std::vector<uint8_t> listData(it + infoDataSize, it + infoDataSize + listDataSize);
> +
> + if (infoDataSize > 0) {
> + ByteStreamBuffer buffer(&*it, infoData.size());
infoData is only used for its .size(), which is equal to infoDataSize.
This comment applies to other locations below too.
> + 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();
> + }
> + }
> +
> + it += infoDataSize;
> + ByteStreamBuffer buffer(&*it, listData.size());
Same here.
> + ControlList list = cs->deserialize<ControlList>(buffer);
> + if (buffer.overflow())
> + LOG(IPADataSerializer, Error) << "Failed to deserialize ControlList: buffer overflow";
> +
> + 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,
> + std::vector<uint8_t>::const_iterator dataEnd,
> + ControlSerializer *cs)
> + {
> + if (!cs)
> + LOG(IPADataSerializer, Fatal)
> + << "ControlSerializer not provided for deserialization of ControlInfoMap";
> +
> + if (std::distance(dataBegin, dataEnd) < 4)
> + return {};
> +
> + uint32_t infoDataSize = readPOD<uint32_t>(dataBegin, 0, dataEnd);
> +
> + std::vector<uint8_t>::const_iterator it = dataBegin + 4;
> +
> + if (static_cast<uint32_t>(std::abs(std::distance(it, dataEnd))) < infoDataSize)
> + return {};
> +
> + std::vector<uint8_t> infoData(it, it + infoDataSize);
> +
> + ByteStreamBuffer buffer(&*it, 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,
> + 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..e20682b9
> --- /dev/null
> +++ b/src/libcamera/ipa_data_serializer.cpp
> @@ -0,0 +1,244 @@
> +/* 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
> + */
> +
> +#define DEFINE_POD_SERIALIZER(type) \
> + \
> +template<> \
> +std::tuple<std::vector<uint8_t>, std::vector<int32_t>> \
> +IPADataSerializer<type>::serialize(const type &data, \
> + [[maybe_unused]] ControlSerializer *cs) \
> +{ \
> + std::vector<uint8_t> dataVec; \
> + dataVec.reserve(sizeof(type)); \
> + appendPOD<type>(dataVec, data); \
> + \
> + return { dataVec, {} }; \
> +} \
> + \
> +template<> \
> +type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, \
> + std::vector<uint8_t>::const_iterator dataEnd, \
> + [[maybe_unused]] ControlSerializer *cs) \
> +{ \
> + return readPOD<type>(dataBegin, 0, dataEnd); \
> +} \
> + \
> +template<> \
> +type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \
> + ControlSerializer *cs) \
> +{ \
> + return deserialize(data.cbegin(), data.end(), cs); \
> +} \
> + \
> +template<> \
> +type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \
> + [[maybe_unused]] const std::vector<int32_t> &fds, \
> + ControlSerializer *cs) \
> +{ \
> + return deserialize(data.cbegin(), data.end(), cs); \
> +} \
> + \
> +template<> \
> +type IPADataSerializer<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, \
> + ControlSerializer *cs) \
> +{ \
> + return deserialize(dataBegin, dataEnd, cs); \
> +}
> +
> +DEFINE_POD_SERIALIZER(bool)
> +DEFINE_POD_SERIALIZER(uint8_t)
> +DEFINE_POD_SERIALIZER(uint16_t)
> +DEFINE_POD_SERIALIZER(uint32_t)
> +DEFINE_POD_SERIALIZER(uint64_t)
> +DEFINE_POD_SERIALIZER(int8_t)
> +DEFINE_POD_SERIALIZER(int16_t)
> +DEFINE_POD_SERIALIZER(int32_t)
> +DEFINE_POD_SERIALIZER(int64_t)
> +DEFINE_POD_SERIALIZER(float)
> +DEFINE_POD_SERIALIZER(double)
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 5a4bf0d7..7d8ea00a 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -26,6 +26,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',
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list