libweston backend configuration API candidates

[Pekka Paalanen]

Hi all,

now we have three API candidates with patches (and my own idea[1]
without patches so it doesn't count). Here is what I have gathered, let
me know if I got something wrong.


Giulio's proposal:
https://patchwork.freedesktop.org/patch/67547/

It uses transparent structs that get passed through a generic function
in libweston to the loaded backend. Configuration format is part of
libweston ABI.

My old opinion stays: I am ok with the approach, though we probably
want to version the structs. Something like:

------------------------------- src/compositor.h -------
 
 /* Configuration struct for a backend.
  *
  * This struct carries the configuration for a backend, and it's
  * passed to the backend's init entry point. The backend will
  * likely want to subclass this in order to handle backend specific
  * data.
  */
 struct weston_backend_config {
+	/** Major version for the backend-specific config struct
+	 *
+	 * This version must match exactly what the backend expects, otherwise
+	 * the struct is incompatible.
+	 */
+	uint32_t major_version;
+
+	/** Minor version of the backend-specific config struct
+	 *
+	 * This must be set to sizeof(struct backend-specific config).
+	 * If the value here is smaller than what the backend expects, the
+	 * extra config members will assume their default values.
+	 *
+	 * A value greater than what the backend expects is incompatible.
+	 */
+	size_t minor_version;
 };

ABI churn is likely, but I am not particularly concerned about it for
now.

With such struct versioning, it is possible to add new options without
breaking the ABI, as long as all additions are made to the end of a
struct and nothing existing is ever modified or removed. If existing
things need to be changed, major_version is bumped, which essentially
corresponds to an ABI break. Even then, if wanted, a backend could
choose to support several major versions. Whether all this flexibility
is actually useful, I am not sure. We could probably be fine with only
minor_version, and use library version bumps for major.


Benoit's proposal:
https://patchwork.freedesktop.org/patch/73206/

This is based on Giulio's proposal, except the config structs are now
opaque. The config structs are created and filled with function calls.
These functions are exported in libweston.so, which means it exports
many backend-specific functions. However, these functions do not
require any backend dependencies to be linked in, so no needless
libraries are pulled in.

Config versioning is tied to the library versioning. This makes it
harder to support multiple libweston versions where the only difference
is adding some configuration options. The compositor must use dlsym()
for any functions it can live without but wants to use if available at
runtime.

As a detail, Giulio's proposal has a callback for configuring
(hotplugged) outputs, while Benoit passes the known output
configurations to the backend on start-up. The benefit of the callback
is that output default settings are controlled by the compositor, not
libweston or the backend. That is why I would prefer to have the
callback. The defaults may vary arbitrarily per output, and later
layout could perhaps be part of the configuration.


Quentin's proposal:
https://patchwork.freedesktop.org/patch/73039/
https://patchwork.freedesktop.org/patch/73035/
https://patchwork.freedesktop.org/patch/73037/
https://patchwork.freedesktop.org/patch/73036/
https://patchwork.freedesktop.org/patch/73038/

This proposal first sets out to build a convenience library
libweston.la to create more structure in the source tree. It seems we
sorely needed that, because the assumptions here are the opposite from
what has already landed in upstream. Upstream has compositor.c and
struct weston_compositor as libwayland items, from which we slowly
extract the things that do not belong in libweston. Quentin's proposal
assumes compositor.c to be initally outside of libweston and slowly
moving all applicable bits into libweston under the new lib/ directory.
This confusion has to be taken into account when looking at the pathces.

The essential idea of this API is, that the compositor will register
configuration entry getter functions with libweston. These functions
are per config item type: a getter for ints, a getter for strings, etc.
Backends will then call these getters to retrieve configuration values
one by one.

The configuration options are identified explicitly by { section, key }
name tuples rather than a key name alone in the API. The getters get
also passed in a default value picked by the backend, in case the
compositor does not recognize what it is being asked for.

The benefits of this approach include that the library ABI is very
stable, as it includes only per-type getter functions. The compositor
implementation is also free to pass unrecognized options through:
adding a new option to a backend does not necessarily require updating
the compositor to understand it, as a user can make the setting in a
configuration file and it will be passed as "data" through to the
backend.

Configuring dynamically added outputs is no different: a backend will
just query some more options.

What I see as the downsides here are the (arguable) complexity of
setting up the getters, and not having an explicit way of knowing
whether all your options were actually used - you'd have to track that
yourself in your compositor. Of course, structs have the same problem,
so at least here you could check it if you wanted. Detecting e.g.
mispelled configuration items is fairly hard, because the decision of
what is a valid key or not is hidden inside libweston.

This is also lacking any explicit notion of a transaction. With structs
it's easy: you pass a pointer to a function, and once the function
returns, the configuration is in. Here we need to document which
function calls query which configurations at what time. This can be
awkward with output hotplugging, and the compositor may not even know
when libweston/backend has finished querying the options.

I also see the unrecognized option pass-through to the backend as a
double-edged sword. It fits perfectly, if users are expecting to be
configuring libweston. However, I believe users are primarily using the
compositor, not libweston. The compositor may make assumptions on how
the backend is configured, and if a user can override those
assumptions, it's an opportunity for the user to shoot himself in the
foot by blindly copying instructions from shady websites. Of course, a
compositor can prevent this, but is there enough reason to support
pass-through in the first place?

In summary, Quentin's proposal seems the most flexible, which naturally
makes it the hardest to program for, while also promising a very stable
library ABI.


I can't see an obvious winner in any of the above, they all can be made
to work, and do not seem to have particularly huge disadvantages at the
moment. We need to decide which properties we value the most.

In that light, I think the person who will be implementing the config
API and converting everything over should make the call. It's not like
we can't change it later, it would just be more work.


Thanks,
pq

[1] https://lists.freedesktop.org/archives/wayland-devel/2015-December/026100.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160216/3ca5d8eb/attachment-0001.sig>