[libcamera-devel,RFC,v2,DO,NOT,MERGE] IPA: raspberrypi: Sample IPA data structure IDL definition and output

8916 diff mbox series

In this whole patch, keep in mind that whenever flatbuffers is
mentioned, it could be replaced by anything else, such as protobuf.

Also, in general, naming of anything is flexible. I tried to chose
reasonable placeholders to show how things would work.

I have not yet done the theoretical plumbing. I wanted to get this draft
out first at least, before continuing too far without feedback.

* Introduction

raspberrypi.libcamera.decl shows a sample custom IPA data structure IDL
definition for the raspberrypi pipeline handler. The intent is that I
will implement a custom compiler that will compile this definition into
flatbuffers, as well as the header that will be included in the pipeline
handler and the IPA. flatbuffers will compile the flatbuffers definition
into the flatbuffers header that the included header will use for serdes.

The purpose of this custom IPA data structure definition is to allow
pipeline handlers and IPAs to use custom data structures for configure,
processEvent, and queueFrameAction, rather than the rigid
IPAOperationData. Hooking it up with framebuffers, and the compiler that
I inted to write, will implement serdes for these custom data structures
that the IPAProxies can use.

raspberrypi.fbs shows the intended output of the compiler that I intend
to write, which will be in the input to the flatbuffers compiler.

raspberrypi_wrapper.h shows the other intended output of the of the
compiler that I inted to write. This is the header that will be included
by the pipeline handler and the IPA. The structs that are defined in
this header can be used directly by the pipeline handler and IPA, and
the serialize function and the deserialization constructor are meant to
be used by the IPAProxy (maybe they could be made private and IPAProxy
into a friend class?).

The custom compiler is necessary to place restrictions on the allowed
data definitions and make the definitions cleaner based on the
restrictions, as well as generate a header that is a wrapper around the
flatbuffers-generated header for the pipeline handler and IPA to include.

* Custom data definition file

raspberrypi.libcamera.decl

There are three different types of custom data structures that must be
defined, for the three IPA functions that take IPAOperationData:
- configure (for IPAInterface::configure())
- event (for IPAInterface::processEvent())
- action (for IPAInterface::queueFrameAction)

Discussion point: should configure have a separate input and output data
definition?

Since all three have an "opcode" and a payload, that is all that needs
to be defined. The opcodes can be seen in the enums ConfigOps, EventOps,
and ActionOps. The payloads can be seen in the unions ConfigPayload,
EventPayload, and ActionPayload. Since the opcodes are essentially
selectors into the unions, which field of the union the opcode points to
must be specified as shown in the enums.

Discussion point: ConfigPayload will become an element in a vector, so
there can be multiple instances of in in a single configure() call.
Conversely, EventPayload and ActionPayload are scalars in
processEvent() and queueFrameAction. Should they be vectorized like
ConfigPayload?

Discussion point: can unions be disallowed besides at the top level of
the data definition tree in the payload?

These opcode and payload definitions are the only required components of
the custom data definition. Once these are satisfied, the rest of the
file follows standard flatbuffers rules.

Also, since flatbuffers structs are a bit of a pain, I've decided to
only allow flatbuffers tables in the definition (flatbuffers structs are
not allowed ). In C++ (and C if I ever have to implement that compiler
too...) they're wrapped as structs, though.

Unions cannot have unions as a direct child.

* flatbuffers input file

raspberrypi.fbs is the intended output with raspberrypi.libcamera.decl,
as described above.

Some things to note...

- RPiConfigParameters is not flags. I decided that rather than scanning
  flags and parsing a vector payload based on that, it would be better
  to have a vector of opcode-payload pairs.

- NONE needs to be added to enums to appease flatbuffers. One of the
  many reasons why I think we should have a custom data definition to
  make writing the data definition cleaner for developers.

- In flatbuffers, primitives cannot be members of unions. I'll add a
  global header that defines wrappers around primitives. In the custom
  data definition they are allowed, though. My compiler shall convert
  them to the wrapper.

- libcamera already comes with a really nice ControlList serdes. In the
  flatbuffers global header I'll add the ControlList definition that is
  seen here. It's just defined as a byte array, so we can use
  libcamera's ControlList serializer. It does need to be renamed,
  though.

- As mentioned earlier, RPiConfigParams has a vector of opcode-payload
  pairs, while RPiEventParams and RPiActionParams have a single
  opcode-payload pair

* Custom generated header

raspberrypi_wrapper.h

flatbuffers will compile the flatbuffers input file into a flatbuffers
header that can be included. This file cannot be used directly by
pipeline handlers and IPAs without them directly interfacing with
flatbuffers and its API. Thus, a custom header will also be generated by
the compiler that I'll implement. This custom header implements wrappers
for the data structures defined in the custom data definition file for
pipeline handlers and IPAs to use, as well as serialize and deserialize
functions for the proxies to use. The intended output of this header can
be seen in raspberrypi_wrapper.h.

The structs and enums and unions defined in this header can be used by
pipeline handlers and IPAs as-is (ie. no special API like flatbuffers).

Discussion point: maybe make the serialize function and deserialization
constructor private and make IPAProxy a friend class?

A Serializable class is reference but never defined. This class is meant
to simply have a serialize() function (deserialize is implemented as an
overloaded constructor). This class might not even be necessary.

There are a couple helpers for serializing and deserializing
ControlList such that it can work well with the flatbuffers data
structures. I got the implementation from the test for
ControlSerializer.

There are also a couple helpers for extracting the fds and injecting
the fds. The fd extraction is meant to be done by the IPAProxy prior to
serialization. The IPAProxy can then use sendmsg and recvmsg (or some
similar facility) to send the fds across the process boundary. These fds
can then be injected back into the object after it is deserialized and
reconstructed. The extracted fds are saved in a queue, in post-order.
Since the data container tree is the same before and after serdes, the
order of the fds is guaranteed to match, as the fds are injected also in
post-order.

Serialization, deserialization, fd extraction, and fd injection are all
implemented with recursion. The fd extraction and injection functions
may or may not be be ommitted if no members in the data container tree
contain no fds. This is TBD.

There are a few rules for the compilation...

- unions and enums are as-is

- tables are compiled with its members as member variables, and have
  three constructors, 1) empty default, 2) define all members, and
  3) deserialization constructor. They also get a serialize function
  that takes a flatbuffer builder and outputs a flatbuffer offset. This
  allows recursion to do serialization. Only the top-level
  (RPiConfigureParamsWrapper, RPiEventParamsWrapper,
  RPiActionParamsWrapper) tables get a serialize functions that takes
  void and returns a pair of byte array and length.

- serialization is implemented as:
  - for every member variable:
    - if it is primitive, nop
    - if it is controls, call serializeControlList
    - if it is table, call table's serialize
    - if it is a vector, convert the vector to a vector of flatbuffer
      offsets by serializing every member based on the above three rules
    - if it is a union:
      - switch on opcode:
        - set union type based on opcode
        - set union content based on the same above four rules
  - call flatbuffer's create object with all the serialized members

- deserialization is implemented as a constructor. top-level tables take
  a pointer to a byte array as input, while non-top-level tables take a
  pointer to a byte array casted to the flatbuffers object. If the table
  is used in a vector anywhere in the data definition, there will be
  another deserialization constructor that takes a flatbuffer offset.
  - cast input to flatbuffers offset (if necessary)
  - for every member variable:
    - if it is primitive, set the member variable
    - if it is controls, set member variable with deserializeControlList
    - if it is table, construct the table with the deserialization
      constructor, and set the member variable
    - if it is vector, convert the vector of flatbuffer offsets into the
      vector of the member variable by deserializing based on the above
      three rules
    - if it is union:
      - switch on opcode:
        - set member variable union content based on same above four rules

- fd extraction is implemented as:
  - for every member variable:
    - if it is primitive or contols, nop
    - if it is table, extract the fds from the table, and add to
      extracted fds queue
    - if it is a vector, append the extracted fds for each element of
      the table, according to the above two rules
    - if it is a union:
      - switch on opcode:
        - append extracted fds according to above three rules

- fd injection is implemented as the inverse of fd extraction, so
  instead of enqueueing the fds, the fds are dequeued and assigned

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

---
Chanes in v2:
- add fd support to inteded custom compiler output to ease the
  IPAProxy's job of extracting, sending the fds over the process
  boundary, and reinjecting the fds
- add a declaration to the custom data definition to specify the
  ControlInfoMap (the ControlSerializer needs this)
- add discussion point, if we can disallow unions in data definition
  outside of the top-level payload container
In the commit message:
- add paragraph about fd extraction and injection in "Custom generated
  header" section
- remove paragraph about "fds not supported" in pre-introduction
- add description of fd extraction and injection mechanism in
  compilation rules section of "Custom generated header" section
---
 include/libcamera/ipa/raspberrypi.fbs         | 107 +++
 include/libcamera/ipa/raspberrypi.h           |  18 -
 .../libcamera/ipa/raspberrypi.libcamera.decl  |  83 +++
 include/libcamera/ipa/raspberrypi_wrapper.h   | 658 ++++++++++++++++++
 4 files changed, 848 insertions(+), 18 deletions(-)
 create mode 100644 include/libcamera/ipa/raspberrypi.fbs
 create mode 100644 include/libcamera/ipa/raspberrypi.libcamera.decl
 create mode 100644 include/libcamera/ipa/raspberrypi_wrapper.h

Hi Paul,

Thank you for the patch.

On Wed, Jul 22, 2020 at 09:25:43PM +0900, Paul Elder wrote:
> In this whole patch, keep in mind that whenever flatbuffers is
> mentioned, it could be replaced by anything else, such as protobuf.
> 
> Also, in general, naming of anything is flexible. I tried to chose
> reasonable placeholders to show how things would work.
> 
> I have not yet done the theoretical plumbing. I wanted to get this draft
> out first at least, before continuing too far without feedback.
> 
> * Introduction

Would it be possible to add a meta-introduction to explain what you're
trying to do, before diving into the details ? A general problem
statement would be useful, both as a reference for other developers who
may not be familiar with the topic at hand, and as a way to make sure we
have the same understanding of the problem.

> raspberrypi.libcamera.decl shows a sample custom IPA data structure IDL
> definition for the raspberrypi pipeline handler. The intent is that I
> will implement a custom compiler that will compile this definition into
> flatbuffers, as well as the header that will be included in the pipeline
> handler and the IPA. flatbuffers will compile the flatbuffers definition
> into the flatbuffers header that the included header will use for serdes.
> 
> The purpose of this custom IPA data structure definition is to allow
> pipeline handlers and IPAs to use custom data structures for configure,
> processEvent, and queueFrameAction, rather than the rigid
> IPAOperationData. Hooking it up with framebuffers, and the compiler that
> I inted to write, will implement serdes for these custom data structures
> that the IPAProxies can use.
> 
> raspberrypi.fbs shows the intended output of the compiler that I intend
> to write, which will be in the input to the flatbuffers compiler.

This two-steps procedure would allow making the underlying serialization
library/framework an implementation detail, which has the upside of
simplifying a switch to a different framework. Let's however not forget
that we don't want to spend cycles unnecessarily changing the underlying
implementation, and that we need to guarantee an ABI stability for
closed-source IPAs. Whether that ABI will be the wire representation of
data or a deserialized format produced by the IPAProxy worker remains to
be decided.

> raspberrypi_wrapper.h shows the other intended output of the of the
> compiler that I inted to write. This is the header that will be included
> by the pipeline handler and the IPA. The structs that are defined in
> this header can be used directly by the pipeline handler and IPA, and
> the serialize function and the deserialization constructor are meant to
> be used by the IPAProxy (maybe they could be made private and IPAProxy
> into a friend class?).
> 
> The custom compiler is necessary to place restrictions on the allowed
> data definitions and make the definitions cleaner based on the
> restrictions, as well as generate a header that is a wrapper around the
> flatbuffers-generated header for the pipeline handler and IPA to include.

This may just be a matter of naming and code refactoring, but I want to
minimize the exposure of pipeline handlers and IPA modules to
serialization. In particular, this means that data type names should not
include a 'Wrapper' suffix but should use the native names.

> * Custom data definition file
> 
> raspberrypi.libcamera.decl
> 
> There are three different types of custom data structures that must be
> defined, for the three IPA functions that take IPAOperationData:
> - configure (for IPAInterface::configure())
> - event (for IPAInterface::processEvent())
> - action (for IPAInterface::queueFrameAction)
> 
> Discussion point: should configure have a separate input and output data
> definition?

Are you talking about the existing ipaConfig and result parameters here
? While they currently use the same data type (IPAOperationData), they
carry different information. What's the alternative ? Using a single
inout argument ? I think separating inputs and outputs will result in a
much cleaner solution.

> Since all three have an "opcode" and a payload, that is all that needs
> to be defined.

We currently have an opcode, but that doesn't mean this is the desired
outcome. I see the existing opcode to be more of a quick hack than a
good solution.

> The opcodes can be seen in the enums ConfigOps, EventOps,
> and ActionOps. The payloads can be seen in the unions ConfigPayload,
> EventPayload, and ActionPayload. Since the opcodes are essentially
> selectors into the unions, which field of the union the opcode points to
> must be specified as shown in the enums.

The opcode is currently abused by the IPA configure() operation. Its
original intent was to be an opcode for queueFrameAction and
processEvent, and it's now also used as a bitfield in configure() to
describe the data that is being transported in the IPAOperationData.

> Discussion point: ConfigPayload will become an element in a vector, so
> there can be multiple instances of in in a single configure() call.
> Conversely, EventPayload and ActionPayload are scalars in
> processEvent() and queueFrameAction. Should they be vectorized like
> ConfigPayload?

I'm not sure to follow you. Why will ConfigPayload become an element in
a vector ? Is this because the current usage of the opcode as a bitfield
will be turned into a vector of single opcode + data ? [edit: I think
you answer this question below, and the answer is a yes]

I think it would be better to instead have a struct that contains all
the parameters, with nullable elements. What I mean by that is

struct StaggeredWriteConfig {
	uint32 gainDelay;
	uint32 exposureDelay;
	bool sensorMetadata;
};

struct ConfigResult {
	StaggeredWriteConfig staggeredWriteConfig;
	ControlList sensorControls;
};

