Merge pull request #84 from FedeDP/pipewire_support

Pipewire support + some global fixes

.build.yml

@@ -13,18 +13,20 @@ packages:
   - libjpeg-turbo
   - wayland
   - libdrm
+  - pipewire
 sources:
   - https://github.com/FedeDP/Clightd
 tasks:
   - prepare: |
       cd Clightd
-      mkdir build build-no-gamma build-no-dpms build-no-ddc build-no-screen build-no-yoctolight build-no-extras
-      (cd build && cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_GAMMA=1 -DENABLE_DPMS=1 -DENABLE_DDC=1 -DENABLE_SCREEN=1 -DENABLE_YOCTOLIGHT=1 ..)
-      (cd build-no-gamma && cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_DPMS=1 -DENABLE_DDC=1 -DENABLE_SCREEN=1 -DENABLE_YOCTOLIGHT=1 ..)
-      (cd build-no-dpms && cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_GAMMA=1 -DENABLE_DDC=1 -DENABLE_SCREEN=1 -DENABLE_YOCTOLIGHT=1 ..)
-      (cd build-no-ddc && cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_GAMMA=1 -DENABLE_DPMS=1 -DENABLE_SCREEN=1 -DENABLE_YOCTOLIGHT=1 ..)
-      (cd build-no-screen && cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_GAMMA=1 -DENABLE_DPMS=1 -DENABLE_DDC=1 -DENABLE_YOCTOLIGHT=1 ..)
-      (cd build-no-yoctolight && cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_GAMMA=1 -DENABLE_DPMS=1 -DENABLE_DDC=1 -DENABLE_SCREEN=1 ..)
+      mkdir build build-no-gamma build-no-dpms build-no-ddc build-no-screen build-no-yoctolight build-no-pipewire build-no-extras
+      (cd build && cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_GAMMA=1 -DENABLE_DPMS=1 -DENABLE_DDC=1 -DENABLE_SCREEN=1 -DENABLE_YOCTOLIGHT=1 -DENABLE_PIPEWIRE=1 ..)
+      (cd build-no-gamma && cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_DPMS=1 -DENABLE_DDC=1 -DENABLE_SCREEN=1 -DENABLE_YOCTOLIGHT=1 -DENABLE_PIPEWIRE=1 ..)
+      (cd build-no-dpms && cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_GAMMA=1 -DENABLE_DDC=1 -DENABLE_SCREEN=1 -DENABLE_YOCTOLIGHT=1 -DENABLE_PIPEWIRE=1 ..)
+      (cd build-no-ddc && cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_GAMMA=1 -DENABLE_DPMS=1 -DENABLE_SCREEN=1 -DENABLE_YOCTOLIGHT=1 -DENABLE_PIPEWIRE=1 ..)
+      (cd build-no-screen && cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_GAMMA=1 -DENABLE_DPMS=1 -DENABLE_DDC=1 -DENABLE_YOCTOLIGHT=1 -DENABLE_PIPEWIRE=1 ..)
+      (cd build-no-yoctolight && cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_GAMMA=1 -DENABLE_DPMS=1 -DENABLE_DDC=1 -DENABLE_SCREEN=1 -DENABLE_PIPEWIRE=1 ..)
+      (cd build-no-pipewire && cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_GAMMA=1 -DENABLE_DPMS=1 -DENABLE_DDC=1 -DENABLE_SCREEN=1 -DENABLE_YOCTOLIGHT=1 ..)
       (cd build-no-extras && cmake -DCMAKE_BUILD_TYPE=Debug ..)
   - build: |
       cd Clightd
@@ -34,6 +36,7 @@ tasks:
       (cd build-no-ddc && make)
       (cd build-no-screen && make)
       (cd build-no-yoctolight && make)
+      (cd build-no-pipewire && make)
       (cd build-no-extras && make)
 triggers:
   - action: email

CMakeLists.txt

