Upstream issues

[arcnmx]

Here's a dumping ground for various problems and limitations that I've run into while working with the wp api. Need to clean up and report some of these maybe.

• [ ] wp_spa_pod_parser_new_object gives you a string instead of the much more useful numeric ID, I can do my own id table lookups thanks :disappointed:
• [ ] no apparent way to get property flags despite the wp_spa_pod_get_property docs implying otherwise?
• [ ] no apparent way to get flags or know if a WpSpaPod is const or not
• [ ] WpObjectInterest has no accessor for self.gtype
• [ ] the gir xml contains a bunch of inaccurate information
• [ ] please rethink using lua-style GVariants as a poor man's ABI - why should a module be required to decode a goddamn <{"1": <whatever>}> a{sv} variant as an array, what is wrong with you why is this such a lua-centric worldview... there is no reason why all plugins must be subjected to lua's idiosyncrasies :<
• [ ] pipewire-rs requiring bindgen at build time has been a longstanding issue that prevents us from publishing to docs.rs and introduces a bunch of hacks into the build and cfgs
• [ ] cargo package --list --allow-dirty -p wireplumber-sys omits critical files if sys/generate/Cargo.toml exists, despite gitignore and an explicit include = ["generate/etc"] maybe report this as a cargo bug? it presumably believes that sys/generate is a sub-crate and ignores its files

[tom-a-wagner]

Regarding inaccurate gir xml:

-i '/_:repository/_:package[not(@name)]' -t attr -n name -v wireplumber-0.4, this has been patched in https://gitlab.freedesktop.org/pipewire/wireplumber/-/merge_requests/448.

I'll try making MRs to fix the other issues soon.

Why is the shared object name being patched? (-u '//_:namespace[@name="Wp"]/@shared-library' -v wireplumber-0.4.so.0) \ At least on fedora, the upstream naming libwireplumber-0.4.so.0 is correct, and wireplumber-0.4.so.0 doesn't exist, and manually building wireplumber also results in the same name.

[tom-a-wagner]

Regarding the -i '///_:type[not(@name) and @c:type="pw_permission"]' -t attr -n name -v guint64 gir patch, while a guint64 does have the same layout as a pw_permission, the generated binding is just wrong, since we want to work with pw_permission, no guint64.

I asked about how to patch this in the gnome rust matrix channel a while ago when I was looking into how to patch the gir file myself, and we agreed that the whole array element should probably be replaced with a gpointer and the method should be bound manually.

A little verbose, but this works:

# Fix type of array parameter using non-gobject-introspection array child type
# These types are missing the `name` attribute since they don't have gir definitions.
# Replace the whole array with a gpointer.
xmlstarlet ed -L \
    -d '//_:class[@name="Client"]/_:method[@name="update_permissions_array"]/_:parameters/_:parameter[@name="permissions"]/_:array' \
    -s '//_:class[@name="Client"]/_:method[@name="update_permissions_array"]/_:parameters/_:parameter[@name="permissions" and not(_:type)]' \
        -t elem -n type \
    Wp-0.4.gir
xmlstarlet ed -L \
    -i '//_:class[@name="Client"]/_:method[@name="update_permissions_array"]/_:parameters/_:parameter[@name="permissions"]/_:type[not(@name)]' \
        -t attr -n name -v gpointer \
    -i '//_:class[@name="Client"]/_:method[@name="update_permissions_array"]/_:parameters/_:parameter[@name="permissions"]/_:type[not(@c:type)]' \
        -t attr -n c:type -v 'const pw_permission*' \
    Wp-0.4.gir

Thoughts?

[arcnmx]

-i '/_:repository/_:package[not(@name)]' -t attr -n name -v wireplumber-0.4, this has been patched in https://gitlab.freedesktop.org/pipewire/wireplumber/-/merge_requests/448.

I had noticed, but left it in just to keep generation working with older versions. It's probably a good idea to clean up and remove the ones that are unnecessary.

Why is the shared object name being patched? (-u '//_:namespace[@name="Wp"]/@shared-library' -v wireplumber-0.4.so.0) At least on fedora, the upstream naming libwireplumber-0.4.so.0 is correct, and wireplumber-0.4.so.0 doesn't exist, and manually building wireplumber also results in the same name.