with ConfigResult being the type returned by configure(), and both
staggeredWriteConfig and sensorControls being nullable types
(effectively pointers in C++ code). We wouldn't need an opcode.

> Discussion point: can unions be disallowed besides at the top level of
> the data definition tree in the payload?

I'd rather disallow unions completely :-)

> These opcode and payload definitions are the only required components of
> the custom data definition. Once these are satisfied, the rest of the
> file follows standard flatbuffers rules.
> 
> Also, since flatbuffers structs are a bit of a pain, I've decided to
> only allow flatbuffers tables in the definition (flatbuffers structs are
> not allowed ). In C++ (and C if I ever have to implement that compiler
> too...) they're wrapped as structs, though.

I think this should be an implementation detail. I'd rather not use the
keyword "table" in the IDL exposed to the pipeline handler and IPA.

Generally speaking, as our main language target is C++, I think we
should also use an IDL syntax that ressembles C++ where we have the
option to do so. Among the IDLs I've looked at (flatbuffers, protobuf
and mojo), mojo seems the most familiar to me. It has the advantages (in
my opinion) to define types in a way that focusses on the data instead
of the protocol (see the "message" keyword in the protobuf IDL) or
storage (see the "table" keyword in the flatbuffers IDL). I would like
pipeline handlers and IPAs to avoid thinking about IPC as much as
possible.

One possible advantage of using the mojo IDL is that we could leverage
the parser, which is written in Python.

> Unions cannot have unions as a direct child.
> 
> * flatbuffers input file
> 
> raspberrypi.fbs is the intended output with raspberrypi.libcamera.decl,
> as described above.
> 
> Some things to note...
> 
> - RPiConfigParameters is not flags. I decided that rather than scanning
>   flags and parsing a vector payload based on that, it would be better
>   to have a vector of opcode-payload pairs.

I think this answers my above question.

> - NONE needs to be added to enums to appease flatbuffers. One of the
>   many reasons why I think we should have a custom data definition to
>   make writing the data definition cleaner for developers.

Could you explain why this is ?

> - In flatbuffers, primitives cannot be members of unions. I'll add a
>   global header that defines wrappers around primitives. In the custom
>   data definition they are allowed, though. My compiler shall convert
>   them to the wrapper.
> 
> - libcamera already comes with a really nice ControlList serdes. In the
>   flatbuffers global header I'll add the ControlList definition that is
>   seen here. It's just defined as a byte array, so we can use
>   libcamera's ControlList serializer. It does need to be renamed,
>   though.

Note that we can refactor the ControlList serializer/deserializer if
needed.

> - As mentioned earlier, RPiConfigParams has a vector of opcode-payload
>   pairs, while RPiEventParams and RPiActionParams have a single
>   opcode-payload pair
> 
> * Custom generated header
> 
> raspberrypi_wrapper.h
> 
> flatbuffers will compile the flatbuffers input file into a flatbuffers
> header that can be included. This file cannot be used directly by
> pipeline handlers and IPAs without them directly interfacing with
> flatbuffers and its API. Thus, a custom header will also be generated by
> the compiler that I'll implement. This custom header implements wrappers
> for the data structures defined in the custom data definition file for
> pipeline handlers and IPAs to use, as well as serialize and deserialize
> functions for the proxies to use. The intended output of this header can
> be seen in raspberrypi_wrapper.h.
> 
> The structs and enums and unions defined in this header can be used by
> pipeline handlers and IPAs as-is (ie. no special API like flatbuffers).
> 
> Discussion point: maybe make the serialize function and deserialization
> constructor private and make IPAProxy a friend class?

Friendship isn't inherited in C++, so you wouldn't be able to call the
functions from a subclass of IPAProxy. I don't see a problem in making
those methods public. They will be in generated code, and nobody should
notice.

> A Serializable class is reference but never defined. This class is meant
> to simply have a serialize() function (deserialize is implemented as an
> overloaded constructor). This class might not even be necessary.

I suppose you will define it (unless it ends up not being necessary)
though ? You can't inherit from a class that isn't defined :-)

> There are a couple helpers for serializing and deserializing
> ControlList such that it can work well with the flatbuffers data
> structures. I got the implementation from the test for
> ControlSerializer.
> 
> There are also a couple helpers for extracting the fds and injecting
> the fds. The fd extraction is meant to be done by the IPAProxy prior to
> serialization. The IPAProxy can then use sendmsg and recvmsg (or some
> similar facility) to send the fds across the process boundary. These fds
> can then be injected back into the object after it is deserialized and
> reconstructed. The extracted fds are saved in a queue, in post-order.
> Since the data container tree is the same before and after serdes, the
> order of the fds is guaranteed to match, as the fds are injected also in
> post-order.

Wouldn't it be simpler if the serialize and deserialize functions were
given a serialization context that contained both the flatbuffer context
and a vector of fds ? We wouldn't need separate fd handling functions.

Note that C++ also makes it possible to move some of the serialization
and deserialization implementation out of the generated wrapper classes
using templates. Something along the lines of the following.

- Base class

template<typename T>
class Serializer
{
};

- Generated class

template<>
class Serializer<MyDataType>
{
public:
	static void serialize(MyDataType &input, Buffer *buffer)
	{
		...
	}
};

- Usage

	MyDataType foo(...);
	...
	Serializer::serialize(foo, buffer);

This could possibly help keeping the wrapper header lean and clean, and
moving serialization code to a generated .cpp file.

> Serialization, deserialization, fd extraction, and fd injection are all
> implemented with recursion. The fd extraction and injection functions
> may or may not be be ommitted if no members in the data container tree
> contain no fds. This is TBD.
> 
> There are a few rules for the compilation...
> 
> - unions and enums are as-is
> 
> - tables are compiled with its members as member variables, and have
>   three constructors, 1) empty default, 2) define all members, and
>   3) deserialization constructor. They also get a serialize function
>   that takes a flatbuffer builder and outputs a flatbuffer offset. This
>   allows recursion to do serialization. Only the top-level
>   (RPiConfigureParamsWrapper, RPiEventParamsWrapper,
>   RPiActionParamsWrapper) tables get a serialize functions that takes
>   void and returns a pair of byte array and length.
> 
> - serialization is implemented as:
>   - for every member variable:
>     - if it is primitive, nop
>     - if it is controls, call serializeControlList
>     - if it is table, call table's serialize
>     - if it is a vector, convert the vector to a vector of flatbuffer
>       offsets by serializing every member based on the above three rules
>     - if it is a union:
>       - switch on opcode:
>         - set union type based on opcode
>         - set union content based on the same above four rules
>   - call flatbuffer's create object with all the serialized members
> 
> - deserialization is implemented as a constructor. top-level tables take
>   a pointer to a byte array as input, while non-top-level tables take a
>   pointer to a byte array casted to the flatbuffers object. If the table
>   is used in a vector anywhere in the data definition, there will be
>   another deserialization constructor that takes a flatbuffer offset.
>   - cast input to flatbuffers offset (if necessary)
>   - for every member variable:
>     - if it is primitive, set the member variable
>     - if it is controls, set member variable with deserializeControlList
>     - if it is table, construct the table with the deserialization
>       constructor, and set the member variable
>     - if it is vector, convert the vector of flatbuffer offsets into the
>       vector of the member variable by deserializing based on the above
>       three rules
>     - if it is union:
>       - switch on opcode:
>         - set member variable union content based on same above four rules
> 
> - fd extraction is implemented as:
>   - for every member variable:
>     - if it is primitive or contols, nop
>     - if it is table, extract the fds from the table, and add to
>       extracted fds queue
>     - if it is a vector, append the extracted fds for each element of
>       the table, according to the above two rules
>     - if it is a union:
>       - switch on opcode:
>         - append extracted fds according to above three rules
> 
> - fd injection is implemented as the inverse of fd extraction, so
>   instead of enqueueing the fds, the fds are dequeued and assigned
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

I think this is a good base for discussions. What I'm missing though is
an example of how this is used by the pipeline handler and IPA. Reading
all your explanation I expect this to be fairly transparent.

A few additional questions that come to my mind:

- How do you foresee handling of closed-source IPAs that don't want to
  link to libcamera ? We currently have a C-based API, which may be
  cumbersome to keep.

- At what level do you think we should handle the IPA ABI compatibility
  ? If this is handled at the wire level, does flatbuffer document the
  serialized format ? I came across
  https://docs.google.com/document/d/13pv9cFh5YKuBggDBQ1-AL8VReF-IYpFOFpRfvWFrwio/edit
  and
  https://docs.google.com/document/d/1jNcsxOdO3Al52s6lIrMOOgY7KXB7TJ8wGGWstAHiTd8/edit
  which may be interesting readings.

