[RFC PATCH v1 8/8] utils: codegen: ipc: Simplify deserialization

Barnabás Pőcze barnabas.pocze at ideasonboard.com
Wed May 21 09:44:33 CEST 2025


Hi


2025. 05. 20. 17:18 keltezéssel, Jacopo Mondi írta:
> Hi Barnabás
> 
> On Thu, May 15, 2025 at 02:00:12PM +0200, Barnabás Pőcze wrote:
>> First, introduce the `SeriReader` type, which is a collection
>> of bytes and file descriptors, with the appropriate methods to
>> consume the first couples bytes / file descriptors.
>>
>> Then a new method is added to `IPCMessage` that returns an appropriately
>> constructed `SeriReader` for parsing the message contents.
>>
>> Then three of the four `deserialize()` overloads are removed, the
>> remaining one is converted to have a single `SeriReader` and an
>> optional `ControlSerializer` as arguments.
>>
>> The remaining `deserialize()` function is also changed to return an
>> `std::optional` to be able to report deserialization failure.
>>
>> There is also a more fundamental change in the serialization: previously,
>> the number of bytes taken up by an item has been written before the serialized
>> bytes (and conditionally the number of file descriptors) when the item is
>> serialized as part of a struct, array, map, function parameter list. This
>> is changed: the number of bytes and file descriptors are *not* serialized
>> into the final buffer. This affords some simplification of the serialization
>> related code paths, but most importantly, it greatly simplifies and unifies
>> how an object is (de)serialized because the deserialization of every object
>> becomes completely self-contained.
>>
>> As a consequence of that, strings now include their lengths as part of the
>> string serialization, and it is not left to an "upper" layer.
>>
>> Another consequence is that an "out parameter" of a remote function call
>> must be deserialized if a later out parameter is needed, even if itself
>> is not. This does not appear to be a great limitation since in all
>> situations presently none of the out parameters are ignored.
>>
>> Finally, the code generation templates are adapted to the above changes.
>> This allows the simplification of the deserialization templates as now
>> calling `IPADataSerializer<T>::deserialize(reader, &controlSerializer_)`
>> is appropriate for any type.
> 
> Wow, there is quite some to unpack here.
> 
> I wonder if the above changes related to the API, such as using
> optional<> might be broken out to make things easier for review, but I
> presume it might be hard.
> 
> Anyway, first question first: a class for "Deserializing" (reading
> from an array of Bytes) has been added, while the "Serialization"
> (marshalling data to an array of Bytes) is still left to each
> specialization of the IPADataSerializer<> class.
> 
> Was this by choice ?

I would like to add something like "SeriWriter" to do away with
returning vectors everywhere.


> 
> The question also comes from the fact we'll need a serializer for all
> the work to be done on ControlList in the public API. Do you plan to
> use SeriReader in that context as well ?

Not necessarily, some parts of it can be reused, but this class specifically
created to be used in `IPADataSerializer`.


> 
>>
>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=269
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
>> ---
>>   .../libcamera/internal/ipa_data_serializer.h  | 228 +++--------
>>   include/libcamera/internal/ipc_pipe.h         |   3 +
>>   include/libcamera/internal/serialization.h    |  91 +++++
>>   src/libcamera/ipa_data_serializer.cpp         | 356 ++++--------------
>>   src/libcamera/ipc_pipe.cpp                    |   5 +
>>   test/ipc/unixsocket_ipc.cpp                   |  18 +-
>>   .../generated_serializer_test.cpp             |  17 +-
>>   .../ipa_data_serializer_test.cpp              |  32 +-
>>   .../module_ipa_proxy.cpp.tmpl                 |  30 +-
>>   .../module_ipa_proxy.h.tmpl                   |   5 +-
>>   .../module_ipa_proxy_worker.cpp.tmpl          |   5 +-
>>   .../libcamera_templates/proxy_functions.tmpl  | 113 +-----
>>   .../libcamera_templates/serializer.tmpl       | 238 +-----------
>>   13 files changed, 316 insertions(+), 825 deletions(-)
>>   create mode 100644 include/libcamera/internal/serialization.h
>>
>> diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h
>> index 564f59e25..0d03729a1 100644
>> --- a/include/libcamera/internal/ipa_data_serializer.h
>> +++ b/include/libcamera/internal/ipa_data_serializer.h
>> @@ -7,6 +7,7 @@
>>
>>   #pragma once
>>
>> +#include <optional>
>>   #include <stdint.h>
>>   #include <string.h>
>>   #include <tuple>
>> @@ -23,6 +24,7 @@
>>   #include <libcamera/ipa/ipa_interface.h>
>>
>>   #include "libcamera/internal/control_serializer.h"
>> +#include "libcamera/internal/serialization.h"
>>
>>   namespace libcamera {
>>
>> @@ -39,26 +41,6 @@ void appendPOD(std::vector<uint8_t> &vec, T val)
>>   	memcpy(&*(vec.end() - byteWidth), &val, byteWidth);
>>   }
>>
>> -template<typename T,
>> -	 std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>
>> -T readPOD(std::vector<uint8_t>::const_iterator it, size_t pos,
>> -	  std::vector<uint8_t>::const_iterator end)
>> -{
>> -	ASSERT(pos + it < end);
>> -
>> -	T ret = 0;
>> -	memcpy(&ret, &(*(it + pos)), sizeof(ret));
>> -
>> -	return ret;
>> -}
>> -
>> -template<typename T,
>> -	 std::enable_if_t<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, typename = void>
>> @@ -68,20 +50,8 @@ public:
>>   	static std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>
>>   	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<SharedFD> &fds,
>> -			     ControlSerializer *cs = nullptr);
>> -	static T deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> -			     std::vector<uint8_t>::const_iterator dataEnd,
>> -			     std::vector<SharedFD>::const_iterator fdsBegin,
>> -			     std::vector<SharedFD>::const_iterator fdsEnd,
>> -			     ControlSerializer *cs = nullptr);
>> +	[[nodiscard]] static std::optional<T>
>> +	deserialize(SeriReader &reader, ControlSerializer *cs = nullptr);
>>   };
>>
>>   #ifndef __DOXYGEN__
>> @@ -121,9 +91,6 @@ public:
>>   			std::tie(dvec, fvec) =
>>   				IPADataSerializer<V>::serialize(it, cs);
>>
>> -			appendPOD<uint32_t>(dataVec, dvec.size());
>> -			appendPOD<uint32_t>(dataVec, fvec.size());
>> -
> 
> You should update the comment that describes the serialized format,
> for all specializations where the serialized formats is changed to
> remove the in-line sizes
> 
> 
>>   			dataVec.insert(dataVec.end(), dvec.begin(), dvec.end());
>>   			fdsVec.insert(fdsVec.end(), fvec.begin(), fvec.end());
>>   		}
>> @@ -131,52 +98,25 @@ public:
>>   		return { dataVec, fdsVec };
>>   	}
>>
>> -	static std::vector<V> deserialize(std::vector<uint8_t> &data, ControlSerializer *cs = nullptr)
>> +	[[nodiscard]] static std::optional<std::vector<V>>
>> +	deserialize(SeriReader &reader, ControlSerializer *cs = nullptr)
>>   	{
>> -		return deserialize(data.cbegin(), data.cend(), cs);
>> -	}
>> +		uint32_t vecLen;
>> +		if (!reader.read(vecLen))
>> +			return {};
> 
> Took me a while to understand this one. You're reading the first 4
> bytes integer that contains the serialized vector size.
> 
> The API of:
> 
> 	template<typename... Ts>
> 	[[nodiscard]] bool read(Ts &...xs) {}
> 
> it's a bit alien to me, as
> 1) It returns a pointer to the serialized data, and that's fine, but
> 2) Loads the content of the serialized data (of size sizeof...(xs))
>     into ...(xs)
> 
> As an aging C programmer I would have expected an API that uses a
> pointer for an output argument, but this would certainly make
> impractical to use the (Ts &...xs) magic...

Pointers could be used, I just like references better (e.g. no nullability issues).


> 
>>
>> -	static std::vector<V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> -					  std::vector<uint8_t>::const_iterator dataEnd,
>> -					  ControlSerializer *cs = nullptr)
>> -	{
>> -		std::vector<SharedFD> fds;
>> -		return deserialize(dataBegin, dataEnd, fds.cbegin(), fds.cend(), cs);
>> -	}
>> +		std::vector<V> ret;
>> +		ret.reserve(vecLen);
> 
> Is
>                  std::vector<V> ret(vecLen)
> 
> different ?