Oh, I omitted lib for no particular reason other than that it happened to work and I wasn't aware of what the correct convention was for that field. My gir file contains an absolute path to the library, which is why that patch exists since I don't want that to appear in the output. It is not relevant to upstream wp.

Regarding the -i '///_:type[not(@name) and @c:type="pw_permission"]' -t attr -n name -v guint64 gir patch, while a guint64 does have the same layout as a pw_permission, the generated binding is just wrong, since we want to work with pw_permission, no guint64.

That was just the first hack I could think of to appease gir and allow it to generate/compile, the note is indeed there to remind us to fix it!

Thoughts?

If the goal is just to appease gir, we can be less verbose and just change name to gpointer to generate the correct sys binding:

diff --git a/ci/wp-gir-filter.sh b/ci/wp-gir-filter.sh
index 2749ade8..96ae40b6 100755
--- a/ci/wp-gir-filter.sh
+++ b/ci/wp-gir-filter.sh
@@ -1,8 +1,7 @@
 #!/usr/bin/env bash

-# note: a pw_permission is actually 2x uint32
 exec xmlstarlet ed \
-   -i '///_:type[not(@name) and @c:type="pw_permission"]' -t attr -n name -v guint64 \
+   -i '///_:type[not(@name) and @c:type="pw_permission"]' -t attr -n name -v 'gpointer' \
    -u '///_:constant[@c:type="WP_LOG_LEVEL_TRACE"]/@value' -v $((1<<8)) \
    -u '///_:constant[@c:type="WP_PIPEWIRE_OBJECT_FEATURES_ALL"]/@value' -v $((992|17)) \
    -i '///_:record[@c:type="WpIteratorMethods"]' -t attr -n glib:get-type -v wp_iterator_methods_get_type \
diff --git a/sys/generate/src/Wp-0.4.gir b/sys/generate/src/Wp-0.4.gir
index b38952e0..6542ff2b 100644
--- a/sys/generate/src/Wp-0.4.gir
+++ b/sys/generate/src/Wp-0.4.gir
@@ -94,7 +94,7 @@ An object id of -1 can be used to set the default object permissions for this cl
           <parameter name="permissions" transfer-ownership="none">
             <doc xml:space="preserve" filename="docs/wp-gtkdoc.h" line="66">an array of permissions per object id</doc>
             <array length="0" zero-terminated="0" c:type="const pw_permission*">
-              <type c:type="pw_permission" name="guint64"/>
+              <type c:type="pw_permission" name="gpointer"/>
             </array>
           </parameter>
         </parameters>
diff --git a/sys/generate/src/lib.rs b/sys/generate/src/lib.rs
index 94b8c3a4..03ec4c7f 100644
--- a/sys/generate/src/lib.rs
+++ b/sys/generate/src/lib.rs
@@ -1870,7 +1870,7 @@ extern "C" {
     pub fn wp_client_get_type() -> GType;
     pub fn wp_client_send_error(self_: *mut WpClient, id: u32, res: c_int, message: *const c_char);
     pub fn wp_client_update_permissions(self_: *mut WpClient, n_perm: c_uint, ...);
-    pub fn wp_client_update_permissions_array(self_: *mut WpClient, n_perm: c_uint, permissions: *const u64);
+    pub fn wp_client_update_permissions_array(self_: *mut WpClient, n_perm: c_uint, permissions: *const pw_permission);

     //=========================================================================
     // WpComponentLoader

pw_permission isn't a pointer type obviously, but since we have to implement the high-level binding manually it doesn't really matter. There are many (other) cases where I wish gir could be configured via toml to replace parameter types, I kind of wonder if we could make a fake gir package for pipewire to fix this for all functions that use pw_* types?

[tom-a-wagner]

Missing version attribute has now been fixed in upstream for structs, enums and functions with https://gitlab.freedesktop.org/pipewire/wireplumber/-/merge_requests/533 and a couple previous MRs improving the doxygen-to-gtkdoc converter.

So we can probably drop those patches sometime after the next wireplumber release.

Properties and signals haven't been fixed upstream yet, because the doxygen-to-gtkdoc converter can't quite handle these yet.

[tom-a-wagner]

I kind of wonder if we could make a fake gir package for pipewire to fix this for all functions that use pw_* types?

This would be nice, but not sure if it's possible.

I wondered how the gdk4-wayland crate handles their non-gir wayland types, and they patch the gir to have those functions return a gpointer instead, and then implement the methods manually.