> ---
> Chanes in v2:
> - add fd support to inteded custom compiler output to ease the
>   IPAProxy's job of extracting, sending the fds over the process
>   boundary, and reinjecting the fds
> - add a declaration to the custom data definition to specify the
>   ControlInfoMap (the ControlSerializer needs this)
> - add discussion point, if we can disallow unions in data definition
>   outside of the top-level payload container
> In the commit message:
> - add paragraph about fd extraction and injection in "Custom generated
>   header" section
> - remove paragraph about "fds not supported" in pre-introduction
> - add description of fd extraction and injection mechanism in
>   compilation rules section of "Custom generated header" section
> ---
>  include/libcamera/ipa/raspberrypi.fbs         | 107 +++
>  include/libcamera/ipa/raspberrypi.h           |  18 -
>  .../libcamera/ipa/raspberrypi.libcamera.decl  |  83 +++
>  include/libcamera/ipa/raspberrypi_wrapper.h   | 658 ++++++++++++++++++
>  4 files changed, 848 insertions(+), 18 deletions(-)
>  create mode 100644 include/libcamera/ipa/raspberrypi.fbs
>  create mode 100644 include/libcamera/ipa/raspberrypi.libcamera.decl
>  create mode 100644 include/libcamera/ipa/raspberrypi_wrapper.h
> 
> diff --git a/include/libcamera/ipa/raspberrypi.fbs b/include/libcamera/ipa/raspberrypi.fbs
> new file mode 100644
> index 0000000..dcc1918
> --- /dev/null
> +++ b/include/libcamera/ipa/raspberrypi.fbs
> @@ -0,0 +1,107 @@
> +enum RPiConfigParameters:uint16 {
> +	NONE = 0,
> +	RPI_IPA_CONFIG_LS_TABLE,
> +	RPI_IPA_CONFIG_STAGGERED_WRITE,
> +	RPI_IPA_CONFIG_SENSOR,
> +	RPI_IPA_CONFIG_SEND_FD,
> +}
> +
> +enum RPiEvents:uint16 {
> +	NONE = 0,
> +	RPI_IPA_EVENT_SIGNAL_STAT_READY,
> +	RPI_IPA_EVENT_SIGNAL_ISP_PREPARE,
> +	RPI_IPA_EVENT_QUEUE_REQUEST,
> +	RPI_IPA_EVENT_SEND_FD,
> +}
> +
> +enum RPiActions:uint16 {
> +	NONE = 0,
> +	RPI_IPA_ACTION_V4L2_SET_STAGGERED,
> +	RPI_IPA_ACTION_V4L2_SET_ISP,
> +	RPI_IPA_ACTION_STATS_METADATA_COMPLETE,
> +	RPI_IPA_ACTION_RUN_ISP,
> +	RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME,
> +	RPI_IPA_ACTION_EMBEDDED_COMPLETE,
> +}
> +
> +// TODO define this for all primitive types, and make global
> +table UnsignedInt {
> +	value:uint32;
> +}
> +
> +table SignedInt {
> +	value:int32;
> +}
> +
> +// TODO make this global
> +// TODO rename this
> +table ControlList {
> +	list:[uint8];
> +}
> +
> +// Data containers
> +
> +table RPiStaggeredWritePayload {
> +	gainDelay:uint32;
> +	exposureDelay:uint32;
> +	sensorMetadata:uint32;
> +}
> +
> +table RPiIspPreparePayload {
> +	embeddedbufferId:uint32;
> +	bayerbufferId:uint32;
> +}
> +
> +table RPiStatsCompletePayload {
> +	bufferId:uint32;
> +	controls:ControlList;
> +}
> +
> +
> +// Payload unions
> +
> +union RPiConfigureUnion {
> +	lsTableHandle:UnsignedInt,
> +	staggeredWriteResult:RPiStaggeredWritePayload,
> +	controls:ControlList,
> +	bufferFd:SignedInt,
> +}
> +
> +table RPiConfigurePayload {
> +        op:RPiConfigParameters;
> +	payload:RPiConfigureUnion;
> +}
> +
> +union RPiEventPayload {
> +	bufferId:UnsignedInt,
> +	ispPrepare:RPiIspPreparePayload,
> +	controls:ControlList,
> +	bufferFd:SignedInt,
> +}
> +
> +union RPiActionPayload {
> +	bufferId:UnsignedInt,
> +	statsComplete:RPiStatsCompletePayload,
> +	controls:ControlList,
> +}
> +
> +
> +// IPA function parameters
> +
> +table RPiConfigParams {
> +	params:[RPiConfigurePayload];
> +}
> +
> +table RPiEventParams {
> +	ev:RPiEvents;
> +	payload:RPiEventPayload;
> +}
> +
> +table RPiActionParams {
> +	op:RPiActions;
> +	payload:RPiActionPayload;
> +}
> +
> +root_type RPiConfigureParams;
> +root_type RPiEventParams;
> +root_type RPiActionParams;
> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> index a493776..69b3808 100644
> --- a/include/libcamera/ipa/raspberrypi.h
> +++ b/include/libcamera/ipa/raspberrypi.h
> @@ -10,24 +10,6 @@
>  #include <libcamera/control_ids.h>
>  #include <libcamera/controls.h>
>  
> -enum RPiConfigParameters {
> -	RPI_IPA_CONFIG_LS_TABLE = (1 << 0),
> -	RPI_IPA_CONFIG_STAGGERED_WRITE = (1 << 1),
> -	RPI_IPA_CONFIG_SENSOR = (1 << 2),
> -};
> -
> -enum RPiOperations {
> -	RPI_IPA_ACTION_V4L2_SET_STAGGERED = 1,
> -	RPI_IPA_ACTION_V4L2_SET_ISP,
> -	RPI_IPA_ACTION_STATS_METADATA_COMPLETE,
> -	RPI_IPA_ACTION_RUN_ISP,
> -	RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME,
> -	RPI_IPA_ACTION_EMBEDDED_COMPLETE,
> -	RPI_IPA_EVENT_SIGNAL_STAT_READY,
> -	RPI_IPA_EVENT_SIGNAL_ISP_PREPARE,
> -	RPI_IPA_EVENT_QUEUE_REQUEST,
> -};
> -
>  enum RPiIpaMask {
>  	ID		= 0x0ffff,
>  	STATS		= 0x10000,
> diff --git a/include/libcamera/ipa/raspberrypi.libcamera.decl b/include/libcamera/ipa/raspberrypi.libcamera.decl
> new file mode 100644
> index 0000000..851ae6c
> --- /dev/null
> +++ b/include/libcamera/ipa/raspberrypi.libcamera.decl
> @@ -0,0 +1,83 @@
> +namespace RPi;
> +
> +// this is necessary for de/serializeControlList()
> +// define in include/libcamera/ipa/raspberrypi.h
> +ControlInfoMap RPiControls;
> +
> +// opcodes
> +// these must be enums, and must be named as shown
> +
> +// required
> +// must specify which union option each value corresponds to
> +enum ConfigOps {
> +	RPI_IPA_CONFIG_LS_TABLE:lsTableHandle,
> +	RPI_IPA_CONFIG_STAGGERED_WRITE:staggeredWriteResult,
> +	RPI_IPA_CONFIG_SENSOR:controls,
> +	RPI_IPA_CONFIG_SEND_FD:bufferFd,
> +}
> +
> +// required
> +// must specify which union option each value corresponds to
> +enum EventOps {
> +	RPI_IPA_EVENT_SIGNAL_STAT_READY:bufferId,
> +	RPI_IPA_EVENT_SIGNAL_ISP_PREPARE:ispPrepare,
> +	RPI_IPA_EVENT_QUEUE_REQUEST:controls,
> +	RPI_IPA_EVENT_SEND_FD:bufferFd,
> +}
> +
> +// required
> +// must specify which union option each value corresponds to
> +enum ActionOps {
> +	RPI_IPA_ACTION_V4L2_SET_STAGGERED:controls,
> +	RPI_IPA_ACTION_V4L2_SET_ISP:controls,
> +	RPI_IPA_ACTION_STATS_METADATA_COMPLETE:statsComplete,
> +	RPI_IPA_ACTION_RUN_ISP:bufferId,
> +	RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME:bufferId,
> +	RPI_IPA_ACTION_EMBEDDED_COMPLETE:bufferId,
> +}
> +
> +// Custom Data containers
> +
> +table RPiStaggeredWritePayload {
> +	gainDelay:uint32;
> +	exposureDelay:uint32;
> +	sensorMetadata:uint32;
> +}
> +
> +table RPiIspPreparePayload {
> +	embeddedbufferId:uint32;
> +	bayerbufferId:uint32;
> +}
> +
> +table RPiStatsCompletePayload {
> +	bufferId:uint32;
> +	controls:ControlList;
> +}
> +
> +
> +// First level payload unions
> +// these must be unions, and must be named as shown
> +
> +// required
> +// this one is actually in a vector of payload objects (op-union pair)
> +union ConfigPayload {
> +	lsTableHandle:uint32,
> +	staggeredWriteResult:RPiStaggeredWritePayload,
> +	controls:ControlList,
> +	bufferFd:int32,
> +}
> +
> +// required
> +union EventPayload {
> +	bufferId:uint32,
> +	ispPrepare:RPiIspPreparePayload,
> +	controls:ControlList,
> +	bufferFd:int32,
> +}
> +
> +// required
> +union ActionPayload {
> +	bufferId:uint32,
> +	statsComplete:RPiStatsCompletePayload,
> +	controls:ControlList,
> +}
> diff --git a/include/libcamera/ipa/raspberrypi_wrapper.h b/include/libcamera/ipa/raspberrypi_wrapper.h
> new file mode 100644
> index 0000000..eab51d3
> --- /dev/null
> +++ b/include/libcamera/ipa/raspberrypi_wrapper.h
> @@ -0,0 +1,658 @@
> +// will be automatically generated by custom compiler
> +
> +#ifndef __LIBCAMERA_IPA_INTERFACE_RASPBERRYPI_FB_H__
> +#define __LIBCAMERA_IPA_INTERFACE_RASPBERRYPI_FB_H__
> +
> +#include "raspberrypi_generated.h"
> +
> +namespace libcamera {
> +
> +// TODO this (and deserialize) should go global
> +inline flatbuffers::Offset<ControlList> serializeControlList(flatbuffers::FlatBufferBuilder &builder, ControlList &list)
> +{
> +	ControlSerializer serializer;
> +
> +	// TODO need a way to get the info map (param? #defined param?)
> +	size_t size = serializer.binarySize(RPiControls);
> +	std::vector<uint8_t> infoData(size);
> +	ByteStreamBuffer buffer(infoData.data(), infoData.size());
> +	serializer.serialize(RPiControls, buffer);
> +
> +	size = serializer.binarySize(list);
> +	std::vector<uint8_t> listData(size);
> +	buffer = ByteStreamBuffer(listData.data(), listData.size());
> +	serializer.serialize(list, buffer);
> +
> +	// don't need to error check; just write empty vector
> +	return CreateControlListDirect(builder, listData);
> +}
> +
> +// TODO fix the input type for this (like ControlListFB or something?)
> +inline ControlList deserializeControlList(ControlList *obj)
> +{
> +	ControlSerializer deserializer;
> +	std::vector<uint8_t> buf = std::vector(obj->begin(), obj->end());
> +
> +	std::vector<uint8_t> infoData(size);
> +	BytesStreamBuffer buffer(const_cast<const uint8_t *>(infoData.data()), infoData.size());
> +	ControlInfoMap infoMap = deserializer.deserialize<ControlInfoMap>(buffer);
> +	// TODO what to do for error checking?
> +
> +	std::vector<uint8_t> listData(size);
> +	buffer = ByteStreamBuffer(const_cast<const uint8_t *>(listData.data()), listData.size());
> +	ControlList list = deserializer.deserialize<ControlList>(buffer);
> +
> +	return list;
> +}
> +
> +
> +enum RPiConfigParametersWrapper {
> +	RPI_IPA_CONFIG_LS_TABLE = 1,
> +	RPI_IPA_CONFIG_STAGGERED_WRITE = 2,
> +	RPI_IPA_CONFIG_SENSOR = 3,
> +	RPI_IPA_CONFIG_SEND_FD = 4,
> +};
> +
> +enum RPiEventsWrapper {
> +  RPI_IPA_EVENT_SIGNAL_STAT_READY = 1,
> +  RPI_IPA_EVENT_SIGNAL_ISP_PREPARE = 2,
> +  RPI_IPA_EVENT_QUEUE_REQUEST = 3,
> +  RPI_IPA_EVENT_SEND_FD = 4,
> +};
> +
> +enum RPiActionsWrapper {
> +	RPI_IPA_ACTION_V4L2_SET_STAGGERED,
> +	RPI_IPA_ACTION_V4L2_SET_ISP,
> +	RPI_IPA_ACTION_STATS_METADATA_COMPLETE,
> +	RPI_IPA_ACTION_RUN_ISP,
> +	RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME,
> +	RPI_IPA_ACTION_EMBEDDED_COMPLETE,
> +};
> +
> +struct RPiStaggeredWritePayloadWrapper : Serializable
> +{
> +	RPiStaggeredWritePayloadWrapper() {}
> +
> +	RPiStaggeredWritePayloadWrapper(uint32_t gainDelay, uint32_t exposureDelay, uint32_t sensorMetadata) : gainDelay_(gainDelay), exposureDelay_(exposureDelay), sensorMetadata_(sensorMetadata) {}
> +
> +	RPiStaggeredWritePayloadWrapper(const RPiStaggeredWritePayload *buf)
> +	{
> +		const RPiStaggeredWritePayload *obj = flatbuffers::GetRoot<RPiStaggeredWritePayload>(buf);
> +
> +		gainDelay_ = obj->gainDelay();
> +		exposureDelay_ = obj->exposureDelay();
> +		sensorMetadata_ = obj->sensorMetadata();
> +	}
> +
> +	flatbuffers::Offset<RPiStaggeredWritePayload> serialize(flatbuffers::FlatBufferBuilder &builder)
> +	{
> +		// - construct compound children depth-first
> +		// - get simple data
> +		// - feed to Create
> +
> +		// nop for gainDelay_
> +
> +		// nop for exposureDelay_
> +
> +		// nop for sensorMetadata_
> +
> +		return CreateRPiStaggeredWritePayload(builder, gainDelay_, exposureDelay_, sensorMetadata_);
> +	}
> +
> +	std::deque<int> extractFds()
> +	{
> +		// contains no fd and no compound children, so return empty
> +		return {};
> +	}
> +
> +	void injectFds(std::deque<int> &fds)
> +	{
> +		// nop
> +	}
> +
> +	uint32_t gainDelay_;
> +	uint32_t exposureDelay_;
> +	uint32_t sensorMetadata_;
> +}
> +
> +struct RPiIspPreparePayloadWrapper : Serializable
> +{
> +	RPiIspPreparePayloadWrapper() {}
> +
> +	RPiIspPreparePayloadWrapper(uint32_t embeddedbufferId, uint32_t bayerbufferId) : embeddedbufferId_(embeddedbufferId), bayerbufferId_(bayerbufferId) {}
> +
> +	RPiIspPreparePayloadWrapper(const RPiIspPreparePayload *buf)
> +	{
> +		const RPiIspPreparePayload *obj = flatbuffers::GetRoot<RPiIspPreparePayload>(buf);
> +
> +		embeddedbufferId_ = obj->embeddedbufferId();
> +		bayerbufferId_ = obj->bayerbufferId();
> +	}
> +
> +	flatbuffers::Offset<RPiIspPreparePayload> serialize(flatbuffers::FlatBufferBuilder &builder)
> +	{
> +		// - construct compound children depth-first
> +		// - get simple data
> +		// - feed to Create
> +
> +		// nop for embeddedbufferId_
> +
> +		// nop for bayerbufferId_
> +
> +		return CreateRPiIspPreparePayload(builder, embeddedbufferId_, bayerbufferId_);
> +	}
> +
> +	// these could be generated for every struct and not just those that
> +	// fds in their tree. that'll save parsing the dependency graph in the
> +	// compiler
> +	std::deque<int> extractFds()
> +	{
> +		return {};
> +	}
> +
> +	void injectFds(std::deque<int> &fds)
> +	{
> +		// nop
> +	}
> +
> +	uint32_t embeddedbufferId_;
> +	uint32_t bayerbufferId_;
> +}
> +
> +struct RPiStatsCompletePayloadWrapper : Serializable
> +{
> +	RPiStatsCompletePayloadWrapper() {}
> +
> +	RPiStatsCompletePayloadWrapper(uint32_t bufferId, ControlList &controls) : bufferId_(bufferId), controls_(controls) {}
> +
> +	// deserialize
> +	RPiStatsCompletePayloadWrapper(const RPiStatsCompletePayload *buf)
> +	{
> +		const RPiStatsCompletePayload *obj = flatbuffers::GetRoot<RPiStatsCompletePayload>(buf);
> +
> +		bufferId_ = obj->bufferId();
> +		controls_ = deserializeControlList(obj->controls());
> +	}
> +
> +	flatbuffers::Offset<RPiStatsCompletePayloadWrapper> serialize(flatbuffers::FlatBufferBuilder &builder)
> +	{
> +		// - construct compound children depth-first
> +		// - get simple data
> +		// - feed to Create
> +
> +		// nop for bufferId_
> +
> +		// serialize controls_
> +		flatbuffers::Offset<ControlList> controls = serializeControlList(builder, controls_);
> +
> +		return CreateRPiStatsCompletePayload(builder, bufferId_, controls);
> +	}
> +
> +	uint32_t bufferId_;
> +	ControlList controls_;
> +}
> +
> +union RPiConfigureUnionWrapper
> +{
> +	uint32_t lsTableHandle_;
> +	RPiStaggeredWritePayload staggeredWriteResult_;
> +	ControlList contols_;
> +	int32_t bufferFd_;
> +}
> +
> +struct RPiConfigurePayloadWrapper : Serializable
> +{
> +	RPiConfigurePayloadWrapper() {}
> +
> +	RPiConfigurePayloadWrapper(enum RPiConfigParametersWrapper op, RPiConfigureUnionWrapper payload) : op_(op), payload_(payload) {}
> +
> +	// deserialize
> +	RPiConfigurePayloadWrapper(const RPiConfigurePayload *buf)
> +	{
> +		// yeah fill this in if you want
> +	}
> +
> +	// deserialize
> +	// TODO need this if is a member of vector
> +	RPiConfigurePayloadWrapper(flatbuffers::Offset<RPiConfigurePayload> &p)
> +	{
> +		// nop for op
> +		op_ = p.op();
> +
> +		// start of union block for payload_
> +		switch (op_) {
> +		case RPI_IPA_CONFIG_LS_TABLE:
> +			payload_.lsTableHandle_ = p.payload_as_lsTableHandle();
> +			break;
> +		case RPI_IPA_CONFIG_STAGGERED_WRITE:
> +			payload_.staggeredWriteResult_ = RPiStaggeredWritePayloadWrapper(p.payload_as_staggeredWriteResult);
> +			break;
> +		case RPI_IPA_CONFIG_SENSOR:
> +			payload_.controls_ = deserializeControlList(p.payload_as_controls());
> +			break;
> +		case RPI_IPA_CONFIG_SEND_FD:
> +			payload_.bufferFd_ = p.payload_as_bufferFd();
> +			break;
> +		}
> +		// end of union block for payload_
> +	}
> +
> +	flatbuffers::Offset<RPiConfigurePayload> serialize(flatbuffers::FlatBufferBuilder &builder)
> +	{
> +		// - construct compound children depth-first
> +		// - get simple data
> +		// - feed to Create
> +
> +		// nop for op_
> +		RPiConfigureUnion payloadType;
> +		flatbuffers::Offset<void> payload;
> +
> +		// start of union block for payload_
> +		switch (op_) {
> +		case RPI_IPA_CONFIG_LS_TABLE:
> +			payloadType = RPiConfigureUnion_lsTableHandle;
> +			payload = CreateUnsignedInt(builder, payload_.lsTableHandle_);
> +			break;
> +		case RPI_IPA_CONFIG_STAGGERED_WRITE:
> +			payloadType = RPiConfigureUnion_staggeredWriteResult;
> +			payload = payload_.staggeredWriteResult_.serialize(builder);
> +			break;
> +		case RPI_IPA_CONFIG_SENSOR:
> +			// controls
> +			payloadType = RPiConfigureUnion_controls;
> +			payload = serializeControlList(builder, payload_.controls_);
> +			break;
> +		case RPI_IPA_CONFIG_SEND_FD:
> +			payloadType = RPiConfigureUnion_bufferFd;
> +			payload = CreateSignedInt(builder, payload_.bufferFd_);
> +			break;
> +		}
> +		// end of union block for payload_
> +
> +		return CreateRPiConfigurePayload(builder, op_, payloadType, payload);
> +	}
> +
> +	std::deque<int> extractFds()
> +	{
> +		std::deque<int> fds;
> +
> +		// nop for op_, since it has no children, and is not fd
> +
> +		// start of union block for payload_
> +		switch (op_) {
> +		case RPI_IPA_CONFIG_LS_TABLE:
> +			// value is not fd
> +			break;
> +		case RPI_IPA_CONFIG_STAGGERED_WRITE:
> +			std::deque<int> pfds = payload_.staggeredWriteResult_.extractFds();
> +			fds.insert(fds.end(), pfds,begin(), pfds.end());
> +			break;
> +		case RPI_IPA_CONFIG_SENSOR:
> +			// value is not fd
> +			break;
> +		case RPI_IPA_CONFIG_SEND_FD:
> +			fds.push_back(payload.bufferFd_);
> +			break;
> +		}
> +		// end of union block for payload_
> +
> +		return fds;
> +	}
> +
> +	void injectFds(std::deque<int> &fds)
> +	{
> +		// nop for op_
> +
> +		// start of union block for payload_
> +		switch (op_) {
> +		case RPI_IPA_CONFIG_LS_TABLE:
> +			break;
> +		case RPI_IPA_CONFIG_STAGGERED_WRITE:
> +			payload_.staggeredWriteResult_.injectFds(fds);
> +			break;
> +		case RPI_IPA_CONFIG_SENSOR:
> +			break;
> +		case RPI_IPA_CONFIG_SEND_FD:
> +			payload_.bufferFd_ = fds.at(0);
> +			fds.pop_front();
> +			break;
> +		}
> +		// end of union block for payload_
> +	}
> +
> +	enum RPiConfigParametersWrapper op_;
> +	RPiConfigureUnionWrapper payload_;
> +}
> +
> +union RPiEventPayloadWrapper
> +{
> +	uint32_t bufferId_;
> +	RPiIspPreparePayloadWrapper ispPrepare_;
> +	ControlList controls_;
> +	int32_t bufferFd_;
> +}
> +
> +union RPiActionPayloadWrapper
> +{
> +	uint32_t bufferId_;
> +	RPiStatsCompletePayloadWrapper statsComplete_;
> +	ControlList controls_;
> +}
> +
> +struct RPiConfigureParamsWrapper : Serializable
> +{
> +	RPiConfigureParamsWrapper() {}
> +
> +	RPiConfigureParamsWrapper(std::vector<RPiConfigurePayloadWrapper> payload) : payload_(payload) {}
> +
> +	// deserialize
> +	// only top-level gets fds
> +	RPiConfigureParamsWrapper(const uint8_t *buf, std::dequeue<int> fds)
> +	{
> +		const RPiConfigureParams *obj = flatbuffers::GetRoot<RPiConfigureParams>(buf);
> +
> +		// start of vector block for payload_
> +		flatbuffers::Vector<flatbuffers::Offset<RPiConfigurePayload>> *payload = obj->params();
> +		std::transform(payload->begin(), payload->end(), std::back_inserter(payload_),
> +				[](flatbuffers::Offset<RPiConfigurePayload> &p) -> RPiConfigurePayloadWrapper {
> +					return RPiConfigurePayloadWrapper(p);
> +				});
> +		// end of vector block for payload_
> +
> +
> +		// after constructing all the children, we need to inject the fds
> +		injecetFds(fds);
> +	}
> +
> +	flatbuffers::Offset<RPiConfigureParams> serialize(flatbuffers::FlatBufferBuilder &builder)
> +	{
> +		// - construct compound children depth-first
> +		// - get simple data
> +		// - feed to Create
> +
> +		// start of vector block for payload_
> +		std::vector<flatbuffers::Offset<RPiConfigurePayload>> payload;
> +		std::transform(payload_.begin(), payload_.end(), std::back_inserter(payload),
> +				[](RPiConfigurePayloadWrapper &p) -> flatbuffers::Offset<RPiConfigurePayload> { return p.serialize(builder); });
> +		// end of vector block for payload_
> +
> +		// use direct if there's vector as a member
> +		return CreateRPiConfigureParamsDirect(builder, payload);
> +	}
> +
> +	// this is only for root_type
> +	std::deque<int> extractFds()
> +	{
> +		std::deque<int> fds;
> +
> +		// start of vector block for payload_
> +		for (RPiConfigurePayloadWrapper &payload : payload_)  {
> +			std::deque<int> &pfds = payload_.extractFds();
> +			fds.insert(fds.end(), pfds.begin(), pfds.end());
> +		}
> +		// end of vector block for payload_
> +
> +		return fds;
> +	}
> +
> +	void injectFds(std::deque<int> &fds)
> +	{
> +		// start of vector block for payload_
> +		for (RPiConfigurePayloadWrapper &payload : payload_) 
> +			payload_.injectFds(fds);
> +		// end of vector block for payload_
> +	}
> +
> +	// this is only for root_type
> +	// returns buffer pointer, buffer size (in bytes), and vector of fds
> +	// the fds are matched by index; the index maps to full path in object
> +	// tree, generated by the special compiler
> +	std::tuple<uint8_t *, size_t, std::deque<int>> serialize()
> +	{
> +		flatbuffers::FlatBufferBuilder builder(1024);
> +		builder.Finish(this.serialize(builder));
> +
> +		uint8_t *buf = builder.GetBufferPointer();
> +		size_t size = builder.GetSize();
> +		return {buf, size, extractFds()};
> +	}
> +
> +	std::vector<RPiConfigurePayloadWrapper> payload_;
> +}
> +
> +struct RPiEventParamsWrapper : Serializable
> +{
> +	RPiEventParamsWrapper() {}
> +
> +	RPiEventParamsWrapper(enum RPiEventsWrapper ev, RPiEventPayloadWrapper payload) : ev_(ev), payload_(payload) {}
> +
> +	// deserialize
> +	// root_type needs fds
> +	RPiEventParamsWrapper(const uint8_t *buf, std::deque<int> fds)
> +	{
> +		const RPiEventParams *obj = flatbuffers::GetRoot<RPiEventParams>(buf);
> +
> +		ev_ = obj->ev();
> +
> +		// start of union block for payload_
> +		switch (ev_) {
> +		case RPI_IPA_EVENT_SIGNAL_STAT_READY:
> +			payload_.bufferId_ = obj->payload_as_bufferId()->value();
> +			break;
> +		case RPI_IPA_EVENT_SIGNAL_ISP_PREPARE:
> +			payload_.ispPrepare_ = RPiIspPreparePayloadWrapper(obj->payload_as_ispPrepare());
> +			break;
> +		case RPI_IPA_EVENT_QUEUE_REQUEST:
> +			payload_.controls_ = deserializeControlList(obj->payload_as_controls());
> +			break;
> +		case RPI_IPA_EVENT_SEND_FD:
> +			payload_.bufferFd_ = obj->payload_as_bufferFd()->value();
> +			break;
> +		}
> +		// end of union block for payload_
> +
> +		injectFds(fds);
> +	}
> +
> +	flatbuffers::Offset<RPiEventParams> serialize(flatbuffers::FlatBufferBuilder &builder)
> +	{
> +		// - construct compound children depth-first
> +		// - get simple data
> +		// - feed to Create
> +
> +		// nop for ev_
> +
> +		// start of union block for payload_
> +		RPiEventPayload payloadType;
> +		flatbuffers::Offset<void> payload;
> +
> +		switch (ev_) {
> +		case RPI_IPA_EVENT_SIGNAL_STAT_READY:
> +			payloadType = RPiEventPayload_bufferId;
> +			payload = CreateUnsignedInt(builder, payload_.bufferId_);
> +			break;
> +		case RPI_IPA_EVENT_SIGNAL_ISP_PREPARE:
> +			payloadType = RPiEventPayload_ispPrepare;
> +			payload = payload_.ispPrepare_.serialize(builder);
> +			break;
> +		case RPI_IPA_EVENT_QUEUE_REQUEST:
> +			payloadType = RPiEventPayload_controls;
> +			payload = serializeControlList(builder, payload_.controls_);
> +			break;
> +		case RPI_IPA_EVENT_SEND_FD:
> +			payloadType = RPiEventPayload_bufferFd;
> +			payload = CreateSignedInt(builder, payload_.bufferFd_);
> +			break;
> +		}
> +		// end of union block
> +
> +		return CreateRPiEventParams(builder, ev_, payloadType, payload);
> +	}
> +
> +	std::deque<int> extractFds()
> +	{
> +		std::deque<int> fds;
> +
> +		// nop for ev_
> +
> +		// start of union block for payload_
> +		switch (ev_) {
> +		case RPI_IPA_EVENT_SIGNAL_STAT_READY:
> +			// non-fd scalar
> +			break;
> +		case RPI_IPA_EVENT_SIGNAL_ISP_PREPARE:
> +			std::deque<int> &pfds = payload_.ispPrepare_.extractFds();
> +			fds.insert(fds.end(), pfds.begin(), pfds.end());
> +			break;
> +		case RPI_IPA_EVENT_QUEUE_REQUEST:
> +			// non-fd controls
> +			break;
> +		case RPI_IPA_EVENT_SEND_FD:
> +			fds.push_back(payload_.bufferFd_);
> +			break;
> +		}
> +		// end of union block for payload_
> +
> +		return fds;
> +	}
> +
> +	void injectFds(std::deque<int> &fds)
> +	{
> +		// nop for ev_
> +
> +		// start of union block for payload_
> +		switch (ev_) {
> +		case RPI_IPA_EVENT_SIGNAL_STAT_READY:
> +			break;
> +		case RPI_IPA_EVENT_SIGNAL_ISP_PREPARE:
> +			payload_.ispPrepare_.injectFds(fds);
> +			break;
> +		case RPI_IPA_EVENT_QUEUE_REQUEST:
> +			break;
> +		case RPI_IPA_EVENT_SEND_FD:
> +			payload_.bufferFd_ = fds.at(0);
> +			fds.pop_front();
> +			break;
> +		}
> +		// end of union block for payload_
> +	}
> +
> +	// this is only for root_type
> +	// returns buffer pointer, buffer size (in bytes), and vector of fds
> +	// the fds are matched by index; the index maps to full path in object
> +	// tree, generated by the special compiler
> +	std::tuple<uint8_t *, size_t, std::deque<int>> serialize()
> +	{
> +		flatbuffers::FlatBufferBuilder builder(1024);
> +		builder.Finish(this.serialize(builder));
> +
> +		uint8_t *buf = builder.GetBufferPointer();
> +		size_t size = builder.GetSize();
> +		return {buf, size, extractFds()};
> +	}
> +
> +	enum RPiEventsWrapper ev_;
> +	RPiEventPayloadWrapper payload_;
> +}
> +
> +struct RPiActionParamsWrapper : Serializable
> +{
> +	RPiActionParamsWrapper() {}
> +
> +	RPiActionParamsWrapper(enum RPiActionsWrapper op, RPiActionPayloadWrapper payload) : op_(op), payload_(payload) {}
> +
> +	// deserialize
> +	// root_type needs fds
> +	RPiActionParamsWrapper(const uint8_t *buf, std::deque<int> &fds)
> +	{
> +		const RPiActionParams *obj = flatbuffers::GetRoot<RPiActionParams>(buf);
> +
> +		op_ = obj->op();
> +
> +		// start of union block for payload_
> +		switch (op_) {
> +		case RPI_IPA_ACTION_V4L2_SET_STAGGERED:
> +		case RPI_IPA_ACTION_V4L2_SET_ISP:
> +			payload_.controls_ = deserializeControlList(obj->payload_as_controls());
> +			break;
> +		case RPI_IPA_ACTION_STATS_METADATA_COMPLETE:
> +			payload_.statsComplete_ = RPiStatsCompletePayloadWrapper(obj->payload_as_statsComplete());
> +			break;
> +		case RPI_IPA_ACTION_RUN_ISP:
> +		case RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME:
> +		case RPI_IPA_ACTION_EMBEDDED_COMPLETE:
> +			// UnsignedInt needs ->value()
> +			payload_.bufferId_ = obj->payload_as_bufferId()->value();
> +			break;
> +		}
> +		// end of union block for payload_
> +
> +		injectFds(fds);
> +	}
> +
> +	flatbuffers::Offset<RPiEventParams> serialize(flatbuffers::FlatBufferBuilder &builder)
> +	{
> +		// - construct compound children depth-first
> +		// - get simple data
> +		// - feed to Create
> +
> +		// nop for op_
> +
> +		// start of union block for payload_
> +		RPiActionPayload payloadType;
> +		flatbuffers::Offset<void> payload;
> +
> +		switch (op_) {
> +		case RPI_IPA_ACTION_V4L2_SET_STAGGERED:
> +		case RPI_IPA_ACTION_V4L2_SET_ISP:
> +			payloadType = RPiActionPayload_controls;
> +			payload = serializeControlList(builder, payload_.controls_);
> +			break;
> +		case RPI_IPA_ACTION_STATS_METADATA_COMPLETE:
> +			payloadType = RPiActionPayload_statsComplete;
> +			payload = payload_.statsComplete_.serialize(builder);
> +			break;
> +		case RPI_IPA_ACTION_RUN_ISP:
> +		case RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME:
> +		case RPI_IPA_ACTION_EMBEDDED_COMPLETE:
> +			payloadType = RPiActionPayload_bufferId;
> +			payload = CreateUnsignedInt(builder, payload_.bufferId_);
> +			break;
> +		}
> +		// end of union block for payload_
> +
> +		return CreateRPiActionParams(builder, op_, payloadType, payload);
> +	}
> +
> +	// root_type needs it even if it's empty, because of top-level serialize()
> +	std::deque<int> extractFds()
> +	{
> +		return {};
> +	}
> +
> +	std::deque<int> injectFds()
> +	{
> +		// nop
> +	}
> +
> +	// this is only for root_type
> +	// returns buffer pointer, buffer size (in bytes), and vector of fds
> +	// the fds are matched by index; the index maps to full path in object
> +	// tree, generated by the special compiler
> +	std::tuple<uint8_t *, size_t, std::deque<int>> serialize()
> +	{
> +		flatbuffers::FlatBufferBuilder builder(1024);
> +		builder.Finish(this.serialize(builder));
> +
> +		uint8_t *buf = builder.GetBufferPointer();
> +		size_t size = builder.GetSize();
> +		return {buf, size, extractFds()};
> +	}
> +
> +	enum RPiActionsWrapper op_;
> +	RPiActionPayloadWrapper payload_;
> +}
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_IPA_INTERFACE_RASPBERRYPI_FB_H__ */

Hi Laurent,

Thank you for the review.

On Fri, Jul 24, 2020 at 06:42:52AM +0300, Laurent Pinchart wrote:
> Hi Paul,
> 
> Thank you for the patch.
> 
> On Wed, Jul 22, 2020 at 09:25:43PM +0900, Paul Elder wrote:
> > In this whole patch, keep in mind that whenever flatbuffers is
> > mentioned, it could be replaced by anything else, such as protobuf.
> > 
> > Also, in general, naming of anything is flexible. I tried to chose
> > reasonable placeholders to show how things would work.
> > 
> > I have not yet done the theoretical plumbing. I wanted to get this draft
> > out first at least, before continuing too far without feedback.
> > 
> > * Introduction
> 
> Would it be possible to add a meta-introduction to explain what you're
> trying to do, before diving into the details ? A general problem
> statement would be useful, both as a reference for other developers who
> may not be familiar with the topic at hand, and as a way to make sure we
> have the same understanding of the problem.

Ah, yes.

> > raspberrypi.libcamera.decl shows a sample custom IPA data structure IDL
> > definition for the raspberrypi pipeline handler. The intent is that I
> > will implement a custom compiler that will compile this definition into
> > flatbuffers, as well as the header that will be included in the pipeline
> > handler and the IPA. flatbuffers will compile the flatbuffers definition
> > into the flatbuffers header that the included header will use for serdes.
> > 
> > The purpose of this custom IPA data structure definition is to allow
> > pipeline handlers and IPAs to use custom data structures for configure,
> > processEvent, and queueFrameAction, rather than the rigid
> > IPAOperationData. Hooking it up with framebuffers, and the compiler that
> > I inted to write, will implement serdes for these custom data structures
> > that the IPAProxies can use.
> > 
> > raspberrypi.fbs shows the intended output of the compiler that I intend
> > to write, which will be in the input to the flatbuffers compiler.
> 
> This two-steps procedure would allow making the underlying serialization
> library/framework an implementation detail, which has the upside of
> simplifying a switch to a different framework. Let's however not forget

That was the idea :)

> that we don't want to spend cycles unnecessarily changing the underlying
> implementation, and that we need to guarantee an ABI stability for
> closed-source IPAs. Whether that ABI will be the wire representation of
> data or a deserialized format produced by the IPAProxy worker remains to
> be decided.
> 
> > raspberrypi_wrapper.h shows the other intended output of the of the
> > compiler that I inted to write. This is the header that will be included
> > by the pipeline handler and the IPA. The structs that are defined in
> > this header can be used directly by the pipeline handler and IPA, and
> > the serialize function and the deserialization constructor are meant to
> > be used by the IPAProxy (maybe they could be made private and IPAProxy
> > into a friend class?).
> > 
> > The custom compiler is necessary to place restrictions on the allowed
> > data definitions and make the definitions cleaner based on the
> > restrictions, as well as generate a header that is a wrapper around the
> > flatbuffers-generated header for the pipeline handler and IPA to include.
> 
> This may just be a matter of naming and code refactoring, but I want to
> minimize the exposure of pipeline handlers and IPA modules to
> serialization. In particular, this means that data type names should not
> include a 'Wrapper' suffix but should use the native names.

Yeah, we can do that.

Should I remove the postfix underscore from the struct member names too
then?

> > * Custom data definition file
> > 
> > raspberrypi.libcamera.decl
> > 
> > There are three different types of custom data structures that must be
> > defined, for the three IPA functions that take IPAOperationData:
> > - configure (for IPAInterface::configure())
> > - event (for IPAInterface::processEvent())
> > - action (for IPAInterface::queueFrameAction)
> > 
> > Discussion point: should configure have a separate input and output data
> > definition?
> 
> Are you talking about the existing ipaConfig and result parameters here
> ? While they currently use the same data type (IPAOperationData), they
> carry different information. What's the alternative ? Using a single
> inout argument ? I think separating inputs and outputs will result in a
> much cleaner solution.

I was wondering if we should make the configure input and output data
types different. To mirror IPAOperationData (to a certain extent), I
made them the same data type. Maybe different data type is better. More
flexible and more partitioned.

> > Since all three have an "opcode" and a payload, that is all that needs
> > to be defined.
> 
> We currently have an opcode, but that doesn't mean this is the desired
> outcome. I see the existing opcode to be more of a quick hack than a
> good solution.

Well, for each IPA function, you want to be able to pass in different
things. I think we do need an opcode, or at least some indicator as to
what is contained in the data. Since the number of IPA functions is
limited, we have to make the data flexible.

I guess the alternative, as you mention below, is to have a struct with
nullable fields. Then instead of building an opcode-payload vector you
could just stuff all the data that you want in one struct, and the
receiver could just check which ones aren't null.

> > The opcodes can be seen in the enums ConfigOps, EventOps,
> > and ActionOps. The payloads can be seen in the unions ConfigPayload,
> > EventPayload, and ActionPayload. Since the opcodes are essentially
> > selectors into the unions, which field of the union the opcode points to
> > must be specified as shown in the enums.
> 
> The opcode is currently abused by the IPA configure() operation. Its
> original intent was to be an opcode for queueFrameAction and
> processEvent, and it's now also used as a bitfield in configure() to
> describe the data that is being transported in the IPAOperationData.

Yeah I noticed. That's why I translated it to a vector of opcode-payload
pairs :)