Yes, that will fill the vector with `vecLen` default constructed elements,
while `reserve()` only allocates storage for `vecLen` elements.


> 
>>
>> -	static std::vector<V> deserialize(std::vector<uint8_t> &data, std::vector<SharedFD> &fds,
>> -					  ControlSerializer *cs = nullptr)
>> -	{
>> -		return deserialize(data.cbegin(), data.cend(), fds.cbegin(), fds.cend(), cs);
>> -	}
>> -
>> -	static std::vector<V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> -					  std::vector<uint8_t>::const_iterator dataEnd,
>> -					  std::vector<SharedFD>::const_iterator fdsBegin,
>> -					  [[maybe_unused]] std::vector<SharedFD>::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<SharedFD>::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;
>> +			auto item = IPADataSerializer<V>::deserialize(reader, cs);
>> +			if (!item)
>> +				return {};
>> +
>> +			ret.emplace_back(std::move(*item));
> 
> Does std::move() make any practical difference ? The data have to
> copied to one container to the other anyway, don't they ?

If you have a non-trivially copyable type, then yes. For example,
a vector of `SharedFD` does benefit from the `std::move()` here.
(And by extension everything that has `SharedFd`, e.g. `IPABuffer` in `core.mojom`.)


> 
>>   		}
>>
>> -		return ret;
>> +		return std::move(ret);
> 
> Here as well, does move() make any difference ? I presume it does if
> the type V underlying the vector is move-constructable ?

As it turns out, this move is redundant, `return ret` will move implicitly,
so no difference here.


