Update Wireplumber API to 0.5

[tokyovigilante]

Wireplumber 0.5 has an incompatible API. This is a first WIP crack at updating. Would fix #2873 (but should not be merged currently).

Two current problems:

• Ignores the results of the two setup calls changed which are now async.
• Doesn't actually seem to work, adding extra logging shows that volume is not actually returned.

However it does build at least... I don't know the WP API well enough to add much more.

[EDIT]: As below now updated to use the new async API and working, ready for review.

[FilippoBonazziSUSE]

wireplumber upstream has stated that there are "no planned API/ABI changes during RCs" (with 0.4.90 being RC1), so it should be safe to start upgrading the code to the 0.5 API now.

[AngryPenguinPL]

Hi. I applied this patch with wireplumber 0.4.90 and see this compile error:

 ../src/modules/wireplumber.cpp:21:14: error: no matching function for call to 'wp_core_new'
DEBUG util.py:463:     21 |   wp_core_ = wp_core_new(NULL, NULL);
DEBUG util.py:463:        |              ^~~~~~~~~~~
DEBUG util.py:463:  /usr/include/wireplumber-0.5/wp/core.h:45:10: note: candidate function not viable: requires 3 arguments, but 2 were provided
DEBUG util.py:463:     45 | WpCore * wp_core_new (GMainContext * context, WpConf * conf,
DEBUG util.py:463:        |          ^            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
DEBUG util.py:463:     46 |     WpProperties * properties);
DEBUG util.py:463:        |     ~~~~~~~~~~~~~~~~~~~~~~~~~
DEBUG util.py:463:  1 error generated.

Anyway wireplumber 0.5.0 was just released, so is possible to target stable API/ABI https://gitlab.freedesktop.org/pipewire/wireplumber/-/commit/59d190a2bd400f3b093f99b16fc0fb06f6cb2cfe

[Arnavion]

Ignores the results of the two setup calls changed which are now async.

Doesn't actually seem to work, adding extra logging shows that volume is not actually returned.

The code loads components via wp_core_load_component and then calls wp_plugin_find to find the plugins in those components. So since the wp_core_load_component calls are async now, the wp_plugin_find calls will end up running before the components have been loaded.

You probably need to make an actual callback, and move those calls and the rest of the WirePlumber::WirePlumber ctor body after the loadRequiredApiModules() call into that callback.

[tokyovigilante]

You probably need to make an actual callback, and move those calls and the rest of the WirePlumber::WirePlumber ctor body after the loadRequiredApiModules() call into that callback.

Thanks for the feedback, have had another crack at this and seems to be working now.

[khaneliman]

Could you also run a clang-format on it to fix up the check.

[tokyovigilante]

Ok have run clang-format and clang-tidy, and removed the trace logging. Two remaining questions, I see clang-tidy in particular has touched much more than my changes (largely around implicit nullptr conversions), I assume there are no issues changing those also?

Secondly, the 0.4.81-0.4.90 RCs needed WP_LOCAL_LOG_TOPIC defined, so I did, however the 0.5.0 release removes this. I'm happy to remove it, it doesn't affect the Waybar logs, but this will cause builds against the RC versions in distros to fail until they update to the 0.5.0 release. Probably better to track a released API version, but let me know.

Will rebase and squash if people are otherwise happy.

[EDIT]: Edited description of 0.5.0 RC logging requirement changes for sense.

[Arnavion]

Compatibility with 0.4.9x is not a requirement for OpenSUSE at least.

[FilippoBonazziSUSE]

Secondly, the 0.5.0 release (so not the 0.4.81-0.4.90 RCs) needs WP_LOCAL_LOG_TOPIC defined, so I did, however the 0.5.0 release removes this.

Probably something wrong in this sentence. Anyway releasing waybar against wireplumber 0.5.0 and not some arbitrary old RCs sounds fine to me. Distros that were quick to update to 0.4.8x and RC versions of wireplumber will be quick to update to 0.5.0 (e.g. openSUSE has almost merged 0.5.0).

[tokyovigilante]

Probably something wrong in this sentence.

Fixed, thanks.

Anyway releasing waybar against wireplumber 0.5.0 and not some arbitrary old RCs sounds fine to me. Distros that were quick to update to 0.4.8x and RC versions of wireplumber will be quick to update to 0.5.0

Makes sense, Fedora rawhide has already updated to 0.5.0, and Fedora 40 has it in testing (0.4.81 was released). 39 and earlier are still on 0.4.17.

[colemickens]

Since I'm (edit: very, very) eager to see this merge, I thought I'd mention to OP that the formatting tests are still failing (maybe didn't re-run, idk?)

[tokyovigilante]

Since I'm (edit: very, very) eager to see this merge, I thought I'd mention to OP that the formatting tests are still failing (maybe didn't re-run, idk?)

I was waiting for a couple more ACKs but have squashed, rebased and repushed to trigger a recheck. All linter warnings addressed AFAICT.

clang-format is failing because of an issue in the dwl module (I note all the PRs are currently failing, presumably for this reason).

--- ./src/modules/dwl/tags.cpp	(original)
+++ ./src/modules/dwl/tags.cpp	(reformatted)
@@ -155,7 +155,7 @@
 }
 
 Tags::~Tags() {
-  if(output_status_) {
+  if (output_status_) {
     zdwl_ipc_output_v2_destroy(output_status_);
   }
   if (status_manager_) {

clang-tidy says:

  Error: 'wp/wp.h' file not found
  Warning: method 'onDefaultNodesApiLoaded' can be made static
  Warning: method 'onMixerApiLoaded' can be made static
  INFO:CPP Linter:0 clang-format-checks-failed
  INFO:CPP Linter:3 clang-tidy-checks-failed
  INFO:CPP Linter:3 checks-failed

Clearly the missing include is a red herring, and the callbacks already are static? So no idea there.

 static void onDefaultNodesApiLoaded(WpObject* p, GAsyncResult* res,
                                     waybar::modules::Wireplumber* self);
 static void onMixerApiLoaded(WpObject* p, GAsyncResult* res, waybar::modules::Wireplumber* self);

Anyway, works for me and I don't see any reason not to merge based on feedback received so far.

Ironically I actually need to build with GCC (on Fedora rawhide) rather than Clang due to a linker failure in the backlight module.

[Arnavion]

The clang-tidy error is because it runs in a Debian container (alexays/waybar:debian) that installs libwireplumber-0.4-dev

In fact, even the linux/build CI that passes on this PR is no longer testing the wireplumber module at all, because all those images have the 0.4 package installed.

Run-time dependency wireplumber-0.5 found: NO (tried pkgconfig and cmake)

[staticssleever668]

Ironically I actually need to build with GCC (on Fedora rawhide) rather than Clang due to a linker failure in the backlight module.

Perhaps #2638 could help with that. :)

[Alexays]

Thanks, next Waybar version will so not be backward compatible with older version of wireplumber.

[tokyovigilante]

Thanks all! @staticssleever668 yup able to build master with clang now.