> > Discussion point: ConfigPayload will become an element in a vector, so
> > there can be multiple instances of in in a single configure() call.
> > Conversely, EventPayload and ActionPayload are scalars in
> > processEvent() and queueFrameAction. Should they be vectorized like
> > ConfigPayload?
> 
> I'm not sure to follow you. Why will ConfigPayload become an element in
> a vector ? Is this because the current usage of the opcode as a bitfield
> will be turned into a vector of single opcode + data ? [edit: I think
> you answer this question below, and the answer is a yes]

The answer is yes :)

> I think it would be better to instead have a struct that contains all
> the parameters, with nullable elements. What I mean by that is
> 
> struct StaggeredWriteConfig {
> 	uint32 gainDelay;
> 	uint32 exposureDelay;
> 	bool sensorMetadata;
> };
> 
> struct ConfigResult {
> 	StaggeredWriteConfig staggeredWriteConfig;
> 	ControlList sensorControls;
> };
> 
> with ConfigResult being the type returned by configure(), and both
> staggeredWriteConfig and sensorControls being nullable types
> (effectively pointers in C++ code). We wouldn't need an opcode.

Yeah, that would be good. Instead of a vector of partially-used structs
with opcode indicators.

How would the fields be nulled, though? How can you tell the difference
between a zero value and a nulled value? If we add null flag that'll
break the nice struct abstraction.