> 
>>   	}
>>   };
>>
>> @@ -218,18 +158,12 @@ public:
>>   			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());
>> -
> 
> Please update the comment here as well
> 
>>   			dataVec.insert(dataVec.end(), dvec.begin(), dvec.end());
>>   			fdsVec.insert(fdsVec.end(), fvec.begin(), fvec.end());
>>   		}
>> @@ -237,63 +171,25 @@ public:
>>   		return { dataVec, fdsVec };
>>   	}
>>
>> -	static std::map<K, V> deserialize(std::vector<uint8_t> &data, ControlSerializer *cs = nullptr)
>> -	{
>> -		return deserialize(data.cbegin(), data.cend(), cs);
>> -	}
>> -
>> -	static std::map<K, V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> -					  std::vector<uint8_t>::const_iterator dataEnd,
>> -					  ControlSerializer *cs = nullptr)
>> +	[[nodiscard]] static std::optional<std::map<K, V>>
>> +	deserialize(SeriReader &reader, ControlSerializer *cs = nullptr)
>>   	{
>> -		std::vector<SharedFD> fds;
>> -		return deserialize(dataBegin, dataEnd, fds.cbegin(), fds.cend(), cs);
>> -	}
>> +		uint32_t mapLen;
>> +		if (!reader.read(mapLen))
>> +			return {};
>>
>> -	static std::map<K, V> deserialize(std::vector<uint8_t> &data, std::vector<SharedFD> &fds,
>> -					  ControlSerializer *cs = nullptr)
>> -	{
>> -		return deserialize(data.cbegin(), data.cend(), fds.cbegin(), fds.cend(), cs);
>> -	}
>> -
>> -	static std::map<K, V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> -					  std::vector<uint8_t>::const_iterator dataEnd,
>> -					  std::vector<SharedFD>::const_iterator fdsBegin,
>> -					  [[maybe_unused]] std::vector<SharedFD>::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<SharedFD>::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;
>> +			auto key = IPADataSerializer<K>::deserialize(reader, cs);
>> +			if (!key)
>> +				return {};
>> +
>> +			auto value = IPADataSerializer<V>::deserialize(reader, cs);
>> +			if (!value)
>> +				return {};
>> +
>> +			ret.try_emplace(std::move(*key), std::move(*value));
> 
> The number of lines saved is impressive!
> 
>>   		}
>>
>>   		return ret;
>> @@ -314,33 +210,14 @@ public:
>>   		return { dataVec, {} };
>>   	}
>>
>> -	static Flags<E> deserialize(std::vector<uint8_t> &data,
>> -				    [[maybe_unused]] ControlSerializer *cs = nullptr)
>> -	{
>> -		return deserialize(data.cbegin(), data.cend());
>> -	}
>> -
>> -	static Flags<E> deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> -				    std::vector<uint8_t>::const_iterator dataEnd,
>> -				    [[maybe_unused]] ControlSerializer *cs = nullptr)
>> +	[[nodiscard]] static std::optional<Flags<E>>
>> +	deserialize(SeriReader &reader, [[maybe_unused]] ControlSerializer *cs = nullptr)
>>   	{
>> -		return Flags<E>{ static_cast<E>(readPOD<uint32_t>(dataBegin, 0, dataEnd)) };
>> -	}
>> +		uint32_t value;
>> +		if (!reader.read(value))
>> +			return {};
>>
>> -	static Flags<E> deserialize(std::vector<uint8_t> &data,
>> -				    [[maybe_unused]] std::vector<SharedFD> &fds,
>> -				    [[maybe_unused]] ControlSerializer *cs = nullptr)
>> -	{
>> -		return deserialize(data.cbegin(), data.cend());
>> -	}
>> -
>> -	static Flags<E> deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> -				    std::vector<uint8_t>::const_iterator dataEnd,
>> -				    [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin,
>> -				    [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,
>> -				    [[maybe_unused]] ControlSerializer *cs = nullptr)
>> -	{
>> -		return deserialize(dataBegin, dataEnd);
>> +		return Flags<E>{ static_cast<E>(value) };
>>   	}
>>   };
>>
>> @@ -360,33 +237,14 @@ public:
>>   		return { dataVec, {} };
>>   	}
>>
>> -	static E deserialize(std::vector<uint8_t> &data,
>> -			     [[maybe_unused]] ControlSerializer *cs = nullptr)
>> +	[[nodiscard]] static std::optional<E>
>> +	deserialize(SeriReader &reader, [[maybe_unused]] ControlSerializer *cs = nullptr)
>>   	{
>> -		return deserialize(data.cbegin(), data.cend());
>> -	}
>> +		U value;
>> +		if (!reader.read(value))
>> +			return {};
>>
>> -	static E deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> -			     std::vector<uint8_t>::const_iterator dataEnd,
>> -			     [[maybe_unused]] ControlSerializer *cs = nullptr)
>> -	{
>> -		return static_cast<E>(readPOD<U>(dataBegin, 0, dataEnd));
>> -	}
>> -
>> -	static E deserialize(std::vector<uint8_t> &data,
>> -			    [[maybe_unused]] std::vector<SharedFD> &fds,
>> -			    [[maybe_unused]] ControlSerializer *cs = nullptr)
>> -	{
>> -		return deserialize(data.cbegin(), data.cend());
>> -	}
>> -
>> -	static E deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> -			     std::vector<uint8_t>::const_iterator dataEnd,
>> -			     [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin,
>> -			     [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,
>> -			     [[maybe_unused]] ControlSerializer *cs = nullptr)
>> -	{
>> -		return deserialize(dataBegin, dataEnd);
>> +		return static_cast<E>(value);
>>   	}
>>   };
>>
>> diff --git a/include/libcamera/internal/ipc_pipe.h b/include/libcamera/internal/ipc_pipe.h
>> index 418c4622f..2b6cde042 100644
>> --- a/include/libcamera/internal/ipc_pipe.h
>> +++ b/include/libcamera/internal/ipc_pipe.h
>> @@ -14,6 +14,7 @@
>>   #include <libcamera/base/signal.h>
>>
>>   #include "libcamera/internal/ipc_unixsocket.h"
>> +#include "libcamera/internal/serialization.h"
>>
>>   namespace libcamera {
>>
>> @@ -40,6 +41,8 @@ public:
>>   	const std::vector<uint8_t> &data() const { return data_; }
>>   	const std::vector<SharedFD> &fds() const { return fds_; }
>>
>> +	SeriReader reader() const { return { data(), fds() }; }
>> +
>>   private:
>>   	Header header_;
>>
>> diff --git a/include/libcamera/internal/serialization.h b/include/libcamera/internal/serialization.h
>> new file mode 100644
>> index 000000000..daa7e5438
>> --- /dev/null
>> +++ b/include/libcamera/internal/serialization.h
>> @@ -0,0 +1,91 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2025, Ideas On Board Oy
>> + *
>> + * Data (de)serialization helper structures
>> + */
>> +#pragma once
>> +
> 
> Missing a few includes
> 
>> +#include <optional>
>> +#include <string.h>
>> +#include <tuple>
>> +
>> +#include <libcamera/base/shared_fd.h>
>> +#include <libcamera/base/span.h>
>> +
>> +namespace libcamera {
>> +
>> +#ifndef __DOXYGEN__
>> +class SeriReader
>> +{
>> +public:
>> +	SeriReader(Span<const std::byte> data, Span<const SharedFD> fds = {})
>> +		: data_(data),
>> +		  fds_(fds)
>> +	{
>> +	}
>> +
>> +	SeriReader(Span<const uint8_t> data, Span<const SharedFD> fds = {})
>> +		: data_(reinterpret_cast<const std::byte *>(data.data()),
>> +			reinterpret_cast<const std::byte *>(data.data() + data.size())),
> 
> ah so uint8_t is not directly convertible to a byte ?

`std::byte` is its own thing, so not convertible; and I prefer that because it's
harder to misuse than `uint8_t` (~ `unsigned char`).


> 
>> +		  fds_(fds)
>> +	{
>> +	}
>> +
>> +	[[nodiscard]] const std::byte *consume(std::size_t s)
>> +	{
>> +		if (data_.size() < s)
>> +			return nullptr;
>> +
>> +		const auto *p = data_.data();
>> +		data_ = data_.subspan(s);
>> +
>> +		return p;
>> +	}
>> +
>> +	template<typename... Ts>
>> +	[[nodiscard]] bool read(Ts &...xs)
> 
> My head didn't explode just because you already showed me this trick
> 
>> +	{
>> +		static_assert((std::is_trivially_copyable_v<Ts> && ...));
>> +
>> +		const auto *p = consume((sizeof(xs) + ...));
> 
> So we read the whole space required to deserialize multiple xs
> 
>> +		if (p)
>> +			((memcpy(&xs, p, sizeof(xs)), p += sizeof(xs)), ...);
> 
> And we copy them one by one
> 
> brilliant
> 
>> +
>> +		return p;
> 
> 
> 
>> +	}
>> +
>> +	template<typename... Ts>
>> +	[[nodiscard]] auto read()
>> +	{
>> +		std::tuple<Ts...> xs;
>> +		bool ok = std::apply([&](auto &...vs) {
>> +			return read(vs...);
>> +		}, xs);
>> +
>> +		if constexpr (sizeof...(Ts) == 1)
>> +			return ok ? std::optional(std::get<0>(xs)) : std::nullopt;
>> +		else
>> +			return ok ? std::optional(xs) : std::nullopt;
> 
> If this last version doesn't work for sizeof...(Ts) == 1
> (I know that, I tried removing the if branch) does it mean that
> 
> 		if constexpr (sizeof...(Ts) == 1)
> 
> is compile-time evaluated and the compiler knows what option to pick.

Yes.


> 
> Now the main question, do you foresee an use for template argument
> packs ? As far as I understood it, all the exiting code works with a
> single argument, or have I missed something ?

This was written before I converted things, and as it turns out, this
overload of `read()` is not really used, so it may be removed.


> 
> I'll stop the review here for now, enough material to ponder up for me :)
> 
> Thanks
>    j
> 
>> +	}
>> +
>> +	[[nodiscard]] const SharedFD *nextFd()
>> +	{
>> +		if (fds_.empty())
>> +			return nullptr;
>> +
>> +		const auto *p = fds_.data();
>> +		fds_ = fds_.subspan(1);
>> +
>> +		return p;
>> +	}
>> +
>> +	[[nodiscard]] bool empty() const { return data_.empty() && fds_.empty(); }
>> +
>> +private:
>> +	Span<const std::byte> data_;
>> +	Span<const SharedFD> fds_;
>> +};
>> +#endif
>> +
>> +} /* namespace libcamera */
>> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp
>> index 0537f785b..979a1e0db 100644
>> --- a/src/libcamera/ipa_data_serializer.cpp
>> +++ b/src/libcamera/ipa_data_serializer.cpp
>> @@ -50,42 +50,6 @@ namespace {
>>    * 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 */
>>
>>   /**
>> @@ -106,80 +70,13 @@ namespace {
>>
>>   /**
>>    * \fn template<typename T> IPADataSerializer<T>::deserialize(
>> - * 	const std::vector<uint8_t> &data,
>> + * 	SeriReader &reader,
>>    * 	ControlSerializer *cs = nullptr)
>> - * \brief Deserialize byte vector into an object
>> + * \brief Deserialize bytes and file descriptors vector into an object
>>    * \tparam T Type of object to deserialize to
>> - * \param[in] data Byte vector to deserialize from
>> + * \param[in] reader Source of bytes and file descriptors
>>    * \param[in] cs ControlSerializer
>>    *
>> - * This version of deserialize() can be used if the object type \a T and its
>> - * members don't have any SharedFD.
>> - *
>> - * \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 SharedFD.
>> - *
>> - * \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<SharedFD> &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 SharedFD.
>> - *
>> - * \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<SharedFD>::const_iterator fdsBegin,
>> - * 	std::vector<SharedFD>::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 SharedFD.
>> - *
>>    * \a cs is only necessary if the object type \a T or its members contain
>>    * ControlList or ControlInfoMap.
>>    *
>> @@ -202,37 +99,11 @@ IPADataSerializer<type>::serialize(const type &data,			\
>>   }									\
>>   									\
>>   template<>								\
>> -type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, \
>> -					  std::vector<uint8_t>::const_iterator dataEnd, \
>> -					  [[maybe_unused]] ControlSerializer *cs) \
>> +std::optional<type> IPADataSerializer<type>::deserialize(SeriReader &reader, \
>> +							 [[maybe_unused]] ControlSerializer *cs) \
>>   {									\
>> -	return readPOD<type>(dataBegin, 0, dataEnd);			\
>> +	return reader.read<type>();					\
>>   }									\
>> -									\
>> -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<SharedFD> &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<SharedFD>::const_iterator fdsBegin, \
>> -					  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd, \
>> -					  ControlSerializer *cs)	\
>> -{									\
>> -	return deserialize(dataBegin, dataEnd, cs);			\
>> -}
>>
>>   DEFINE_POD_SERIALIZER(bool)
>>   DEFINE_POD_SERIALIZER(uint8_t)
>> @@ -256,44 +127,29 @@ std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>
>>   IPADataSerializer<std::string>::serialize(const std::string &data,
>>   					  [[maybe_unused]] ControlSerializer *cs)
>>   {
>> -	return { { data.cbegin(), data.end() }, {} };
>> -}
>> +	std::vector<uint8_t> dataVec;
>>
>> -template<>
>> -std::string
>> -IPADataSerializer<std::string>::deserialize(const std::vector<uint8_t> &data,
>> -					    [[maybe_unused]] ControlSerializer *cs)
>> -{
>> -	return { data.cbegin(), data.cend() };
>> -}
>> +	ASSERT(data.size() <= std::numeric_limits<uint32_t>::max());
>> +	appendPOD<uint32_t>(dataVec, data.size());
>> +	dataVec.insert(dataVec.end(), data.c_str(), data.c_str() + data.size());
>>
>> -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 };
>> +	return { dataVec, {} };
>>   }
>>
>>   template<>
>> -std::string
>> -IPADataSerializer<std::string>::deserialize(const std::vector<uint8_t> &data,
>> -					    [[maybe_unused]] const std::vector<SharedFD> &fds,
>> +std::optional<std::string>
>> +IPADataSerializer<std::string>::deserialize(SeriReader &reader,
>>   					    [[maybe_unused]] ControlSerializer *cs)
>>   {
>> -	return { data.cbegin(), data.cend() };
>> -}
>> +	uint32_t length;
>> +	if (!reader.read(length))
>> +		return {};
>>
>> -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<SharedFD>::const_iterator fdsBegin,
>> -					    [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,
>> -					    [[maybe_unused]] ControlSerializer *cs)
>> -{
>> -	return { dataBegin, dataEnd };
>> +	const auto *p = reader.consume(length);
>> +	if (!p)
>> +		return {};
>> +
>> +	return { { reinterpret_cast<const char *>(p), std::size_t(length) } };
>>   }
>>
>>   /*
>> @@ -356,73 +212,43 @@ IPADataSerializer<ControlList>::serialize(const ControlList &data, ControlSerial
>>   }
>>
>>   template<>
>> -ControlList
>> -IPADataSerializer<ControlList>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> -					    std::vector<uint8_t>::const_iterator dataEnd,
>> +std::optional<ControlList>
>> +IPADataSerializer<ControlList>::deserialize(SeriReader &reader,
>>   					    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::distance(it, dataEnd)) < infoDataSize + listDataSize)
>> +	uint32_t infoDataSize, listDataSize;
>> +	if (!reader.read(infoDataSize, listDataSize))
>>   		return {};
>>
>>   	if (infoDataSize > 0) {
>> -		ByteStreamBuffer buffer(&*it, infoDataSize);
>> +		const auto *p = reader.consume(infoDataSize);
>> +		if (!p)
>> +			return {};
>> +
>> +		ByteStreamBuffer buffer(reinterpret_cast<const uint8_t *>(p), infoDataSize);
>>   		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();
>> +			return {};
>>   		}
>>   	}
>>
>> -	it += infoDataSize;
>> -	ByteStreamBuffer buffer(&*it, listDataSize);
>> +	const auto *p = reader.consume(listDataSize);
>> +
>> +	ByteStreamBuffer buffer(reinterpret_cast<const uint8_t *>(p), listDataSize);
>>   	ControlList list = cs->deserialize<ControlList>(buffer);
>> -	if (buffer.overflow())
>> +	if (buffer.overflow()) {
>>   		LOG(IPADataSerializer, Error) << "Failed to deserialize ControlList: buffer overflow";
>> +		return {};
>> +	}
>>
>> -	return list;
>> -}
>> -
>> -template<>
>> -ControlList
>> -IPADataSerializer<ControlList>::deserialize(const std::vector<uint8_t> &data,
>> -					    ControlSerializer *cs)
>> -{
>> -	return deserialize(data.cbegin(), data.end(), cs);
>> -}
>> -
>> -template<>
>> -ControlList
>> -IPADataSerializer<ControlList>::deserialize(const std::vector<uint8_t> &data,
>> -					    [[maybe_unused]] const std::vector<SharedFD> &fds,
>> -					    ControlSerializer *cs)
>> -{
>> -	return deserialize(data.cbegin(), data.end(), cs);
>> -}
>> -
>> -template<>
>> -ControlList
>> -IPADataSerializer<ControlList>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> -					    std::vector<uint8_t>::const_iterator dataEnd,
>> -					    [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin,
>> -					    [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,
>> -					    ControlSerializer *cs)
>> -{
>> -	return deserialize(dataBegin, dataEnd, cs);
>> +	return std::move(list);
>>   }
>>
>>   /*
>> @@ -458,57 +284,28 @@ IPADataSerializer<ControlInfoMap>::serialize(const ControlInfoMap &map,
>>   }
>>
>>   template<>
>> -ControlInfoMap
>> -IPADataSerializer<ControlInfoMap>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> -					       std::vector<uint8_t>::const_iterator dataEnd,
>> +std::optional<ControlInfoMap>
>> +IPADataSerializer<ControlInfoMap>::deserialize(SeriReader &reader,
>>   					       ControlSerializer *cs)
>>   {
>>   	if (!cs)
>>   		LOG(IPADataSerializer, Fatal)
>>   			<< "ControlSerializer not provided for deserialization of ControlInfoMap";
>>
>> -	if (std::distance(dataBegin, dataEnd) < 4)
>> +	uint32_t infoDataSize;
>> +	if (!reader.read(infoDataSize))
>>   		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::distance(it, dataEnd)) < infoDataSize)
>> +	const auto *p = reader.consume(infoDataSize);
>> +	if (!p)
>>   		return {};
>>
>> -	ByteStreamBuffer buffer(&*it, infoDataSize);
>> +	ByteStreamBuffer buffer(reinterpret_cast<const uint8_t *>(p), infoDataSize);
>>   	ControlInfoMap map = cs->deserialize<ControlInfoMap>(buffer);
>> +	if (buffer.overflow())
>> +		return {};
>>
>> -	return map;
>> -}
>> -
>> -template<>
>> -ControlInfoMap
>> -IPADataSerializer<ControlInfoMap>::deserialize(const std::vector<uint8_t> &data,
>> -					       ControlSerializer *cs)
>> -{
>> -	return deserialize(data.cbegin(), data.end(), cs);
>> -}
>> -
>> -template<>
>> -ControlInfoMap
>> -IPADataSerializer<ControlInfoMap>::deserialize(const std::vector<uint8_t> &data,
>> -					       [[maybe_unused]] const std::vector<SharedFD> &fds,
>> -					       ControlSerializer *cs)
>> -{
>> -	return deserialize(data.cbegin(), data.end(), cs);
>> -}
>> -
>> -template<>
>> -ControlInfoMap
>> -IPADataSerializer<ControlInfoMap>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> -					       std::vector<uint8_t>::const_iterator dataEnd,
>> -					       [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin,
>> -					       [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,
>> -					       ControlSerializer *cs)
>> -{
>> -	return deserialize(dataBegin, dataEnd, cs);
>> +	return std::move(map);
>>   }
>>
>>   /*
>> @@ -542,27 +339,23 @@ IPADataSerializer<SharedFD>::serialize(const SharedFD &data,
>>   }
>>
>>   template<>
>> -SharedFD IPADataSerializer<SharedFD>::deserialize([[maybe_unused]] std::vector<uint8_t>::const_iterator dataBegin,
>> -						  [[maybe_unused]] std::vector<uint8_t>::const_iterator dataEnd,
>> -						  std::vector<SharedFD>::const_iterator fdsBegin,
>> -						  std::vector<SharedFD>::const_iterator fdsEnd,
>> -						  [[maybe_unused]] ControlSerializer *cs)
>> +std::optional<SharedFD>
>> +IPADataSerializer<SharedFD>::deserialize(SeriReader &reader,
>> +					 [[maybe_unused]] ControlSerializer *cs)
>>   {
>> -	ASSERT(std::distance(dataBegin, dataEnd) >= 4);
>> +	uint32_t valid;
>>
>> -	uint32_t valid = readPOD<uint32_t>(dataBegin, 0, dataEnd);
>> +	if (!reader.read(valid))
>> +		return {};
>>
>> -	ASSERT(!(valid && std::distance(fdsBegin, fdsEnd) < 1));
>> +	if (!valid)
>> +		return SharedFD{};
>>
>> -	return valid ? *fdsBegin : SharedFD();
>> -}
>> +	const auto *fd = reader.nextFd();
>> +	if (!fd)
>> +		return {};
>>
>> -template<>
>> -SharedFD IPADataSerializer<SharedFD>::deserialize(const std::vector<uint8_t> &data,
>> -						  const std::vector<SharedFD> &fds,
>> -						  [[maybe_unused]] ControlSerializer *cs)
>> -{
>> -	return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end());
>> +	return *fd;
>>   }
>>
>>   /*
>> @@ -594,30 +387,19 @@ IPADataSerializer<FrameBuffer::Plane>::serialize(const FrameBuffer::Plane &data,
>>   }
>>
>>   template<>
>> -FrameBuffer::Plane
>> -IPADataSerializer<FrameBuffer::Plane>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> -						   std::vector<uint8_t>::const_iterator dataEnd,
>> -						   std::vector<SharedFD>::const_iterator fdsBegin,
>> -						   [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,
>> +std::optional<FrameBuffer::Plane>
>> +IPADataSerializer<FrameBuffer::Plane>::deserialize(SeriReader &reader,
>>   						   [[maybe_unused]] ControlSerializer *cs)
>>   {
>> -	FrameBuffer::Plane ret;
>> -
>> -	ret.fd = IPADataSerializer<SharedFD>::deserialize(dataBegin, dataBegin + 4,
>> -							  fdsBegin, fdsBegin + 1);
>> -	ret.offset = readPOD<uint32_t>(dataBegin, 4, dataEnd);
>> -	ret.length = readPOD<uint32_t>(dataBegin, 8, dataEnd);
>> +	auto fd = IPADataSerializer<SharedFD>::deserialize(reader);
>> +	if (!fd)
>> +		return {};
>>
>> -	return ret;
>> -}
>> +	uint32_t offset, length;
>> +	if (!reader.read(offset, length))
>> +		return {};
>>
>> -template<>
>> -FrameBuffer::Plane
>> -IPADataSerializer<FrameBuffer::Plane>::deserialize(const std::vector<uint8_t> &data,
>> -						   const std::vector<SharedFD> &fds,
>> -						   ControlSerializer *cs)
>> -{
>> -	return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs);
>> +	return { { std::move(*fd), offset, length } };
>>   }
>>
>>   #endif /* __DOXYGEN__ */
>> diff --git a/src/libcamera/ipc_pipe.cpp b/src/libcamera/ipc_pipe.cpp
>> index 548299d05..71d0c2cb4 100644
>> --- a/src/libcamera/ipc_pipe.cpp
>> +++ b/src/libcamera/ipc_pipe.cpp
>> @@ -148,6 +148,11 @@ IPCUnixSocket::Payload IPCMessage::payload() const
>>    * \brief Returns a const reference to the vector containing file descriptors
>>    */
>>
>> +/**
>> + * \fn IPCMessage::reader() const
>> + * \brief Returns a `SeriReader` instance for parsing the message
>> + */
>> +
>>   /**
>>    * \class IPCPipe
>>    * \brief IPC message pipe for IPA isolation
>> diff --git a/test/ipc/unixsocket_ipc.cpp b/test/ipc/unixsocket_ipc.cpp
>> index df7d9c2b4..c0e174ba9 100644
>> --- a/test/ipc/unixsocket_ipc.cpp
>> +++ b/test/ipc/unixsocket_ipc.cpp
>> @@ -102,7 +102,14 @@ private:
>>   		}
>>
>>   		case CmdSetAsync: {
>> -			value_ = IPADataSerializer<int32_t>::deserialize(ipcMessage.data());
>> +			SeriReader reader = ipcMessage.reader();
>> +			auto value = IPADataSerializer<int32_t>::deserialize(reader);
>> +			if (!value) {
>> +				cerr << "Failed to deserialize value" << endl;
>> +				stop(-ENODATA);
>> +			} else {
>> +				value_ = *value;
>> +			}
>>   			break;
>>   		}
>>   		}
>> @@ -155,7 +162,14 @@ protected:
>>   			return ret;
>>   		}
>>
>> -		return IPADataSerializer<int32_t>::deserialize(buf.data());
>> +		SeriReader reader = buf.reader();
>> +		auto value = IPADataSerializer<int32_t>::deserialize(reader);
>> +		if (!value) {
>> +			cerr << "Failed to deserialize value" << endl;
>> +			return -ENODATA;
>> +		}
>> +
>> +		return *value;
>>   	}
>>
>>   	int exit()
>> diff --git a/test/serialization/generated_serializer/generated_serializer_test.cpp b/test/serialization/generated_serializer/generated_serializer_test.cpp
>> index dd6968850..0640e741d 100644
>> --- a/test/serialization/generated_serializer/generated_serializer_test.cpp
>> +++ b/test/serialization/generated_serializer/generated_serializer_test.cpp
>> @@ -43,7 +43,7 @@ if (struct1.field != struct2.field) {				\
>>   }
>>
>>
>> -		ipa::test::TestStruct t, u;
>> +		ipa::test::TestStruct t;
>>
>>   		t.m = {
>>   			{ "a", "z" },
>> @@ -71,8 +71,13 @@ if (struct1.field != struct2.field) {				\
>>
>>   		std::tie(serialized, ignore) =
>>   			IPADataSerializer<ipa::test::TestStruct>::serialize(t);
>> +		SeriReader reader(serialized);
>>
>> -		u = IPADataSerializer<ipa::test::TestStruct>::deserialize(serialized);
>> +		auto optu = IPADataSerializer<ipa::test::TestStruct>::deserialize(reader);
>> +		if (!optu)
>> +			return TestFail;
>> +
>> +		auto &u = *optu;
>>
>>   		if (!equals(t.m, u.m))
>>   			return TestFail;
>> @@ -91,12 +96,16 @@ if (struct1.field != struct2.field) {				\
>>
>>   		/* Test vector of generated structs */
>>   		std::vector<ipa::test::TestStruct> v = { t, u };
>> -		std::vector<ipa::test::TestStruct> w;
>>
>>   		std::tie(serialized, ignore) =
>>   			IPADataSerializer<vector<ipa::test::TestStruct>>::serialize(v);
>> +		reader = SeriReader(serialized);
>> +
>> +		auto optw = IPADataSerializer<vector<ipa::test::TestStruct>>::deserialize(reader);
>> +		if (!optw)
>> +			return TestFail;
>>
>> -		w = IPADataSerializer<vector<ipa::test::TestStruct>>::deserialize(serialized);
>> +		auto &w = *optw;
>>
>>   		if (!equals(v[0].m, w[0].m) ||
>>   		    !equals(v[1].m, w[1].m))
>> diff --git a/test/serialization/ipa_data_serializer_test.cpp b/test/serialization/ipa_data_serializer_test.cpp
>> index afea93a6c..3873808a6 100644
>> --- a/test/serialization/ipa_data_serializer_test.cpp
>> +++ b/test/serialization/ipa_data_serializer_test.cpp
>> @@ -52,7 +52,9 @@ int testPodSerdes(T in)
>>   	std::vector<SharedFD> fds;
>>
>>   	std::tie(buf, fds) = IPADataSerializer<T>::serialize(in);
>> -	T out = IPADataSerializer<T>::deserialize(buf, fds);
>> +	SeriReader reader(buf, fds);
>> +
>> +	auto out = IPADataSerializer<T>::deserialize(reader);
>>   	if (in == out)
>>   		return TestPass;
>>
>> @@ -71,7 +73,9 @@ int testVectorSerdes(const std::vector<T> &in,
>>   	std::vector<SharedFD> fds;
>>
>>   	std::tie(buf, fds) = IPADataSerializer<std::vector<T>>::serialize(in, cs);
>> -	std::vector<T> out = IPADataSerializer<std::vector<T>>::deserialize(buf, fds, cs);
>> +	SeriReader reader(buf, fds);
>> +
>> +	auto out = IPADataSerializer<std::vector<T>>::deserialize(reader, cs);
>>   	if (in == out)
>>   		return TestPass;
>>
>> @@ -91,7 +95,9 @@ int testMapSerdes(const std::map<K, V> &in,
>>   	std::vector<SharedFD> fds;
>>
>>   	std::tie(buf, fds) = IPADataSerializer<std::map<K, V>>::serialize(in, cs);
>> -	std::map<K, V> out = IPADataSerializer<std::map<K, V>>::deserialize(buf, fds, cs);
>> +	SeriReader reader(buf, fds);
>> +
>> +	auto out = IPADataSerializer<std::map<K, V>>::deserialize(reader, cs);
>>   	if (in == out)
>>   		return TestPass;
>>
>> @@ -171,17 +177,27 @@ private:
>>   		std::tie(listBuf, std::ignore) =
>>   			IPADataSerializer<ControlList>::serialize(list, &cs);
>>
>> -		const ControlInfoMap infoMapOut =
>> -			IPADataSerializer<ControlInfoMap>::deserialize(infoMapBuf, &cs);
>> +		SeriReader listReader(listBuf);
>> +		SeriReader infoMapReader(infoMapBuf);
>> +
>> +		auto infoMapOut = IPADataSerializer<ControlInfoMap>::deserialize(infoMapReader, &cs);
>> +		if (!infoMapOut) {
>> +			cerr << "`ControlInfoMap` cannot be deserialized" << endl;
>> +			return TestFail;
>> +		}
>>
>> -		ControlList listOut = IPADataSerializer<ControlList>::deserialize(listBuf, &cs);
>> +		auto listOut = IPADataSerializer<ControlList>::deserialize(listReader, &cs);
>> +		if (!listOut) {
>> +			cerr << "`ControlList` cannot be deserialized" << endl;
>> +			return TestFail;
>> +		}
>>
>> -		if (!SerializationTest::equals(infoMap, infoMapOut)) {
>> +		if (!SerializationTest::equals(infoMap, *infoMapOut)) {
>>   			cerr << "Deserialized map doesn't match original" << endl;
>>   			return TestFail;
>>   		}
>>
>> -		if (!SerializationTest::equals(list, listOut)) {
>> +		if (!SerializationTest::equals(list, *listOut)) {
>>   			cerr << "Deserialized list doesn't match original" << endl;
>>   			return TestFail;
>>   		}
>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>> index 9a3aadbd2..843260b4b 100644
>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>> @@ -107,13 +107,13 @@ namespace {{ns}} {
>>   {% if interface_event.methods|length > 0 %}
>>   void {{proxy_name}}::recvMessage(const IPCMessage &data)
>>   {
>> -	size_t dataSize = data.data().size();
>> +	SeriReader reader = data.reader();
>>   	{{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);
>>
>>   	switch (_cmd) {
>>   {%- for method in interface_event.methods %}
>>   	case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {
>> -		{{method.mojom_name}}IPC(data.data().cbegin(), dataSize, data.fds());
>> +		{{method.mojom_name}}IPC(reader);
>>   		break;
>>   	}
>>   {%- endfor %}
>> @@ -211,18 +211,23 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
>>   		return;
>>   {%- endif %}
>>   	}
>> +{% if has_output %}
>> +	SeriReader _outputReader = _ipcOutputBuf.reader();
>> +{% endif -%}
>>   {% if method|method_return_value != "void" %}
>> -	{{method|method_return_value}} _retValue = IPADataSerializer<{{method|method_return_value}}>::deserialize(_ipcOutputBuf.data(), 0);
>> -
>> -{{proxy_funcs.deserialize_call(method|method_param_outputs, '_ipcOutputBuf.data()', '_ipcOutputBuf.fds()', init_offset = method|method_return_value|byte_width|int)}}
>> +	auto _retValue = IPADataSerializer<{{method|method_return_value}}>::deserialize(_outputReader);
>> +	ASSERT(_retValue);
>> +{% endif -%}
>>
>> -	return _retValue;
>> +{% if has_output %}
>> +	{{proxy_funcs.deserialize_call(method|method_param_outputs, '_outputReader')}}
>> +	ASSERT(_outputReader.empty());
>> +{% endif -%}
>>
>> -{% elif method|method_param_outputs|length > 0 %}
>> -{{proxy_funcs.deserialize_call(method|method_param_outputs, '_ipcOutputBuf.data()', '_ipcOutputBuf.fds()')}}
>> +{% if method|method_return_value != "void" %}
>> +	return std::move(*_retValue);
>>   {% endif -%}
>>   }
>> -
>>   {% endfor %}
>>
>>   {% for method in interface_event.methods %}
>> @@ -232,12 +237,9 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
>>   	{{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
>>   }
>>
>> -void {{proxy_name}}::{{method.mojom_name}}IPC(
>> -	[[maybe_unused]] std::vector<uint8_t>::const_iterator data,
>> -	[[maybe_unused]] size_t dataSize,
>> -	[[maybe_unused]] const std::vector<SharedFD> &fds)
>> +void {{proxy_name}}::{{method.mojom_name}}IPC([[maybe_unused]] SeriReader &reader)
>>   {
>> -{{proxy_funcs.deserialize_call(method.parameters, 'data', 'fds', false, true, true, 'dataSize')}}
>> +	{{proxy_funcs.deserialize_call(method.parameters, 'reader', false, true)}}
>>   	{{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
>>   }
>>   {% endfor %}
>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
>> index a0312a7c1..945a4ded9 100644
>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
>> @@ -53,10 +53,7 @@ private:
>>   {% endfor %}
>>   {% for method in interface_event.methods %}
>>   {{proxy_funcs.func_sig(proxy_name, method, "Thread", false)|indent(8, true)}};
>> -	void {{method.mojom_name}}IPC(
>> -		std::vector<uint8_t>::const_iterator data,
>> -		size_t dataSize,
>> -		const std::vector<SharedFD> &fds);
>> +	void {{method.mojom_name}}IPC(SeriReader &reader);
>>   {% endfor %}
>>
>>   	/* Helper class to invoke async functions in another thread. */
>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
>> index 1f990d3f9..de6378be0 100644
>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
>> @@ -71,6 +71,7 @@ public:
>>   		}
>>
>>   		IPCMessage _ipcMessage(_message);
>> +		SeriReader _reader = _ipcMessage.reader();
>>
>>   		{{cmd_enum_name}} _cmd = static_cast<{{cmd_enum_name}}>(_ipcMessage.header().cmd);
>>
>> @@ -85,7 +86,9 @@ public:
>>   {%- if method.mojom_name == "configure" %}
>>   			controlSerializer_.reset();
>>   {%- endif %}
>> -		{{proxy_funcs.deserialize_call(method|method_param_inputs, '_ipcMessage.data()', '_ipcMessage.fds()', false, true)|indent(16, true)}}
>> +			{{proxy_funcs.deserialize_call(method|method_param_inputs, '_reader', false, true)|indent(16, true)}}
>> +			ASSERT(_reader.empty());
>> +
>>   {% for param in method|method_param_outputs %}
>>   			{{param|name}} {{param.mojom_name}};
>>   {% endfor %}
>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl
>> index 01e2567ca..b6835ca35 100644
>> --- a/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl
>> +++ b/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl
>> @@ -64,15 +64,6 @@
>>   );
>>   {%- endfor %}
>>
>> -{%- if params|length > 1 %}
>> -{%- for param in params %}
>> -	appendPOD<uint32_t>({{buf}}, {{param.mojom_name}}Buf.size());
>> -{%- if param|has_fd %}
>> -	appendPOD<uint32_t>({{buf}}, {{param.mojom_name}}Fds.size());
>> -{%- endif %}
>> -{%- endfor %}
>> -{%- endif %}
>> -
>>   {%- for param in params %}
>>   	{{buf}}.insert({{buf}}.end(), {{param.mojom_name}}Buf.begin(), {{param.mojom_name}}Buf.end());
>>   {%- endfor %}
>> @@ -84,104 +75,28 @@
>>   {%- endfor %}
>>   {%- endmacro -%}
>>
>> -
>> -{#
>> - # \brief Deserialize a single object from data buffer and fd vector
>> - #
>> - # \param pointer If true, deserialize the object into a dereferenced pointer
>> - # \param iter If true, treat \a buf as an iterator instead of a vector
>> - # \param data_size Variable that holds the size of the vector referenced by \a buf
>> - #
>> - # Generate code to deserialize a single object, as specified in \a param,
>> - # from \a buf data buffer and \a fds fd vector.
>> - # This code is meant to be used by macro deserialize_call.
>> - #}
>> -{%- macro deserialize_param(param, pointer, loop, buf, fds, iter, data_size) -%}
>> -{{"*" if pointer}}{{param.mojom_name}} =
>> -IPADataSerializer<{{param|name_full}}>::deserialize(
>> -	{{buf}}{{- ".cbegin()" if not iter}} + {{param.mojom_name}}Start,
>> -{%- if loop.last and not iter %}
>> -	{{buf}}.cend()
>> -{%- elif not iter %}
>> -	{{buf}}.cbegin() + {{param.mojom_name}}Start + {{param.mojom_name}}BufSize
>> -{%- elif iter and loop.length == 1 %}
>> -	{{buf}} + {{data_size}}
>> -{%- else %}
>> -	{{buf}} + {{param.mojom_name}}Start + {{param.mojom_name}}BufSize
>> -{%- endif -%}
>> -{{- "," if param|has_fd}}
>> -{%- if param|has_fd %}
>> -	{{fds}}.cbegin() + {{param.mojom_name}}FdStart,
>> -{%- if loop.last %}
>> -	{{fds}}.cend()
>> -{%- else %}
>> -	{{fds}}.cbegin() + {{param.mojom_name}}FdStart + {{param.mojom_name}}FdsSize
>> -{%- endif -%}
>> -{%- endif -%}
>> -{{- "," if param|needs_control_serializer}}
>> -{%- if param|needs_control_serializer %}
>> -	&controlSerializer_
>> -{%- endif -%}
>> -);
>> -{%- endmacro -%}
>> -
>> -
>>   {#
>> - # \brief Deserialize multiple objects from data buffer and fd vector
>> + # \brief Deserialize multiple objects
>>    #
>>    # \param pointer If true, deserialize objects into pointers, and adds a null check.
>>    # \param declare If true, declare the objects in addition to deserialization.
>> - # \param iter if true, treat \a buf as an iterator instead of a vector
>> - # \param data_size Variable that holds the size of the vector referenced by \a buf
>> - #
>> - # Generate code to deserialize multiple objects, as specified in \a params
>> - # (which are the parameters to some function), from \a buf data buffer and
>> - # \a fds fd vector.
>> - # This code is meant to be used by the proxy, for deserializing after IPC calls.
>>    #
>> - # \todo Avoid intermediate vectors
>> + # Generate code to deserialize multiple objects, as specified in \a params,
>> + # from \a reader. This code is meant to be used by the proxy, for deserializing
>> + # after IPC calls.
>>    #}
>> -{%- macro deserialize_call(params, buf, fds, pointer = true, declare = false, iter = false, data_size = '', init_offset = 0) -%}
>> -{% set ns = namespace(size_offset = init_offset) %}
>> -{%- if params|length > 1 %}
>> +{%- macro deserialize_call(params, reader, pointer = true, declare = false) -%}
>>   {%- for param in params %}
>> -	[[maybe_unused]] const size_t {{param.mojom_name}}BufSize = readPOD<uint32_t>({{buf}}, {{ns.size_offset}}
>> -{%- if iter -%}
>> -, {{buf}} + {{data_size}}
>> -{%- endif -%}
>> -);
>> -	{%- set ns.size_offset = ns.size_offset + 4 %}
>> -{%- if param|has_fd %}
>> -	[[maybe_unused]] const size_t {{param.mojom_name}}FdsSize = readPOD<uint32_t>({{buf}}, {{ns.size_offset}}
>> -{%- if iter -%}
>> -, {{buf}} + {{data_size}}
>> -{%- endif -%}
>> -);
>> -	{%- set ns.size_offset = ns.size_offset + 4 %}
>> -{%- endif %}
>> -{%- endfor %}
>> -{%- endif %}
>> -{% for param in params %}
>> -{%- if loop.first %}
>> -	const size_t {{param.mojom_name}}Start = {{ns.size_offset}};
>> -{%- else %}
>> -	const size_t {{param.mojom_name}}Start = {{loop.previtem.mojom_name}}Start + {{loop.previtem.mojom_name}}BufSize;
>> -{%- endif %}
>> -{%- endfor %}
>> -{% for param in params|with_fds %}
>> -{%- if loop.first %}
>> -	const size_t {{param.mojom_name}}FdStart = 0;
>> +	auto param_{{param.mojom_name}} = IPADataSerializer<{{param|name_full}}>::deserialize({{reader}}, &controlSerializer_);
>> +	ASSERT(param_{{param.mojom_name}});
>> +
>> +{%- if pointer %}
>> +	if ({{param.mojom_name}})
>> +		*{{param.mojom_name}} = std::move(*param_{{param.mojom_name}});
>> +{%- elif declare %}
>> +	{{param|name}} &{{param.mojom_name}} = *param_{{param.mojom_name}};
>>   {%- else %}
>> -	const size_t {{param.mojom_name}}FdStart = {{loop.previtem.mojom_name}}FdStart + {{loop.previtem.mojom_name}}FdsSize;
>> +	{{param.mojom_name}} = std::move(*param_{{param.mojom_name}});
>>   {%- endif %}
>> -{%- endfor %}
>> -{% for param in params %}
>> -	{%- if pointer %}
>> -	if ({{param.mojom_name}}) {
>> -{{deserialize_param(param, pointer, loop, buf, fds, iter, data_size)|indent(16, True)}}
>> -	}
>> -	{%- else %}
>> -	{{param|name + " " if declare}}{{deserialize_param(param, pointer, loop, buf, fds, iter, data_size)|indent(8)}}
>> -	{%- endif %}
>>   {% endfor %}
>>   {%- endmacro -%}
>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl b/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl
>> index e316dd88a..9e9dd0ca6 100644
>> --- a/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl
>> +++ b/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl
>> @@ -2,22 +2,6 @@
>>    # SPDX-License-Identifier: LGPL-2.1-or-later
>>    # Copyright (C) 2020, Google Inc.
>>   -#}
>> -{#
>> - # \brief Verify that there is enough bytes to deserialize
>> - #
>> - # Generate code that verifies that \a size is not greater than \a dataSize.
>> - # Otherwise log an error with \a name and \a typename.
>> - #}
>> -{%- macro check_data_size(size, dataSize, name, typename) %}
>> -		if ({{dataSize}} < {{size}}) {
>> -			LOG(IPADataSerializer, Error)
>> -				<< "Failed to deserialize " << "{{name}}"
>> -				<< ": not enough {{typename}}, expected "
>> -				<< ({{size}}) << ", got " << ({{dataSize}});
>> -			return ret;
>> -		}
>> -{%- endmacro %}
>> -
>>
>>   {#
>>    # \brief Serialize a field into return vector
>> @@ -42,15 +26,10 @@
>>   		retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());
>>   		retFds.insert(retFds.end(), {{field.mojom_name}}Fds.begin(), {{field.mojom_name}}Fds.end());
>>   {%- elif field|is_controls %}
>> -		if (data.{{field.mojom_name}}.size() > 0) {
>> -			std::vector<uint8_t> {{field.mojom_name}};
>> -			std::tie({{field.mojom_name}}, std::ignore) =
>> -				IPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}}, cs);
>> -			appendPOD<uint32_t>(retData, {{field.mojom_name}}.size());
>> -			retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());
>> -		} else {
>> -			appendPOD<uint32_t>(retData, 0);
>> -		}
>> +		std::vector<uint8_t> {{field.mojom_name}};
>> +		std::tie({{field.mojom_name}}, std::ignore) =
>> +			IPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}}, cs);
>> +		retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());
>>   {%- elif field|is_plain_struct or field|is_array or field|is_map or field|is_str %}
>>   		std::vector<uint8_t> {{field.mojom_name}};
>>   	{%- if field|has_fd %}
>> @@ -65,10 +44,6 @@
>>   			IPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}});
>>   	{%- else %}
>>   			IPADataSerializer<{{field|name_full}}>::serialize(data.{{field.mojom_name}}, cs);
>> -	{%- endif %}
>> -		appendPOD<uint32_t>(retData, {{field.mojom_name}}.size());
>> -	{%- if field|has_fd %}
>> -		appendPOD<uint32_t>(retData, {{field.mojom_name}}Fds.size());
>>   	{%- endif %}
>>   		retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());
>>   	{%- if field|has_fd %}
>> @@ -79,89 +54,6 @@
>>   {%- endif %}
>>   {%- endmacro %}
>>
>> -
>> -{#
>> - # \brief Deserialize a field into return struct
>> - #
>> - # Generate code to deserialize \a field into object ret.
>> - # This code is meant to be used by the IPADataSerializer specialization.
>> - #}
>> -{%- macro deserializer_field(field, loop) %}
>> -{% if field|is_pod or field|is_enum %}
>> -	{%- set field_size = (field|bit_width|int / 8)|int %}
>> -		{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}
>> -		ret.{{field.mojom_name}} = IPADataSerializer<{{field|name_full}}>::deserialize(m, m + {{field_size}});
>> -	{%- if not loop.last %}
>> -		m += {{field_size}};
>> -		dataSize -= {{field_size}};
>> -	{%- endif %}
>> -{% elif field|is_fd %}
>> -	{%- set field_size = 4 %}
>> -		{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}
>> -		ret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field_size}}, n, n + 1, cs);
>> -	{%- if not loop.last %}
>> -		m += {{field_size}};
>> -		dataSize -= {{field_size}};
>> -		n += ret.{{field.mojom_name}}.isValid() ? 1 : 0;
>> -		fdsSize -= ret.{{field.mojom_name}}.isValid() ? 1 : 0;
>> -	{%- endif %}
>> -{% elif field|is_controls %}
>> -	{%- set field_size = 4 %}
>> -		{{- check_data_size(field_size, 'dataSize', field.mojom_name + 'Size', 'data')}}
>> -		const size_t {{field.mojom_name}}Size = readPOD<uint32_t>(m, 0, dataEnd);
>> -		m += {{field_size}};
>> -		dataSize -= {{field_size}};
>> -	{%- set field_size = field.mojom_name + 'Size' -%}
>> -		{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}
>> -		if ({{field.mojom_name}}Size > 0)
>> -			ret.{{field.mojom_name}} =
>> -				IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field.mojom_name}}Size, cs);
>> -	{%- if not loop.last %}
>> -		m += {{field_size}};
>> -		dataSize -= {{field_size}};
>> -	{%- endif %}
>> -{% elif field|is_plain_struct or field|is_array or field|is_map or field|is_str %}
>> -	{%- set field_size = 4 %}
>> -		{{- check_data_size(field_size, 'dataSize', field.mojom_name + 'Size', 'data')}}
>> -		const size_t {{field.mojom_name}}Size = readPOD<uint32_t>(m, 0, dataEnd);
>> -		m += {{field_size}};
>> -		dataSize -= {{field_size}};
>> -	{%- if field|has_fd %}
>> -	{%- set field_size = 4 %}
>> -		{{- check_data_size(field_size, 'dataSize', field.mojom_name + 'FdsSize', 'data')}}
>> -		const size_t {{field.mojom_name}}FdsSize = readPOD<uint32_t>(m, 0, dataEnd);
>> -		m += {{field_size}};
>> -		dataSize -= {{field_size}};
>> -		{{- check_data_size(field.mojom_name + 'FdsSize', 'fdsSize', field.mojom_name, 'fds')}}
>> -	{%- endif %}
>> -	{%- set field_size = field.mojom_name + 'Size' -%}
>> -		{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}
>> -		ret.{{field.mojom_name}} =
>> -	{%- if field|is_str %}
>> -			IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field.mojom_name}}Size);
>> -	{%- elif field|has_fd and (field|is_array or field|is_map) %}
>> -			IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field.mojom_name}}Size, n, n + {{field.mojom_name}}FdsSize, cs);
>> -	{%- elif field|has_fd and (not (field|is_array or field|is_map)) %}
>> -			IPADataSerializer<{{field|name_full}}>::deserialize(m, m + {{field.mojom_name}}Size, n, n + {{field.mojom_name}}FdsSize, cs);
>> -	{%- elif (not field|has_fd) and (field|is_array or field|is_map) %}
>> -			IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field.mojom_name}}Size, cs);
>> -	{%- else %}
>> -			IPADataSerializer<{{field|name_full}}>::deserialize(m, m + {{field.mojom_name}}Size, cs);
>> -	{%- endif %}
>> -	{%- if not loop.last %}
>> -		m += {{field_size}};
>> -		dataSize -= {{field_size}};
>> -	{%- if field|has_fd %}
>> -		n += {{field.mojom_name}}FdsSize;
>> -		fdsSize -= {{field.mojom_name}}FdsSize;
>> -	{%- endif %}
>> -	{%- endif %}
>> -{% else %}
>> -		/* Unknown deserialization for {{field.mojom_name}}. */
>> -{%- endif %}
>> -{%- endmacro %}
>> -
>> -
>>   {#
>>    # \brief Serialize a struct
>>    #
>> @@ -194,126 +86,30 @@
>>
>>
>>   {#
>> - # \brief Deserialize a struct that has fds
>> + # \brief Deserialize a struct
>>    #
>> - # Generate code for IPADataSerializer specialization, for deserializing
>> - # \a struct, in the case that \a struct has file descriptors.
>> + # Generate code for IPADataSerializer specialization, for deserializing \a struct.
>>    #}
>> -{%- macro deserializer_fd(struct) %}
>> -	static {{struct|name_full}}
>> -	deserialize(std::vector<uint8_t> &data,
>> -		    std::vector<SharedFD> &fds,
>> -{%- if struct|needs_control_serializer %}
>> -		    ControlSerializer *cs)
>> -{%- else %}
>> -		    ControlSerializer *cs = nullptr)
>> -{%- endif %}
>> -	{
>> -		return IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), fds.cbegin(), fds.cend(), cs);
>> -	}
>> -
>> +{%- macro deserializer(struct) %}
>>   {# \todo Don't inline this function #}
>> -	static {{struct|name_full}}
>> -	deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> -		    std::vector<uint8_t>::const_iterator dataEnd,
>> -		    std::vector<SharedFD>::const_iterator fdsBegin,
>> -		    std::vector<SharedFD>::const_iterator fdsEnd,
>> +	[[nodiscard]] static std::optional<{{struct|name_full}}>
>> +	deserialize(SeriReader &reader,
>>   {%- if struct|needs_control_serializer %}
>>   		    ControlSerializer *cs)
>>   {%- else %}
>>   		    [[maybe_unused]] ControlSerializer *cs = nullptr)
>>   {%- endif %}
>>   	{
>> -		{{struct|name_full}} ret;
>> -		std::vector<uint8_t>::const_iterator m = dataBegin;
>> -		std::vector<SharedFD>::const_iterator n = fdsBegin;
>> -
>> -		size_t dataSize = std::distance(dataBegin, dataEnd);
>> -		[[maybe_unused]] size_t fdsSize = std::distance(fdsBegin, fdsEnd);
>> -{%- for field in struct.fields -%}
>> -{{deserializer_field(field, loop)}}
>> +{%- for field in struct.fields %}
>> +		auto {{field.mojom_name}} = IPADataSerializer<{{field|name_full}}>::deserialize(reader, cs);
>> +		if (!{{field.mojom_name}})
>> +			return {};
>>   {%- endfor %}
>> -		return ret;
>> -	}
>> -{%- endmacro %}
>> -
>> -{#
>> - # \brief Deserialize a struct that has fds, using non-fd
>> - #
>> - # Generate code for IPADataSerializer specialization, for deserializing
>> - # \a struct, in the case that \a struct has no file descriptors but requires
>> - # deserializers with file descriptors.
>> - #}
>> -{%- macro deserializer_fd_simple(struct) %}
>> -	static {{struct|name_full}}
>> -	deserialize(std::vector<uint8_t> &data,
>> -		    [[maybe_unused]] std::vector<SharedFD> &fds,
>> -		    ControlSerializer *cs = nullptr)
>> -	{
>> -		return IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), cs);
>> -	}
>> -
>> -	static {{struct|name_full}}
>> -	deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> -		    std::vector<uint8_t>::const_iterator dataEnd,
>> -		    [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin,
>> -		    [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,
>> -		    ControlSerializer *cs = nullptr)
>> -	{
>> -		return IPADataSerializer<{{struct|name_full}}>::deserialize(dataBegin, dataEnd, cs);
>> -	}
>> -{%- endmacro %}
>> -
>> -
>> -{#
>> - # \brief Deserialize a struct that has no fds
>> - #
>> - # Generate code for IPADataSerializer specialization, for deserializing
>> - # \a struct, in the case that \a struct does not have file descriptors.
>> - #}
>> -{%- macro deserializer_no_fd(struct) %}
>> -	static {{struct|name_full}}
>> -	deserialize(std::vector<uint8_t> &data,
>> -{%- if struct|needs_control_serializer %}
>> -		    ControlSerializer *cs)
>> -{%- else %}
>> -		    ControlSerializer *cs = nullptr)
>> -{%- endif %}
>> -	{
>> -		return IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), cs);
>> -	}
>>
>> -{# \todo Don't inline this function #}
>> -	static {{struct|name_full}}
>> -	deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> -		    std::vector<uint8_t>::const_iterator dataEnd,
>> -{%- if struct|needs_control_serializer %}
>> -		    ControlSerializer *cs)
>> -{%- else %}
>> -		    [[maybe_unused]] ControlSerializer *cs = nullptr)
>> -{%- endif %}
>> -	{
>> -		{{struct|name_full}} ret;
>> -		std::vector<uint8_t>::const_iterator m = dataBegin;
>> -
>> -		size_t dataSize = std::distance(dataBegin, dataEnd);
>> -{%- for field in struct.fields -%}
>> -{{deserializer_field(field, loop)}}
>> +		return { {
>> +{%- for field in struct.fields %}
>> +			std::move(*{{field.mojom_name}}),
>>   {%- endfor %}
>> -		return ret;
>> +		} };
>>   	}
>>   {%- endmacro %}
>> -
>> -{#
>> - # \brief Deserialize a struct
>> - #
>> - # Generate code for IPADataSerializer specialization, for deserializing \a struct.
>> - #}
>> -{%- macro deserializer(struct) %}
>> -{%- if struct|has_fd %}
>> -{{deserializer_fd(struct)}}
>> -{%- else %}
>> -{{deserializer_no_fd(struct)}}
>> -{{deserializer_fd_simple(struct)}}
>> -{%- endif %}
>> -{%- endmacro %}
>> --
>> 2.49.0
>>



More information about the libcamera-devel mailing list