@@ -99,6 +99,7 @@ optional_dep(DPMS "x11;xext;libdrm;wayland-client" "DPMS" protocol/org_kde_kwin_
 optional_dep(SCREEN "x11" "screen emitted brightness" protocol/wlr-screencopy-unstable-v1.xml src/modules/screen_plugins)
 optional_dep(DDC "ddcutil>=0.9.5" "external monitor backlight")
 optional_dep(YOCTOLIGHT "libusb-1.0" "Yoctolight usb als devices support")
+optional_dep(PIPEWIRE "libpipewire-0.3" "Enable pipewire camera sensor support")
 
 # Convert ld flag list from list to space separated string.
 string(REPLACE ";" " " COMBINED_LDFLAGS "${COMBINED_LDFLAGS}")
@@ -111,7 +112,7 @@ set_target_properties(
 
 # Installation of targets (must be before file configuration to work)
 install(TARGETS ${PROJECT_NAME}
-        RUNTIME DESTINATION "${CMAKE_INSTALL_LIBEXECDIR}")
+        RUNTIME DESTINATION "${CMAKE_INSTALL_FULL_LIBEXECDIR}")
 
 set(SCRIPT_DIR "${CMAKE_CURRENT_SOURCE_DIR}/Scripts")
 
@@ -142,6 +143,8 @@ if(SYSTEMD_BASE_FOUND)
 
     install(FILES ${CMAKE_CURRENT_BINARY_DIR}/clightd.service
         DESTINATION ${SYSTEMD_SERVICE_DIR})
+
+    set(SYSTEMD_SERVICE "SystemdService=clightd.service")
 endif()
 
 # Install dbus service

Scripts/clightd.service

@@ -7,8 +7,14 @@ Requires=@[email protected]
 Type=dbus
 BusName=org.clightd.clightd
 User=root
-# Default backlight vcp code; update if needed
+# Default backlight vcp code; update if needed.
+# Moreover, you can also specify per-monitor BL VCP value,
+# using CLIGHTD_BL_VCP_$mon_id, where mon_id is the monitor identifier 
+# as seen by Clightd; you can explore them using: 
+# $ busctl call org.clightd.clightd /org/clightd/clightd/Backlight2 org.clightd.clightd.Backlight2 Get
 Environment=CLIGHTD_BL_VCP=0x10
+# Default pipewire runtime dir watched by Clightd
+Environment=CLIGHTD_PIPEWIRE_RUNTIME_DIR=/run/user/1000/
 ExecStart=@CMAKE_INSTALL_FULL_LIBEXECDIR@/clightd
 Restart=on-failure
 RestartSec=5

Scripts/org.clightd.clightd.service

@@ -2,4 +2,4 @@
 Name=org.clightd.clightd
 Exec=@CMAKE_INSTALL_FULL_LIBEXECDIR@/clightd
 User=root
-SystemdService=clightd.service
+@SYSTEMD_SERVICE@

TODO.md

@@ -30,22 +30,59 @@
 - [x] Drop bl_store_vpcode() and only load vpcode from CLIGHTD_BL_VCP env?
 - [x] Add CLIGHTD_BL_VCP Environment variable to systemd script with a comment thus it is simple to update it if needed
 - [x] Expose Max and Internal properties
-- [ ] Update dbus api wiki
+- [x] Update dbus api wiki
 - [x] add a page about monitor hotplugging (dep on ddcutil >= 1.2.0 and refresh time!)
 - [x] Investigate memleaks (related to ddca_redetect_displays()?) -> see here: https://github.com/rockowitz/ddcutil/issues/202
 - [x] Instead of 30s sleep, use an udev_monitor on drm subsystem?
+- [x] Add support for monitor id specific CLIGHTD_BL_VCP env
 
 ### KbdBacklight
 - [x] call sd_bus_emit_object_added() sd_bus_emit_object_removed() When object path are created/deleted
 - [x] Fix: udev_reference is a snapshot of an udev device at a current time. Wrong!
+- [x] Fixed (small) memleak
+
+### ALS
+- [x] Fix: avoid using cached udev_dev reference in loop (thus always returning same ambient brightness read during a Capture request)
+- [x] Fixed EIO errors
+
+### Sensor
+- [x] Only emit Sensor.Changed signal for added/removed devices
+
+### Pipewire
+- [x] Support pipewire for Camera sensor? This would allow multiple application sharing camera
+- [x] Pipewire run as root needs XDG_RUNTIME_DIR env -> workaround: find the first /run/user folder and use that
+- [x] Unify camera settings between camera and pipewire sensors... ?
+- [x] Support monitor sensor api for pipewire
+- [x] Fix segfault
+- [x] Fix subsequent Capture
+- [x] Add a CLIGHTD_PW_RUNTIME_DIR env variable (in clightd.service, see CLIGHTD_BL_CODE) that defaults to /run/user/1000/. If the env variable is empty -> disable pipewire. If folder does not exist: disable pipewire. Otherwise: inotify on folder to wait for socket to appear. If socket is already there, immediately start monitoring.
+- [x] Document the new env variable!
+- [x] Use caller uid instead of defaulting to first found user during Capture!
+- [x] Use a map to store list of nodes?
+- [x] Free list of nodes upon exit!
+- [x] Fix xdg_runtime_dir set to create monitor
+- [x] Fix memleaks
+- [x] Support crop settings
+- [x] Test crop
+- [x] Drop crop API support for both pipewire and webcam; they add lots of complexity while giving no real perf improvements considering we are using small frames
+- [x] Fix: pipewire capture while webcam is already owned by another app freezes during pw_loop_iterate()
 
 ### Generic
 - [x] When built with ddcutil, clightd.service should be started after systemd-modules-load.service
 - [x] Show commit hash in version
+- [x] All api that require eg Xauth or xdg rutime user, fallback at automatically fetching a default value given the caller:
+- [x] test X
+- [x] test wl
+- [x] Document the new behavior!
+- [x] Fix clightd not cleanly exiting when built with DDC or YOCTOLIGHT (most probably libusb or whatever is creating another thread that is stealing the signal!)
+- [x] do not use strlen() to only check that strign is not empty
 
 ## 5.x
 - [ ] Keep it up to date with possible ddcutil api changes
 
+### Pipewire
+- [ ] Fix set_camera_setting() impl -> how to get current value? how to set a new value?
+
 ## 6.x (api break release)
 
 ### Generic
@@ -54,9 +91,15 @@
 - [ ] Drop old BACKLIGHT module -> in case, drop {Lower,Raise,Set}All from clightd polkit policy
 - [ ] Rename Backlight2 to Backlight
 
-### Pipewire
-- [ ] merge pipewire work
+### Move to user service (?)
 - [ ] move clightd to user service
+- [ ] Drop polkit and use sd_session_is_active() 
+- [ ] Add udev rules for yoctolight, als, keyboard and backlight modules for "clightd" 
+- [ ] Drop useless API params (eg: DISPLAY, XAUTHORITY, XDG_RUNTIME_DIR etc etc)
+- [ ] # groupadd clightd
+- [ ] # usermod -aG clightd myusername
+- [ ] # echo 'KERNEL=="i2c-[0-9]*", GROUP="clightd"' >> /etc/udev/rules.d/10-local_i2c_group.rules
+... would break https://github.com/FedeDP/Clight/issues/144 ...
 
 ## Ideas
 - [ ] follow ddcci kernel driver and in case, drop ddcutil and add the kernel driver as clightd opt-dep

src/commons.h

@@ -20,8 +20,9 @@
 #include <module/module_easy.h>
 
 #define SIZE(x) (sizeof(x) / sizeof(*x))
-#define _ctor_     __attribute__((constructor (101))) // Used for Sensors registering
-#define _dtor_     __attribute__((destructor (101)))  // Used for libusb dtor
+
+#define _ctor_     __attribute__((constructor (101))) // Used for plugins registering (sensor, gamma, dpms, screen) and libusb/libpipewire init
+#define _dtor_     __attribute__((destructor (101)))  // Used for libusb and libpipewire dtor
 
 /* Used by dpms, gamma and screen*/
 #define UNSUPPORTED             INT_MIN 

src/modules/backlight2.c

@@ -6,6 +6,7 @@
 
 #define BL_SUBSYSTEM        "backlight"
 #define DRM_SUBSYSTEM       "drm"
+#define BL_VCP_ENV          "CLIGHTD_BL_VCP"
 
 typedef struct {
     double target_pct;
@@ -104,14 +105,14 @@ MODULE("BACKLIGHT2");
     static DDCA_Vcp_Feature_Code br_code = 0x10;
 
     static void bl_load_vpcode(void) {
-        if (getenv("CLIGHTD_BL_VCP")) {
-            br_code = strtol(getenv("CLIGHTD_BL_VCP"), NULL, 16);
-            m_log("Set 0x%x vcp code.\n", br_code);
+        if (getenv(BL_VCP_ENV)) {
+            br_code = strtol(getenv(BL_VCP_ENV), NULL, 16);
+            m_log("Set default 0x%x vcp code.\n", br_code);
         }
     }
 
     static void get_info_id(char *id, const int size, const DDCA_Display_Info *dinfo) {
-        if (!strlen(dinfo->sn) || !strcasecmp(dinfo->sn, "Unspecified")) {
+        if ((dinfo->sn[0] == '\0') || !strcasecmp(dinfo->sn, "Unspecified")) {
             switch(dinfo->path.io_mode) {
                 case DDCA_IO_I2C:
                     snprintf(id, size, "/dev/i2c-%d", dinfo->path.path.i2c_busno);
@@ -423,9 +424,17 @@ static int set_external_backlight(bl_t *bl, int value) {
 #ifdef DDC_PRESENT
     DDCA_Display_Handle dh = NULL;
     if (!ddca_open_display2(bl->dev, false, &dh)) {
+        DDCA_Vcp_Feature_Code specific_br_code;
+        char specific_br_env[64];
+        snprintf(specific_br_env, sizeof(specific_br_env), BL_VCP_ENV"_%s", bl->sn);
+        if (getenv(specific_br_env)) {
+            specific_br_code = strtol(getenv(specific_br_env), NULL, 16);
+        } else {
+            specific_br_code = br_code;
+        }
         int8_t new_sh = (value >> 8) & 0xff;
         int8_t new_sl = value & 0xff;
-        ret = ddca_set_non_table_vcp_value(dh, br_code, new_sh, new_sl);
+        ret = ddca_set_non_table_vcp_value(dh, specific_br_code, new_sh, new_sl);
         ddca_close_display(dh);
     }
 #endif

src/modules/dpms.c

@@ -2,6 +2,7 @@
 
 #include "dpms.h"
 #include "polkit.h"
+#include "bus_utils.h"
 
 static int method_getdpms(sd_bus_message *m, void *userdata, sd_bus_error *ret_error);
 static int method_setdpms(sd_bus_message *m, void *userdata, sd_bus_error *ret_error);
@@ -94,6 +95,8 @@ static int method_getdpms(sd_bus_message *m, void *userdata, sd_bus_error *ret_e
         return r;
     }
 
+    bus_sender_fill_creds(m); // used by PW plugin
+
     /*
      * Note: this is freed by drm plugin if it is an empty string
      * to get a default drm device.
@@ -148,6 +151,8 @@ static int method_setdpms(sd_bus_message *m, void *userdata, sd_bus_error *ret_e
         return -EINVAL;
     }
 
+    bus_sender_fill_creds(m); // used by PW plugin
+
     /*
      * Note: this is freed by drm plugin if it is an empty string
      * to get a default drm device.

src/modules/dpms_plugins/xorg.c

@@ -1,6 +1,7 @@
-#include <X11/Xlib.h>
+#include "xorg_utils.h"
 #include <X11/extensions/dpms.h>
 #include "dpms.h"
+#include "bus_utils.h"
 
 DPMS("Xorg");
 
@@ -17,9 +18,7 @@ static int get(const char **display, const char *xauthority) {
     CARD16 s;
     int ret = WRONG_PLUGIN;
 
-    setenv("XAUTHORITY", xauthority, 1);
-
-    Display *dpy = XOpenDisplay(*display);
+    Display *dpy = fetch_xorg_display(display, xauthority);
     if (dpy) {
         if (DPMSCapable(dpy)) {
             DPMSInfo(dpy, &s, &onoff);
@@ -28,19 +27,14 @@ static int get(const char **display, const char *xauthority) {
             ret = UNSUPPORTED;
         }
         XCloseDisplay(dpy);
-    }
-
-    unsetenv("XAUTHORITY");
+    }    
     return ret;
 }
 
 static int set(const char **display, const char *xauthority, int dpms_level) {
     int ret = WRONG_PLUGIN;
 
-    /* set xauthority cookie */
-    setenv("XAUTHORITY", xauthority, 1);
-
-    Display *dpy = XOpenDisplay(*display);
+    Display *dpy = fetch_xorg_display(display, xauthority);
     if (dpy) {
         if (DPMSCapable(dpy)) {
             DPMSEnable(dpy);
@@ -52,8 +46,5 @@ static int set(const char **display, const char *xauthority, int dpms_level) {
         }
         XCloseDisplay(dpy);
     }
-
-    /* Drop xauthority cookie */
-    unsetenv("XAUTHORITY");
     return ret;
 }

src/modules/gamma.c

@@ -10,6 +10,7 @@
 #include <module/map.h>
 #include <math.h>
 #include "gamma.h"
+#include "bus_utils.h"
 
 static unsigned short get_red(int temp);
 static unsigned short get_green(int temp);
@@ -269,6 +270,8 @@ static int method_setgamma(sd_bus_message *m, void *userdata, sd_bus_error *ret_
     if (temp < 1000 || temp > 10000) {
         error = EINVAL;
     } else {
+        bus_sender_fill_creds(m);
+
         gamma_client *sc = map_get(clients, display);
         if (!sc) {
             sc = fetch_client(userdata, display, env, &error);
@@ -311,6 +314,8 @@ static int method_getgamma(sd_bus_message *m, void *userdata, sd_bus_error *ret_
         return r;
     }
 
+    bus_sender_fill_creds(m); // used by PW plugin
+
     gamma_client *cl = map_get(clients, display);
     if (cl) {
         temp = cl->current_temp;

src/modules/gamma_plugins/xorg.c

@@ -1,5 +1,7 @@
 #include "gamma.h"
 #include <commons.h>
+#include "bus_utils.h"
+#include "xorg_utils.h"
 #include <X11/extensions/Xrandr.h>
 
 typedef struct {
@@ -12,9 +14,7 @@ GAMMA("Xorg");
 static int validate(const char **id, const char *env, void **priv_data) {
     int ret = WRONG_PLUGIN;
 
-    setenv("XAUTHORITY", env, 1);
-
-    Display *dpy = XOpenDisplay(*id);
+    Display *dpy = fetch_xorg_display(id, env);
     if (dpy) {
         int screen = DefaultScreen(dpy);
         Window root = RootWindow(dpy, screen);
@@ -30,8 +30,6 @@ static int validate(const char **id, const char *env, void **priv_data) {
             XCloseDisplay(dpy);
         }
     }
-
-    unsetenv("XAUTHORITY");
     return ret;
 }
 

src/modules/keyboard.c

@@ -72,7 +72,7 @@ static bool evaluate(void) {
 }
 
 static void init(void) {
-    kbds = map_new(true, dtor_kbd);
+    kbds = map_new(false, dtor_kbd);
     sd_bus_add_object_manager(bus, NULL, object_path);
     int r = sd_bus_add_object_vtable(bus,
                                  NULL,
@@ -141,6 +141,7 @@ static void destroy(void) {
 static void dtor_kbd(void *data) {
     kbd_t *k = (kbd_t *)data;
     sd_bus_slot_unref(k->slot);
+    free((void *)k->sysname);
     free(k);
 }
 

src/modules/screen.c

@@ -1,6 +1,7 @@
 #ifdef SCREEN_PRESENT
 
 #include "screen.h"
+#include "bus_utils.h"
 
 #define MONITOR_ILL_MAX              255
 
@@ -117,6 +118,8 @@ static int method_getbrightness(sd_bus_message* m, void* userdata, sd_bus_error*
         return r;
     }
 
+    bus_sender_fill_creds(m);
+
     screen_plugin *plugin = userdata;
     int br = WRONG_PLUGIN;
     if (!plugin) {

src/modules/screen_plugins/fb.c

@@ -15,7 +15,7 @@ static int get_frame_brightness(const char *id, const char *env) {
     int ret = WRONG_PLUGIN;
 
     struct udev_device *dev = NULL;
-    if (!id || !strlen(id)) {
+    if (id == NULL || id[0] == '\0') {
         /* Fetch first matching device from udev */
         get_udev_device(NULL, FB_SUBSYSTEM, NULL, NULL, &dev);
         if (!dev) {