> > Discussion point: can unions be disallowed besides at the top level of
> > the data definition tree in the payload?
> 
> I'd rather disallow unions completely :-)

Woooo :)

> > These opcode and payload definitions are the only required components of
> > the custom data definition. Once these are satisfied, the rest of the
> > file follows standard flatbuffers rules.
> > 
> > Also, since flatbuffers structs are a bit of a pain, I've decided to
> > only allow flatbuffers tables in the definition (flatbuffers structs are
> > not allowed ). In C++ (and C if I ever have to implement that compiler
> > too...) they're wrapped as structs, though.
> 
> I think this should be an implementation detail. I'd rather not use the
> keyword "table" in the IDL exposed to the pipeline handler and IPA.

Sure, we can change that. Perks of custom IDL :)

> Generally speaking, as our main language target is C++, I think we
> should also use an IDL syntax that ressembles C++ where we have the
> option to do so. Among the IDLs I've looked at (flatbuffers, protobuf
> and mojo), mojo seems the most familiar to me. It has the advantages (in
> my opinion) to define types in a way that focusses on the data instead
> of the protocol (see the "message" keyword in the protobuf IDL) or
> storage (see the "table" keyword in the flatbuffers IDL). I would like
> pipeline handlers and IPAs to avoid thinking about IPC as much as
> possible.

Yeah, I think that pipeline handlers and IPAs shouldn't have to worry
about IPC. Though I think we can still enforce some rules to make IPC
life easier, like mandating what root data structures are required and
in what form. It's like an entry point.

> One possible advantage of using the mojo IDL is that we could leverage
> the parser, which is written in Python.

Ooh yeah maybe. We'd still need custom code generation though.

> > Unions cannot have unions as a direct child.
> > 
> > * flatbuffers input file
> > 
> > raspberrypi.fbs is the intended output with raspberrypi.libcamera.decl,
> > as described above.
> > 
> > Some things to note...
> > 
> > - RPiConfigParameters is not flags. I decided that rather than scanning
> >   flags and parsing a vector payload based on that, it would be better
> >   to have a vector of opcode-payload pairs.
> 
> I think this answers my above question.
> 
> > - NONE needs to be added to enums to appease flatbuffers. One of the
> >   many reasons why I think we should have a custom data definition to
> >   make writing the data definition cleaner for developers.
> 
> Could you explain why this is ?

I think flatbuffers was whining that there was no zero value defined in
the enum, so I just added a dummy placeholder NONE to hold the zero
value.

> > - In flatbuffers, primitives cannot be members of unions. I'll add a
> >   global header that defines wrappers around primitives. In the custom
> >   data definition they are allowed, though. My compiler shall convert
> >   them to the wrapper.
> > 
> > - libcamera already comes with a really nice ControlList serdes. In the
> >   flatbuffers global header I'll add the ControlList definition that is
> >   seen here. It's just defined as a byte array, so we can use
> >   libcamera's ControlList serializer. It does need to be renamed,
> >   though.
> 
> Note that we can refactor the ControlList serializer/deserializer if
> needed.
> 
> > - As mentioned earlier, RPiConfigParams has a vector of opcode-payload
> >   pairs, while RPiEventParams and RPiActionParams have a single
> >   opcode-payload pair
> > 
> > * Custom generated header
> > 
> > raspberrypi_wrapper.h
> > 
> > flatbuffers will compile the flatbuffers input file into a flatbuffers
> > header that can be included. This file cannot be used directly by
> > pipeline handlers and IPAs without them directly interfacing with
> > flatbuffers and its API. Thus, a custom header will also be generated by
> > the compiler that I'll implement. This custom header implements wrappers
> > for the data structures defined in the custom data definition file for
> > pipeline handlers and IPAs to use, as well as serialize and deserialize
> > functions for the proxies to use. The intended output of this header can
> > be seen in raspberrypi_wrapper.h.
> > 
> > The structs and enums and unions defined in this header can be used by
> > pipeline handlers and IPAs as-is (ie. no special API like flatbuffers).
> > 
> > Discussion point: maybe make the serialize function and deserialization
> > constructor private and make IPAProxy a friend class?
> 
> Friendship isn't inherited in C++, so you wouldn't be able to call the
> functions from a subclass of IPAProxy. I don't see a problem in making
> those methods public. They will be in generated code, and nobody should
> notice.

Ah, okay.

> > A Serializable class is reference but never defined. This class is meant
> > to simply have a serialize() function (deserialize is implemented as an
> > overloaded constructor). This class might not even be necessary.
> 
> I suppose you will define it (unless it ends up not being necessary)
> though ? You can't inherit from a class that isn't defined :-)

Yeah, I did define it :) We need a type that the IPAProxy can agree on
with all the different pipeline handlers and IPAs.

> > There are a couple helpers for serializing and deserializing
> > ControlList such that it can work well with the flatbuffers data
> > structures. I got the implementation from the test for
> > ControlSerializer.
> > 
> > There are also a couple helpers for extracting the fds and injecting
> > the fds. The fd extraction is meant to be done by the IPAProxy prior to
> > serialization. The IPAProxy can then use sendmsg and recvmsg (or some
> > similar facility) to send the fds across the process boundary. These fds
> > can then be injected back into the object after it is deserialized and
> > reconstructed. The extracted fds are saved in a queue, in post-order.
> > Since the data container tree is the same before and after serdes, the
> > order of the fds is guaranteed to match, as the fds are injected also in
> > post-order.
> 
> Wouldn't it be simpler if the serialize and deserialize functions were
> given a serialization context that contained both the flatbuffer context
> and a vector of fds ? We wouldn't need separate fd handling functions.

Oh yeah maybe. So serialize would output an fd vector in addition to the
serialized object tree, and deserialize would take both as an input?

> Note that C++ also makes it possible to move some of the serialization
> and deserialization implementation out of the generated wrapper classes
> using templates. Something along the lines of the following.
> 
> - Base class
> 
> template<typename T>
> class Serializer
> {
> };
> 
> - Generated class
> 
> template<>
> class Serializer<MyDataType>
> {
> public:
> 	static void serialize(MyDataType &input, Buffer *buffer)
> 	{
> 		...
> 	}
> };
> 
> - Usage
> 
> 	MyDataType foo(...);
> 	...
> 	Serializer::serialize(foo, buffer);
> 
> This could possibly help keeping the wrapper header lean and clean, and
> moving serialization code to a generated .cpp file.

Ooh yeah that'll be nice.

