[{"id":36588,"web_url":"https://patchwork.libcamera.org/comment/36588/","msgid":"<gdoghqz4ruaoqdwjpl7lvdtotpdt2jw2t3s3wbb6obk4f2sm7l@26zafvg5qunq>","date":"2025-11-01T11:19:30","subject":"Re: [RFC PATCH v2 2/2] utils: codegen: ipc: Simplify deserialization","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Tue, Oct 14, 2025 at 01:57:52PM +0200, Barnabás Pőcze wrote:\n> First, introduce the `SeriReader` type, which is a collection\n> of bytes and file descriptors, with the appropriate methods to\n> consume the first couples bytes / file descriptors.\n>\n> Then a new method is added to `IPCMessage` that returns an appropriately\n> constructed `SeriReader` for parsing the message contents.\n>\n> Then three of the four `deserialize()` overloads are removed, the\n> remaining one is converted to have a single `SeriReader` and an\n> optional `ControlSerializer` as arguments.\n>\n> The remaining `deserialize()` function is also changed to return an\n> `std::optional` to be able to report deserialization failure.\n>\n> There is also a more fundamental change in the serialization: previously,\n> the number of bytes taken up by an item has been written before the serialized\n> bytes (and conditionally the number of file descriptors) when the item is\n> serialized as part of a struct, array, map, function parameter list. This\n> is changed: the number of bytes and file descriptors are *not* serialized\n> into the final buffer. This affords some simplification of the serialization\n> related code paths, but most importantly, it greatly simplifies and unifies\n> how an object is (de)serialized because the deserialization of every object\n> becomes completely self-contained.\n>\n> As a consequence of that, strings now include their lengths as part of the\n> string serialization, and it is not left to an \"upper\" layer.\n>\n> Another consequence is that an \"out parameter\" of a remote function call\n> must be deserialized if a later out parameter is needed, even if itself\n> is not. This does not appear to be a great limitation since in all\n> situations presently none of the out parameters are ignored.\n>\n> Finally, the code generation templates are adapted to the above changes.\n> This allows the simplification of the deserialization templates as now\n> calling `IPADataSerializer<T>::deserialize(reader, &controlSerializer_)`\n> is appropriate for any type.\n>\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=269\n> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> ---\n> change in v2:\n>   * rebase\n>   * adjust comments\n>\n> v1: https://patchwork.libcamera.org/patch/23373/\n> ---\n>  .../libcamera/internal/ipa_data_serializer.h  | 234 +++---------\n>  include/libcamera/internal/ipc_pipe.h         |   3 +\n>  include/libcamera/internal/meson.build        |   1 +\n>  include/libcamera/internal/serialization.h    |  86 +++++\n>  src/libcamera/ipa_data_serializer.cpp         | 352 ++++--------------\n>  src/libcamera/ipc_pipe.cpp                    |   5 +\n>  src/libcamera/meson.build                     |   1 +\n>  src/libcamera/serialization.cpp               |  60 +++\n>  test/ipc/unixsocket_ipc.cpp                   |  18 +-\n>  .../generated_serializer_test.cpp             |  17 +-\n>  .../ipa_data_serializer_test.cpp              |  32 +-\n>  .../module_ipa_proxy.cpp.tmpl                 |  30 +-\n>  .../module_ipa_proxy.h.tmpl                   |   5 +-\n>  .../module_ipa_proxy_worker.cpp.tmpl          |   5 +-\n>  .../libcamera_templates/proxy_functions.tmpl  | 113 +-----\n>  .../libcamera_templates/serializer.tmpl       | 238 +-----------\n>  16 files changed, 372 insertions(+), 828 deletions(-)\n\nThat's quite dense.\n\nBefore getting into the details, I would like to better understand how\nthis could be unified with the control serialization effort you're\npushing for MetadataList.\n\nI think that, if we want to get into redesigining the serialization of\nat the IPC border, we should consider it in the larger re-design\nof the libcamera API C requirement.\n\nSo let me digress a bit.\n\nMetadataList addresses the first need to serialize controls at the API\nlevel in order to avoid concurrent access to a storage that doesn't\nsupport thread-safe access.\n\nAs we know the application thread could access the control list while\nthe libcamera thread populates it. The solution we designed is to\nserialize controls into a binary buffer and implement an API on top of\nit that guarantees safe read access to a portion of the buffer while\ndirecting write access to a portion of the buffer that is not\navailable to readers.\n\nThe MetadataList class implements a control serialization protocol\nwhile we have a different controls serialization protocol for IPC\nimplemented by the ControlSerializer class and used over IPC.\n\nDo you envision re-implementing any of the two on top of the other ?\nAs MetadataList is \"newer\" I presume the IPC ControlSerializer should\nbe re-implemented on top the serialization format once broken-out from\nMetadataList ?\n\nIf that's the plan, are there implications on the this patch or, as\nI understand it, the IPADataSerializer still delegates the actual\ncontrol serialization format to the ControlSerializer implementation\nand your patch only changes how the data around serialized ControlList\nand ControlInfoMap are read ?\n\nNow on the patch\n\n\n>  create mode 100644 include/libcamera/internal/serialization.h\n>  create mode 100644 src/libcamera/serialization.cpp\n>\n> diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h\n> index 927f5c4c7..3b2e0029b 100644\n> --- a/include/libcamera/internal/ipa_data_serializer.h\n> +++ b/include/libcamera/internal/ipa_data_serializer.h\n> @@ -7,6 +7,7 @@\n>\n>  #pragma once\n>\n> +#include <optional>\n>  #include <stdint.h>\n>  #include <string.h>\n>  #include <tuple>\n> @@ -23,6 +24,7 @@\n>  #include <libcamera/ipa/ipa_interface.h>\n>\n>  #include \"libcamera/internal/control_serializer.h\"\n> +#include \"libcamera/internal/serialization.h\"\n>\n>  namespace libcamera {\n>\n> @@ -39,26 +41,6 @@ void appendPOD(std::vector<uint8_t> &vec, T val)\n>  \tmemcpy(&*(vec.end() - byteWidth), &val, byteWidth);\n>  }\n>\n> -template<typename T,\n> -\t std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>\n> -T readPOD(std::vector<uint8_t>::const_iterator it, size_t pos,\n> -\t  std::vector<uint8_t>::const_iterator end)\n> -{\n> -\tASSERT(pos + it < end);\n> -\n> -\tT ret = 0;\n> -\tmemcpy(&ret, &(*(it + pos)), sizeof(ret));\n> -\n> -\treturn ret;\n> -}\n> -\n> -template<typename T,\n> -\t std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>\n> -T readPOD(std::vector<uint8_t> &vec, size_t pos)\n> -{\n> -\treturn readPOD<T>(vec.cbegin(), pos, vec.end());\n> -}\n> -\n>  } /* namespace */\n>\n>  template<typename T, typename = void>\n> @@ -68,20 +50,8 @@ public:\n>  \tstatic std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>\n>  \tserialize(const T &data, ControlSerializer *cs = nullptr);\n>\n> -\tstatic T deserialize(const std::vector<uint8_t> &data,\n> -\t\t\t     ControlSerializer *cs = nullptr);\n> -\tstatic T deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t\t     std::vector<uint8_t>::const_iterator dataEnd,\n> -\t\t\t     ControlSerializer *cs = nullptr);\n> -\n> -\tstatic T deserialize(const std::vector<uint8_t> &data,\n> -\t\t\t     const std::vector<SharedFD> &fds,\n> -\t\t\t     ControlSerializer *cs = nullptr);\n> -\tstatic T deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t\t     std::vector<uint8_t>::const_iterator dataEnd,\n> -\t\t\t     std::vector<SharedFD>::const_iterator fdsBegin,\n> -\t\t\t     std::vector<SharedFD>::const_iterator fdsEnd,\n> -\t\t\t     ControlSerializer *cs = nullptr);\n> +\t[[nodiscard]] static std::optional<T>\n> +\tdeserialize(SeriReader &reader, ControlSerializer *cs = nullptr);\n\nnice!\n\n>  };\n>\n>  #ifndef __DOXYGEN__\n> @@ -93,8 +63,6 @@ public:\n>   *\n>   * For every element in the vector:\n>   *\n> - * 4 bytes - uint32_t Size of element, in bytes\n> - * 4 bytes - uint32_t Number of fds for the element\n>   * X bytes - Serialized element\n>   *\n>   * \\todo Support elements that are references\n> @@ -121,9 +89,6 @@ public:\n>  \t\t\tstd::tie(dvec, fvec) =\n>  \t\t\t\tIPADataSerializer<V>::serialize(it, cs);\n>\n> -\t\t\tappendPOD<uint32_t>(dataVec, dvec.size());\n> -\t\t\tappendPOD<uint32_t>(dataVec, fvec.size());\n> -\n>  \t\t\tdataVec.insert(dataVec.end(), dvec.begin(), dvec.end());\n>  \t\t\tfdsVec.insert(fdsVec.end(), fvec.begin(), fvec.end());\n>  \t\t}\n> @@ -131,49 +96,22 @@ public:\n>  \t\treturn { dataVec, fdsVec };\n>  \t}\n>\n> -\tstatic std::vector<V> deserialize(std::vector<uint8_t> &data, ControlSerializer *cs = nullptr)\n> +\t[[nodiscard]] static std::optional<std::vector<V>>\n> +\tdeserialize(SeriReader &reader, ControlSerializer *cs = nullptr)\n>  \t{\n> -\t\treturn deserialize(data.cbegin(), data.cend(), cs);\n> -\t}\n> +\t\tuint32_t vecLen;\n> +\t\tif (!reader.read(vecLen))\n> +\t\t\treturn {};\n>\n> -\tstatic std::vector<V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t\t\t\t  std::vector<uint8_t>::const_iterator dataEnd,\n> -\t\t\t\t\t  ControlSerializer *cs = nullptr)\n> -\t{\n> -\t\tstd::vector<SharedFD> fds;\n> -\t\treturn deserialize(dataBegin, dataEnd, fds.cbegin(), fds.cend(), cs);\n> -\t}\n> +\t\tstd::vector<V> ret;\n> +\t\tret.reserve(vecLen);\n>\n> -\tstatic std::vector<V> deserialize(std::vector<uint8_t> &data, std::vector<SharedFD> &fds,\n> -\t\t\t\t\t  ControlSerializer *cs = nullptr)\n> -\t{\n> -\t\treturn deserialize(data.cbegin(), data.cend(), fds.cbegin(), fds.cend(), cs);\n> -\t}\n> -\n> -\tstatic std::vector<V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t\t\t\t  std::vector<uint8_t>::const_iterator dataEnd,\n> -\t\t\t\t\t  std::vector<SharedFD>::const_iterator fdsBegin,\n> -\t\t\t\t\t  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,\n> -\t\t\t\t\t  ControlSerializer *cs = nullptr)\n> -\t{\n> -\t\tuint32_t vecLen = readPOD<uint32_t>(dataBegin, 0, dataEnd);\n> -\t\tstd::vector<V> ret(vecLen);\n> -\n> -\t\tstd::vector<uint8_t>::const_iterator dataIter = dataBegin + 4;\n> -\t\tstd::vector<SharedFD>::const_iterator fdIter = fdsBegin;\n>  \t\tfor (uint32_t i = 0; i < vecLen; i++) {\n> -\t\t\tuint32_t sizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd);\n> -\t\t\tuint32_t sizeofFds = readPOD<uint32_t>(dataIter, 4, dataEnd);\n> -\t\t\tdataIter += 8;\n> -\n> -\t\t\tret[i] = IPADataSerializer<V>::deserialize(dataIter,\n> -\t\t\t\t\t\t\t\t   dataIter + sizeofData,\n> -\t\t\t\t\t\t\t\t   fdIter,\n> -\t\t\t\t\t\t\t\t   fdIter + sizeofFds,\n> -\t\t\t\t\t\t\t\t   cs);\n> -\n> -\t\t\tdataIter += sizeofData;\n> -\t\t\tfdIter += sizeofFds;\nMuch nicer from a code point of view\n\nLet me address this in pieces\n\n> +\t\t\tauto item = IPADataSerializer<V>::deserialize(reader, cs);\n\nitem (optionally) contains a variable of type V which has been copied\nfrom the binary buffer\n\n> +\t\t\tif (!item)\n> +\t\t\t\treturn {};\n> +\n> +\t\t\tret.emplace_back(std::move(*item));\n\nThen we move item and call emplace_back which calls the move\nconstructor of type V.\n\nHowever, since the data from the serialized buffer have already been\ncopied to 'item' do move() and emplace_back() actually help ? Won't we\nend up doing a copy inside ret anyway ?\n\n>  \t\t}\n>\n>  \t\treturn ret;\n> @@ -187,11 +125,7 @@ public:\n>   *\n>   * For every pair in the map:\n>   *\n> - * 4 bytes - uint32_t Size of key, in bytes\n> - * 4 bytes - uint32_t Number of fds for the key\n>   * X bytes - Serialized key\n> - * 4 bytes - uint32_t Size of value, in bytes\n> - * 4 bytes - uint32_t Number of fds for the value\n>   * X bytes - Serialized value\n>   *\n>   * \\todo Support keys or values that are references\n> @@ -218,18 +152,12 @@ public:\n>  \t\t\tstd::tie(dvec, fvec) =\n>  \t\t\t\tIPADataSerializer<K>::serialize(it.first, cs);\n>\n> -\t\t\tappendPOD<uint32_t>(dataVec, dvec.size());\n> -\t\t\tappendPOD<uint32_t>(dataVec, fvec.size());\n> -\n>  \t\t\tdataVec.insert(dataVec.end(), dvec.begin(), dvec.end());\n>  \t\t\tfdsVec.insert(fdsVec.end(), fvec.begin(), fvec.end());\n>\n>  \t\t\tstd::tie(dvec, fvec) =\n>  \t\t\t\tIPADataSerializer<V>::serialize(it.second, cs);\n>\n> -\t\t\tappendPOD<uint32_t>(dataVec, dvec.size());\n> -\t\t\tappendPOD<uint32_t>(dataVec, fvec.size());\n> -\n>  \t\t\tdataVec.insert(dataVec.end(), dvec.begin(), dvec.end());\n>  \t\t\tfdsVec.insert(fdsVec.end(), fvec.begin(), fvec.end());\n>  \t\t}\n> @@ -237,63 +165,27 @@ public:\n>  \t\treturn { dataVec, fdsVec };\n>  \t}\n>\n> -\tstatic std::map<K, V> deserialize(std::vector<uint8_t> &data, ControlSerializer *cs = nullptr)\n> +\t[[nodiscard]] static std::optional<std::map<K, V>>\n> +\tdeserialize(SeriReader &reader, ControlSerializer *cs = nullptr)\n>  \t{\n> -\t\treturn deserialize(data.cbegin(), data.cend(), cs);\n> -\t}\n> +\t\tuint32_t mapLen;\n> +\t\tif (!reader.read(mapLen))\n> +\t\t\treturn {};\n>\n> -\tstatic std::map<K, V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t\t\t\t  std::vector<uint8_t>::const_iterator dataEnd,\n> -\t\t\t\t\t  ControlSerializer *cs = nullptr)\n> -\t{\n> -\t\tstd::vector<SharedFD> fds;\n> -\t\treturn deserialize(dataBegin, dataEnd, fds.cbegin(), fds.cend(), cs);\n> -\t}\n> -\n> -\tstatic std::map<K, V> deserialize(std::vector<uint8_t> &data, std::vector<SharedFD> &fds,\n> -\t\t\t\t\t  ControlSerializer *cs = nullptr)\n> -\t{\n> -\t\treturn deserialize(data.cbegin(), data.cend(), fds.cbegin(), fds.cend(), cs);\n> -\t}\n> -\n> -\tstatic std::map<K, V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t\t\t\t  std::vector<uint8_t>::const_iterator dataEnd,\n> -\t\t\t\t\t  std::vector<SharedFD>::const_iterator fdsBegin,\n> -\t\t\t\t\t  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,\n> -\t\t\t\t\t  ControlSerializer *cs = nullptr)\n> -\t{\n>  \t\tstd::map<K, V> ret;\n>\n> -\t\tuint32_t mapLen = readPOD<uint32_t>(dataBegin, 0, dataEnd);\n> -\n> -\t\tstd::vector<uint8_t>::const_iterator dataIter = dataBegin + 4;\n> -\t\tstd::vector<SharedFD>::const_iterator fdIter = fdsBegin;\n>  \t\tfor (uint32_t i = 0; i < mapLen; i++) {\n> -\t\t\tuint32_t sizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd);\n> -\t\t\tuint32_t sizeofFds = readPOD<uint32_t>(dataIter, 4, dataEnd);\n> -\t\t\tdataIter += 8;\n> -\n> -\t\t\tK key = IPADataSerializer<K>::deserialize(dataIter,\n> -\t\t\t\t\t\t\t\t  dataIter + sizeofData,\n> -\t\t\t\t\t\t\t\t  fdIter,\n> -\t\t\t\t\t\t\t\t  fdIter + sizeofFds,\n> -\t\t\t\t\t\t\t\t  cs);\n> -\n> -\t\t\tdataIter += sizeofData;\n> -\t\t\tfdIter += sizeofFds;\n> -\t\t\tsizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd);\n> -\t\t\tsizeofFds = readPOD<uint32_t>(dataIter, 4, dataEnd);\n> -\t\t\tdataIter += 8;\n> -\n> -\t\t\tconst V value = IPADataSerializer<V>::deserialize(dataIter,\n> -\t\t\t\t\t\t\t\t\t  dataIter + sizeofData,\n> -\t\t\t\t\t\t\t\t\t  fdIter,\n> -\t\t\t\t\t\t\t\t\t  fdIter + sizeofFds,\n> -\t\t\t\t\t\t\t\t\t  cs);\n> -\t\t\tret.insert({ key, value });\n> -\n> -\t\t\tdataIter += sizeofData;\n> -\t\t\tfdIter += sizeofFds;\n> +\t\t\tauto key = IPADataSerializer<K>::deserialize(reader, cs);\n> +\t\t\tif (!key)\n> +\t\t\t\treturn {};\n> +\n> +\t\t\tauto value = IPADataSerializer<V>::deserialize(reader, cs);\n> +\t\t\tif (!value)\n> +\t\t\t\treturn {};\n> +\n> +\t\t\tauto [it, inserted] = ret.try_emplace(std::move(*key), std::move(*value));\n> +\t\t\tif (!inserted)\n> +\t\t\t\treturn {};\n>  \t\t}\n>\n>  \t\treturn ret;\n> @@ -314,33 +206,14 @@ public:\n>  \t\treturn { dataVec, {} };\n>  \t}\n>\n> -\tstatic Flags<E> deserialize(std::vector<uint8_t> &data,\n> -\t\t\t\t    [[maybe_unused]] ControlSerializer *cs = nullptr)\n> +\t[[nodiscard]] static std::optional<Flags<E>>\n> +\tdeserialize(SeriReader &reader, [[maybe_unused]] ControlSerializer *cs = nullptr)\n>  \t{\n> -\t\treturn deserialize(data.cbegin(), data.cend());\n> -\t}\n> +\t\tuint32_t value;\n> +\t\tif (!reader.read(value))\n> +\t\t\treturn {};\n>\n> -\tstatic Flags<E> deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t\t\t    std::vector<uint8_t>::const_iterator dataEnd,\n> -\t\t\t\t    [[maybe_unused]] ControlSerializer *cs = nullptr)\n> -\t{\n> -\t\treturn Flags<E>{ static_cast<E>(readPOD<uint32_t>(dataBegin, 0, dataEnd)) };\n> -\t}\n> -\n> -\tstatic Flags<E> deserialize(std::vector<uint8_t> &data,\n> -\t\t\t\t    [[maybe_unused]] std::vector<SharedFD> &fds,\n> -\t\t\t\t    [[maybe_unused]] ControlSerializer *cs = nullptr)\n> -\t{\n> -\t\treturn deserialize(data.cbegin(), data.cend());\n> -\t}\n> -\n> -\tstatic Flags<E> deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t\t\t    std::vector<uint8_t>::const_iterator dataEnd,\n> -\t\t\t\t    [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin,\n> -\t\t\t\t    [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,\n> -\t\t\t\t    [[maybe_unused]] ControlSerializer *cs = nullptr)\n> -\t{\n> -\t\treturn deserialize(dataBegin, dataEnd);\n> +\t\treturn Flags<E>{ static_cast<E>(value) };\n>  \t}\n>  };\n>\n> @@ -360,33 +233,14 @@ public:\n>  \t\treturn { dataVec, {} };\n>  \t}\n>\n> -\tstatic E deserialize(std::vector<uint8_t> &data,\n> -\t\t\t     [[maybe_unused]] ControlSerializer *cs = nullptr)\n> -\t{\n> -\t\treturn deserialize(data.cbegin(), data.cend());\n> -\t}\n> -\n> -\tstatic E deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t\t     std::vector<uint8_t>::const_iterator dataEnd,\n> -\t\t\t     [[maybe_unused]] ControlSerializer *cs = nullptr)\n> -\t{\n> -\t\treturn static_cast<E>(readPOD<U>(dataBegin, 0, dataEnd));\n> -\t}\n> -\n> -\tstatic E deserialize(std::vector<uint8_t> &data,\n> -\t\t\t     [[maybe_unused]] std::vector<SharedFD> &fds,\n> -\t\t\t     [[maybe_unused]] ControlSerializer *cs = nullptr)\n> +\t[[nodiscard]] static std::optional<E>\n> +\tdeserialize(SeriReader &reader, [[maybe_unused]] ControlSerializer *cs = nullptr)\n>  \t{\n> -\t\treturn deserialize(data.cbegin(), data.cend());\n> -\t}\n> +\t\tU value;\n> +\t\tif (!reader.read(value))\n> +\t\t\treturn {};\n>\n> -\tstatic E deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t\t     std::vector<uint8_t>::const_iterator dataEnd,\n> -\t\t\t     [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin,\n> -\t\t\t     [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,\n> -\t\t\t     [[maybe_unused]] ControlSerializer *cs = nullptr)\n> -\t{\n> -\t\treturn deserialize(dataBegin, dataEnd);\n> +\t\treturn static_cast<E>(value);\n>  \t}\n>  };\n>\n> diff --git a/include/libcamera/internal/ipc_pipe.h b/include/libcamera/internal/ipc_pipe.h\n> index 418c4622f..2b6cde042 100644\n> --- a/include/libcamera/internal/ipc_pipe.h\n> +++ b/include/libcamera/internal/ipc_pipe.h\n> @@ -14,6 +14,7 @@\n>  #include <libcamera/base/signal.h>\n>\n>  #include \"libcamera/internal/ipc_unixsocket.h\"\n> +#include \"libcamera/internal/serialization.h\"\n>\n>  namespace libcamera {\n>\n> @@ -40,6 +41,8 @@ public:\n>  \tconst std::vector<uint8_t> &data() const { return data_; }\n>  \tconst std::vector<SharedFD> &fds() const { return fds_; }\n>\n> +\tSeriReader reader() const { return { data(), fds() }; }\n> +\n>  private:\n>  \tHeader header_;\n>\n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index 45c299f6a..a11a52a01 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -39,6 +39,7 @@ libcamera_internal_headers = files([\n>      'process.h',\n>      'pub_key.h',\n>      'request.h',\n> +    'serialization.h',\n>      'shared_mem_object.h',\n>      'source_paths.h',\n>      'sysfs.h',\n> diff --git a/include/libcamera/internal/serialization.h b/include/libcamera/internal/serialization.h\n> new file mode 100644\n> index 000000000..f20fd8ca7\n> --- /dev/null\n> +++ b/include/libcamera/internal/serialization.h\n> @@ -0,0 +1,86 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board Oy\n> + *\n> + * Data (de)serialization helper structures\n> + */\n> +#pragma once\n> +\n> +#include <cstddef>\n> +#include <optional>\n> +#include <stdint.h>\n> +#include <string.h>\n> +#include <tuple>\n> +#include <type_traits>\n> +\n> +#include <libcamera/base/shared_fd.h>\n> +#include <libcamera/base/span.h>\n> +\n> +namespace libcamera {\n> +\n> +class SeriReader\n> +{\n> +public:\n> +\tSeriReader(Span<const uint8_t> data, Span<const SharedFD> fds = {})\n> +\t\t: data_(reinterpret_cast<const std::byte *>(data.data()),\n> +\t\t\treinterpret_cast<const std::byte *>(data.data() + data.size())),\n> +\t\t  fds_(fds)\n> +\t{\n> +\t}\n> +\n> +\t[[nodiscard]] bool empty() const { return data_.empty() && fds_.empty(); }\n> +\n> +\t[[nodiscard]] const std::byte *consume(std::size_t s)\n> +\t{\n> +\t\tif (data_.size() < s)\n> +\t\t\treturn nullptr;\n> +\n> +\t\tconst auto *p = data_.data();\n> +\t\tdata_ = data_.subspan(s);\n> +\n> +\t\treturn p;\n> +\t}\n\nI'm missing why this need to be public, shouldn't the callers use the\nread() functions to access data instead of accessing the raw buffer ?\n\nI see 4 occurrences, and it seems the reason is to support handling of\nstrings, ControlInfoMap and ControlList. Is this because they're just\na bit special ?\n\n> +\n> +\ttemplate<typename... Ts>\n\nNow, not that I'm against template packs, but I see this being only\nused to do things like\n\n\tif (!reader.read(offset, length))\n\nisn't it equivalent to do\n\n\tif (!reader.read(offset))\n\tif (!reader.read(length))\n\nDo you envision other usage for supporting multiple template arguments ?\n\n\n> +\t[[nodiscard]] bool read(Ts &...xs)\n> +\t{\n> +\t\tstatic_assert((std::is_trivially_copyable_v<Ts> && ...));\n> +\n> +\t\tconst auto *p = consume((sizeof(xs) + ...));\n> +\t\tif (p)\n> +\t\t\t((memcpy(&xs, p, sizeof(xs)), p += sizeof(xs)), ...);\n> +\n> +\t\treturn p;\n> +\t}\n> +\n> +\ttemplate<typename... Ts>\n> +\t[[nodiscard]] auto read()\n> +\t{\n> +\t\tstd::tuple<Ts...> xs;\n> +\t\tbool ok = std::apply([&](auto &...vs) {\n> +\t\t\treturn read(vs...);\n> +\t\t}, xs);\n\nif I\n\n                if (!ok)\n                        return std::nullopt;\n\nhere and remove the \"ok ? : \" below\n\n../include/libcamera/internal/serialization.h:67:37: error: inconsistent deduction for auto return type: ‘std::nullopt_t’ and then ‘std::optional<double>’\n   67 |                         return std::optional(std::get<0>(xs));\n\n\n\n> +\n> +\t\tif constexpr (sizeof...(Ts) == 1)\n> +\t\t\treturn ok ? std::optional(std::get<0>(xs)) : std::nullopt;\n> +\t\telse\n> +\t\t\treturn ok ? std::optional(xs) : std::nullopt;\n\nWhile it works this way....\n\nNot complaining about it, just wondering why the compiler can deduce\nin one case and not in the other\n\n> +\t}\n> +\n> +\t[[nodiscard]] const SharedFD *popFd()\n> +\t{\n> +\t\tif (fds_.empty())\n> +\t\t\treturn nullptr;\n> +\n> +\t\tconst auto *p = fds_.data();\n> +\t\tfds_ = fds_.subspan(1);\n> +\n> +\t\treturn p;\n> +\t}\n> +\n> +private:\n> +\tSpan<const std::byte> data_;\n> +\tSpan<const SharedFD> fds_;\n> +};\n> +\n> +} /* namespace libcamera */\n\nDo you plan a SeriWriter ? :)\n\nThe rest looks good (I didn't look at the mojo part) and there's no\ndoubt this is better.\n\nHowever, I would like to see it tested with an IPC running isolated.\nDid you have chance to give it a spin ?\n\nThanks\n  j\n\n\n> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp\n> index 0537f785b..ecb68c94e 100644\n> --- a/src/libcamera/ipa_data_serializer.cpp\n> +++ b/src/libcamera/ipa_data_serializer.cpp\n> @@ -50,42 +50,6 @@ namespace {\n>   * generated IPA proxies.\n>   */\n>\n> -/**\n> - * \\fn template<typename T> T readPOD(std::vector<uint8_t>::iterator it, size_t pos,\n> - * \t\t\t\t      std::vector<uint8_t>::iterator end)\n> - * \\brief Read POD from byte vector, in little-endian order\n> - * \\tparam T Type of POD to read\n> - * \\param[in] it Iterator of byte vector to read from\n> - * \\param[in] pos Index in byte vector to read from\n> - * \\param[in] end Iterator marking end of byte vector\n> - *\n> - * This function is meant to be used by the IPA data serializer, and the\n> - * generated IPA proxies.\n> - *\n> - * If the \\a pos plus the byte-width of the desired POD is past \\a end, it is\n> - * a fata error will occur, as it means there is insufficient data for\n> - * deserialization, which should never happen.\n> - *\n> - * \\return The POD read from \\a it at index \\a pos\n> - */\n> -\n> -/**\n> - * \\fn template<typename T> T readPOD(std::vector<uint8_t> &vec, size_t pos)\n> - * \\brief Read POD from byte vector, in little-endian order\n> - * \\tparam T Type of POD to read\n> - * \\param[in] vec Byte vector to read from\n> - * \\param[in] pos Index in vec to start reading from\n> - *\n> - * This function is meant to be used by the IPA data serializer, and the\n> - * generated IPA proxies.\n> - *\n> - * If the \\a pos plus the byte-width of the desired POD is past the end of\n> - * \\a vec, a fatal error will occur, as it means there is insufficient data\n> - * for deserialization, which should never happen.\n> - *\n> - * \\return The POD read from \\a vec at index \\a pos\n> - */\n> -\n>  } /* namespace */\n>\n>  /**\n> @@ -106,80 +70,13 @@ namespace {\n>\n>  /**\n>   * \\fn template<typename T> IPADataSerializer<T>::deserialize(\n> - * \tconst std::vector<uint8_t> &data,\n> + * \tSeriReader &reader,\n>   * \tControlSerializer *cs = nullptr)\n> - * \\brief Deserialize byte vector into an object\n> + * \\brief Deserialize bytes and file descriptors vector into an object\n>   * \\tparam T Type of object to deserialize to\n> - * \\param[in] data Byte vector to deserialize from\n> + * \\param[in] reader Source of bytes and file descriptors\n>   * \\param[in] cs ControlSerializer\n>   *\n> - * This version of deserialize() can be used if the object type \\a T and its\n> - * members don't have any SharedFD.\n> - *\n> - * \\a cs is only necessary if the object type \\a T or its members contain\n> - * ControlList or ControlInfoMap.\n> - *\n> - * \\return The deserialized object\n> - */\n> -\n> -/**\n> - * \\fn template<typename T> IPADataSerializer<T>::deserialize(\n> - * \tstd::vector<uint8_t>::const_iterator dataBegin,\n> - * \tstd::vector<uint8_t>::const_iterator dataEnd,\n> - * \tControlSerializer *cs = nullptr)\n> - * \\brief Deserialize byte vector into an object\n> - * \\tparam T Type of object to deserialize to\n> - * \\param[in] dataBegin Begin iterator of byte vector to deserialize from\n> - * \\param[in] dataEnd End iterator of byte vector to deserialize from\n> - * \\param[in] cs ControlSerializer\n> - *\n> - * This version of deserialize() can be used if the object type \\a T and its\n> - * members don't have any SharedFD.\n> - *\n> - * \\a cs is only necessary if the object type \\a T or its members contain\n> - * ControlList or ControlInfoMap.\n> - *\n> - * \\return The deserialized object\n> - */\n> -\n> -/**\n> - * \\fn template<typename T> IPADataSerializer<T>::deserialize(\n> - * \tconst std::vector<uint8_t> &data,\n> - * \tconst std::vector<SharedFD> &fds,\n> - * \tControlSerializer *cs = nullptr)\n> - * \\brief Deserialize byte vector and fd vector into an object\n> - * \\tparam T Type of object to deserialize to\n> - * \\param[in] data Byte vector to deserialize from\n> - * \\param[in] fds Fd vector to deserialize from\n> - * \\param[in] cs ControlSerializer\n> - *\n> - * This version of deserialize() (or the iterator version) must be used if\n> - * the object type \\a T or its members contain SharedFD.\n> - *\n> - * \\a cs is only necessary if the object type \\a T or its members contain\n> - * ControlList or ControlInfoMap.\n> - *\n> - * \\return The deserialized object\n> - */\n> -\n> -/**\n> - * \\fn template<typename T> IPADataSerializer::deserialize(\n> - * \tstd::vector<uint8_t>::const_iterator dataBegin,\n> - * \tstd::vector<uint8_t>::const_iterator dataEnd,\n> - * \tstd::vector<SharedFD>::const_iterator fdsBegin,\n> - * \tstd::vector<SharedFD>::const_iterator fdsEnd,\n> - * \tControlSerializer *cs = nullptr)\n> - * \\brief Deserialize byte vector and fd vector into an object\n> - * \\tparam T Type of object to deserialize to\n> - * \\param[in] dataBegin Begin iterator of byte vector to deserialize from\n> - * \\param[in] dataEnd End iterator of byte vector to deserialize from\n> - * \\param[in] fdsBegin Begin iterator of fd vector to deserialize from\n> - * \\param[in] fdsEnd End iterator of fd vector to deserialize from\n> - * \\param[in] cs ControlSerializer\n> - *\n> - * This version of deserialize() (or the vector version) must be used if\n> - * the object type \\a T or its members contain SharedFD.\n> - *\n>   * \\a cs is only necessary if the object type \\a T or its members contain\n>   * ControlList or ControlInfoMap.\n>   *\n> @@ -202,37 +99,11 @@ IPADataSerializer<type>::serialize(const type &data,\t\t\t\\\n>  }\t\t\t\t\t\t\t\t\t\\\n>  \t\t\t\t\t\t\t\t\t\\\n>  template<>\t\t\t\t\t\t\t\t\\\n> -type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, \\\n> -\t\t\t\t\t  std::vector<uint8_t>::const_iterator dataEnd, \\\n> -\t\t\t\t\t  [[maybe_unused]] ControlSerializer *cs) \\\n> +std::optional<type> IPADataSerializer<type>::deserialize(SeriReader &reader, \\\n> +\t\t\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n>  {\t\t\t\t\t\t\t\t\t\\\n> -\treturn readPOD<type>(dataBegin, 0, dataEnd);\t\t\t\\\n> +\treturn reader.read<type>();\t\t\t\t\t\\\n>  }\t\t\t\t\t\t\t\t\t\\\n> -\t\t\t\t\t\t\t\t\t\\\n> -template<>\t\t\t\t\t\t\t\t\\\n> -type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \\\n> -\t\t\t\t\t  ControlSerializer *cs)\t\\\n> -{\t\t\t\t\t\t\t\t\t\\\n> -\treturn deserialize(data.cbegin(), data.end(), cs);\t\t\\\n> -}\t\t\t\t\t\t\t\t\t\\\n> -\t\t\t\t\t\t\t\t\t\\\n> -template<>\t\t\t\t\t\t\t\t\\\n> -type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \\\n> -\t\t\t\t\t  [[maybe_unused]] const std::vector<SharedFD> &fds, \\\n> -\t\t\t\t\t  ControlSerializer *cs)\t\\\n> -{\t\t\t\t\t\t\t\t\t\\\n> -\treturn deserialize(data.cbegin(), data.end(), cs);\t\t\\\n> -}\t\t\t\t\t\t\t\t\t\\\n> -\t\t\t\t\t\t\t\t\t\\\n> -template<>\t\t\t\t\t\t\t\t\\\n> -type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, \\\n> -\t\t\t\t\t  std::vector<uint8_t>::const_iterator dataEnd, \\\n> -\t\t\t\t\t  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin, \\\n> -\t\t\t\t\t  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd, \\\n> -\t\t\t\t\t  ControlSerializer *cs)\t\\\n> -{\t\t\t\t\t\t\t\t\t\\\n> -\treturn deserialize(dataBegin, dataEnd, cs);\t\t\t\\\n> -}\n>\n>  DEFINE_POD_SERIALIZER(bool)\n>  DEFINE_POD_SERIALIZER(uint8_t)\n> @@ -256,44 +127,29 @@ std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>\n>  IPADataSerializer<std::string>::serialize(const std::string &data,\n>  \t\t\t\t\t  [[maybe_unused]] ControlSerializer *cs)\n>  {\n> -\treturn { { data.cbegin(), data.end() }, {} };\n> -}\n> +\tstd::vector<uint8_t> dataVec;\n>\n> -template<>\n> -std::string\n> -IPADataSerializer<std::string>::deserialize(const std::vector<uint8_t> &data,\n> -\t\t\t\t\t    [[maybe_unused]] ControlSerializer *cs)\n> -{\n> -\treturn { data.cbegin(), data.cend() };\n> -}\n> +\tASSERT(data.size() <= std::numeric_limits<uint32_t>::max());\n> +\tappendPOD<uint32_t>(dataVec, data.size());\n> +\tdataVec.insert(dataVec.end(), data.c_str(), data.c_str() + data.size());\n>\n> -template<>\n> -std::string\n> -IPADataSerializer<std::string>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t\t\t\t    std::vector<uint8_t>::const_iterator dataEnd,\n> -\t\t\t\t\t    [[maybe_unused]] ControlSerializer *cs)\n> -{\n> -\treturn { dataBegin, dataEnd };\n> +\treturn { dataVec, {} };\n>  }\n>\n>  template<>\n> -std::string\n> -IPADataSerializer<std::string>::deserialize(const std::vector<uint8_t> &data,\n> -\t\t\t\t\t    [[maybe_unused]] const std::vector<SharedFD> &fds,\n> +std::optional<std::string>\n> +IPADataSerializer<std::string>::deserialize(SeriReader &reader,\n>  \t\t\t\t\t    [[maybe_unused]] ControlSerializer *cs)\n>  {\n> -\treturn { data.cbegin(), data.cend() };\n> -}\n> +\tuint32_t length;\n> +\tif (!reader.read(length))\n> +\t\treturn {};\n>\n> -template<>\n> -std::string\n> -IPADataSerializer<std::string>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t\t\t\t    std::vector<uint8_t>::const_iterator dataEnd,\n> -\t\t\t\t\t    [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin,\n> -\t\t\t\t\t    [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,\n> -\t\t\t\t\t    [[maybe_unused]] ControlSerializer *cs)\n> -{\n> -\treturn { dataBegin, dataEnd };\n> +\tconst auto *p = reader.consume(length);\n> +\tif (!p)\n> +\t\treturn {};\n> +\n> +\treturn { { reinterpret_cast<const char *>(p), std::size_t(length) } };\n>  }\n>\n>  /*\n> @@ -356,75 +212,45 @@ IPADataSerializer<ControlList>::serialize(const ControlList &data, ControlSerial\n>  }\n>\n>  template<>\n> -ControlList\n> -IPADataSerializer<ControlList>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t\t\t\t    std::vector<uint8_t>::const_iterator dataEnd,\n> +std::optional<ControlList>\n> +IPADataSerializer<ControlList>::deserialize(SeriReader &reader,\n>  \t\t\t\t\t    ControlSerializer *cs)\n>  {\n>  \tif (!cs)\n>  \t\tLOG(IPADataSerializer, Fatal)\n>  \t\t\t<< \"ControlSerializer not provided for deserialization of ControlList\";\n>\n> -\tif (std::distance(dataBegin, dataEnd) < 8)\n> -\t\treturn {};\n> -\n> -\tuint32_t infoDataSize = readPOD<uint32_t>(dataBegin, 0, dataEnd);\n> -\tuint32_t listDataSize = readPOD<uint32_t>(dataBegin, 4, dataEnd);\n> -\n> -\tstd::vector<uint8_t>::const_iterator it = dataBegin + 8;\n> -\n> -\tif (infoDataSize + listDataSize < infoDataSize ||\n> -\t    static_cast<uint32_t>(std::distance(it, dataEnd)) < infoDataSize + listDataSize)\n> +\tuint32_t infoDataSize, listDataSize;\n> +\tif (!reader.read(infoDataSize, listDataSize))\n>  \t\treturn {};\n>\n>  \tif (infoDataSize > 0) {\n> -\t\tByteStreamBuffer buffer(&*it, infoDataSize);\n> +\t\tconst auto *p = reader.consume(infoDataSize);\n> +\t\tif (!p)\n> +\t\t\treturn {};\n> +\n> +\t\tByteStreamBuffer buffer(reinterpret_cast<const uint8_t *>(p), infoDataSize);\n>  \t\tControlInfoMap map = cs->deserialize<ControlInfoMap>(buffer);\n>  \t\t/* It's fine if map is empty. */\n>  \t\tif (buffer.overflow()) {\n>  \t\t\tLOG(IPADataSerializer, Error)\n>  \t\t\t\t<< \"Failed to deserialize ControlLists's ControlInfoMap: buffer overflow\";\n> -\t\t\treturn ControlList();\n> +\t\t\treturn {};\n>  \t\t}\n>  \t}\n>\n> -\tit += infoDataSize;\n> -\tByteStreamBuffer buffer(&*it, listDataSize);\n> +\tconst auto *p = reader.consume(listDataSize);\n> +\n> +\tByteStreamBuffer buffer(reinterpret_cast<const uint8_t *>(p), listDataSize);\n>  \tControlList list = cs->deserialize<ControlList>(buffer);\n> -\tif (buffer.overflow())\n> +\tif (buffer.overflow()) {\n>  \t\tLOG(IPADataSerializer, Error) << \"Failed to deserialize ControlList: buffer overflow\";\n> +\t\treturn {};\n> +\t}\n>\n>  \treturn list;\n>  }\n>\n> -template<>\n> -ControlList\n> -IPADataSerializer<ControlList>::deserialize(const std::vector<uint8_t> &data,\n> -\t\t\t\t\t    ControlSerializer *cs)\n> -{\n> -\treturn deserialize(data.cbegin(), data.end(), cs);\n> -}\n> -\n> -template<>\n> -ControlList\n> -IPADataSerializer<ControlList>::deserialize(const std::vector<uint8_t> &data,\n> -\t\t\t\t\t    [[maybe_unused]] const std::vector<SharedFD> &fds,\n> -\t\t\t\t\t    ControlSerializer *cs)\n> -{\n> -\treturn deserialize(data.cbegin(), data.end(), cs);\n> -}\n> -\n> -template<>\n> -ControlList\n> -IPADataSerializer<ControlList>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t\t\t\t    std::vector<uint8_t>::const_iterator dataEnd,\n> -\t\t\t\t\t    [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin,\n> -\t\t\t\t\t    [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,\n> -\t\t\t\t\t    ControlSerializer *cs)\n> -{\n> -\treturn deserialize(dataBegin, dataEnd, cs);\n> -}\n> -\n>  /*\n>   * const ControlInfoMap is serialized as:\n>   *\n> @@ -458,59 +284,30 @@ IPADataSerializer<ControlInfoMap>::serialize(const ControlInfoMap &map,\n>  }\n>\n>  template<>\n> -ControlInfoMap\n> -IPADataSerializer<ControlInfoMap>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t\t\t\t       std::vector<uint8_t>::const_iterator dataEnd,\n> +std::optional<ControlInfoMap>\n> +IPADataSerializer<ControlInfoMap>::deserialize(SeriReader &reader,\n>  \t\t\t\t\t       ControlSerializer *cs)\n>  {\n>  \tif (!cs)\n>  \t\tLOG(IPADataSerializer, Fatal)\n>  \t\t\t<< \"ControlSerializer not provided for deserialization of ControlInfoMap\";\n>\n> -\tif (std::distance(dataBegin, dataEnd) < 4)\n> +\tuint32_t infoDataSize;\n> +\tif (!reader.read(infoDataSize))\n>  \t\treturn {};\n>\n> -\tuint32_t infoDataSize = readPOD<uint32_t>(dataBegin, 0, dataEnd);\n> -\n> -\tstd::vector<uint8_t>::const_iterator it = dataBegin + 4;\n> -\n> -\tif (static_cast<uint32_t>(std::distance(it, dataEnd)) < infoDataSize)\n> +\tconst auto *p = reader.consume(infoDataSize);\n> +\tif (!p)\n>  \t\treturn {};\n>\n> -\tByteStreamBuffer buffer(&*it, infoDataSize);\n> +\tByteStreamBuffer buffer(reinterpret_cast<const uint8_t *>(p), infoDataSize);\n>  \tControlInfoMap map = cs->deserialize<ControlInfoMap>(buffer);\n> +\tif (buffer.overflow())\n> +\t\treturn {};\n>\n>  \treturn map;\n>  }\n>\n> -template<>\n> -ControlInfoMap\n> -IPADataSerializer<ControlInfoMap>::deserialize(const std::vector<uint8_t> &data,\n> -\t\t\t\t\t       ControlSerializer *cs)\n> -{\n> -\treturn deserialize(data.cbegin(), data.end(), cs);\n> -}\n> -\n> -template<>\n> -ControlInfoMap\n> -IPADataSerializer<ControlInfoMap>::deserialize(const std::vector<uint8_t> &data,\n> -\t\t\t\t\t       [[maybe_unused]] const std::vector<SharedFD> &fds,\n> -\t\t\t\t\t       ControlSerializer *cs)\n> -{\n> -\treturn deserialize(data.cbegin(), data.end(), cs);\n> -}\n> -\n> -template<>\n> -ControlInfoMap\n> -IPADataSerializer<ControlInfoMap>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t\t\t\t       std::vector<uint8_t>::const_iterator dataEnd,\n> -\t\t\t\t\t       [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin,\n> -\t\t\t\t\t       [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,\n> -\t\t\t\t\t       ControlSerializer *cs)\n> -{\n> -\treturn deserialize(dataBegin, dataEnd, cs);\n> -}\n> -\n>  /*\n>   * SharedFD instances are serialized into four bytes that tells if the SharedFD\n>   * is valid or not. If it is valid, then for serialization the fd will be\n> @@ -542,27 +339,23 @@ IPADataSerializer<SharedFD>::serialize(const SharedFD &data,\n>  }\n>\n>  template<>\n> -SharedFD IPADataSerializer<SharedFD>::deserialize([[maybe_unused]] std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t\t\t\t\t  [[maybe_unused]] std::vector<uint8_t>::const_iterator dataEnd,\n> -\t\t\t\t\t\t  std::vector<SharedFD>::const_iterator fdsBegin,\n> -\t\t\t\t\t\t  std::vector<SharedFD>::const_iterator fdsEnd,\n> -\t\t\t\t\t\t  [[maybe_unused]] ControlSerializer *cs)\n> +std::optional<SharedFD>\n> +IPADataSerializer<SharedFD>::deserialize(SeriReader &reader,\n> +\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs)\n>  {\n> -\tASSERT(std::distance(dataBegin, dataEnd) >= 4);\n> +\tuint32_t valid;\n>\n> -\tuint32_t valid = readPOD<uint32_t>(dataBegin, 0, dataEnd);\n> +\tif (!reader.read(valid))\n> +\t\treturn {};\n>\n> -\tASSERT(!(valid && std::distance(fdsBegin, fdsEnd) < 1));\n> +\tif (!valid)\n> +\t\treturn SharedFD{};\n>\n> -\treturn valid ? *fdsBegin : SharedFD();\n> -}\n> +\tconst auto *fd = reader.popFd();\n> +\tif (!fd)\n> +\t\treturn {};\n>\n> -template<>\n> -SharedFD IPADataSerializer<SharedFD>::deserialize(const std::vector<uint8_t> &data,\n> -\t\t\t\t\t\t  const std::vector<SharedFD> &fds,\n> -\t\t\t\t\t\t  [[maybe_unused]] ControlSerializer *cs)\n> -{\n> -\treturn deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end());\n> +\treturn *fd;\n>  }\n>\n>  /*\n> @@ -594,30 +387,19 @@ IPADataSerializer<FrameBuffer::Plane>::serialize(const FrameBuffer::Plane &data,\n>  }\n>\n>  template<>\n> -FrameBuffer::Plane\n> -IPADataSerializer<FrameBuffer::Plane>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t\t\t\t\t   std::vector<uint8_t>::const_iterator dataEnd,\n> -\t\t\t\t\t\t   std::vector<SharedFD>::const_iterator fdsBegin,\n> -\t\t\t\t\t\t   [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,\n> +std::optional<FrameBuffer::Plane>\n> +IPADataSerializer<FrameBuffer::Plane>::deserialize(SeriReader &reader,\n>  \t\t\t\t\t\t   [[maybe_unused]] ControlSerializer *cs)\n>  {\n> -\tFrameBuffer::Plane ret;\n> -\n> -\tret.fd = IPADataSerializer<SharedFD>::deserialize(dataBegin, dataBegin + 4,\n> -\t\t\t\t\t\t\t  fdsBegin, fdsBegin + 1);\n> -\tret.offset = readPOD<uint32_t>(dataBegin, 4, dataEnd);\n> -\tret.length = readPOD<uint32_t>(dataBegin, 8, dataEnd);\n> +\tauto fd = IPADataSerializer<SharedFD>::deserialize(reader);\n> +\tif (!fd)\n> +\t\treturn {};\n>\n> -\treturn ret;\n> -}\n> +\tuint32_t offset, length;\n> +\tif (!reader.read(offset, length))\n> +\t\treturn {};\n>\n> -template<>\n> -FrameBuffer::Plane\n> -IPADataSerializer<FrameBuffer::Plane>::deserialize(const std::vector<uint8_t> &data,\n> -\t\t\t\t\t\t   const std::vector<SharedFD> &fds,\n> -\t\t\t\t\t\t   ControlSerializer *cs)\n> -{\n> -\treturn deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs);\n> +\treturn { { std::move(*fd), offset, length } };\n>  }\n>\n>  #endif /* __DOXYGEN__ */\n> diff --git a/src/libcamera/ipc_pipe.cpp b/src/libcamera/ipc_pipe.cpp\n> index 548299d05..71d0c2cb4 100644\n> --- a/src/libcamera/ipc_pipe.cpp\n> +++ b/src/libcamera/ipc_pipe.cpp\n> @@ -148,6 +148,11 @@ IPCUnixSocket::Payload IPCMessage::payload() const\n>   * \\brief Returns a const reference to the vector containing file descriptors\n>   */\n>\n> +/**\n> + * \\fn IPCMessage::reader() const\n> + * \\brief Returns a `SeriReader` instance for parsing the message\n> + */\n> +\n>  /**\n>   * \\class IPCPipe\n>   * \\brief IPC message pipe for IPA isolation\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 5b9b86f21..1834ff9b1 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -49,6 +49,7 @@ libcamera_internal_sources = files([\n>      'pipeline_handler.cpp',\n>      'process.cpp',\n>      'pub_key.cpp',\n> +    'serialization.cpp',\n>      'shared_mem_object.cpp',\n>      'source_paths.cpp',\n>      'sysfs.cpp',\n> diff --git a/src/libcamera/serialization.cpp b/src/libcamera/serialization.cpp\n> new file mode 100644\n> index 000000000..1b1d3d38e\n> --- /dev/null\n> +++ b/src/libcamera/serialization.cpp\n> @@ -0,0 +1,60 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board Oy\n> + *\n> + * Data (de)serialization helper structures\n> + */\n> +\n> +#include \"libcamera/internal/serialization.h\"\n> +\n> +namespace libcamera {\n> +\n> +/**\n> + * \\class SeriReader\n> + * \\brief Class to consume a byte and file descriptor buffer\n> + */\n> +\n> +/**\n> + * \\fn SeriReader::SeriReader(Span<const uint8_t> data, Span<const SharedFD> fds)\n> + * \\brief Constructor\n> + */\n> +\n> +/**\n> + * \\fn SeriReader::empty() const\n> + * \\brief Check if there is at least one byte or file descriptor to read\n> + */\n> +\n> +/**\n> + * \\fn SeriReader::consume(std::size_t s)\n> + * \\brief Consume \\a s bytes\n> + * \\return A pointer to the start of the data, or \\a nullptr if there are not\n> + *         enough bytes left.\n> + */\n> +\n> +/**\n> + * \\fn template<typename... Ts> SeriReader::read(Ts &...xs)\n> + * \\brief Read bytes into \\a xs\n> + * \\return \\a true on success, or \\a false if there are not enough bytes left\n> + *\n> + * This function reads \\a sizeof(xs) bytes into \\a xs, for each parameter\n> + * in the argument pack, in order.\n> + */\n> +\n> +/**\n> + * \\fn template<typename... Ts> SeriReader::read()\n> + * \\brief Extract \\a Ts objects\n> + * \\return \\a std::nullopt if there are not enough bytes left\n> + *\n> + * This function creates an \\a std::tuple with types \\a Ts, and then tries to read\n> + * enough bytes into each element, in order. If that succeeds, the \\a std::tuple\n> + * (or just the value if \\a Ts has a single element) is returned in an \\a std::optional.\n> + */\n> +\n> +/**\n> + * \\fn SeriReader::popFd()\n> + * \\brief Try to consume a single file descriptor\n> + * \\return A pointer to the file descriptor, or \\a nullptr if there are not\n> + *         enough file descriptors left.\n> + */\n> +\n> +} /* namespace libcamera */\n> diff --git a/test/ipc/unixsocket_ipc.cpp b/test/ipc/unixsocket_ipc.cpp\n> index f3e3c09ef..c314e4514 100644\n> --- a/test/ipc/unixsocket_ipc.cpp\n> +++ b/test/ipc/unixsocket_ipc.cpp\n> @@ -102,7 +102,14 @@ private:\n>  \t\t}\n>\n>  \t\tcase CmdSetAsync: {\n> -\t\t\tvalue_ = IPADataSerializer<int32_t>::deserialize(ipcMessage.data());\n> +\t\t\tSeriReader reader = ipcMessage.reader();\n> +\t\t\tauto value = IPADataSerializer<int32_t>::deserialize(reader);\n> +\t\t\tif (!value) {\n> +\t\t\t\tcerr << \"Failed to deserialize value\" << endl;\n> +\t\t\t\tstop(-ENODATA);\n> +\t\t\t} else {\n> +\t\t\t\tvalue_ = *value;\n> +\t\t\t}\n>  \t\t\tbreak;\n>  \t\t}\n>  \t\t}\n> @@ -155,7 +162,14 @@ protected:\n>  \t\t\treturn ret;\n>  \t\t}\n>\n> -\t\treturn IPADataSerializer<int32_t>::deserialize(buf.data());\n> +\t\tSeriReader reader = buf.reader();\n> +\t\tauto value = IPADataSerializer<int32_t>::deserialize(reader);\n> +\t\tif (!value) {\n> +\t\t\tcerr << \"Failed to deserialize value\" << endl;\n> +\t\t\treturn -ENODATA;\n> +\t\t}\n> +\n> +\t\treturn *value;\n>  \t}\n>\n>  \tint exit()\n> diff --git a/test/serialization/generated_serializer/generated_serializer_test.cpp b/test/serialization/generated_serializer/generated_serializer_test.cpp\n> index dd6968850..0640e741d 100644\n> --- a/test/serialization/generated_serializer/generated_serializer_test.cpp\n> +++ b/test/serialization/generated_serializer/generated_serializer_test.cpp\n> @@ -43,7 +43,7 @@ if (struct1.field != struct2.field) {\t\t\t\t\\\n>  }\n>\n>\n> -\t\tipa::test::TestStruct t, u;\n> +\t\tipa::test::TestStruct t;\n>\n>  \t\tt.m = {\n>  \t\t\t{ \"a\", \"z\" },\n> @@ -71,8 +71,13 @@ if (struct1.field != struct2.field) {\t\t\t\t\\\n>\n>  \t\tstd::tie(serialized, ignore) =\n>  \t\t\tIPADataSerializer<ipa::test::TestStruct>::serialize(t);\n> +\t\tSeriReader reader(serialized);\n>\n> -\t\tu = IPADataSerializer<ipa::test::TestStruct>::deserialize(serialized);\n> +\t\tauto optu = IPADataSerializer<ipa::test::TestStruct>::deserialize(reader);\n> +\t\tif (!optu)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tauto &u = *optu;\n>\n>  \t\tif (!equals(t.m, u.m))\n>  \t\t\treturn TestFail;\n> @@ -91,12 +96,16 @@ if (struct1.field != struct2.field) {\t\t\t\t\\\n>\n>  \t\t/* Test vector of generated structs */\n>  \t\tstd::vector<ipa::test::TestStruct> v = { t, u };\n> -\t\tstd::vector<ipa::test::TestStruct> w;\n>\n>  \t\tstd::tie(serialized, ignore) =\n>  \t\t\tIPADataSerializer<vector<ipa::test::TestStruct>>::serialize(v);\n> +\t\treader = SeriReader(serialized);\n> +\n> +\t\tauto optw = IPADataSerializer<vector<ipa::test::TestStruct>>::deserialize(reader);\n> +\t\tif (!optw)\n> +\t\t\treturn TestFail;\n>\n> -\t\tw = IPADataSerializer<vector<ipa::test::TestStruct>>::deserialize(serialized);\n> +\t\tauto &w = *optw;\n>\n>  \t\tif (!equals(v[0].m, w[0].m) ||\n>  \t\t    !equals(v[1].m, w[1].m))\n> diff --git a/test/serialization/ipa_data_serializer_test.cpp b/test/serialization/ipa_data_serializer_test.cpp\n> index afea93a6c..3873808a6 100644\n> --- a/test/serialization/ipa_data_serializer_test.cpp\n> +++ b/test/serialization/ipa_data_serializer_test.cpp\n> @@ -52,7 +52,9 @@ int testPodSerdes(T in)\n>  \tstd::vector<SharedFD> fds;\n>\n>  \tstd::tie(buf, fds) = IPADataSerializer<T>::serialize(in);\n> -\tT out = IPADataSerializer<T>::deserialize(buf, fds);\n> +\tSeriReader reader(buf, fds);\n> +\n> +\tauto out = IPADataSerializer<T>::deserialize(reader);\n>  \tif (in == out)\n>  \t\treturn TestPass;\n>\n> @@ -71,7 +73,9 @@ int testVectorSerdes(const std::vector<T> &in,\n>  \tstd::vector<SharedFD> fds;\n>\n>  \tstd::tie(buf, fds) = IPADataSerializer<std::vector<T>>::serialize(in, cs);\n> -\tstd::vector<T> out = IPADataSerializer<std::vector<T>>::deserialize(buf, fds, cs);\n> +\tSeriReader reader(buf, fds);\n> +\n> +\tauto out = IPADataSerializer<std::vector<T>>::deserialize(reader, cs);\n>  \tif (in == out)\n>  \t\treturn TestPass;\n>\n> @@ -91,7 +95,9 @@ int testMapSerdes(const std::map<K, V> &in,\n>  \tstd::vector<SharedFD> fds;\n>\n>  \tstd::tie(buf, fds) = IPADataSerializer<std::map<K, V>>::serialize(in, cs);\n> -\tstd::map<K, V> out = IPADataSerializer<std::map<K, V>>::deserialize(buf, fds, cs);\n> +\tSeriReader reader(buf, fds);\n> +\n> +\tauto out = IPADataSerializer<std::map<K, V>>::deserialize(reader, cs);\n>  \tif (in == out)\n>  \t\treturn TestPass;\n>\n> @@ -171,17 +177,27 @@ private:\n>  \t\tstd::tie(listBuf, std::ignore) =\n>  \t\t\tIPADataSerializer<ControlList>::serialize(list, &cs);\n>\n> -\t\tconst ControlInfoMap infoMapOut =\n> -\t\t\tIPADataSerializer<ControlInfoMap>::deserialize(infoMapBuf, &cs);\n> +\t\tSeriReader listReader(listBuf);\n> +\t\tSeriReader infoMapReader(infoMapBuf);\n> +\n> +\t\tauto infoMapOut = IPADataSerializer<ControlInfoMap>::deserialize(infoMapReader, &cs);\n> +\t\tif (!infoMapOut) {\n> +\t\t\tcerr << \"`ControlInfoMap` cannot be deserialized\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n>\n> -\t\tControlList listOut = IPADataSerializer<ControlList>::deserialize(listBuf, &cs);\n> +\t\tauto listOut = IPADataSerializer<ControlList>::deserialize(listReader, &cs);\n> +\t\tif (!listOut) {\n> +\t\t\tcerr << \"`ControlList` cannot be deserialized\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n>\n> -\t\tif (!SerializationTest::equals(infoMap, infoMapOut)) {\n> +\t\tif (!SerializationTest::equals(infoMap, *infoMapOut)) {\n>  \t\t\tcerr << \"Deserialized map doesn't match original\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n>\n> -\t\tif (!SerializationTest::equals(list, listOut)) {\n> +\t\tif (!SerializationTest::equals(list, *listOut)) {\n>  \t\t\tcerr << \"Deserialized list doesn't match original\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n> 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\n> index 0d03c1df0..6146b7932 100644\n> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n> @@ -195,29 +195,34 @@ namespace {{ns}} {\n>  \t\treturn;\n>  {%- endif %}\n>  \t}\n> +{% if has_output %}\n> +\tSeriReader _outputReader = _ipcOutputBuf.reader();\n> +{% endif -%}\n>  {% if method|method_return_value != \"void\" %}\n> -\t{{method|method_return_value}} _retValue = IPADataSerializer<{{method|method_return_value}}>::deserialize(_ipcOutputBuf.data(), 0);\n> -\n> -{{proxy_funcs.deserialize_call(method|method_param_outputs, '_ipcOutputBuf.data()', '_ipcOutputBuf.fds()', init_offset = method|method_return_value|byte_width|int)}}\n> +\tauto _retValue = IPADataSerializer<{{method|method_return_value}}>::deserialize(_outputReader);\n> +\tASSERT(_retValue);\n> +{% endif -%}\n>\n> -\treturn _retValue;\n> +{% if has_output %}\n> +\t{{proxy_funcs.deserialize_call(method|method_param_outputs, '_outputReader')}}\n> +\tASSERT(_outputReader.empty());\n> +{% endif -%}\n>\n> -{% elif method|method_param_outputs|length > 0 %}\n> -{{proxy_funcs.deserialize_call(method|method_param_outputs, '_ipcOutputBuf.data()', '_ipcOutputBuf.fds()')}}\n> +{% if method|method_return_value != \"void\" %}\n> +\treturn std::move(*_retValue);\n>  {% endif -%}\n>  }\n> -\n>  {% endfor %}\n>\n>  void {{proxy_name}}Isolated::recvMessage(const IPCMessage &data)\n>  {\n> -\tsize_t dataSize = data.data().size();\n> +\tSeriReader reader = data.reader();\n>  \t{{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);\n>\n>  \tswitch (_cmd) {\n>  {%- for method in interface_event.methods %}\n>  \tcase {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {\n> -\t\t{{method.mojom_name}}Handler(data.data().cbegin(), dataSize, data.fds());\n> +\t\t{{method.mojom_name}}Handler(reader);\n>  \t\tbreak;\n>  \t}\n>  {%- endfor %}\n> @@ -227,12 +232,9 @@ void {{proxy_name}}Isolated::recvMessage(const IPCMessage &data)\n>  }\n>\n>  {% for method in interface_event.methods %}\n> -void {{proxy_name}}Isolated::{{method.mojom_name}}Handler(\n> -\t[[maybe_unused]] std::vector<uint8_t>::const_iterator data,\n> -\t[[maybe_unused]] size_t dataSize,\n> -\t[[maybe_unused]] const std::vector<SharedFD> &fds)\n> +void {{proxy_name}}Isolated::{{method.mojom_name}}Handler([[maybe_unused]] SeriReader &reader)\n>  {\n> -{{proxy_funcs.deserialize_call(method.parameters, 'data', 'fds', false, true, true, 'dataSize')}}\n> +\t{{proxy_funcs.deserialize_call(method.parameters, 'reader', false, true)}}\n>  \t{{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});\n>  }\n>  {% endfor %}\n> 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\n> index ef280ca42..f33529f71 100644\n> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n> @@ -123,10 +123,7 @@ private:\n>  \tvoid recvMessage(const IPCMessage &data);\n>\n>  {% for method in interface_event.methods %}\n> -\tvoid {{method.mojom_name}}Handler(\n> -\t\tstd::vector<uint8_t>::const_iterator data,\n> -\t\tsize_t dataSize,\n> -\t\tconst std::vector<SharedFD> &fds);\n> +\tvoid {{method.mojom_name}}Handler(SeriReader &reader);\n>  {% endfor %}\n>\n>  \tstd::unique_ptr<IPCPipeUnixSocket> ipc_;\n> 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\n> index 1f990d3f9..de6378be0 100644\n> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl\n> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl\n> @@ -71,6 +71,7 @@ public:\n>  \t\t}\n>\n>  \t\tIPCMessage _ipcMessage(_message);\n> +\t\tSeriReader _reader = _ipcMessage.reader();\n>\n>  \t\t{{cmd_enum_name}} _cmd = static_cast<{{cmd_enum_name}}>(_ipcMessage.header().cmd);\n>\n> @@ -85,7 +86,9 @@ public:\n>  {%- if method.mojom_name == \"configure\" %}\n>  \t\t\tcontrolSerializer_.reset();\n>  {%- endif %}\n> -\t\t{{proxy_funcs.deserialize_call(method|method_param_inputs, '_ipcMessage.data()', '_ipcMessage.fds()', false, true)|indent(16, true)}}\n> +\t\t\t{{proxy_funcs.deserialize_call(method|method_param_inputs, '_reader', false, true)|indent(16, true)}}\n> +\t\t\tASSERT(_reader.empty());\n> +\n>  {% for param in method|method_param_outputs %}\n>  \t\t\t{{param|name}} {{param.mojom_name}};\n>  {% endfor %}\n> diff --git a/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl\n> index 01e2567ca..b6835ca35 100644\n> --- a/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl\n> +++ b/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl\n> @@ -64,15 +64,6 @@\n>  );\n>  {%- endfor %}\n>\n> -{%- if params|length > 1 %}\n> -{%- for param in params %}\n> -\tappendPOD<uint32_t>({{buf}}, {{param.mojom_name}}Buf.size());\n> -{%- if param|has_fd %}\n> -\tappendPOD<uint32_t>({{buf}}, {{param.mojom_name}}Fds.size());\n> -{%- endif %}\n> -{%- endfor %}\n> -{%- endif %}\n> -\n>  {%- for param in params %}\n>  \t{{buf}}.insert({{buf}}.end(), {{param.mojom_name}}Buf.begin(), {{param.mojom_name}}Buf.end());\n>  {%- endfor %}\n> @@ -84,104 +75,28 @@\n>  {%- endfor %}\n>  {%- endmacro -%}\n>\n> -\n> -{#\n> - # \\brief Deserialize a single object from data buffer and fd vector\n> - #\n> - # \\param pointer If true, deserialize the object into a dereferenced pointer\n> - # \\param iter If true, treat \\a buf as an iterator instead of a vector\n> - # \\param data_size Variable that holds the size of the vector referenced by \\a buf\n> - #\n> - # Generate code to deserialize a single object, as specified in \\a param,\n> - # from \\a buf data buffer and \\a fds fd vector.\n> - # This code is meant to be used by macro deserialize_call.\n> - #}\n> -{%- macro deserialize_param(param, pointer, loop, buf, fds, iter, data_size) -%}\n> -{{\"*\" if pointer}}{{param.mojom_name}} =\n> -IPADataSerializer<{{param|name_full}}>::deserialize(\n> -\t{{buf}}{{- \".cbegin()\" if not iter}} + {{param.mojom_name}}Start,\n> -{%- if loop.last and not iter %}\n> -\t{{buf}}.cend()\n> -{%- elif not iter %}\n> -\t{{buf}}.cbegin() + {{param.mojom_name}}Start + {{param.mojom_name}}BufSize\n> -{%- elif iter and loop.length == 1 %}\n> -\t{{buf}} + {{data_size}}\n> -{%- else %}\n> -\t{{buf}} + {{param.mojom_name}}Start + {{param.mojom_name}}BufSize\n> -{%- endif -%}\n> -{{- \",\" if param|has_fd}}\n> -{%- if param|has_fd %}\n> -\t{{fds}}.cbegin() + {{param.mojom_name}}FdStart,\n> -{%- if loop.last %}\n> -\t{{fds}}.cend()\n> -{%- else %}\n> -\t{{fds}}.cbegin() + {{param.mojom_name}}FdStart + {{param.mojom_name}}FdsSize\n> -{%- endif -%}\n> -{%- endif -%}\n> -{{- \",\" if param|needs_control_serializer}}\n> -{%- if param|needs_control_serializer %}\n> -\t&controlSerializer_\n> -{%- endif -%}\n> -);\n> -{%- endmacro -%}\n> -\n> -\n>  {#\n> - # \\brief Deserialize multiple objects from data buffer and fd vector\n> + # \\brief Deserialize multiple objects\n>   #\n>   # \\param pointer If true, deserialize objects into pointers, and adds a null check.\n>   # \\param declare If true, declare the objects in addition to deserialization.\n> - # \\param iter if true, treat \\a buf as an iterator instead of a vector\n> - # \\param data_size Variable that holds the size of the vector referenced by \\a buf\n> - #\n> - # Generate code to deserialize multiple objects, as specified in \\a params\n> - # (which are the parameters to some function), from \\a buf data buffer and\n> - # \\a fds fd vector.\n> - # This code is meant to be used by the proxy, for deserializing after IPC calls.\n>   #\n> - # \\todo Avoid intermediate vectors\n> + # Generate code to deserialize multiple objects, as specified in \\a params,\n> + # from \\a reader. This code is meant to be used by the proxy, for deserializing\n> + # after IPC calls.\n>   #}\n> -{%- macro deserialize_call(params, buf, fds, pointer = true, declare = false, iter = false, data_size = '', init_offset = 0) -%}\n> -{% set ns = namespace(size_offset = init_offset) %}\n> -{%- if params|length > 1 %}\n> +{%- macro deserialize_call(params, reader, pointer = true, declare = false) -%}\n>  {%- for param in params %}\n> -\t[[maybe_unused]] const size_t {{param.mojom_name}}BufSize = readPOD<uint32_t>({{buf}}, {{ns.size_offset}}\n> -{%- if iter -%}\n> -, {{buf}} + {{data_size}}\n> -{%- endif -%}\n> -);\n> -\t{%- set ns.size_offset = ns.size_offset + 4 %}\n> -{%- if param|has_fd %}\n> -\t[[maybe_unused]] const size_t {{param.mojom_name}}FdsSize = readPOD<uint32_t>({{buf}}, {{ns.size_offset}}\n> -{%- if iter -%}\n> -, {{buf}} + {{data_size}}\n> -{%- endif -%}\n> -);\n> -\t{%- set ns.size_offset = ns.size_offset + 4 %}\n> -{%- endif %}\n> -{%- endfor %}\n> -{%- endif %}\n> -{% for param in params %}\n> -{%- if loop.first %}\n> -\tconst size_t {{param.mojom_name}}Start = {{ns.size_offset}};\n> -{%- else %}\n> -\tconst size_t {{param.mojom_name}}Start = {{loop.previtem.mojom_name}}Start + {{loop.previtem.mojom_name}}BufSize;\n> -{%- endif %}\n> -{%- endfor %}\n> -{% for param in params|with_fds %}\n> -{%- if loop.first %}\n> -\tconst size_t {{param.mojom_name}}FdStart = 0;\n> +\tauto param_{{param.mojom_name}} = IPADataSerializer<{{param|name_full}}>::deserialize({{reader}}, &controlSerializer_);\n> +\tASSERT(param_{{param.mojom_name}});\n> +\n> +{%- if pointer %}\n> +\tif ({{param.mojom_name}})\n> +\t\t*{{param.mojom_name}} = std::move(*param_{{param.mojom_name}});\n> +{%- elif declare %}\n> +\t{{param|name}} &{{param.mojom_name}} = *param_{{param.mojom_name}};\n>  {%- else %}\n> -\tconst size_t {{param.mojom_name}}FdStart = {{loop.previtem.mojom_name}}FdStart + {{loop.previtem.mojom_name}}FdsSize;\n> +\t{{param.mojom_name}} = std::move(*param_{{param.mojom_name}});\n>  {%- endif %}\n> -{%- endfor %}\n> -{% for param in params %}\n> -\t{%- if pointer %}\n> -\tif ({{param.mojom_name}}) {\n> -{{deserialize_param(param, pointer, loop, buf, fds, iter, data_size)|indent(16, True)}}\n> -\t}\n> -\t{%- else %}\n> -\t{{param|name + \" \" if declare}}{{deserialize_param(param, pointer, loop, buf, fds, iter, data_size)|indent(8)}}\n> -\t{%- endif %}\n>  {% endfor %}\n>  {%- endmacro -%}\n> diff --git a/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl b/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl\n> index e316dd88a..9e9dd0ca6 100644\n> --- a/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl\n> +++ b/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl\n> @@ -2,22 +2,6 @@\n>   # SPDX-License-Identifier: LGPL-2.1-or-later\n>   # Copyright (C) 2020, Google Inc.\n>  -#}\n> -{#\n> - # \\brief Verify that there is enough bytes to deserialize\n> - #\n> - # Generate code that verifies that \\a size is not greater than \\a dataSize.\n> - # Otherwise log an error with \\a name and \\a typename.\n> - #}\n> -{%- macro check_data_size(size, dataSize, name, typename) %}\n> -\t\tif ({{dataSize}} < {{size}}) {\n> -\t\t\tLOG(IPADataSerializer, Error)\n> -\t\t\t\t<< \"Failed to deserialize \" << \"{{name}}\"\n> -\t\t\t\t<< \": not enough {{typename}}, expected \"\n> -\t\t\t\t<< ({{size}}) << \", got \" << ({{dataSize}});\n> -\t\t\treturn ret;\n> -\t\t}\n> -{%- endmacro %}\n> -\n>\n>  {#\n>   # \\brief Serialize a field into return vector\n> @@ -42,15 +26,10 @@\n>  \t\tretData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());\n>  \t\tretFds.insert(retFds.end(), {{field.mojom_name}}Fds.begin(), {{field.mojom_name}}Fds.end());\n>  {%- elif field|is_controls %}\n> -\t\tif (data.{{field.mojom_name}}.size() > 0) {\n> -\t\t\tstd::vector<uint8_t> {{field.mojom_name}};\n> -\t\t\tstd::tie({{field.mojom_name}}, std::ignore) =\n> -\t\t\t\tIPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}}, cs);\n> -\t\t\tappendPOD<uint32_t>(retData, {{field.mojom_name}}.size());\n> -\t\t\tretData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());\n> -\t\t} else {\n> -\t\t\tappendPOD<uint32_t>(retData, 0);\n> -\t\t}\n> +\t\tstd::vector<uint8_t> {{field.mojom_name}};\n> +\t\tstd::tie({{field.mojom_name}}, std::ignore) =\n> +\t\t\tIPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}}, cs);\n> +\t\tretData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());\n>  {%- elif field|is_plain_struct or field|is_array or field|is_map or field|is_str %}\n>  \t\tstd::vector<uint8_t> {{field.mojom_name}};\n>  \t{%- if field|has_fd %}\n> @@ -65,10 +44,6 @@\n>  \t\t\tIPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}});\n>  \t{%- else %}\n>  \t\t\tIPADataSerializer<{{field|name_full}}>::serialize(data.{{field.mojom_name}}, cs);\n> -\t{%- endif %}\n> -\t\tappendPOD<uint32_t>(retData, {{field.mojom_name}}.size());\n> -\t{%- if field|has_fd %}\n> -\t\tappendPOD<uint32_t>(retData, {{field.mojom_name}}Fds.size());\n>  \t{%- endif %}\n>  \t\tretData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());\n>  \t{%- if field|has_fd %}\n> @@ -79,89 +54,6 @@\n>  {%- endif %}\n>  {%- endmacro %}\n>\n> -\n> -{#\n> - # \\brief Deserialize a field into return struct\n> - #\n> - # Generate code to deserialize \\a field into object ret.\n> - # This code is meant to be used by the IPADataSerializer specialization.\n> - #}\n> -{%- macro deserializer_field(field, loop) %}\n> -{% if field|is_pod or field|is_enum %}\n> -\t{%- set field_size = (field|bit_width|int / 8)|int %}\n> -\t\t{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}\n> -\t\tret.{{field.mojom_name}} = IPADataSerializer<{{field|name_full}}>::deserialize(m, m + {{field_size}});\n> -\t{%- if not loop.last %}\n> -\t\tm += {{field_size}};\n> -\t\tdataSize -= {{field_size}};\n> -\t{%- endif %}\n> -{% elif field|is_fd %}\n> -\t{%- set field_size = 4 %}\n> -\t\t{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}\n> -\t\tret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field_size}}, n, n + 1, cs);\n> -\t{%- if not loop.last %}\n> -\t\tm += {{field_size}};\n> -\t\tdataSize -= {{field_size}};\n> -\t\tn += ret.{{field.mojom_name}}.isValid() ? 1 : 0;\n> -\t\tfdsSize -= ret.{{field.mojom_name}}.isValid() ? 1 : 0;\n> -\t{%- endif %}\n> -{% elif field|is_controls %}\n> -\t{%- set field_size = 4 %}\n> -\t\t{{- check_data_size(field_size, 'dataSize', field.mojom_name + 'Size', 'data')}}\n> -\t\tconst size_t {{field.mojom_name}}Size = readPOD<uint32_t>(m, 0, dataEnd);\n> -\t\tm += {{field_size}};\n> -\t\tdataSize -= {{field_size}};\n> -\t{%- set field_size = field.mojom_name + 'Size' -%}\n> -\t\t{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}\n> -\t\tif ({{field.mojom_name}}Size > 0)\n> -\t\t\tret.{{field.mojom_name}} =\n> -\t\t\t\tIPADataSerializer<{{field|name}}>::deserialize(m, m + {{field.mojom_name}}Size, cs);\n> -\t{%- if not loop.last %}\n> -\t\tm += {{field_size}};\n> -\t\tdataSize -= {{field_size}};\n> -\t{%- endif %}\n> -{% elif field|is_plain_struct or field|is_array or field|is_map or field|is_str %}\n> -\t{%- set field_size = 4 %}\n> -\t\t{{- check_data_size(field_size, 'dataSize', field.mojom_name + 'Size', 'data')}}\n> -\t\tconst size_t {{field.mojom_name}}Size = readPOD<uint32_t>(m, 0, dataEnd);\n> -\t\tm += {{field_size}};\n> -\t\tdataSize -= {{field_size}};\n> -\t{%- if field|has_fd %}\n> -\t{%- set field_size = 4 %}\n> -\t\t{{- check_data_size(field_size, 'dataSize', field.mojom_name + 'FdsSize', 'data')}}\n> -\t\tconst size_t {{field.mojom_name}}FdsSize = readPOD<uint32_t>(m, 0, dataEnd);\n> -\t\tm += {{field_size}};\n> -\t\tdataSize -= {{field_size}};\n> -\t\t{{- check_data_size(field.mojom_name + 'FdsSize', 'fdsSize', field.mojom_name, 'fds')}}\n> -\t{%- endif %}\n> -\t{%- set field_size = field.mojom_name + 'Size' -%}\n> -\t\t{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}\n> -\t\tret.{{field.mojom_name}} =\n> -\t{%- if field|is_str %}\n> -\t\t\tIPADataSerializer<{{field|name}}>::deserialize(m, m + {{field.mojom_name}}Size);\n> -\t{%- elif field|has_fd and (field|is_array or field|is_map) %}\n> -\t\t\tIPADataSerializer<{{field|name}}>::deserialize(m, m + {{field.mojom_name}}Size, n, n + {{field.mojom_name}}FdsSize, cs);\n> -\t{%- elif field|has_fd and (not (field|is_array or field|is_map)) %}\n> -\t\t\tIPADataSerializer<{{field|name_full}}>::deserialize(m, m + {{field.mojom_name}}Size, n, n + {{field.mojom_name}}FdsSize, cs);\n> -\t{%- elif (not field|has_fd) and (field|is_array or field|is_map) %}\n> -\t\t\tIPADataSerializer<{{field|name}}>::deserialize(m, m + {{field.mojom_name}}Size, cs);\n> -\t{%- else %}\n> -\t\t\tIPADataSerializer<{{field|name_full}}>::deserialize(m, m + {{field.mojom_name}}Size, cs);\n> -\t{%- endif %}\n> -\t{%- if not loop.last %}\n> -\t\tm += {{field_size}};\n> -\t\tdataSize -= {{field_size}};\n> -\t{%- if field|has_fd %}\n> -\t\tn += {{field.mojom_name}}FdsSize;\n> -\t\tfdsSize -= {{field.mojom_name}}FdsSize;\n> -\t{%- endif %}\n> -\t{%- endif %}\n> -{% else %}\n> -\t\t/* Unknown deserialization for {{field.mojom_name}}. */\n> -{%- endif %}\n> -{%- endmacro %}\n> -\n> -\n>  {#\n>   # \\brief Serialize a struct\n>   #\n> @@ -194,126 +86,30 @@\n>\n>\n>  {#\n> - # \\brief Deserialize a struct that has fds\n> + # \\brief Deserialize a struct\n>   #\n> - # Generate code for IPADataSerializer specialization, for deserializing\n> - # \\a struct, in the case that \\a struct has file descriptors.\n> + # Generate code for IPADataSerializer specialization, for deserializing \\a struct.\n>   #}\n> -{%- macro deserializer_fd(struct) %}\n> -\tstatic {{struct|name_full}}\n> -\tdeserialize(std::vector<uint8_t> &data,\n> -\t\t    std::vector<SharedFD> &fds,\n> -{%- if struct|needs_control_serializer %}\n> -\t\t    ControlSerializer *cs)\n> -{%- else %}\n> -\t\t    ControlSerializer *cs = nullptr)\n> -{%- endif %}\n> -\t{\n> -\t\treturn IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), fds.cbegin(), fds.cend(), cs);\n> -\t}\n> -\n> +{%- macro deserializer(struct) %}\n>  {# \\todo Don't inline this function #}\n> -\tstatic {{struct|name_full}}\n> -\tdeserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t    std::vector<uint8_t>::const_iterator dataEnd,\n> -\t\t    std::vector<SharedFD>::const_iterator fdsBegin,\n> -\t\t    std::vector<SharedFD>::const_iterator fdsEnd,\n> +\t[[nodiscard]] static std::optional<{{struct|name_full}}>\n> +\tdeserialize(SeriReader &reader,\n>  {%- if struct|needs_control_serializer %}\n>  \t\t    ControlSerializer *cs)\n>  {%- else %}\n>  \t\t    [[maybe_unused]] ControlSerializer *cs = nullptr)\n>  {%- endif %}\n>  \t{\n> -\t\t{{struct|name_full}} ret;\n> -\t\tstd::vector<uint8_t>::const_iterator m = dataBegin;\n> -\t\tstd::vector<SharedFD>::const_iterator n = fdsBegin;\n> -\n> -\t\tsize_t dataSize = std::distance(dataBegin, dataEnd);\n> -\t\t[[maybe_unused]] size_t fdsSize = std::distance(fdsBegin, fdsEnd);\n> -{%- for field in struct.fields -%}\n> -{{deserializer_field(field, loop)}}\n> +{%- for field in struct.fields %}\n> +\t\tauto {{field.mojom_name}} = IPADataSerializer<{{field|name_full}}>::deserialize(reader, cs);\n> +\t\tif (!{{field.mojom_name}})\n> +\t\t\treturn {};\n>  {%- endfor %}\n> -\t\treturn ret;\n> -\t}\n> -{%- endmacro %}\n> -\n> -{#\n> - # \\brief Deserialize a struct that has fds, using non-fd\n> - #\n> - # Generate code for IPADataSerializer specialization, for deserializing\n> - # \\a struct, in the case that \\a struct has no file descriptors but requires\n> - # deserializers with file descriptors.\n> - #}\n> -{%- macro deserializer_fd_simple(struct) %}\n> -\tstatic {{struct|name_full}}\n> -\tdeserialize(std::vector<uint8_t> &data,\n> -\t\t    [[maybe_unused]] std::vector<SharedFD> &fds,\n> -\t\t    ControlSerializer *cs = nullptr)\n> -\t{\n> -\t\treturn IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), cs);\n> -\t}\n> -\n> -\tstatic {{struct|name_full}}\n> -\tdeserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t    std::vector<uint8_t>::const_iterator dataEnd,\n> -\t\t    [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin,\n> -\t\t    [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,\n> -\t\t    ControlSerializer *cs = nullptr)\n> -\t{\n> -\t\treturn IPADataSerializer<{{struct|name_full}}>::deserialize(dataBegin, dataEnd, cs);\n> -\t}\n> -{%- endmacro %}\n> -\n> -\n> -{#\n> - # \\brief Deserialize a struct that has no fds\n> - #\n> - # Generate code for IPADataSerializer specialization, for deserializing\n> - # \\a struct, in the case that \\a struct does not have file descriptors.\n> - #}\n> -{%- macro deserializer_no_fd(struct) %}\n> -\tstatic {{struct|name_full}}\n> -\tdeserialize(std::vector<uint8_t> &data,\n> -{%- if struct|needs_control_serializer %}\n> -\t\t    ControlSerializer *cs)\n> -{%- else %}\n> -\t\t    ControlSerializer *cs = nullptr)\n> -{%- endif %}\n> -\t{\n> -\t\treturn IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), cs);\n> -\t}\n>\n> -{# \\todo Don't inline this function #}\n> -\tstatic {{struct|name_full}}\n> -\tdeserialize(std::vector<uint8_t>::const_iterator dataBegin,\n> -\t\t    std::vector<uint8_t>::const_iterator dataEnd,\n> -{%- if struct|needs_control_serializer %}\n> -\t\t    ControlSerializer *cs)\n> -{%- else %}\n> -\t\t    [[maybe_unused]] ControlSerializer *cs = nullptr)\n> -{%- endif %}\n> -\t{\n> -\t\t{{struct|name_full}} ret;\n> -\t\tstd::vector<uint8_t>::const_iterator m = dataBegin;\n> -\n> -\t\tsize_t dataSize = std::distance(dataBegin, dataEnd);\n> -{%- for field in struct.fields -%}\n> -{{deserializer_field(field, loop)}}\n> +\t\treturn { {\n> +{%- for field in struct.fields %}\n> +\t\t\tstd::move(*{{field.mojom_name}}),\n>  {%- endfor %}\n> -\t\treturn ret;\n> +\t\t} };\n>  \t}\n>  {%- endmacro %}\n> -\n> -{#\n> - # \\brief Deserialize a struct\n> - #\n> - # Generate code for IPADataSerializer specialization, for deserializing \\a struct.\n> - #}\n> -{%- macro deserializer(struct) %}\n> -{%- if struct|has_fd %}\n> -{{deserializer_fd(struct)}}\n> -{%- else %}\n> -{{deserializer_no_fd(struct)}}\n> -{{deserializer_fd_simple(struct)}}\n> -{%- endif %}\n> -{%- endmacro %}\n> --\n> 2.51.0","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 04899C3241\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat,  1 Nov 2025 11:19:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2492960A8A;\n\tSat,  1 Nov 2025 12:19:36 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3E388606D5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat,  1 Nov 2025 12:19:34 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:6462:5de2:153:f9b8:5024:faa2])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 71134666;\n\tSat,  1 Nov 2025 12:17:42 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"c0JmqVR0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761995862;\n\tbh=//jl0sW3q/uYacgSC1M+x39dD5ihhhPMbdZVXisl7AM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=c0JmqVR0of8ZDYfT2IXYgIZ0Loobv+0HMgLjFWTZRJznhfl5QcIviDKCeQZyUVbyW\n\tkNjygls+YbZBc3Gy+uj8MgfNgR25AYXdM1NYVk1oZkdkuZNN2+g3WDI42vLtAsLPCQ\n\tWpLYCe9wpAL1JWXFmca2e8J13kMVANGwAu1Pv3Eg=","Date":"Sat, 1 Nov 2025 12:19:30 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 2/2] utils: codegen: ipc: Simplify deserialization","Message-ID":"<gdoghqz4ruaoqdwjpl7lvdtotpdt2jw2t3s3wbb6obk4f2sm7l@26zafvg5qunq>","References":"<20251014115752.466522-1-barnabas.pocze@ideasonboard.com>\n\t<20251014115752.466522-2-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20251014115752.466522-2-barnabas.pocze@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":36650,"web_url":"https://patchwork.libcamera.org/comment/36650/","msgid":"<c5ed583a-2685-4c00-9058-0c4567722bee@ideasonboard.com>","date":"2025-11-03T12:20:58","subject":"Re: [RFC PATCH v2 2/2] utils: codegen: ipc: Simplify deserialization","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 11. 01. 12:19 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Tue, Oct 14, 2025 at 01:57:52PM +0200, Barnabás Pőcze wrote:\n>> First, introduce the `SeriReader` type, which is a collection\n>> of bytes and file descriptors, with the appropriate methods to\n>> consume the first couples bytes / file descriptors.\n>>\n>> Then a new method is added to `IPCMessage` that returns an appropriately\n>> constructed `SeriReader` for parsing the message contents.\n>>\n>> Then three of the four `deserialize()` overloads are removed, the\n>> remaining one is converted to have a single `SeriReader` and an\n>> optional `ControlSerializer` as arguments.\n>>\n>> The remaining `deserialize()` function is also changed to return an\n>> `std::optional` to be able to report deserialization failure.\n>>\n>> There is also a more fundamental change in the serialization: previously,\n>> the number of bytes taken up by an item has been written before the serialized\n>> bytes (and conditionally the number of file descriptors) when the item is\n>> serialized as part of a struct, array, map, function parameter list. This\n>> is changed: the number of bytes and file descriptors are *not* serialized\n>> into the final buffer. This affords some simplification of the serialization\n>> related code paths, but most importantly, it greatly simplifies and unifies\n>> how an object is (de)serialized because the deserialization of every object\n>> becomes completely self-contained.\n>>\n>> As a consequence of that, strings now include their lengths as part of the\n>> string serialization, and it is not left to an \"upper\" layer.\n>>\n>> Another consequence is that an \"out parameter\" of a remote function call\n>> must be deserialized if a later out parameter is needed, even if itself\n>> is not. This does not appear to be a great limitation since in all\n>> situations presently none of the out parameters are ignored.\n>>\n>> Finally, the code generation templates are adapted to the above changes.\n>> This allows the simplification of the deserialization templates as now\n>> calling `IPADataSerializer<T>::deserialize(reader, &controlSerializer_)`\n>> is appropriate for any type.\n>>\n>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=269\n>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n>> ---\n>> change in v2:\n>>    * rebase\n>>    * adjust comments\n>>\n>> v1: https://patchwork.libcamera.org/patch/23373/\n>> ---\n>>   .../libcamera/internal/ipa_data_serializer.h  | 234 +++---------\n>>   include/libcamera/internal/ipc_pipe.h         |   3 +\n>>   include/libcamera/internal/meson.build        |   1 +\n>>   include/libcamera/internal/serialization.h    |  86 +++++\n>>   src/libcamera/ipa_data_serializer.cpp         | 352 ++++--------------\n>>   src/libcamera/ipc_pipe.cpp                    |   5 +\n>>   src/libcamera/meson.build                     |   1 +\n>>   src/libcamera/serialization.cpp               |  60 +++\n>>   test/ipc/unixsocket_ipc.cpp                   |  18 +-\n>>   .../generated_serializer_test.cpp             |  17 +-\n>>   .../ipa_data_serializer_test.cpp              |  32 +-\n>>   .../module_ipa_proxy.cpp.tmpl                 |  30 +-\n>>   .../module_ipa_proxy.h.tmpl                   |   5 +-\n>>   .../module_ipa_proxy_worker.cpp.tmpl          |   5 +-\n>>   .../libcamera_templates/proxy_functions.tmpl  | 113 +-----\n>>   .../libcamera_templates/serializer.tmpl       | 238 +-----------\n>>   16 files changed, 372 insertions(+), 828 deletions(-)\n> \n> That's quite dense.\n> \n> Before getting into the details, I would like to better understand how\n> this could be unified with the control serialization effort you're\n> pushing for MetadataList.\n> \n> I think that, if we want to get into redesigining the serialization of\n> at the IPC border, we should consider it in the larger re-design\n> of the libcamera API C requirement.\n> \n> So let me digress a bit.\n> \n> MetadataList addresses the first need to serialize controls at the API\n> level in order to avoid concurrent access to a storage that doesn't\n> support thread-safe access.\n> \n> As we know the application thread could access the control list while\n> the libcamera thread populates it. The solution we designed is to\n> serialize controls into a binary buffer and implement an API on top of\n> it that guarantees safe read access to a portion of the buffer while\n> directing write access to a portion of the buffer that is not\n> available to readers.\n> \n> The MetadataList class implements a control serialization protocol\n> while we have a different controls serialization protocol for IPC\n> implemented by the ControlSerializer class and used over IPC.\n> \n> Do you envision re-implementing any of the two on top of the other ?\n> As MetadataList is \"newer\" I presume the IPC ControlSerializer should\n> be re-implemented on top the serialization format once broken-out from\n> MetadataList ?\n\nSorry, I don't have a good answer here. This will depend on how/if/when\n`MetadataList` is merged, and whether or not there is a will to unify the two.\nI am unsure what the best course of action is at the moment.\n\n\n> \n> If that's the plan, are there implications on the this patch or, as\n> I understand it, the IPADataSerializer still delegates the actual\n> control serialization format to the ControlSerializer implementation\n> and your patch only changes how the data around serialized ControlList\n> and ControlInfoMap are read ?\n\nThat is correct, the serialization of control values, lists, info maps, etc.\nis not changed.\n\n\n> \n> Now on the patch\n> \n> \n>>   create mode 100644 include/libcamera/internal/serialization.h\n>>   create mode 100644 src/libcamera/serialization.cpp\n>>\n>> diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h\n>> index 927f5c4c7..3b2e0029b 100644\n>> --- a/include/libcamera/internal/ipa_data_serializer.h\n>> +++ b/include/libcamera/internal/ipa_data_serializer.h\n>> @@ -7,6 +7,7 @@\n>>\n>>   #pragma once\n>>\n>> +#include <optional>\n>>   #include <stdint.h>\n>>   #include <string.h>\n>>   #include <tuple>\n>> @@ -23,6 +24,7 @@\n>>   #include <libcamera/ipa/ipa_interface.h>\n>>\n>>   #include \"libcamera/internal/control_serializer.h\"\n>> +#include \"libcamera/internal/serialization.h\"\n>>\n>>   namespace libcamera {\n>>\n>> @@ -39,26 +41,6 @@ void appendPOD(std::vector<uint8_t> &vec, T val)\n>>   \tmemcpy(&*(vec.end() - byteWidth), &val, byteWidth);\n>>   }\n>>\n>> -template<typename T,\n>> -\t std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>\n>> -T readPOD(std::vector<uint8_t>::const_iterator it, size_t pos,\n>> -\t  std::vector<uint8_t>::const_iterator end)\n>> -{\n>> -\tASSERT(pos + it < end);\n>> -\n>> -\tT ret = 0;\n>> -\tmemcpy(&ret, &(*(it + pos)), sizeof(ret));\n>> -\n>> -\treturn ret;\n>> -}\n>> -\n>> -template<typename T,\n>> -\t std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>\n>> -T readPOD(std::vector<uint8_t> &vec, size_t pos)\n>> -{\n>> -\treturn readPOD<T>(vec.cbegin(), pos, vec.end());\n>> -}\n>> -\n>>   } /* namespace */\n>>\n>>   template<typename T, typename = void>\n>> @@ -68,20 +50,8 @@ public:\n>>   \tstatic std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>\n>>   \tserialize(const T &data, ControlSerializer *cs = nullptr);\n>>\n>> -\tstatic T deserialize(const std::vector<uint8_t> &data,\n>> -\t\t\t     ControlSerializer *cs = nullptr);\n>> -\tstatic T deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n>> -\t\t\t     std::vector<uint8_t>::const_iterator dataEnd,\n>> -\t\t\t     ControlSerializer *cs = nullptr);\n>> -\n>> -\tstatic T deserialize(const std::vector<uint8_t> &data,\n>> -\t\t\t     const std::vector<SharedFD> &fds,\n>> -\t\t\t     ControlSerializer *cs = nullptr);\n>> -\tstatic T deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n>> -\t\t\t     std::vector<uint8_t>::const_iterator dataEnd,\n>> -\t\t\t     std::vector<SharedFD>::const_iterator fdsBegin,\n>> -\t\t\t     std::vector<SharedFD>::const_iterator fdsEnd,\n>> -\t\t\t     ControlSerializer *cs = nullptr);\n>> +\t[[nodiscard]] static std::optional<T>\n>> +\tdeserialize(SeriReader &reader, ControlSerializer *cs = nullptr);\n> \n> nice!\n> \n>>   };\n>>\n>>   #ifndef __DOXYGEN__\n>> @@ -93,8 +63,6 @@ public:\n>>    *\n>>    * For every element in the vector:\n>>    *\n>> - * 4 bytes - uint32_t Size of element, in bytes\n>> - * 4 bytes - uint32_t Number of fds for the element\n>>    * X bytes - Serialized element\n>>    *\n>>    * \\todo Support elements that are references\n>> @@ -121,9 +89,6 @@ public:\n>>   \t\t\tstd::tie(dvec, fvec) =\n>>   \t\t\t\tIPADataSerializer<V>::serialize(it, cs);\n>>\n>> -\t\t\tappendPOD<uint32_t>(dataVec, dvec.size());\n>> -\t\t\tappendPOD<uint32_t>(dataVec, fvec.size());\n>> -\n>>   \t\t\tdataVec.insert(dataVec.end(), dvec.begin(), dvec.end());\n>>   \t\t\tfdsVec.insert(fdsVec.end(), fvec.begin(), fvec.end());\n>>   \t\t}\n>> @@ -131,49 +96,22 @@ public:\n>>   \t\treturn { dataVec, fdsVec };\n>>   \t}\n>>\n>> -\tstatic std::vector<V> deserialize(std::vector<uint8_t> &data, ControlSerializer *cs = nullptr)\n>> +\t[[nodiscard]] static std::optional<std::vector<V>>\n>> +\tdeserialize(SeriReader &reader, ControlSerializer *cs = nullptr)\n>>   \t{\n>> -\t\treturn deserialize(data.cbegin(), data.cend(), cs);\n>> -\t}\n>> +\t\tuint32_t vecLen;\n>> +\t\tif (!reader.read(vecLen))\n>> +\t\t\treturn {};\n>>\n>> -\tstatic std::vector<V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n>> -\t\t\t\t\t  std::vector<uint8_t>::const_iterator dataEnd,\n>> -\t\t\t\t\t  ControlSerializer *cs = nullptr)\n>> -\t{\n>> -\t\tstd::vector<SharedFD> fds;\n>> -\t\treturn deserialize(dataBegin, dataEnd, fds.cbegin(), fds.cend(), cs);\n>> -\t}\n>> +\t\tstd::vector<V> ret;\n>> +\t\tret.reserve(vecLen);\n>>\n>> -\tstatic std::vector<V> deserialize(std::vector<uint8_t> &data, std::vector<SharedFD> &fds,\n>> -\t\t\t\t\t  ControlSerializer *cs = nullptr)\n>> -\t{\n>> -\t\treturn deserialize(data.cbegin(), data.cend(), fds.cbegin(), fds.cend(), cs);\n>> -\t}\n>> -\n>> -\tstatic std::vector<V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n>> -\t\t\t\t\t  std::vector<uint8_t>::const_iterator dataEnd,\n>> -\t\t\t\t\t  std::vector<SharedFD>::const_iterator fdsBegin,\n>> -\t\t\t\t\t  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,\n>> -\t\t\t\t\t  ControlSerializer *cs = nullptr)\n>> -\t{\n>> -\t\tuint32_t vecLen = readPOD<uint32_t>(dataBegin, 0, dataEnd);\n>> -\t\tstd::vector<V> ret(vecLen);\n>> -\n>> -\t\tstd::vector<uint8_t>::const_iterator dataIter = dataBegin + 4;\n>> -\t\tstd::vector<SharedFD>::const_iterator fdIter = fdsBegin;\n>>   \t\tfor (uint32_t i = 0; i < vecLen; i++) {\n>> -\t\t\tuint32_t sizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd);\n>> -\t\t\tuint32_t sizeofFds = readPOD<uint32_t>(dataIter, 4, dataEnd);\n>> -\t\t\tdataIter += 8;\n>> -\n>> -\t\t\tret[i] = IPADataSerializer<V>::deserialize(dataIter,\n>> -\t\t\t\t\t\t\t\t   dataIter + sizeofData,\n>> -\t\t\t\t\t\t\t\t   fdIter,\n>> -\t\t\t\t\t\t\t\t   fdIter + sizeofFds,\n>> -\t\t\t\t\t\t\t\t   cs);\n>> -\n>> -\t\t\tdataIter += sizeofData;\n>> -\t\t\tfdIter += sizeofFds;\n> Much nicer from a code point of view\n> \n> Let me address this in pieces\n> \n>> +\t\t\tauto item = IPADataSerializer<V>::deserialize(reader, cs);\n> \n> item (optionally) contains a variable of type V which has been copied\n> from the binary buffer\n> \n>> +\t\t\tif (!item)\n>> +\t\t\t\treturn {};\n>> +\n>> +\t\t\tret.emplace_back(std::move(*item));\n> \n> Then we move item and call emplace_back which calls the move\n> constructor of type V.\n> \n> However, since the data from the serialized buffer have already been\n> copied to 'item' do move() and emplace_back() actually help ? Won't we\n> end up doing a copy inside ret anyway ?\n\nThe move constructor of `V` might be more efficient that the copy constructor.\nThis is especially relevant if `V` contains e.g. `SharedFD` or `std::string`\nor other \"non trivial\" types. For trivially copyable things (e.g. just integers\nhere and there), there is no difference.\n\n\n> \n>>   \t\t}\n>>\n>>   \t\treturn ret;\n>> @@ -187,11 +125,7 @@ public:\n>>    *\n>>    * For every pair in the map:\n>>    *\n>> - * 4 bytes - uint32_t Size of key, in bytes\n>> - * 4 bytes - uint32_t Number of fds for the key\n>>    * X bytes - Serialized key\n>> - * 4 bytes - uint32_t Size of value, in bytes\n>> - * 4 bytes - uint32_t Number of fds for the value\n>>    * X bytes - Serialized value\n>>    *\n>>    * \\todo Support keys or values that are references\n>> @@ -218,18 +152,12 @@ public:\n>>   \t\t\tstd::tie(dvec, fvec) =\n>>   \t\t\t\tIPADataSerializer<K>::serialize(it.first, cs);\n>>\n>> -\t\t\tappendPOD<uint32_t>(dataVec, dvec.size());\n>> -\t\t\tappendPOD<uint32_t>(dataVec, fvec.size());\n>> -\n>>   \t\t\tdataVec.insert(dataVec.end(), dvec.begin(), dvec.end());\n>>   \t\t\tfdsVec.insert(fdsVec.end(), fvec.begin(), fvec.end());\n>>\n>>   \t\t\tstd::tie(dvec, fvec) =\n>>   \t\t\t\tIPADataSerializer<V>::serialize(it.second, cs);\n>>\n>> -\t\t\tappendPOD<uint32_t>(dataVec, dvec.size());\n>> -\t\t\tappendPOD<uint32_t>(dataVec, fvec.size());\n>> -\n>>   \t\t\tdataVec.insert(dataVec.end(), dvec.begin(), dvec.end());\n>>   \t\t\tfdsVec.insert(fdsVec.end(), fvec.begin(), fvec.end());\n>>   \t\t}\n>> @@ -237,63 +165,27 @@ public:\n>>   \t\treturn { dataVec, fdsVec };\n>>   \t}\n>>\n>> -\tstatic std::map<K, V> deserialize(std::vector<uint8_t> &data, ControlSerializer *cs = nullptr)\n>> +\t[[nodiscard]] static std::optional<std::map<K, V>>\n>> +\tdeserialize(SeriReader &reader, ControlSerializer *cs = nullptr)\n>>   \t{\n>> -\t\treturn deserialize(data.cbegin(), data.cend(), cs);\n>> -\t}\n>> +\t\tuint32_t mapLen;\n>> +\t\tif (!reader.read(mapLen))\n>> +\t\t\treturn {};\n>>\n>> -\tstatic std::map<K, V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n>> -\t\t\t\t\t  std::vector<uint8_t>::const_iterator dataEnd,\n>> -\t\t\t\t\t  ControlSerializer *cs = nullptr)\n>> -\t{\n>> -\t\tstd::vector<SharedFD> fds;\n>> -\t\treturn deserialize(dataBegin, dataEnd, fds.cbegin(), fds.cend(), cs);\n>> -\t}\n>> -\n>> -\tstatic std::map<K, V> deserialize(std::vector<uint8_t> &data, std::vector<SharedFD> &fds,\n>> -\t\t\t\t\t  ControlSerializer *cs = nullptr)\n>> -\t{\n>> -\t\treturn deserialize(data.cbegin(), data.cend(), fds.cbegin(), fds.cend(), cs);\n>> -\t}\n>> -\n>> -\tstatic std::map<K, V> deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n>> -\t\t\t\t\t  std::vector<uint8_t>::const_iterator dataEnd,\n>> -\t\t\t\t\t  std::vector<SharedFD>::const_iterator fdsBegin,\n>> -\t\t\t\t\t  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,\n>> -\t\t\t\t\t  ControlSerializer *cs = nullptr)\n>> -\t{\n>>   \t\tstd::map<K, V> ret;\n>>\n>> -\t\tuint32_t mapLen = readPOD<uint32_t>(dataBegin, 0, dataEnd);\n>> -\n>> -\t\tstd::vector<uint8_t>::const_iterator dataIter = dataBegin + 4;\n>> -\t\tstd::vector<SharedFD>::const_iterator fdIter = fdsBegin;\n>>   \t\tfor (uint32_t i = 0; i < mapLen; i++) {\n>> -\t\t\tuint32_t sizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd);\n>> -\t\t\tuint32_t sizeofFds = readPOD<uint32_t>(dataIter, 4, dataEnd);\n>> -\t\t\tdataIter += 8;\n>> -\n>> -\t\t\tK key = IPADataSerializer<K>::deserialize(dataIter,\n>> -\t\t\t\t\t\t\t\t  dataIter + sizeofData,\n>> -\t\t\t\t\t\t\t\t  fdIter,\n>> -\t\t\t\t\t\t\t\t  fdIter + sizeofFds,\n>> -\t\t\t\t\t\t\t\t  cs);\n>> -\n>> -\t\t\tdataIter += sizeofData;\n>> -\t\t\tfdIter += sizeofFds;\n>> -\t\t\tsizeofData = readPOD<uint32_t>(dataIter, 0, dataEnd);\n>> -\t\t\tsizeofFds = readPOD<uint32_t>(dataIter, 4, dataEnd);\n>> -\t\t\tdataIter += 8;\n>> -\n>> -\t\t\tconst V value = IPADataSerializer<V>::deserialize(dataIter,\n>> -\t\t\t\t\t\t\t\t\t  dataIter + sizeofData,\n>> -\t\t\t\t\t\t\t\t\t  fdIter,\n>> -\t\t\t\t\t\t\t\t\t  fdIter + sizeofFds,\n>> -\t\t\t\t\t\t\t\t\t  cs);\n>> -\t\t\tret.insert({ key, value });\n>> -\n>> -\t\t\tdataIter += sizeofData;\n>> -\t\t\tfdIter += sizeofFds;\n>> +\t\t\tauto key = IPADataSerializer<K>::deserialize(reader, cs);\n>> +\t\t\tif (!key)\n>> +\t\t\t\treturn {};\n>> +\n>> +\t\t\tauto value = IPADataSerializer<V>::deserialize(reader, cs);\n>> +\t\t\tif (!value)\n>> +\t\t\t\treturn {};\n>> +\n>> +\t\t\tauto [it, inserted] = ret.try_emplace(std::move(*key), std::move(*value));\n>> +\t\t\tif (!inserted)\n>> +\t\t\t\treturn {};\n>>   \t\t}\n>>\n>>   \t\treturn ret;\n>> @@ -314,33 +206,14 @@ public:\n>>   \t\treturn { dataVec, {} };\n>>   \t}\n>>\n>> -\tstatic Flags<E> deserialize(std::vector<uint8_t> &data,\n>> -\t\t\t\t    [[maybe_unused]] ControlSerializer *cs = nullptr)\n>> +\t[[nodiscard]] static std::optional<Flags<E>>\n>> +\tdeserialize(SeriReader &reader, [[maybe_unused]] ControlSerializer *cs = nullptr)\n>>   \t{\n>> -\t\treturn deserialize(data.cbegin(), data.cend());\n>> -\t}\n>> +\t\tuint32_t value;\n>> +\t\tif (!reader.read(value))\n>> +\t\t\treturn {};\n>>\n>> -\tstatic Flags<E> deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n>> -\t\t\t\t    std::vector<uint8_t>::const_iterator dataEnd,\n>> -\t\t\t\t    [[maybe_unused]] ControlSerializer *cs = nullptr)\n>> -\t{\n>> -\t\treturn Flags<E>{ static_cast<E>(readPOD<uint32_t>(dataBegin, 0, dataEnd)) };\n>> -\t}\n>> -\n>> -\tstatic Flags<E> deserialize(std::vector<uint8_t> &data,\n>> -\t\t\t\t    [[maybe_unused]] std::vector<SharedFD> &fds,\n>> -\t\t\t\t    [[maybe_unused]] ControlSerializer *cs = nullptr)\n>> -\t{\n>> -\t\treturn deserialize(data.cbegin(), data.cend());\n>> -\t}\n>> -\n>> -\tstatic Flags<E> deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n>> -\t\t\t\t    std::vector<uint8_t>::const_iterator dataEnd,\n>> -\t\t\t\t    [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin,\n>> -\t\t\t\t    [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,\n>> -\t\t\t\t    [[maybe_unused]] ControlSerializer *cs = nullptr)\n>> -\t{\n>> -\t\treturn deserialize(dataBegin, dataEnd);\n>> +\t\treturn Flags<E>{ static_cast<E>(value) };\n>>   \t}\n>>   };\n>>\n>> @@ -360,33 +233,14 @@ public:\n>>   \t\treturn { dataVec, {} };\n>>   \t}\n>>\n>> -\tstatic E deserialize(std::vector<uint8_t> &data,\n>> -\t\t\t     [[maybe_unused]] ControlSerializer *cs = nullptr)\n>> -\t{\n>> -\t\treturn deserialize(data.cbegin(), data.cend());\n>> -\t}\n>> -\n>> -\tstatic E deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n>> -\t\t\t     std::vector<uint8_t>::const_iterator dataEnd,\n>> -\t\t\t     [[maybe_unused]] ControlSerializer *cs = nullptr)\n>> -\t{\n>> -\t\treturn static_cast<E>(readPOD<U>(dataBegin, 0, dataEnd));\n>> -\t}\n>> -\n>> -\tstatic E deserialize(std::vector<uint8_t> &data,\n>> -\t\t\t     [[maybe_unused]] std::vector<SharedFD> &fds,\n>> -\t\t\t     [[maybe_unused]] ControlSerializer *cs = nullptr)\n>> +\t[[nodiscard]] static std::optional<E>\n>> +\tdeserialize(SeriReader &reader, [[maybe_unused]] ControlSerializer *cs = nullptr)\n>>   \t{\n>> -\t\treturn deserialize(data.cbegin(), data.cend());\n>> -\t}\n>> +\t\tU value;\n>> +\t\tif (!reader.read(value))\n>> +\t\t\treturn {};\n>>\n>> -\tstatic E deserialize(std::vector<uint8_t>::const_iterator dataBegin,\n>> -\t\t\t     std::vector<uint8_t>::const_iterator dataEnd,\n>> -\t\t\t     [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin,\n>> -\t\t\t     [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,\n>> -\t\t\t     [[maybe_unused]] ControlSerializer *cs = nullptr)\n>> -\t{\n>> -\t\treturn deserialize(dataBegin, dataEnd);\n>> +\t\treturn static_cast<E>(value);\n>>   \t}\n>>   };\n>>\n>> diff --git a/include/libcamera/internal/ipc_pipe.h b/include/libcamera/internal/ipc_pipe.h\n>> index 418c4622f..2b6cde042 100644\n>> --- a/include/libcamera/internal/ipc_pipe.h\n>> +++ b/include/libcamera/internal/ipc_pipe.h\n>> @@ -14,6 +14,7 @@\n>>   #include <libcamera/base/signal.h>\n>>\n>>   #include \"libcamera/internal/ipc_unixsocket.h\"\n>> +#include \"libcamera/internal/serialization.h\"\n>>\n>>   namespace libcamera {\n>>\n>> @@ -40,6 +41,8 @@ public:\n>>   \tconst std::vector<uint8_t> &data() const { return data_; }\n>>   \tconst std::vector<SharedFD> &fds() const { return fds_; }\n>>\n>> +\tSeriReader reader() const { return { data(), fds() }; }\n>> +\n>>   private:\n>>   \tHeader header_;\n>>\n>> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n>> index 45c299f6a..a11a52a01 100644\n>> --- a/include/libcamera/internal/meson.build\n>> +++ b/include/libcamera/internal/meson.build\n>> @@ -39,6 +39,7 @@ libcamera_internal_headers = files([\n>>       'process.h',\n>>       'pub_key.h',\n>>       'request.h',\n>> +    'serialization.h',\n>>       'shared_mem_object.h',\n>>       'source_paths.h',\n>>       'sysfs.h',\n>> diff --git a/include/libcamera/internal/serialization.h b/include/libcamera/internal/serialization.h\n>> new file mode 100644\n>> index 000000000..f20fd8ca7\n>> --- /dev/null\n>> +++ b/include/libcamera/internal/serialization.h\n>> @@ -0,0 +1,86 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2025, Ideas On Board Oy\n>> + *\n>> + * Data (de)serialization helper structures\n>> + */\n>> +#pragma once\n>> +\n>> +#include <cstddef>\n>> +#include <optional>\n>> +#include <stdint.h>\n>> +#include <string.h>\n>> +#include <tuple>\n>> +#include <type_traits>\n>> +\n>> +#include <libcamera/base/shared_fd.h>\n>> +#include <libcamera/base/span.h>\n>> +\n>> +namespace libcamera {\n>> +\n>> +class SeriReader\n>> +{\n>> +public:\n>> +\tSeriReader(Span<const uint8_t> data, Span<const SharedFD> fds = {})\n>> +\t\t: data_(reinterpret_cast<const std::byte *>(data.data()),\n>> +\t\t\treinterpret_cast<const std::byte *>(data.data() + data.size())),\n>> +\t\t  fds_(fds)\n>> +\t{\n>> +\t}\n>> +\n>> +\t[[nodiscard]] bool empty() const { return data_.empty() && fds_.empty(); }\n>> +\n>> +\t[[nodiscard]] const std::byte *consume(std::size_t s)\n>> +\t{\n>> +\t\tif (data_.size() < s)\n>> +\t\t\treturn nullptr;\n>> +\n>> +\t\tconst auto *p = data_.data();\n>> +\t\tdata_ = data_.subspan(s);\n>> +\n>> +\t\treturn p;\n>> +\t}\n> \n> I'm missing why this need to be public, shouldn't the callers use the\n> read() functions to access data instead of accessing the raw buffer ?\n> \n> I see 4 occurrences, and it seems the reason is to support handling of\n> strings, ControlInfoMap and ControlList. Is this because they're just\n> a bit special ?\n\nI think it's fine to have it as public. The idea is that `consume()` is for\nsimply consuming a number of bytes for arbitrary processing. This is especially\nrelevant for e.g. strings as you have noted, where one wants to get the bytes\nof the string, and the use that to construct the `std::string` instance. Or if\na different mechanism is needed to make sense of the bytes, e.g. ControlSerializer.\n\nWhile `read()` is mostly from extracting data into objects of concrete (mostly integral)\ntypes. I see the `read()` overloads as convenience functions on top of `consume()`.\n\n\n> \n>> +\n>> +\ttemplate<typename... Ts>\n> \n> Now, not that I'm against template packs, but I see this being only\n> used to do things like\n> \n> \tif (!reader.read(offset, length))\n> \n> isn't it equivalent to do\n> \n> \tif (!reader.read(offset))\n> \tif (!reader.read(length))\n> \n> Do you envision other usage for supporting multiple template arguments ?\n\nYes it is equivalent, but the advantage of the single call is that it\ncalculates the necessary amount of bytes once and only does a single check.\n\nI also believe it is more convenient than a longer `if` chain.\n\n\n> \n> \n>> +\t[[nodiscard]] bool read(Ts &...xs)\n>> +\t{\n>> +\t\tstatic_assert((std::is_trivially_copyable_v<Ts> && ...));\n>> +\n>> +\t\tconst auto *p = consume((sizeof(xs) + ...));\n>> +\t\tif (p)\n>> +\t\t\t((memcpy(&xs, p, sizeof(xs)), p += sizeof(xs)), ...);\n>> +\n>> +\t\treturn p;\n>> +\t}\n>> +\n>> +\ttemplate<typename... Ts>\n>> +\t[[nodiscard]] auto read()\n>> +\t{\n>> +\t\tstd::tuple<Ts...> xs;\n>> +\t\tbool ok = std::apply([&](auto &...vs) {\n>> +\t\t\treturn read(vs...);\n>> +\t\t}, xs);\n> \n> if I\n> \n>                  if (!ok)\n>                          return std::nullopt;\n> \n> here and remove the \"ok ? : \" below\n> \n> ../include/libcamera/internal/serialization.h:67:37: error: inconsistent deduction for auto return type: ‘std::nullopt_t’ and then ‘std::optional<double>’\n>     67 |                         return std::optional(std::get<0>(xs));\n> \n> \n> \n>> +\n>> +\t\tif constexpr (sizeof...(Ts) == 1)\n>> +\t\t\treturn ok ? std::optional(std::get<0>(xs)) : std::nullopt;\n>> +\t\telse\n>> +\t\t\treturn ok ? std::optional(xs) : std::nullopt;\n> \n> While it works this way....\n> \n> Not complaining about it, just wondering why the compiler can deduce\n> in one case and not in the other\n\nWell, the rules for determining the type of `(x ? y : z)` are different\nfrom the rules of return type deduction. The ternary operator is more\n\"permissive\" in a sense.\n\n\n> \n>> +\t}\n>> +\n>> +\t[[nodiscard]] const SharedFD *popFd()\n>> +\t{\n>> +\t\tif (fds_.empty())\n>> +\t\t\treturn nullptr;\n>> +\n>> +\t\tconst auto *p = fds_.data();\n>> +\t\tfds_ = fds_.subspan(1);\n>> +\n>> +\t\treturn p;\n>> +\t}\n>> +\n>> +private:\n>> +\tSpan<const std::byte> data_;\n>> +\tSpan<const SharedFD> fds_;\n>> +};\n>> +\n>> +} /* namespace libcamera */\n> \n> Do you plan a SeriWriter ? :)\n\nWell, I would like to. Currently serialization creates a lot of temporary\n`std::vector`s. It would be nice to eliminate them.\n\n\n> \n> The rest looks good (I didn't look at the mojo part) and there's no\n> doubt this is better.\n> \n> However, I would like to see it tested with an IPC running isolated.\n> Did you have chance to give it a spin ?\n\nI have been using this since the first submission, so I have tested it\nmany times on rpi and rkisp1.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> Thanks\n>    j\n> \n> [...]","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 32B59BDE4C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  3 Nov 2025 12:21:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2D490609D8;\n\tMon,  3 Nov 2025 13:21:04 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8EA40606A0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  3 Nov 2025 13:21:02 +0100 (CET)","from [192.168.33.39] (185.221.140.239.nat.pool.zt.hu\n\t[185.221.140.239])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 378C999F;\n\tMon,  3 Nov 2025 13:19:09 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"vheo5z+2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1762172349;\n\tbh=+78+1j01HEt0DDCl7cJzhWHJ5O7JqrZO/KZgepNTYTA=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=vheo5z+2BkeFsUOULBVxQ6V02EIPLB0lE6BerUb3GeBWpz57GmdYW/APrVqSP3E/v\n\tDWN4FHkxSvt8qjVijm8k9k9tAoAkVNDWgzI8iWX5M4nM5tBtEvy1MS3xmz3FR7mBpm\n\tZX0cDCkfgyuTo109eoHlU+elGmmDD83fd4ZgP8Iw=","Message-ID":"<c5ed583a-2685-4c00-9058-0c4567722bee@ideasonboard.com>","Date":"Mon, 3 Nov 2025 13:20:58 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v2 2/2] utils: codegen: ipc: Simplify deserialization","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20251014115752.466522-1-barnabas.pocze@ideasonboard.com>\n\t<20251014115752.466522-2-barnabas.pocze@ideasonboard.com>\n\t<gdoghqz4ruaoqdwjpl7lvdtotpdt2jw2t3s3wbb6obk4f2sm7l@26zafvg5qunq>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<gdoghqz4ruaoqdwjpl7lvdtotpdt2jw2t3s3wbb6obk4f2sm7l@26zafvg5qunq>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]