[libcamera-devel] [PATCH v4 08/37] libcamera: Add IPADataSerializer
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Nov 26 17:28:12 CET 2020
Hi Paul,
On Thu, Nov 26, 2020 at 06:49:35PM +0900, paul.elder at ideasonboard.com wrote:
> On Fri, Nov 20, 2020 at 10:43:43AM +0200, Laurent Pinchart wrote:
> > On Fri, Nov 06, 2020 at 07:36:38PM +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 at ideasonboard.com>
> > >
> > > ---
> > > 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 | 1004 +++++++++++++++++
> > > src/libcamera/ipa_data_serializer.cpp | 178 +++
> > > src/libcamera/meson.build | 1 +
> > > 3 files changed, 1183 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..9a18f914
> > > --- /dev/null
> > > +++ b/include/libcamera/internal/ipa_data_serializer.h
> > > @@ -0,0 +1,1004 @@
> > > +/* 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 <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, T val)
> >
> > An std::enable_if<std::is_arithmetic_v<T>> would help ensuring there's
> > no misuse of this template.
>
> Ah okay.
>
> > > +{
> > > + size_t byteWidth = sizeof(val);
> > > + std::vector<uint8_t> v(byteWidth);
> > > + memcpy(&v[0], &val, byteWidth);
> > > +
> > > + vec.insert(vec.end(), v.begin(), v.end());
> >
> > vec.resize(vec.size() + byteWidth);
> > memcpy(vec.end() - byteWidth, &val, byteWidth);
> >
> > If we want to be efficient I think we'll need to implement our own
> > container instead of using std::vector<>, in order to minimize the
> > number of reallocations.
> >
> > In general I think the serialization and deserialization code have room
> > for optimization, which is completely expected. Perfection can wait for
> > a follow-up series ;-) I'd like to focus on the interface with the
> > pipeline handlers and the base IPC mechanism as the target to get this
> > series merged. The internal implementation can then be reworked later.
> >
> > > +}
> > > +
> > > +template<typename T>
> > > +T readPOD(std::vector<uint8_t> &vec, size_t pos)
> > > +{
> > > + T ret = 0;
> > > + size_t byteWidth = sizeof(ret);
> > > + if (pos + byteWidth > vec.size())
> >
> > There's a risk of an integer overflow if the incoming message has a size
> > set to UINT_MAX for instance. This check will pass, and you'll write
> > data to vec[-1] below.
>
> I've since removed this and it takes the iterators and calls the
> iterator version instead, so that should solve the integer overflow
> issue.
>
> > > + LOG(IPADataSerializer, Fatal)
> > > + << "Not enough data to deserialize POD";
> >
> > If you want to make this fatal, then there should be checks in the
> > (generated) caller code to handle the issue gracefully. We don't want to
> > crash because of a malformed message.
>
> So the caller has protections against trying to read more data than
> exists, either because that's what the message said, or because the
> message got truncated (eg. in the generated code):
>
> if (dataSize < 4) {
> LOG(IPADataSerializer, Error)
> << "Failed to deserialize " << "staggeredWriteResultSize"
> << ": not enough data, expected "
> << (4) << ", got " << (dataSize);
> return ret;
> }
> const size_t staggeredWriteResultSize = readPOD<uint32_t>(m, 0, dataEnd);
> m += 4;
> dataSize -= 4;
>
> So if readPOD really receives not enough data, then something is
> /really/ wrong. Or is the caller's protection sufficient and we can
> remove the Fatal from readPOD?
With guaranteed protection in the caller, we would at least turn this
into an ASSERT().
> > > +
> > > + memcpy(&ret, &vec[pos], byteWidth);
> > > + return ret;
> > > +}
> > > +
> > > +template<typename T>
> > > +T readPOD(std::vector<uint8_t>::iterator it, size_t pos,
> > > + std::vector<uint8_t>::iterator end)
> > > +{
> > > + T ret = 0;
> > > + size_t byteWidth = sizeof(ret);
> > > +
> > > + if (pos + it >= end)
> > > + LOG(IPADataSerializer, Fatal)
> > > + << "Not enough data to deserialize POD";
> > > +
> > > + std::vector<uint8_t> v(it + pos, it + pos + byteWidth);
> > > + memcpy(&ret, v.data(), byteWidth);
> > > + return ret;
> > > +}
> >
> > Do we need both functions ?
>
> No, I made the vector one call the iterator one.
>
> > As commented in the review of 04/37, using a Span<const uint8_t> would
> > likely be best for deserialization.
> >
> > > +
> > > +} /* namespace */
> > > +
> > > +template<typename T>
> > > +class IPADataSerializer
> > > +{
> > > +#ifdef __DOXYGEN__
> > > +public:
> > > + static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> > > + serialize(T data, ControlSerializer *cs);
> >
> > s/T/const T &/
> >
> > > +
> > > + static T deserialize(std::vector<uint8_t> &data,
> >
> > const. Same everywhere below.
> >
> > > + ControlSerializer *cs);
> > > + static T deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + std::vector<uint8_t>::iterator dataEnd,
> > > + ControlSerializer *cs);
> > > +
> > > + static T deserialize(std::vector<uint8_t> &data,
> > > + std::vector<int32_t> &fds,
> > > + ControlSerializer *cs);
> > > + static T deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + std::vector<uint8_t>::iterator dataEnd,
> > > + std::vector<int32_t>::iterator fdsBegin,
> > > + std::vector<int32_t>::iterator fdsEnd,
> > > + ControlSerializer *cs);
> >
> > If you switch to span, I think all these could be merged into a single
> > function.
> >
> > static T deserialize(Span<const uint8_t> data,
> > Span<const int32_t> fds,
> > ControlSerializer *cs);
> >
> > This can be called with
> >
> > deserialize<T>(data, {}, cs);
> >
> > when no fds is available.
>
> Yeah, that would be nice...
>
> > > +#endif /* __DOXYGEN__ */
> > > +};
> > > +
> > > +#ifndef __DOXYGEN__
> > > +
> > > +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.begin(), data.end(), cs);
> > > + }
> > > +
> > > + static std::vector<V> deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + std::vector<uint8_t>::iterator dataEnd,
> > > + ControlSerializer *cs = nullptr)
> > > + {
> > > + std::vector<int32_t> fds;
> > > + return deserialize(dataBegin, dataEnd, fds.begin(), fds.end(), cs);
> > > + }
> > > +
> > > + static std::vector<V> deserialize(std::vector<uint8_t> &data, std::vector<int32_t> &fds,
> > > + ControlSerializer *cs = nullptr)
> > > + {
> > > + return deserialize(data.begin(), data.end(), fds.begin(), fds.end(), cs);
> > > + }
> > > +
> > > + static std::vector<V> deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + [[maybe_unused]] std::vector<uint8_t>::iterator dataEnd,
> >
> > dataEnd is used.
> >
> > > + std::vector<int32_t>::iterator fdsBegin,
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsEnd,
> > > + ControlSerializer *cs = nullptr)
> > > + {
> > > + uint32_t vecLen = readPOD<uint32_t>(dataBegin, 0, dataEnd);
> > > + std::vector<V> ret(vecLen);
> > > +
> > > + std::vector<uint8_t>::iterator dataIter = dataBegin + 4;
> > > + std::vector<int32_t>::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);
> > > +
> > > + ret[i] = IPADataSerializer<V>::deserialize(dataIter + 8,
> > > + dataIter + 8 + sizeofData,
> > > + fdIter,
> > > + fdIter + sizeofFds,
> >
> > This could go past fdsEnd.
>
> Oh shoot yeah...
>
> > The deserializer code will need to be hardened, which will likely
> > involve a refactoring. I think it can be merged with a few issues, we'll
> > fix them on top.
>
> For now I could just jam in protection checks like what we have in the
> generated code. I guess at this point the generated code is more
> hardened haha...
Let's stop writing code manually then. We only need a code generator
that is generated from a ... hmmm... :-)
> > These two comments apply to the code below as well.
> >
> > > + cs);
> > > +
> > > + dataIter += 8 + sizeofData;
> > > + fdIter += sizeofFds;
> > > + }
> > > +
> > > + return ret;
> > > + }
> > > +};
> > > +
> > > +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.begin(), data.end(), cs);
> > > + }
> > > +
> > > + static std::map<K, V> deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + std::vector<uint8_t>::iterator dataEnd,
> > > + ControlSerializer *cs = nullptr)
> > > + {
> > > + std::vector<int32_t> fds;
> > > + return deserialize(dataBegin, dataEnd, fds.begin(), 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.begin(), data.end(), fds.begin(), fds.end(), cs);
> > > + }
> > > +
> > > + static std::map<K, V> deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + [[maybe_unused]] std::vector<uint8_t>::iterator dataEnd,
> > > + std::vector<int32_t>::iterator fdsBegin,
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsEnd,
> > > + ControlSerializer *cs = nullptr)
> > > + {
> > > + std::map<K, V> ret;
> > > +
> > > + uint32_t mapLen = readPOD<uint32_t>(dataBegin, 0, dataEnd);
> > > +
> > > + std::vector<uint8_t>::iterator dataIter = dataBegin + 4;
> > > + std::vector<int32_t>::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);
> > > +
> > > + K key = IPADataSerializer<K>::deserialize(dataIter + 8,
> > > + dataIter + 8 + sizeofData,
> > > + fdIter,
> > > + fdIter + sizeofFds,
> > > + cs);
> > > +
> > > + dataIter += 8 + sizeofData;
> > > + fdIter += sizeofFds;
> > > + sizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd);
> > > + sizeofFds = readPOD<uint32_t>(dataIter, 4, dataEnd);
> > > +
> > > + const V value = IPADataSerializer<V>::deserialize(dataIter + 8,
> > > + dataIter + 8 + sizeofData,
> > > + fdIter,
> > > + fdIter + sizeofFds,
> > > + cs);
> > > + ret.insert({key, value});
> > > +
> > > + dataIter += 8 + sizeofData;
> > > + fdIter += sizeofFds;
> > > + }
> > > +
> > > + return ret;
> > > + }
> > > +};
> >
> > All the specializations below can go to a .cpp file.
>
> They can? I thought then they won't be visible to anybody else?
>
> Indeed the compiler complains that the the specialized serializers and
> the proxy workers can't find the appropriate specialization.
Here's what I meant:
diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h
index 574d8f445983..467b964f6d93 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(T data, ControlSerializer *cs);
+ serialize(T data, ControlSerializer *cs = nullptr);
static T deserialize(std::vector<uint8_t> &data,
- ControlSerializer *cs);
+ ControlSerializer *cs = nullptr);
static T deserialize(std::vector<uint8_t>::iterator dataBegin,
std::vector<uint8_t>::iterator dataEnd,
- ControlSerializer *cs);
+ ControlSerializer *cs = nullptr);
static T deserialize(std::vector<uint8_t> &data,
std::vector<int32_t> &fds,
- ControlSerializer *cs);
+ ControlSerializer *cs = nullptr);
static T deserialize(std::vector<uint8_t>::iterator dataBegin,
std::vector<uint8_t>::iterator dataEnd,
std::vector<int32_t>::iterator fdsBegin,
std::vector<int32_t>::iterator fdsEnd,
- ControlSerializer *cs);
+ ControlSerializer *cs = nullptr);
};
#ifndef __DOXYGEN__
@@ -293,64 +293,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.begin(), data.end()); \
- } \
- \
- static type deserialize(std::vector<uint8_t>::iterator dataBegin,\
- [[maybe_unused]] std::vector<uint8_t>::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.begin(), data.end()); \
- } \
- \
- static type deserialize(std::vector<uint8_t>::iterator dataBegin,\
- std::vector<uint8_t>::iterator dataEnd,\
- [[maybe_unused]] std::vector<int32_t>::iterator fdsBegin,\
- [[maybe_unused]] std::vector<int32_t>::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.begin(), 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 7a228e1470df..53ddfcfa80ba 100644
--- a/src/libcamera/ipa_data_serializer.cpp
+++ b/src/libcamera/ipa_data_serializer.cpp
@@ -175,4 +175,62 @@ namespace {
* \return The deserialized object
*/
+#define DECLARE_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>::iterator dataBegin,\
+ [[maybe_unused]] std::vector<uint8_t>::iterator dataEnd,\
+ [[maybe_unused]] ControlSerializer *cs)\
+{ \
+ return readPOD<type>(dataBegin, 0, dataEnd); \
+} \
+ \
+template<> \
+type IPADataSerializer<type>::deserialize(std::vector<uint8_t> &data, \
+ [[maybe_unused]] ControlSerializer *cs)\
+{ \
+ return deserialize(data.begin(), data.end()); \
+} \
+ \
+template<> \
+type IPADataSerializer<type>::deserialize(std::vector<uint8_t> &data, \
+ [[maybe_unused]] std::vector<int32_t> &fds,\
+ [[maybe_unused]] ControlSerializer *cs)\
+{ \
+ return deserialize(data.begin(), data.end()); \
+} \
+ \
+template<> \
+type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::iterator dataBegin,\
+ std::vector<uint8_t>::iterator dataEnd,\
+ [[maybe_unused]] std::vector<int32_t>::iterator fdsBegin,\
+ [[maybe_unused]] std::vector<int32_t>::iterator fdsEnd,\
+ [[maybe_unused]] ControlSerializer *cs)\
+{ \
+ 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 */
Same for the other specilizations. Only specializations that have
template arguments (map and vector) need to be inlined. All the other
ones can go to the .cpp file, and will cause the compiler to generate a
single implementation, instead of inlining them.
> > > +
> > > +#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.begin(), data.end()); \
> > > + } \
> > > + \
> > > + static type deserialize(std::vector<uint8_t>::iterator dataBegin,\
> > > + [[maybe_unused]] std::vector<uint8_t>::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.begin(), data.end()); \
> > > + } \
> > > + \
> > > + static type deserialize(std::vector<uint8_t>::iterator dataBegin,\
> > > + std::vector<uint8_t>::iterator dataEnd,\
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsBegin,\
> > > + [[maybe_unused]] std::vector<int32_t>::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)
> > > +
> > > +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.begin(), data.end()}, {}};
> >
> > Shouldn't we prefix strings by their length, like we do for vectors,
> > instead of relying on the terminating 0 ?
>
> We actually don't rely on the terminating zero. All
> containers of strings (function parameter, struct, vector/map)
> already record the length, and pass the full vector or iterator pair to
> the deserializer, so this is actually sufficient.
>
> I do the same trick with vectors and maps, actually. Notice they don't
> contain their length in bytes, but actually their length in number of
> elements. This is for the same reason; every container (function
> parameter, struct, vector/map) of vectors/maps already contains the length
> in bytes.
>
> > > + }
> > > +
> > > + static std::string deserialize(std::vector<uint8_t> &data,
> > > + [[maybe_unused]] ControlSerializer *cs = nullptr)
> > > + {
> > > + return {data.begin(), data.end()};
> > > + }
> > > +
> > > + static std::string deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + std::vector<uint8_t>::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.begin(), data.end()};
> > > + }
> > > +
> > > + static std::string deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + std::vector<uint8_t>::iterator dataEnd,
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsBegin,
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsEnd,
> > > + [[maybe_unused]] ControlSerializer *cs = nullptr)
> > > + {
> > > + return {dataBegin, dataEnd};
> > > + }
> > > +};
> > > +
> > > +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() };
> >
> > I think you should store an int32_t here, set to -1 when the fd is
> > invalid. Ideally it should contain the index of the entry in the fds
>
> I don't see how that's better than the scheme that I have here now...
The idea, with storing an int32_t, is that deserialization could then be
done using views instead of copying data. The serialized buffer would be
updated in-place to replace the fd in the data vector with the actual
value, and then view classes would allow accessing data directly in the
data vector. That's an optimization for later.
> > vector for valid fds, but that will require a refactoring of the
> > serialization code to pass the data and fds vectors to the serialization
> > functions. It can thus be done later.
> >
> > > + 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.begin(), data.end(), fds.begin(), fds.end());
> > > + }
> > > +
> > > + static FileDescriptor deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + std::vector<uint8_t>::iterator dataEnd,
> > > + std::vector<int32_t>::iterator fdsBegin,
> > > + std::vector<int32_t>::iterator fdsEnd,
> > > + [[maybe_unused]] ControlSerializer *cs = nullptr)
> > > + {
> > > + if (std::distance(dataBegin, dataEnd) < 1)
> > > + LOG(IPADataSerializer, Fatal)
> > > + << "Invalid data to deserialize FileDescriptor";
> > > +
> > > + bool valid = !!(*dataBegin);
> > > +
> > > + if (valid && std::distance(fdsBegin, fdsEnd) < 1)
> > > + LOG(IPADataSerializer, Fatal)
> > > + << "Invalid fds to deserialize FileDescriptor";
> > > +
> > > + return valid ? FileDescriptor(*fdsBegin) : FileDescriptor();
> > > + }
> > > +};
> > > +
> > > +template<>
> > > +class IPADataSerializer<IPASettings>
> > > +{
> > > +public:
> > > + static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> > > + serialize(const IPASettings &data, [[maybe_unused]] ControlSerializer *cs = nullptr)
> > > + {
> > > + return IPADataSerializer<std::string>::serialize(data.configurationFile);
> > > + }
> > > +
> > > + static IPASettings deserialize(std::vector<uint8_t> &data,
> > > + [[maybe_unused]] ControlSerializer *cs = nullptr)
> > > + {
> > > + return { IPADataSerializer<std::string>::deserialize(data.begin(), data.end()) };
> > > + }
> > > +
> > > + static IPASettings deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + std::vector<uint8_t>::iterator dataEnd,
> > > + [[maybe_unused]] ControlSerializer *cs = nullptr)
> > > + {
> > > + return { IPADataSerializer<std::string>::deserialize(dataBegin, dataEnd) };
> > > + }
> > > +
> > > + static IPASettings deserialize(std::vector<uint8_t> &data,
> > > + [[maybe_unused]] std::vector<int32_t> &fds,
> > > + [[maybe_unused]] ControlSerializer *cs = nullptr)
> > > + {
> > > + return { IPADataSerializer<std::string>::deserialize(data.begin(), data.end()) };
> > > + }
> > > +
> > > + static IPASettings deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + std::vector<uint8_t>::iterator dataEnd,
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsBegin,
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsEnd,
> > > + [[maybe_unused]] ControlSerializer *cs = nullptr)
> > > + {
> > > + return { IPADataSerializer<std::string>::deserialize(dataBegin, dataEnd) };
> > > + }
> > > +};
> >
> > Would it make sense to define the IPASettings structure in a .mojom file
> > to avoid manually-written serialization code ? Same for IPAStream and
> > IPABuffer.
>
> For all the structs defined in ipa_interface.h that have no member
> functions, yes. It would mean another header would have to be included
> by generated serializers and proxy workers, but they're generated anyway :)
>
> I'd have to add a serializer for Size, and we'd have to keep
> the FrameBuffer::Plane serializer, though.
A serializer for Size (and likely later for other geometry classes)
would make sense.
> Hm I'll have to shuffle things around in meson though... I'll put it in
> the todo list.
Feel free to give it a go whenever you'll have "free" time ;-)
> > > +
> > > +template<>
> > > +class IPADataSerializer<CameraSensorInfo>
> > > +{
> > > +public:
> > > + static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> > > + serialize(const CameraSensorInfo &data, [[maybe_unused]] ControlSerializer *cs = nullptr)
> > > + {
> > > + std::vector<uint8_t> dataVec;
> > > +
> > > + uint32_t strLen = data.model.size();
> > > + appendPOD<uint32_t>(dataVec, strLen);
> > > +
> > > + dataVec.insert(dataVec.end(), data.model.begin(), data.model.end());
> > > +
> > > + appendPOD<uint32_t>(dataVec, data.bitsPerPixel);
> > > +
> > > + appendPOD<uint32_t>(dataVec, data.activeAreaSize.width);
> > > + appendPOD<uint32_t>(dataVec, data.activeAreaSize.height);
> > > +
> > > + appendPOD<uint32_t>(dataVec, static_cast<uint32_t>(data.analogCrop.x));
> > > + appendPOD<uint32_t>(dataVec, static_cast<uint32_t>(data.analogCrop.y));
> > > + appendPOD<uint32_t>(dataVec, data.analogCrop.width);
> > > + appendPOD<uint32_t>(dataVec, data.analogCrop.height);
> > > +
> > > + appendPOD<uint32_t>(dataVec, data.outputSize.width);
> > > + appendPOD<uint32_t>(dataVec, data.outputSize.height);
> > > +
> > > + appendPOD<uint64_t>(dataVec, data.pixelRate);
> > > +
> > > + appendPOD<uint32_t>(dataVec, data.lineLength);
> > > +
> > > + return {dataVec, {}};
> > > + }
> > > +
> > > + static CameraSensorInfo deserialize(std::vector<uint8_t> &data,
> > > + [[maybe_unused]] ControlSerializer *cs = nullptr)
> > > + {
> > > + return deserialize(data.begin(), data.end());
> > > + }
> > > +
> > > + static CameraSensorInfo deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + [[maybe_unused]] std::vector<uint8_t>::iterator dataEnd,
> > > + [[maybe_unused]] ControlSerializer *cs = nullptr)
> > > + {
> > > + CameraSensorInfo ret;
> > > +
> > > + uint32_t strLen = readPOD<uint32_t>(dataBegin, 0, dataEnd);
> > > + std::string str(dataBegin + 4, dataBegin + 4 + strLen);
> > > + ret.model = str;
> > > +
> > > + std::vector<uint8_t>::iterator it = dataBegin + 4 + strLen;
> > > +
> > > + ret.bitsPerPixel = readPOD<uint32_t>(it, 0, dataEnd);
> > > +
> > > + ret.activeAreaSize.width = readPOD<uint32_t>(it, 4, dataEnd);
> > > + ret.activeAreaSize.height = readPOD<uint32_t>(it, 8, dataEnd);
> > > +
> > > + ret.analogCrop.x = static_cast<int32_t>(readPOD<uint32_t>(it, 12, dataEnd));
> > > + ret.analogCrop.y = static_cast<int32_t>(readPOD<uint32_t>(it, 16, dataEnd));
> > > + ret.analogCrop.width = readPOD<uint32_t>(it, 20, dataEnd);
> > > + ret.analogCrop.height = readPOD<uint32_t>(it, 24, dataEnd);
> > > +
> > > + ret.outputSize.width = readPOD<uint32_t>(it, 28, dataEnd);
> > > + ret.outputSize.height = readPOD<uint32_t>(it, 32, dataEnd);
> > > +
> > > + ret.pixelRate = readPOD<uint64_t>(it, 36, dataEnd);
> > > +
> > > + ret.lineLength = readPOD<uint32_t>(it, 44, dataEnd);
> > > +
> > > + return ret;
> > > + }
> > > +
> > > + static CameraSensorInfo deserialize(std::vector<uint8_t> &data,
> > > + [[maybe_unused]] std::vector<int32_t> &fds,
> > > + [[maybe_unused]] ControlSerializer *cs = nullptr)
> > > + {
> > > + return deserialize(data.begin(), data.end());
> > > + }
> > > +
> > > + static CameraSensorInfo deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + std::vector<uint8_t>::iterator dataEnd,
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsBegin,
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsEnd,
> > > + [[maybe_unused]] ControlSerializer *cs = nullptr)
> > > + {
> > > + return deserialize(dataBegin, dataEnd);
> > > + }
> > > +};
> > > +
> > > +template<>
> > > +class IPADataSerializer<IPAStream>
> > > +{
> > > +public:
> > > + static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> > > + serialize(const IPAStream &data, [[maybe_unused]] ControlSerializer *cs = nullptr)
> > > + {
> > > + std::vector<uint8_t> dataVec;
> > > +
> > > + appendPOD<uint32_t>(dataVec, data.pixelFormat);
> > > +
> > > + appendPOD<uint32_t>(dataVec, data.size.width);
> > > + appendPOD<uint32_t>(dataVec, data.size.height);
> > > +
> > > + return {dataVec, {}};
> > > + }
> > > +
> > > + static IPAStream deserialize(std::vector<uint8_t> &data,
> > > + [[maybe_unused]] ControlSerializer *cs = nullptr)
> > > + {
> > > + return deserialize(data.begin(), data.end());
> > > + }
> > > +
> > > + static IPAStream deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + [[maybe_unused]] std::vector<uint8_t>::iterator dataEnd,
> > > + [[maybe_unused]] ControlSerializer *cs = nullptr)
> > > + {
> > > + IPAStream ret;
> > > +
> > > + ret.pixelFormat = readPOD<uint32_t>(dataBegin, 0, dataEnd);
> > > +
> > > + ret.size.width = readPOD<uint32_t>(dataBegin, 4, dataEnd);
> > > + ret.size.height = readPOD<uint32_t>(dataBegin, 8, dataEnd);
> > > +
> > > + return ret;
> > > + }
> > > +
> > > + static IPAStream deserialize(std::vector<uint8_t> &data,
> > > + [[maybe_unused]] std::vector<int32_t> &fds,
> > > + [[maybe_unused]] ControlSerializer *cs = nullptr)
> > > + {
> > > + return deserialize(data.begin(), data.end());
> > > + }
> > > +
> > > + static IPAStream deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + std::vector<uint8_t>::iterator dataEnd,
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsBegin,
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsEnd,
> > > + [[maybe_unused]] ControlSerializer *cs = nullptr)
> > > + {
> > > + return deserialize(dataBegin, dataEnd);
> > > + }
> > > +};
> > > +
> > > +template<>
> > > +class IPADataSerializer<const ControlList>
> > > +{
> > > +public:
> > > + /* map arg will be generated, since it's per-pipeline anyway. */
> >
> > This means we will serialize the ControlInfoMap every single time we
> > serialize a ControlList. That's very inefficient. The ControlInfoMap
> > instances are supposed to be serialized and transmitted upfront, at
> > configure() time, and then reused.
>
> Yeah...
>
> I'll put this in the todo list as we discussed.
If \todo had a priority argument, it should be high for this one. As
commented today in replies to other patches in the series, I'd really
like to avoid the global ControlInfoMap instances, so this code may need
to be reworked sooner than expected.
> > > + static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> > > + serialize(const ControlList &data, const ControlInfoMap &map,
> > > + ControlSerializer *cs)
> > > + {
> > > + if (!cs)
> > > + LOG(IPADataSerializer, Fatal)
> > > + << "ControlSerializer not provided for serialization of ControlList";
> > > +
> > > + 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 ControlList's ControlInfoMap";
> > > + return {{}, {}};
> > > + }
> > > +
> > > + size = cs->binarySize(data);
> > > + std::vector<uint8_t> listData(size);
> > > + buffer = ByteStreamBuffer(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 const ControlList deserialize(std::vector<uint8_t> &data, ControlSerializer *cs)
> > > + {
> > > + return deserialize(data.begin(), data.end(), cs);
> > > + }
> > > +
> > > + static const ControlList deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + [[maybe_unused]] std::vector<uint8_t>::iterator dataEnd,
> > > + ControlSerializer *cs)
> > > + {
> > > + if (!cs)
> > > + LOG(IPADataSerializer, Fatal)
> > > + << "ControlSerializer not provided for deserialization of ControlList";
> > > +
> > > + uint32_t infoDataSize = readPOD<uint32_t>(dataBegin, 0, dataEnd);
> > > + uint32_t listDataSize = readPOD<uint32_t>(dataBegin, 4, dataEnd);
> > > +
> > > + std::vector<uint8_t>::iterator it = dataBegin + 8;
> > > +
> > > + std::vector<uint8_t> infoData(it, it + infoDataSize);
> > > + std::vector<uint8_t> listData(it + infoDataSize, it + infoDataSize + listDataSize);
> > > +
> > > + 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();
> > > + }
> > > +
> > > + buffer = ByteStreamBuffer(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 const ControlList deserialize(std::vector<uint8_t> &data,
> > > + [[maybe_unused]] std::vector<int32_t> &fds,
> > > + ControlSerializer *cs)
> > > + {
> > > + return deserialize(data.begin(), data.end(), cs);
> > > + }
> > > +
> > > + static const ControlList deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + std::vector<uint8_t>::iterator dataEnd,
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsBegin,
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsEnd,
> > > + ControlSerializer *cs)
> > > + {
> > > + return deserialize(dataBegin, dataEnd, cs);
> > > + }
> > > +};
> > > +
> > > +template<>
> > > +class IPADataSerializer<ControlList>
> >
> > Requiring two specializations, one for const ControlList and one for
> > ControlList, is a sign that something is wrong somewhere. Same for
> > ControlInfoMap below.
> >
> > > +{
> > > +public:
> > > + static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> > > + serialize(const ControlList &data, const ControlInfoMap &map,
> > > + ControlSerializer *cs)
> > > + {
> > > + const ControlList &list_const = const_cast<const ControlList &>(data);
> > > +
> > > + std::vector<uint8_t> dataVec;
> > > + std::tie(dataVec, std::ignore) =
> > > + IPADataSerializer<const ControlList>::serialize(list_const, map, cs);
> > > +
> > > + return {dataVec, {}};
> > > + }
> > > +
> > > + static ControlList deserialize(std::vector<uint8_t> &data,
> > > + ControlSerializer *cs)
> > > + {
> > > + ControlList ret;
> > > + const ControlList &list = ret;
> > > + const_cast<ControlList &>(list) =
> > > + IPADataSerializer<const ControlList>::deserialize(data, cs);
> > > +
> > > + return ret;
> > > + }
> > > +
> > > + static ControlList deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + [[maybe_unused]] std::vector<uint8_t>::iterator dataEnd,
> > > + ControlSerializer *cs)
> > > + {
> > > + ControlList ret;
> > > + const ControlList &list = ret;
> > > + const_cast<ControlList &>(list) =
> > > + IPADataSerializer<const ControlList>::deserialize(dataBegin, dataEnd, cs);
> > > +
> > > + return ret;
> > > + }
> > > +
> > > + static ControlList deserialize(std::vector<uint8_t> &data,
> > > + [[maybe_unused]] std::vector<int32_t> &fds,
> > > + ControlSerializer *cs)
> > > + {
> > > + ControlList ret;
> > > + const ControlList &list = ret;
> > > + const_cast<ControlList &>(list) =
> > > + IPADataSerializer<const ControlList>::deserialize(data, fds, cs);
> > > +
> > > + return ret;
> > > + }
> > > +
> > > + static ControlList deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + std::vector<uint8_t>::iterator dataEnd,
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsBegin,
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsEnd,
> > > + ControlSerializer *cs)
> > > + {
> > > + ControlList ret;
> > > + const ControlList &list = ret;
> > > + const_cast<ControlList &>(list) =
> > > + IPADataSerializer<const ControlList>::deserialize(dataBegin, dataEnd,
> > > + fdsBegin, fdsEnd, cs);
> > > +
> > > + return ret;
> > > + }
> > > +};
> > > +
> > > +template<>
> > > +class IPADataSerializer<const 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())
> > > + return {{}, {}};
> > > +
> > > + std::vector<uint8_t> dataVec;
> > > + appendPOD<uint32_t>(dataVec, infoData.size());
> > > + dataVec.insert(dataVec.end(), infoData.begin(), infoData.end());
> > > +
> > > + return {dataVec, {}};
> > > + }
> > > +
> > > + static const ControlInfoMap deserialize(std::vector<uint8_t> &data,
> > > + ControlSerializer *cs)
> > > + {
> > > + return deserialize(data.begin(), data.end(), cs);
> > > + }
> > > +
> > > + static const ControlInfoMap deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + [[maybe_unused]] std::vector<uint8_t>::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>::iterator it = dataBegin + 4;
> > > +
> > > + std::vector<uint8_t> infoData(it, it + infoDataSize);
> > > +
> > > + ByteStreamBuffer buffer(const_cast<const uint8_t *>(infoData.data()), infoData.size());
> > > + const ControlInfoMap map = cs->deserialize<ControlInfoMap>(buffer);
> > > +
> > > + return map;
> > > + }
> > > +
> > > + static const ControlInfoMap deserialize(std::vector<uint8_t> &data,
> > > + [[maybe_unused]] std::vector<int32_t> &fds,
> > > + ControlSerializer *cs)
> > > + {
> > > + return deserialize(data.begin(), data.end(), cs);
> > > + }
> > > +
> > > + static const ControlInfoMap deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + std::vector<uint8_t>::iterator dataEnd,
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsBegin,
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsEnd,
> > > + ControlSerializer *cs)
> > > + {
> > > + return deserialize(dataBegin, dataEnd, cs);
> > > + }
> > > +};
> > > +
> > > +template<>
> > > +class IPADataSerializer<ControlInfoMap>
> > > +{
> > > +public:
> > > + static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> > > + serialize(const ControlInfoMap &map, [[maybe_unused]] ControlSerializer *cs)
> > > + {
> > > + const ControlInfoMap &map_const = const_cast<const ControlInfoMap &>(map);
> > > +
> > > + std::vector<uint8_t> dataVec;
> > > + std::tie(dataVec, std::ignore) =
> > > + IPADataSerializer<const ControlInfoMap>::serialize(map_const, cs);
> > > +
> > > + return {dataVec, {}};
> > > + }
> > > +
> > > + static ControlInfoMap deserialize(std::vector<uint8_t> &data,
> > > + ControlSerializer *cs)
> > > + {
> > > + ControlInfoMap ret;
> > > + const ControlInfoMap &map = ret;
> > > + const_cast<ControlInfoMap &>(map) =
> > > + IPADataSerializer<const ControlInfoMap>::deserialize(data, cs);
> > > +
> > > + return ret;
> > > + }
> > > +
> > > + static ControlInfoMap deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + [[maybe_unused]] std::vector<uint8_t>::iterator dataEnd,
> > > + ControlSerializer *cs)
> > > + {
> > > + ControlInfoMap ret;
> > > + const ControlInfoMap &map = ret;
> > > + const_cast<ControlInfoMap &>(map) =
> > > + IPADataSerializer<const ControlInfoMap>::deserialize(dataBegin, dataEnd, cs);
> > > +
> > > + return ret;
> > > + }
> > > +
> > > + static ControlInfoMap deserialize(std::vector<uint8_t> &data,
> > > + [[maybe_unused]] std::vector<int32_t> &fds,
> > > + ControlSerializer *cs)
> > > + {
> > > + ControlInfoMap ret;
> > > + const ControlInfoMap &map = ret;
> > > + const_cast<ControlInfoMap &>(map) =
> > > + IPADataSerializer<const ControlInfoMap>::deserialize(data, fds, cs);
> > > +
> > > + return ret;
> > > + }
> > > +
> > > + static ControlInfoMap deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + std::vector<uint8_t>::iterator dataEnd,
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsBegin,
> > > + [[maybe_unused]] std::vector<int32_t>::iterator fdsEnd,
> > > + ControlSerializer *cs)
> > > + {
> > > + ControlInfoMap ret;
> > > + const ControlInfoMap &map = ret;
> > > + const_cast<ControlInfoMap &>(map) =
> > > + IPADataSerializer<const ControlInfoMap>::deserialize(dataBegin, dataEnd,
> > > + fdsBegin, fdsEnd, cs);
> > > +
> > > + return ret;
> > > + }
> > > +};
> > > +
> > > +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.begin(), data.end(), fds.begin(), fds.end(), cs);
> > > + }
> > > +
> > > + static FrameBuffer::Plane deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + [[maybe_unused]] std::vector<uint8_t>::iterator dataEnd,
> > > + std::vector<int32_t>::iterator fdsBegin,
> > > + [[maybe_unused]] std::vector<int32_t>::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;
> > > + }
> > > +};
> > > +
> > > +
> > > +template<>
> > > +class IPADataSerializer<IPABuffer>
> > > +{
> > > +public:
> > > + static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> > > + serialize(const IPABuffer &data, ControlSerializer *cs = nullptr)
> > > + {
> > > + std::vector<uint8_t> dataVec;
> > > +
> > > + appendPOD<uint32_t>(dataVec, data.id);
> > > +
> > > + std::vector<uint8_t> planesDataVec;
> > > + std::vector<int32_t> planesFdsVec;
> > > + std::tie(planesDataVec, planesFdsVec) =
> > > + IPADataSerializer<std::vector<FrameBuffer::Plane>>::serialize(data.planes, cs);
> > > +
> > > + dataVec.insert(dataVec.end(), planesDataVec.begin(), planesDataVec.end());
> > > +
> > > + return {dataVec, planesFdsVec};
> > > + }
> > > +
> > > + static IPABuffer deserialize(std::vector<uint8_t> &data,
> > > + std::vector<int32_t> &fds,
> > > + ControlSerializer *cs = nullptr)
> > > + {
> > > + return deserialize(data.begin(), data.end(), fds.begin(), fds.end(), cs);
> > > + }
> > > +
> > > + static IPABuffer deserialize(std::vector<uint8_t>::iterator dataBegin,
> > > + std::vector<uint8_t>::iterator dataEnd,
> > > + std::vector<int32_t>::iterator fdsBegin,
> > > + std::vector<int32_t>::iterator fdsEnd,
> > > + ControlSerializer *cs = nullptr)
> > > + {
> > > + IPABuffer ret;
> > > +
> > > + ret.id = readPOD<uint32_t>(dataBegin, 0, dataEnd);
> > > +
> > > + ret.planes =
> > > + IPADataSerializer<std::vector<FrameBuffer::Plane>>::deserialize(
> > > + dataBegin + 4, dataEnd, fdsBegin, fdsEnd, cs);
> > > +
> > > + 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..eb3d6362
> > > --- /dev/null
> > > +++ b/src/libcamera/ipa_data_serializer.cpp
> > > @@ -0,0 +1,178 @@
> > > +/* 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.
> > > + */
> > > +
> > > +namespace {
> > > +
> > > +/**
> > > + * \fn template<typename T> void appendPOD(std::vector<uint8_t> &vec, T val)
> >
> > POD also includes structures and arrays (see
> > https://en.cppreference.com/w/cpp/named_req/PODType). The correct term
> > here is "arithmetic type" (see
> > https://en.cppreference.com/w/cpp/types/is_arithmetic).
> > appendArithmetic() sounds a bit wrong though.
>
> Yeah...
>
> > > + * \brief Append uint to end of byte vector, in little-endian order
> >
> > s/uint/\a val/ as it can also be a bool or a float. Same below, the
> > documentation should talk about arithmetic types and values, not uint.
> >
> > > + * \tparam T Type of uint to append
> >
> > Oh, I didn't know about tparam. Interesting.
> >
> > > + * \param[in] vec Byte vector to append to
> > > + * \param[in] val Value of uint 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> &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
> > > + */
> > > +
> > > +/**
> > > + * \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
> > > + */
> > > +
> > > +} /* 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(
> > > + * 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>::iterator dataBegin,
> > > + * std::vector<uint8_t>::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(
> > > + * std::vector<uint8_t> &data,
> > > + * 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>::iterator dataBegin,
> > > + * std::vector<uint8_t>::iterator dataEnd,
> > > + * std::vector<int32_t>::iterator fdsBegin,
> > > + * std::vector<int32_t>::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 99b27e66..01bcffd4 100644
> > > --- a/src/libcamera/meson.build
> > > +++ b/src/libcamera/meson.build
> > > @@ -24,6 +24,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