> > Serialization, deserialization, fd extraction, and fd injection are all
> > implemented with recursion. The fd extraction and injection functions
> > may or may not be be ommitted if no members in the data container tree
> > contain no fds. This is TBD.
> > 
> > There are a few rules for the compilation...
> > 
> > - unions and enums are as-is
> > 
> > - tables are compiled with its members as member variables, and have
> >   three constructors, 1) empty default, 2) define all members, and
> >   3) deserialization constructor. They also get a serialize function
> >   that takes a flatbuffer builder and outputs a flatbuffer offset. This
> >   allows recursion to do serialization. Only the top-level
> >   (RPiConfigureParamsWrapper, RPiEventParamsWrapper,
> >   RPiActionParamsWrapper) tables get a serialize functions that takes
> >   void and returns a pair of byte array and length.
> > 
> > - serialization is implemented as:
> >   - for every member variable:
> >     - if it is primitive, nop
> >     - if it is controls, call serializeControlList
> >     - if it is table, call table's serialize
> >     - if it is a vector, convert the vector to a vector of flatbuffer
> >       offsets by serializing every member based on the above three rules
> >     - if it is a union:
> >       - switch on opcode:
> >         - set union type based on opcode
> >         - set union content based on the same above four rules
> >   - call flatbuffer's create object with all the serialized members
> > 
> > - deserialization is implemented as a constructor. top-level tables take
> >   a pointer to a byte array as input, while non-top-level tables take a
> >   pointer to a byte array casted to the flatbuffers object. If the table
> >   is used in a vector anywhere in the data definition, there will be
> >   another deserialization constructor that takes a flatbuffer offset.
> >   - cast input to flatbuffers offset (if necessary)
> >   - for every member variable:
> >     - if it is primitive, set the member variable
> >     - if it is controls, set member variable with deserializeControlList
> >     - if it is table, construct the table with the deserialization
> >       constructor, and set the member variable
> >     - if it is vector, convert the vector of flatbuffer offsets into the
> >       vector of the member variable by deserializing based on the above
> >       three rules
> >     - if it is union:
> >       - switch on opcode:
> >         - set member variable union content based on same above four rules
> > 
> > - fd extraction is implemented as:
> >   - for every member variable:
> >     - if it is primitive or contols, nop
> >     - if it is table, extract the fds from the table, and add to
> >       extracted fds queue
> >     - if it is a vector, append the extracted fds for each element of
> >       the table, according to the above two rules
> >     - if it is a union:
> >       - switch on opcode:
> >         - append extracted fds according to above three rules
> > 
> > - fd injection is implemented as the inverse of fd extraction, so
> >   instead of enqueueing the fds, the fds are dequeued and assigned
> > 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> I think this is a good base for discussions. What I'm missing though is
> an example of how this is used by the pipeline handler and IPA. Reading
> all your explanation I expect this to be fairly transparent.

I'm working on the plumbing of the model :)

> A few additional questions that come to my mind:
> 
> - How do you foresee handling of closed-source IPAs that don't want to
>   link to libcamera ? We currently have a C-based API, which may be
>   cumbersome to keep.

As we discussed before, I have two ideas. In both cases, the
close-source IPAs would have to expose C IPA functions. The easier (for
me) choice is that we then simply pass the serialized flatbuffer to the
IPA as a buffer and size.

The other idea is that my compiler will also generate a C header. In
that case, the IPAProxy can serialize a C++ struct, and deserialize it
into a C struct, and then pass that to the closed source IPA.

In both cases, we have a problem of how to deserialize/translate the
ControlList. If we go with the first idea, then the closed source IPA
wouldn't know how to deserialize ControlList. If we go with the second
idea, then we would have to provide a C version of ControlList. Or,
maybe we could just document how the ControlList is serialized? And then
closed-source IPAs would have to implement their own ControlList serdes :)

In either case, we would be able to get rid of the C-based API that we
have to manually maintain.

> - At what level do you think we should handle the IPA ABI compatibility
>   ? If this is handled at the wire level, does flatbuffer document the
>   serialized format ? I came across

There is documentation for the flatbuffers format:
https://github.com/dvidelabs/flatcc/blob/master/doc/binary-format.md#flatbuffers-binary-format

I think either way (exposing flatbuffers directly vs exposing C
structs), ABI compatibility shouldn't be an issue. flatbuffers already
has backwards-compatibility built in, and even if we use C structs,
since they're generated from flatbuffers (which is generated from the
data definition), they should also be backwards compatible.

Of course, we'll have to disallow backward-compatibility breaking
changes, like in "Schema evolution examples" here:
https://google.github.io/flatbuffers/flatbuffers_guide_writing_schema.html

But I'm not sure the flatbuffers compiler supports that? Maybe we can
police it when the data definition is updated in libcamera upstream?

>   https://docs.google.com/document/d/13pv9cFh5YKuBggDBQ1-AL8VReF-IYpFOFpRfvWFrwio/edit
>   and
>   https://docs.google.com/document/d/1jNcsxOdO3Al52s6lIrMOOgY7KXB7TJ8wGGWstAHiTd8/edit
>   which may be interesting readings.


Thanks,

Paul

> > ---
> > Chanes in v2:
> > - add fd support to inteded custom compiler output to ease the
> >   IPAProxy's job of extracting, sending the fds over the process
> >   boundary, and reinjecting the fds
> > - add a declaration to the custom data definition to specify the
> >   ControlInfoMap (the ControlSerializer needs this)
> > - add discussion point, if we can disallow unions in data definition
> >   outside of the top-level payload container
> > In the commit message:
> > - add paragraph about fd extraction and injection in "Custom generated
> >   header" section
> > - remove paragraph about "fds not supported" in pre-introduction
> > - add description of fd extraction and injection mechanism in
> >   compilation rules section of "Custom generated header" section
> > ---
> >  include/libcamera/ipa/raspberrypi.fbs         | 107 +++
> >  include/libcamera/ipa/raspberrypi.h           |  18 -
> >  .../libcamera/ipa/raspberrypi.libcamera.decl  |  83 +++
> >  include/libcamera/ipa/raspberrypi_wrapper.h   | 658 ++++++++++++++++++
> >  4 files changed, 848 insertions(+), 18 deletions(-)
> >  create mode 100644 include/libcamera/ipa/raspberrypi.fbs
> >  create mode 100644 include/libcamera/ipa/raspberrypi.libcamera.decl
> >  create mode 100644 include/libcamera/ipa/raspberrypi_wrapper.h
> > 
> > diff --git a/include/libcamera/ipa/raspberrypi.fbs b/include/libcamera/ipa/raspberrypi.fbs
> > new file mode 100644
> > index 0000000..dcc1918
> > --- /dev/null
> > +++ b/include/libcamera/ipa/raspberrypi.fbs
> > @@ -0,0 +1,107 @@
> > +enum RPiConfigParameters:uint16 {
> > +	NONE = 0,
> > +	RPI_IPA_CONFIG_LS_TABLE,
> > +	RPI_IPA_CONFIG_STAGGERED_WRITE,
> > +	RPI_IPA_CONFIG_SENSOR,
> > +	RPI_IPA_CONFIG_SEND_FD,
> > +}
> > +
> > +enum RPiEvents:uint16 {
> > +	NONE = 0,
> > +	RPI_IPA_EVENT_SIGNAL_STAT_READY,
> > +	RPI_IPA_EVENT_SIGNAL_ISP_PREPARE,
> > +	RPI_IPA_EVENT_QUEUE_REQUEST,
> > +	RPI_IPA_EVENT_SEND_FD,
> > +}
> > +
> > +enum RPiActions:uint16 {
> > +	NONE = 0,
> > +	RPI_IPA_ACTION_V4L2_SET_STAGGERED,
> > +	RPI_IPA_ACTION_V4L2_SET_ISP,
> > +	RPI_IPA_ACTION_STATS_METADATA_COMPLETE,
> > +	RPI_IPA_ACTION_RUN_ISP,
> > +	RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME,
> > +	RPI_IPA_ACTION_EMBEDDED_COMPLETE,
> > +}
> > +
> > +// TODO define this for all primitive types, and make global
> > +table UnsignedInt {
> > +	value:uint32;
> > +}
> > +
> > +table SignedInt {
> > +	value:int32;
> > +}
> > +
> > +// TODO make this global
> > +// TODO rename this
> > +table ControlList {
> > +	list:[uint8];
> > +}
> > +
> > +// Data containers
> > +
> > +table RPiStaggeredWritePayload {
> > +	gainDelay:uint32;
> > +	exposureDelay:uint32;
> > +	sensorMetadata:uint32;
> > +}
> > +
> > +table RPiIspPreparePayload {
> > +	embeddedbufferId:uint32;
> > +	bayerbufferId:uint32;
> > +}
> > +
> > +table RPiStatsCompletePayload {
> > +	bufferId:uint32;
> > +	controls:ControlList;
> > +}
> > +
> > +
> > +// Payload unions
> > +
> > +union RPiConfigureUnion {
> > +	lsTableHandle:UnsignedInt,
> > +	staggeredWriteResult:RPiStaggeredWritePayload,
> > +	controls:ControlList,
> > +	bufferFd:SignedInt,
> > +}
> > +
> > +table RPiConfigurePayload {
> > +        op:RPiConfigParameters;
> > +	payload:RPiConfigureUnion;
> > +}
> > +
> > +union RPiEventPayload {
> > +	bufferId:UnsignedInt,
> > +	ispPrepare:RPiIspPreparePayload,
> > +	controls:ControlList,
> > +	bufferFd:SignedInt,
> > +}
> > +
> > +union RPiActionPayload {
> > +	bufferId:UnsignedInt,
> > +	statsComplete:RPiStatsCompletePayload,
> > +	controls:ControlList,
> > +}
> > +
> > +
> > +// IPA function parameters
> > +
> > +table RPiConfigParams {
> > +	params:[RPiConfigurePayload];
> > +}
> > +
> > +table RPiEventParams {
> > +	ev:RPiEvents;
> > +	payload:RPiEventPayload;
> > +}
> > +
> > +table RPiActionParams {
> > +	op:RPiActions;
> > +	payload:RPiActionPayload;
> > +}
> > +
> > +root_type RPiConfigureParams;
> > +root_type RPiEventParams;
> > +root_type RPiActionParams;
> > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> > index a493776..69b3808 100644
> > --- a/include/libcamera/ipa/raspberrypi.h
> > +++ b/include/libcamera/ipa/raspberrypi.h
> > @@ -10,24 +10,6 @@
> >  #include <libcamera/control_ids.h>
> >  #include <libcamera/controls.h>
> >  
> > -enum RPiConfigParameters {
> > -	RPI_IPA_CONFIG_LS_TABLE = (1 << 0),
> > -	RPI_IPA_CONFIG_STAGGERED_WRITE = (1 << 1),
> > -	RPI_IPA_CONFIG_SENSOR = (1 << 2),
> > -};
> > -
> > -enum RPiOperations {
> > -	RPI_IPA_ACTION_V4L2_SET_STAGGERED = 1,
> > -	RPI_IPA_ACTION_V4L2_SET_ISP,
> > -	RPI_IPA_ACTION_STATS_METADATA_COMPLETE,
> > -	RPI_IPA_ACTION_RUN_ISP,
> > -	RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME,
> > -	RPI_IPA_ACTION_EMBEDDED_COMPLETE,
> > -	RPI_IPA_EVENT_SIGNAL_STAT_READY,
> > -	RPI_IPA_EVENT_SIGNAL_ISP_PREPARE,
> > -	RPI_IPA_EVENT_QUEUE_REQUEST,
> > -};
> > -
> >  enum RPiIpaMask {
> >  	ID		= 0x0ffff,
> >  	STATS		= 0x10000,
> > diff --git a/include/libcamera/ipa/raspberrypi.libcamera.decl b/include/libcamera/ipa/raspberrypi.libcamera.decl
> > new file mode 100644
> > index 0000000..851ae6c
> > --- /dev/null
> > +++ b/include/libcamera/ipa/raspberrypi.libcamera.decl
> > @@ -0,0 +1,83 @@
> > +namespace RPi;
> > +
> > +// this is necessary for de/serializeControlList()
> > +// define in include/libcamera/ipa/raspberrypi.h
> > +ControlInfoMap RPiControls;
> > +
> > +// opcodes
> > +// these must be enums, and must be named as shown
> > +
> > +// required
> > +// must specify which union option each value corresponds to
> > +enum ConfigOps {
> > +	RPI_IPA_CONFIG_LS_TABLE:lsTableHandle,
> > +	RPI_IPA_CONFIG_STAGGERED_WRITE:staggeredWriteResult,
> > +	RPI_IPA_CONFIG_SENSOR:controls,
> > +	RPI_IPA_CONFIG_SEND_FD:bufferFd,
> > +}
> > +
> > +// required
> > +// must specify which union option each value corresponds to
> > +enum EventOps {
> > +	RPI_IPA_EVENT_SIGNAL_STAT_READY:bufferId,
> > +	RPI_IPA_EVENT_SIGNAL_ISP_PREPARE:ispPrepare,
> > +	RPI_IPA_EVENT_QUEUE_REQUEST:controls,
> > +	RPI_IPA_EVENT_SEND_FD:bufferFd,
> > +}
> > +
> > +// required
> > +// must specify which union option each value corresponds to
> > +enum ActionOps {
> > +	RPI_IPA_ACTION_V4L2_SET_STAGGERED:controls,
> > +	RPI_IPA_ACTION_V4L2_SET_ISP:controls,
> > +	RPI_IPA_ACTION_STATS_METADATA_COMPLETE:statsComplete,
> > +	RPI_IPA_ACTION_RUN_ISP:bufferId,
> > +	RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME:bufferId,
> > +	RPI_IPA_ACTION_EMBEDDED_COMPLETE:bufferId,
> > +}
> > +
> > +// Custom Data containers
> > +
> > +table RPiStaggeredWritePayload {
> > +	gainDelay:uint32;
> > +	exposureDelay:uint32;
> > +	sensorMetadata:uint32;
> > +}
> > +
> > +table RPiIspPreparePayload {
> > +	embeddedbufferId:uint32;
> > +	bayerbufferId:uint32;
> > +}
> > +
> > +table RPiStatsCompletePayload {
> > +	bufferId:uint32;
> > +	controls:ControlList;
> > +}
> > +
> > +
> > +// First level payload unions
> > +// these must be unions, and must be named as shown
> > +
> > +// required
> > +// this one is actually in a vector of payload objects (op-union pair)
> > +union ConfigPayload {
> > +	lsTableHandle:uint32,
> > +	staggeredWriteResult:RPiStaggeredWritePayload,
> > +	controls:ControlList,
> > +	bufferFd:int32,
> > +}
> > +
> > +// required
> > +union EventPayload {
> > +	bufferId:uint32,
> > +	ispPrepare:RPiIspPreparePayload,
> > +	controls:ControlList,
> > +	bufferFd:int32,
> > +}
> > +
> > +// required
> > +union ActionPayload {
> > +	bufferId:uint32,
> > +	statsComplete:RPiStatsCompletePayload,
> > +	controls:ControlList,
> > +}
> > diff --git a/include/libcamera/ipa/raspberrypi_wrapper.h b/include/libcamera/ipa/raspberrypi_wrapper.h
> > new file mode 100644
> > index 0000000..eab51d3
> > --- /dev/null
> > +++ b/include/libcamera/ipa/raspberrypi_wrapper.h
> > @@ -0,0 +1,658 @@
> > +// will be automatically generated by custom compiler
> > +
> > +#ifndef __LIBCAMERA_IPA_INTERFACE_RASPBERRYPI_FB_H__
> > +#define __LIBCAMERA_IPA_INTERFACE_RASPBERRYPI_FB_H__
> > +
> > +#include "raspberrypi_generated.h"
> > +
> > +namespace libcamera {
> > +
> > +// TODO this (and deserialize) should go global
> > +inline flatbuffers::Offset<ControlList> serializeControlList(flatbuffers::FlatBufferBuilder &builder, ControlList &list)
> > +{
> > +	ControlSerializer serializer;
> > +
> > +	// TODO need a way to get the info map (param? #defined param?)
> > +	size_t size = serializer.binarySize(RPiControls);
> > +	std::vector<uint8_t> infoData(size);
> > +	ByteStreamBuffer buffer(infoData.data(), infoData.size());
> > +	serializer.serialize(RPiControls, buffer);
> > +
> > +	size = serializer.binarySize(list);
> > +	std::vector<uint8_t> listData(size);
> > +	buffer = ByteStreamBuffer(listData.data(), listData.size());
> > +	serializer.serialize(list, buffer);
> > +
> > +	// don't need to error check; just write empty vector
> > +	return CreateControlListDirect(builder, listData);
> > +}
> > +
> > +// TODO fix the input type for this (like ControlListFB or something?)
> > +inline ControlList deserializeControlList(ControlList *obj)
> > +{
> > +	ControlSerializer deserializer;
> > +	std::vector<uint8_t> buf = std::vector(obj->begin(), obj->end());
> > +
> > +	std::vector<uint8_t> infoData(size);
> > +	BytesStreamBuffer buffer(const_cast<const uint8_t *>(infoData.data()), infoData.size());
> > +	ControlInfoMap infoMap = deserializer.deserialize<ControlInfoMap>(buffer);
> > +	// TODO what to do for error checking?
> > +
> > +	std::vector<uint8_t> listData(size);
> > +	buffer = ByteStreamBuffer(const_cast<const uint8_t *>(listData.data()), listData.size());
> > +	ControlList list = deserializer.deserialize<ControlList>(buffer);
> > +
> > +	return list;
> > +}
> > +
> > +
> > +enum RPiConfigParametersWrapper {
> > +	RPI_IPA_CONFIG_LS_TABLE = 1,
> > +	RPI_IPA_CONFIG_STAGGERED_WRITE = 2,
> > +	RPI_IPA_CONFIG_SENSOR = 3,
> > +	RPI_IPA_CONFIG_SEND_FD = 4,
> > +};
> > +
> > +enum RPiEventsWrapper {
> > +  RPI_IPA_EVENT_SIGNAL_STAT_READY = 1,
> > +  RPI_IPA_EVENT_SIGNAL_ISP_PREPARE = 2,
> > +  RPI_IPA_EVENT_QUEUE_REQUEST = 3,
> > +  RPI_IPA_EVENT_SEND_FD = 4,
> > +};
> > +
> > +enum RPiActionsWrapper {
> > +	RPI_IPA_ACTION_V4L2_SET_STAGGERED,
> > +	RPI_IPA_ACTION_V4L2_SET_ISP,
> > +	RPI_IPA_ACTION_STATS_METADATA_COMPLETE,
> > +	RPI_IPA_ACTION_RUN_ISP,
> > +	RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME,
> > +	RPI_IPA_ACTION_EMBEDDED_COMPLETE,
> > +};
> > +
> > +struct RPiStaggeredWritePayloadWrapper : Serializable
> > +{
> > +	RPiStaggeredWritePayloadWrapper() {}
> > +
> > +	RPiStaggeredWritePayloadWrapper(uint32_t gainDelay, uint32_t exposureDelay, uint32_t sensorMetadata) : gainDelay_(gainDelay), exposureDelay_(exposureDelay), sensorMetadata_(sensorMetadata) {}
> > +
> > +	RPiStaggeredWritePayloadWrapper(const RPiStaggeredWritePayload *buf)
> > +	{
> > +		const RPiStaggeredWritePayload *obj = flatbuffers::GetRoot<RPiStaggeredWritePayload>(buf);
> > +
> > +		gainDelay_ = obj->gainDelay();
> > +		exposureDelay_ = obj->exposureDelay();
> > +		sensorMetadata_ = obj->sensorMetadata();
> > +	}
> > +
> > +	flatbuffers::Offset<RPiStaggeredWritePayload> serialize(flatbuffers::FlatBufferBuilder &builder)
> > +	{
> > +		// - construct compound children depth-first
> > +		// - get simple data
> > +		// - feed to Create
> > +
> > +		// nop for gainDelay_
> > +
> > +		// nop for exposureDelay_
> > +
> > +		// nop for sensorMetadata_
> > +
> > +		return CreateRPiStaggeredWritePayload(builder, gainDelay_, exposureDelay_, sensorMetadata_);
> > +	}
> > +
> > +	std::deque<int> extractFds()
> > +	{
> > +		// contains no fd and no compound children, so return empty
> > +		return {};
> > +	}
> > +
> > +	void injectFds(std::deque<int> &fds)
> > +	{
> > +		// nop
> > +	}
> > +
> > +	uint32_t gainDelay_;
> > +	uint32_t exposureDelay_;
> > +	uint32_t sensorMetadata_;
> > +}
> > +
> > +struct RPiIspPreparePayloadWrapper : Serializable
> > +{
> > +	RPiIspPreparePayloadWrapper() {}
> > +
> > +	RPiIspPreparePayloadWrapper(uint32_t embeddedbufferId, uint32_t bayerbufferId) : embeddedbufferId_(embeddedbufferId), bayerbufferId_(bayerbufferId) {}
> > +
> > +	RPiIspPreparePayloadWrapper(const RPiIspPreparePayload *buf)
> > +	{
> > +		const RPiIspPreparePayload *obj = flatbuffers::GetRoot<RPiIspPreparePayload>(buf);
> > +
> > +		embeddedbufferId_ = obj->embeddedbufferId();
> > +		bayerbufferId_ = obj->bayerbufferId();
> > +	}
> > +
> > +	flatbuffers::Offset<RPiIspPreparePayload> serialize(flatbuffers::FlatBufferBuilder &builder)
> > +	{
> > +		// - construct compound children depth-first
> > +		// - get simple data
> > +		// - feed to Create
> > +
> > +		// nop for embeddedbufferId_
> > +
> > +		// nop for bayerbufferId_
> > +
> > +		return CreateRPiIspPreparePayload(builder, embeddedbufferId_, bayerbufferId_);
> > +	}
> > +
> > +	// these could be generated for every struct and not just those that
> > +	// fds in their tree. that'll save parsing the dependency graph in the
> > +	// compiler
> > +	std::deque<int> extractFds()
> > +	{
> > +		return {};
> > +	}
> > +
> > +	void injectFds(std::deque<int> &fds)
> > +	{
> > +		// nop
> > +	}
> > +
> > +	uint32_t embeddedbufferId_;
> > +	uint32_t bayerbufferId_;
> > +}
> > +
> > +struct RPiStatsCompletePayloadWrapper : Serializable
> > +{
> > +	RPiStatsCompletePayloadWrapper() {}
> > +
> > +	RPiStatsCompletePayloadWrapper(uint32_t bufferId, ControlList &controls) : bufferId_(bufferId), controls_(controls) {}
> > +
> > +	// deserialize
> > +	RPiStatsCompletePayloadWrapper(const RPiStatsCompletePayload *buf)
> > +	{
> > +		const RPiStatsCompletePayload *obj = flatbuffers::GetRoot<RPiStatsCompletePayload>(buf);
> > +
> > +		bufferId_ = obj->bufferId();
> > +		controls_ = deserializeControlList(obj->controls());
> > +	}
> > +
> > +	flatbuffers::Offset<RPiStatsCompletePayloadWrapper> serialize(flatbuffers::FlatBufferBuilder &builder)
> > +	{
> > +		// - construct compound children depth-first
> > +		// - get simple data
> > +		// - feed to Create
> > +
> > +		// nop for bufferId_
> > +
> > +		// serialize controls_
> > +		flatbuffers::Offset<ControlList> controls = serializeControlList(builder, controls_);
> > +
> > +		return CreateRPiStatsCompletePayload(builder, bufferId_, controls);
> > +	}
> > +
> > +	uint32_t bufferId_;
> > +	ControlList controls_;
> > +}
> > +
> > +union RPiConfigureUnionWrapper
> > +{
> > +	uint32_t lsTableHandle_;
> > +	RPiStaggeredWritePayload staggeredWriteResult_;
> > +	ControlList contols_;
> > +	int32_t bufferFd_;
> > +}
> > +
> > +struct RPiConfigurePayloadWrapper : Serializable
> > +{
> > +	RPiConfigurePayloadWrapper() {}
> > +
> > +	RPiConfigurePayloadWrapper(enum RPiConfigParametersWrapper op, RPiConfigureUnionWrapper payload) : op_(op), payload_(payload) {}
> > +
> > +	// deserialize
> > +	RPiConfigurePayloadWrapper(const RPiConfigurePayload *buf)
> > +	{
> > +		// yeah fill this in if you want
> > +	}
> > +
> > +	// deserialize
> > +	// TODO need this if is a member of vector
> > +	RPiConfigurePayloadWrapper(flatbuffers::Offset<RPiConfigurePayload> &p)
> > +	{
> > +		// nop for op
> > +		op_ = p.op();
> > +
> > +		// start of union block for payload_
> > +		switch (op_) {
> > +		case RPI_IPA_CONFIG_LS_TABLE:
> > +			payload_.lsTableHandle_ = p.payload_as_lsTableHandle();
> > +			break;
> > +		case RPI_IPA_CONFIG_STAGGERED_WRITE:
> > +			payload_.staggeredWriteResult_ = RPiStaggeredWritePayloadWrapper(p.payload_as_staggeredWriteResult);
> > +			break;
> > +		case RPI_IPA_CONFIG_SENSOR:
> > +			payload_.controls_ = deserializeControlList(p.payload_as_controls());
> > +			break;
> > +		case RPI_IPA_CONFIG_SEND_FD:
> > +			payload_.bufferFd_ = p.payload_as_bufferFd();
> > +			break;
> > +		}
> > +		// end of union block for payload_
> > +	}
> > +
> > +	flatbuffers::Offset<RPiConfigurePayload> serialize(flatbuffers::FlatBufferBuilder &builder)
> > +	{
> > +		// - construct compound children depth-first
> > +		// - get simple data
> > +		// - feed to Create
> > +
> > +		// nop for op_
> > +		RPiConfigureUnion payloadType;
> > +		flatbuffers::Offset<void> payload;
> > +
> > +		// start of union block for payload_
> > +		switch (op_) {
> > +		case RPI_IPA_CONFIG_LS_TABLE:
> > +			payloadType = RPiConfigureUnion_lsTableHandle;
> > +			payload = CreateUnsignedInt(builder, payload_.lsTableHandle_);
> > +			break;
> > +		case RPI_IPA_CONFIG_STAGGERED_WRITE:
> > +			payloadType = RPiConfigureUnion_staggeredWriteResult;
> > +			payload = payload_.staggeredWriteResult_.serialize(builder);
> > +			break;
> > +		case RPI_IPA_CONFIG_SENSOR:
> > +			// controls
> > +			payloadType = RPiConfigureUnion_controls;
> > +			payload = serializeControlList(builder, payload_.controls_);
> > +			break;
> > +		case RPI_IPA_CONFIG_SEND_FD:
> > +			payloadType = RPiConfigureUnion_bufferFd;
> > +			payload = CreateSignedInt(builder, payload_.bufferFd_);
> > +			break;
> > +		}
> > +		// end of union block for payload_
> > +
> > +		return CreateRPiConfigurePayload(builder, op_, payloadType, payload);
> > +	}
> > +
> > +	std::deque<int> extractFds()
> > +	{
> > +		std::deque<int> fds;
> > +
> > +		// nop for op_, since it has no children, and is not fd
> > +
> > +		// start of union block for payload_
> > +		switch (op_) {
> > +		case RPI_IPA_CONFIG_LS_TABLE:
> > +			// value is not fd
> > +			break;
> > +		case RPI_IPA_CONFIG_STAGGERED_WRITE:
> > +			std::deque<int> pfds = payload_.staggeredWriteResult_.extractFds();
> > +			fds.insert(fds.end(), pfds,begin(), pfds.end());
> > +			break;
> > +		case RPI_IPA_CONFIG_SENSOR:
> > +			// value is not fd
> > +			break;
> > +		case RPI_IPA_CONFIG_SEND_FD:
> > +			fds.push_back(payload.bufferFd_);
> > +			break;
> > +		}
> > +		// end of union block for payload_
> > +
> > +		return fds;
> > +	}
> > +
> > +	void injectFds(std::deque<int> &fds)
> > +	{
> > +		// nop for op_
> > +
> > +		// start of union block for payload_
> > +		switch (op_) {
> > +		case RPI_IPA_CONFIG_LS_TABLE:
> > +			break;
> > +		case RPI_IPA_CONFIG_STAGGERED_WRITE:
> > +			payload_.staggeredWriteResult_.injectFds(fds);
> > +			break;
> > +		case RPI_IPA_CONFIG_SENSOR:
> > +			break;
> > +		case RPI_IPA_CONFIG_SEND_FD:
> > +			payload_.bufferFd_ = fds.at(0);
> > +			fds.pop_front();
> > +			break;
> > +		}
> > +		// end of union block for payload_
> > +	}
> > +
> > +	enum RPiConfigParametersWrapper op_;
> > +	RPiConfigureUnionWrapper payload_;
> > +}
> > +
> > +union RPiEventPayloadWrapper
> > +{
> > +	uint32_t bufferId_;
> > +	RPiIspPreparePayloadWrapper ispPrepare_;
> > +	ControlList controls_;
> > +	int32_t bufferFd_;
> > +}
> > +
> > +union RPiActionPayloadWrapper
> > +{
> > +	uint32_t bufferId_;
> > +	RPiStatsCompletePayloadWrapper statsComplete_;
> > +	ControlList controls_;
> > +}
> > +
> > +struct RPiConfigureParamsWrapper : Serializable
> > +{
> > +	RPiConfigureParamsWrapper() {}
> > +
> > +	RPiConfigureParamsWrapper(std::vector<RPiConfigurePayloadWrapper> payload) : payload_(payload) {}
> > +
> > +	// deserialize
> > +	// only top-level gets fds
> > +	RPiConfigureParamsWrapper(const uint8_t *buf, std::dequeue<int> fds)
> > +	{
> > +		const RPiConfigureParams *obj = flatbuffers::GetRoot<RPiConfigureParams>(buf);
> > +
> > +		// start of vector block for payload_
> > +		flatbuffers::Vector<flatbuffers::Offset<RPiConfigurePayload>> *payload = obj->params();
> > +		std::transform(payload->begin(), payload->end(), std::back_inserter(payload_),
> > +				[](flatbuffers::Offset<RPiConfigurePayload> &p) -> RPiConfigurePayloadWrapper {
> > +					return RPiConfigurePayloadWrapper(p);
> > +				});
> > +		// end of vector block for payload_
> > +
> > +
> > +		// after constructing all the children, we need to inject the fds
> > +		injecetFds(fds);
> > +	}
> > +
> > +	flatbuffers::Offset<RPiConfigureParams> serialize(flatbuffers::FlatBufferBuilder &builder)
> > +	{
> > +		// - construct compound children depth-first
> > +		// - get simple data
> > +		// - feed to Create
> > +
> > +		// start of vector block for payload_
> > +		std::vector<flatbuffers::Offset<RPiConfigurePayload>> payload;
> > +		std::transform(payload_.begin(), payload_.end(), std::back_inserter(payload),
> > +				[](RPiConfigurePayloadWrapper &p) -> flatbuffers::Offset<RPiConfigurePayload> { return p.serialize(builder); });
> > +		// end of vector block for payload_
> > +
> > +		// use direct if there's vector as a member
> > +		return CreateRPiConfigureParamsDirect(builder, payload);
> > +	}
> > +
> > +	// this is only for root_type
> > +	std::deque<int> extractFds()
> > +	{
> > +		std::deque<int> fds;
> > +
> > +		// start of vector block for payload_
> > +		for (RPiConfigurePayloadWrapper &payload : payload_)  {
> > +			std::deque<int> &pfds = payload_.extractFds();
> > +			fds.insert(fds.end(), pfds.begin(), pfds.end());
> > +		}
> > +		// end of vector block for payload_
> > +
> > +		return fds;
> > +	}
> > +
> > +	void injectFds(std::deque<int> &fds)
> > +	{
> > +		// start of vector block for payload_
> > +		for (RPiConfigurePayloadWrapper &payload : payload_) 
> > +			payload_.injectFds(fds);
> > +		// end of vector block for payload_
> > +	}
> > +
> > +	// this is only for root_type
> > +	// returns buffer pointer, buffer size (in bytes), and vector of fds
> > +	// the fds are matched by index; the index maps to full path in object
> > +	// tree, generated by the special compiler
> > +	std::tuple<uint8_t *, size_t, std::deque<int>> serialize()
> > +	{
> > +		flatbuffers::FlatBufferBuilder builder(1024);
> > +		builder.Finish(this.serialize(builder));
> > +
> > +		uint8_t *buf = builder.GetBufferPointer();
> > +		size_t size = builder.GetSize();
> > +		return {buf, size, extractFds()};
> > +	}
> > +
> > +	std::vector<RPiConfigurePayloadWrapper> payload_;
> > +}
> > +
> > +struct RPiEventParamsWrapper : Serializable
> > +{
> > +	RPiEventParamsWrapper() {}
> > +
> > +	RPiEventParamsWrapper(enum RPiEventsWrapper ev, RPiEventPayloadWrapper payload) : ev_(ev), payload_(payload) {}
> > +
> > +	// deserialize
> > +	// root_type needs fds
> > +	RPiEventParamsWrapper(const uint8_t *buf, std::deque<int> fds)
> > +	{
> > +		const RPiEventParams *obj = flatbuffers::GetRoot<RPiEventParams>(buf);
> > +
> > +		ev_ = obj->ev();
> > +
> > +		// start of union block for payload_
> > +		switch (ev_) {
> > +		case RPI_IPA_EVENT_SIGNAL_STAT_READY:
> > +			payload_.bufferId_ = obj->payload_as_bufferId()->value();
> > +			break;
> > +		case RPI_IPA_EVENT_SIGNAL_ISP_PREPARE:
> > +			payload_.ispPrepare_ = RPiIspPreparePayloadWrapper(obj->payload_as_ispPrepare());
> > +			break;
> > +		case RPI_IPA_EVENT_QUEUE_REQUEST:
> > +			payload_.controls_ = deserializeControlList(obj->payload_as_controls());
> > +			break;
> > +		case RPI_IPA_EVENT_SEND_FD:
> > +			payload_.bufferFd_ = obj->payload_as_bufferFd()->value();
> > +			break;
> > +		}
> > +		// end of union block for payload_
> > +
> > +		injectFds(fds);
> > +	}
> > +
> > +	flatbuffers::Offset<RPiEventParams> serialize(flatbuffers::FlatBufferBuilder &builder)
> > +	{
> > +		// - construct compound children depth-first
> > +		// - get simple data
> > +		// - feed to Create
> > +
> > +		// nop for ev_
> > +
> > +		// start of union block for payload_
> > +		RPiEventPayload payloadType;
> > +		flatbuffers::Offset<void> payload;
> > +
> > +		switch (ev_) {
> > +		case RPI_IPA_EVENT_SIGNAL_STAT_READY:
> > +			payloadType = RPiEventPayload_bufferId;
> > +			payload = CreateUnsignedInt(builder, payload_.bufferId_);
> > +			break;
> > +		case RPI_IPA_EVENT_SIGNAL_ISP_PREPARE:
> > +			payloadType = RPiEventPayload_ispPrepare;
> > +			payload = payload_.ispPrepare_.serialize(builder);
> > +			break;
> > +		case RPI_IPA_EVENT_QUEUE_REQUEST:
> > +			payloadType = RPiEventPayload_controls;
> > +			payload = serializeControlList(builder, payload_.controls_);
> > +			break;
> > +		case RPI_IPA_EVENT_SEND_FD:
> > +			payloadType = RPiEventPayload_bufferFd;
> > +			payload = CreateSignedInt(builder, payload_.bufferFd_);
> > +			break;
> > +		}
> > +		// end of union block
> > +
> > +		return CreateRPiEventParams(builder, ev_, payloadType, payload);
> > +	}
> > +
> > +	std::deque<int> extractFds()
> > +	{
> > +		std::deque<int> fds;
> > +
> > +		// nop for ev_
> > +
> > +		// start of union block for payload_
> > +		switch (ev_) {
> > +		case RPI_IPA_EVENT_SIGNAL_STAT_READY:
> > +			// non-fd scalar
> > +			break;
> > +		case RPI_IPA_EVENT_SIGNAL_ISP_PREPARE:
> > +			std::deque<int> &pfds = payload_.ispPrepare_.extractFds();
> > +			fds.insert(fds.end(), pfds.begin(), pfds.end());
> > +			break;
> > +		case RPI_IPA_EVENT_QUEUE_REQUEST:
> > +			// non-fd controls
> > +			break;
> > +		case RPI_IPA_EVENT_SEND_FD:
> > +			fds.push_back(payload_.bufferFd_);
> > +			break;
> > +		}
> > +		// end of union block for payload_
> > +
> > +		return fds;
> > +	}
> > +
> > +	void injectFds(std::deque<int> &fds)
> > +	{
> > +		// nop for ev_
> > +
> > +		// start of union block for payload_
> > +		switch (ev_) {
> > +		case RPI_IPA_EVENT_SIGNAL_STAT_READY:
> > +			break;
> > +		case RPI_IPA_EVENT_SIGNAL_ISP_PREPARE:
> > +			payload_.ispPrepare_.injectFds(fds);
> > +			break;
> > +		case RPI_IPA_EVENT_QUEUE_REQUEST:
> > +			break;
> > +		case RPI_IPA_EVENT_SEND_FD:
> > +			payload_.bufferFd_ = fds.at(0);
> > +			fds.pop_front();
> > +			break;
> > +		}
> > +		// end of union block for payload_
> > +	}
> > +
> > +	// this is only for root_type
> > +	// returns buffer pointer, buffer size (in bytes), and vector of fds
> > +	// the fds are matched by index; the index maps to full path in object
> > +	// tree, generated by the special compiler
> > +	std::tuple<uint8_t *, size_t, std::deque<int>> serialize()
> > +	{
> > +		flatbuffers::FlatBufferBuilder builder(1024);
> > +		builder.Finish(this.serialize(builder));
> > +
> > +		uint8_t *buf = builder.GetBufferPointer();
> > +		size_t size = builder.GetSize();
> > +		return {buf, size, extractFds()};
> > +	}
> > +
> > +	enum RPiEventsWrapper ev_;
> > +	RPiEventPayloadWrapper payload_;
> > +}
> > +
> > +struct RPiActionParamsWrapper : Serializable
> > +{
> > +	RPiActionParamsWrapper() {}
> > +
> > +	RPiActionParamsWrapper(enum RPiActionsWrapper op, RPiActionPayloadWrapper payload) : op_(op), payload_(payload) {}
> > +
> > +	// deserialize
> > +	// root_type needs fds
> > +	RPiActionParamsWrapper(const uint8_t *buf, std::deque<int> &fds)
> > +	{
> > +		const RPiActionParams *obj = flatbuffers::GetRoot<RPiActionParams>(buf);
> > +
> > +		op_ = obj->op();
> > +
> > +		// start of union block for payload_
> > +		switch (op_) {
> > +		case RPI_IPA_ACTION_V4L2_SET_STAGGERED:
> > +		case RPI_IPA_ACTION_V4L2_SET_ISP:
> > +			payload_.controls_ = deserializeControlList(obj->payload_as_controls());
> > +			break;
> > +		case RPI_IPA_ACTION_STATS_METADATA_COMPLETE:
> > +			payload_.statsComplete_ = RPiStatsCompletePayloadWrapper(obj->payload_as_statsComplete());
> > +			break;
> > +		case RPI_IPA_ACTION_RUN_ISP:
> > +		case RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME:
> > +		case RPI_IPA_ACTION_EMBEDDED_COMPLETE:
> > +			// UnsignedInt needs ->value()
> > +			payload_.bufferId_ = obj->payload_as_bufferId()->value();
> > +			break;
> > +		}
> > +		// end of union block for payload_
> > +
> > +		injectFds(fds);
> > +	}
> > +
> > +	flatbuffers::Offset<RPiEventParams> serialize(flatbuffers::FlatBufferBuilder &builder)
> > +	{
> > +		// - construct compound children depth-first
> > +		// - get simple data
> > +		// - feed to Create
> > +
> > +		// nop for op_
> > +
> > +		// start of union block for payload_
> > +		RPiActionPayload payloadType;
> > +		flatbuffers::Offset<void> payload;
> > +
> > +		switch (op_) {
> > +		case RPI_IPA_ACTION_V4L2_SET_STAGGERED:
> > +		case RPI_IPA_ACTION_V4L2_SET_ISP:
> > +			payloadType = RPiActionPayload_controls;
> > +			payload = serializeControlList(builder, payload_.controls_);
> > +			break;
> > +		case RPI_IPA_ACTION_STATS_METADATA_COMPLETE:
> > +			payloadType = RPiActionPayload_statsComplete;
> > +			payload = payload_.statsComplete_.serialize(builder);
> > +			break;
> > +		case RPI_IPA_ACTION_RUN_ISP:
> > +		case RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME:
> > +		case RPI_IPA_ACTION_EMBEDDED_COMPLETE:
> > +			payloadType = RPiActionPayload_bufferId;
> > +			payload = CreateUnsignedInt(builder, payload_.bufferId_);
> > +			break;
> > +		}
> > +		// end of union block for payload_
> > +
> > +		return CreateRPiActionParams(builder, op_, payloadType, payload);
> > +	}
> > +
> > +	// root_type needs it even if it's empty, because of top-level serialize()
> > +	std::deque<int> extractFds()
> > +	{
> > +		return {};
> > +	}
> > +
> > +	std::deque<int> injectFds()
> > +	{
> > +		// nop
> > +	}
> > +
> > +	// this is only for root_type
> > +	// returns buffer pointer, buffer size (in bytes), and vector of fds
> > +	// the fds are matched by index; the index maps to full path in object
> > +	// tree, generated by the special compiler
> > +	std::tuple<uint8_t *, size_t, std::deque<int>> serialize()
> > +	{
> > +		flatbuffers::FlatBufferBuilder builder(1024);
> > +		builder.Finish(this.serialize(builder));
> > +
> > +		uint8_t *buf = builder.GetBufferPointer();
> > +		size_t size = builder.GetSize();
> > +		return {buf, size, extractFds()};
> > +	}
> > +
> > +	enum RPiActionsWrapper op_;
> > +	RPiActionPayloadWrapper payload_;
> > +}
> > +
> > +} /* namespace libcamera */
> > +
> > +#endif /* __LIBCAMERA_IPA_INTERFACE_RASPBERRYPI_FB_H__ */
> 
> -- 
> Regards,
> 
> Laurent Pinchart