From 1ad3fdff8a377a6e10532a8b0530765a4b472333 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Danis?= Date: Mon, 11 Aug 2025 16:36:01 +0200 Subject: [PATCH 01/34] bluez5: backend-native: Free command list queue on RFComm free When RFComm conection is closed or lost, the pending commands should be freed, and if a DBus message is attached an error reply should be sent. --- spa/plugins/bluez5/backend-native.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/spa/plugins/bluez5/backend-native.c b/spa/plugins/bluez5/backend-native.c index af62887a3..d4b5beef6 100644 --- a/spa/plugins/bluez5/backend-native.c +++ b/spa/plugins/bluez5/backend-native.c @@ -411,12 +411,23 @@ static void volume_sync_stop_timer(struct rfcomm *rfcomm); static void rfcomm_free(struct rfcomm *rfcomm) { struct updated_call *updated_call; + struct rfcomm_cmd *cmd; spa_list_consume(updated_call, &rfcomm->updated_call_list, link) { spa_list_remove(&updated_call->link); free(updated_call); } + spa_list_consume(cmd, &rfcomm->cmd_send_queue, link) { + if (cmd->msg) { + telephony_send_dbus_method_reply(rfcomm->backend->telephony, cmd->msg, BT_TELEPHONY_ERROR_FAILED, 0); + spa_clear_ptr(cmd->msg, dbus_message_unref); + } + + spa_list_remove(&cmd->link); + free(cmd); + } + codec_switch_stop_timer(rfcomm); if (rfcomm->telephony_ag) { telephony_ag_destroy(rfcomm->telephony_ag); From a50b66651e360241ef43540ecddee0cc76a3edb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Danis?= Date: Fri, 8 Aug 2025 16:26:25 +0200 Subject: [PATCH 02/34] bluez5: backend-native: Add log for call state changes This also to track each call state changes. --- spa/plugins/bluez5/backend-native.c | 30 +++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/spa/plugins/bluez5/backend-native.c b/spa/plugins/bluez5/backend-native.c index d4b5beef6..044ad5bd6 100644 --- a/spa/plugins/bluez5/backend-native.c +++ b/spa/plugins/bluez5/backend-native.c @@ -1911,6 +1911,12 @@ static const struct spa_bt_telephony_ag_callbacks telephony_ag_callbacks = { .set_microphone_volume = hfp_hf_set_microphone_volume, }; +#define hfp_hf_set_call_state(log, obj, new_state) \ +({ \ + spa_log_debug(log, "call id: %u, %u -> %u", obj->id, obj->state, new_state); \ + obj->state = new_state; \ +}) + static void hfp_hf_remove_disconnected_calls(struct rfcomm *rfcomm) { struct impl *backend = rfcomm->backend; @@ -1930,7 +1936,7 @@ static void hfp_hf_remove_disconnected_calls(struct rfcomm *rfcomm) spa_log_debug(backend->log, "call %d -> %s", call->id, found ? "updated" : "disconnected"); if (!found) { - call->state = CALL_STATE_DISCONNECTED; + hfp_hf_set_call_state(backend->log, call, CALL_STATE_DISCONNECTED); telephony_call_notify_updated_props(call); telephony_call_destroy(call); } @@ -2057,7 +2063,7 @@ static bool rfcomm_hfp_hf(struct rfcomm *rfcomm, char* token) spa_list_for_each_safe(call, tcall, &rfcomm->telephony_ag->call_list, link) { if (call->state == CALL_STATE_DIALING || call->state == CALL_STATE_ALERTING || call->state == CALL_STATE_INCOMING || call->state == CALL_STATE_WAITING) { - call->state = CALL_STATE_DISCONNECTED; + hfp_hf_set_call_state(backend->log, call, CALL_STATE_DISCONNECTED); telephony_call_notify_updated_props(call); telephony_call_destroy(call); } @@ -2101,7 +2107,7 @@ static bool rfcomm_hfp_hf(struct rfcomm *rfcomm, char* token) struct spa_bt_telephony_call *call; spa_list_for_each(call, &rfcomm->telephony_ag->call_list, link) { if (call->state == CALL_STATE_DIALING) { - call->state = CALL_STATE_ALERTING; + hfp_hf_set_call_state(backend->log, call, CALL_STATE_ALERTING); telephony_call_notify_updated_props(call); } } @@ -2118,7 +2124,7 @@ static bool rfcomm_hfp_hf(struct rfcomm *rfcomm, char* token) struct spa_bt_telephony_call *call, *tcall; spa_list_for_each_safe(call, tcall, &rfcomm->telephony_ag->call_list, link) { if (call->state == CALL_STATE_ACTIVE) { - call->state = CALL_STATE_DISCONNECTED; + hfp_hf_set_call_state(backend->log, call, CALL_STATE_DISCONNECTED); telephony_call_notify_updated_props(call); telephony_call_destroy(call); } @@ -2128,7 +2134,7 @@ static bool rfcomm_hfp_hf(struct rfcomm *rfcomm, char* token) spa_list_for_each(call, &rfcomm->telephony_ag->call_list, link) { if (call->state == CALL_STATE_DIALING || call->state == CALL_STATE_ALERTING || call->state == CALL_STATE_INCOMING) { - call->state = CALL_STATE_ACTIVE; + hfp_hf_set_call_state(backend->log, call, CALL_STATE_ACTIVE); telephony_call_notify_updated_props(call); } } @@ -2146,7 +2152,7 @@ static bool rfcomm_hfp_hf(struct rfcomm *rfcomm, char* token) bool found_waiting = false; spa_list_for_each_safe(call, tcall, &rfcomm->telephony_ag->call_list, link) { if (call->state == CALL_STATE_WAITING) { - call->state = CALL_STATE_DISCONNECTED; + hfp_hf_set_call_state(backend->log, call, CALL_STATE_DISCONNECTED); telephony_call_notify_updated_props(call); telephony_call_destroy(call); found_waiting = true; @@ -2156,7 +2162,7 @@ static bool rfcomm_hfp_hf(struct rfcomm *rfcomm, char* token) if (!found_waiting) { spa_list_for_each_safe(call, tcall, &rfcomm->telephony_ag->call_list, link) { if (call->state == CALL_STATE_HELD) { - call->state = CALL_STATE_DISCONNECTED; + hfp_hf_set_call_state(backend->log, call, CALL_STATE_DISCONNECTED); telephony_call_notify_updated_props(call); telephony_call_destroy(call); } @@ -2167,10 +2173,10 @@ static bool rfcomm_hfp_hf(struct rfcomm *rfcomm, char* token) spa_list_for_each(call, &rfcomm->telephony_ag->call_list, link) { bool changed = false; if (call->state == CALL_STATE_ACTIVE) { - call->state = CALL_STATE_HELD; + hfp_hf_set_call_state(backend->log, call, CALL_STATE_HELD); changed = true; } else if (call->state == CALL_STATE_HELD) { - call->state = CALL_STATE_ACTIVE; + hfp_hf_set_call_state(backend->log, call, CALL_STATE_ACTIVE); changed = true; } @@ -2182,7 +2188,7 @@ static bool rfcomm_hfp_hf(struct rfcomm *rfcomm, char* token) spa_list_for_each(call, &rfcomm->telephony_ag->call_list, link) { bool changed = false; if (call->state == CALL_STATE_ACTIVE || call->state == CALL_STATE_WAITING) { - call->state = CALL_STATE_HELD; + hfp_hf_set_call_state(backend->log, call, CALL_STATE_HELD); changed = true; } @@ -2431,14 +2437,14 @@ static bool rfcomm_hfp_hf(struct rfcomm *rfcomm, char* token) struct spa_bt_telephony_call *call, *tcall; spa_list_for_each_safe(call, tcall, &rfcomm->telephony_ag->call_list, link) { if (call->state == CALL_STATE_ACTIVE) { - call->state = CALL_STATE_DISCONNECTED; + hfp_hf_set_call_state(backend->log, call, CALL_STATE_DISCONNECTED); telephony_call_notify_updated_props(call); telephony_call_destroy(call); } } spa_list_for_each(call, &rfcomm->telephony_ag->call_list, link) { if (call->state == CALL_STATE_HELD) { - call->state = CALL_STATE_ACTIVE; + hfp_hf_set_call_state(backend->log, call, CALL_STATE_ACTIVE); telephony_call_notify_updated_props(call); } } From 0d0108e954124a302e959cb0f2d2200aa12319bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Tue, 22 Jul 2025 00:17:43 +0200 Subject: [PATCH 03/34] test: spa-utils: utils_snprintf_abort_neg_size: remove valgrind check Death tests are always skipped by the framework when running on valgrind, so there is no need for the check. --- test/test-spa-utils.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/test-spa-utils.c b/test/test-spa-utils.c index 06b8f8d17..43c2ffa2c 100644 --- a/test/test-spa-utils.c +++ b/test/test-spa-utils.c @@ -853,9 +853,6 @@ PWTEST(utils_snprintf_abort_neg_size) size_t size = pwtest_get_iteration(current_test); char dest[8]; - if (RUNNING_ON_VALGRIND) - return PWTEST_SKIP; - spa_scnprintf(dest, size, "1234"); /* expected to abort() */ return PWTEST_FAIL; From fcb318b9c5158ab66ca1d5797b197237eae7d5c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Wed, 23 Jul 2025 18:18:52 +0200 Subject: [PATCH 04/34] pwtest: is_debugger_attached(): rework test Check if the "TracerPid" field in `/proc/self/status` is non-zero. This does not need libcap, and does not depend on capabilities, and it also works under qemu user emulation. --- meson.build | 3 --- test/meson.build | 1 - test/pwtest.c | 43 +++++++++++-------------------------------- 3 files changed, 11 insertions(+), 36 deletions(-) diff --git a/meson.build b/meson.build index 307f1155c..0a01a8f37 100644 --- a/meson.build +++ b/meson.build @@ -381,9 +381,6 @@ libusb_dep = dependency('libusb-1.0', required : get_option('libusb')) summary({'libusb (Bluetooth quirks)': libusb_dep.found()}, bool_yn: true, section: 'Backend') cdata.set('HAVE_LIBUSB', libusb_dep.found()) -cap_lib = dependency('libcap', required : false) -cdata.set('HAVE_LIBCAP', cap_lib.found()) - glib2_dep = dependency('glib-2.0', required : get_option('flatpak')) summary({'GLib-2.0 (Flatpak support)': glib2_dep.found()}, bool_yn: true, section: 'Misc dependencies') flatpak_support = glib2_dep.found() diff --git a/test/meson.build b/test/meson.build index b4f24cf1b..c5671149d 100644 --- a/test/meson.build +++ b/test/meson.build @@ -9,7 +9,6 @@ pwtest_deps = [ pipewire_dep, mathlib, dl_lib, - cap_lib, epoll_shim_dep ] diff --git a/test/pwtest.c b/test/pwtest.c index 7094a59e1..89a385f53 100644 --- a/test/pwtest.c +++ b/test/pwtest.c @@ -19,9 +19,6 @@ #ifdef HAVE_PIDFD_OPEN #include #endif -#ifdef HAVE_LIBCAP -#include -#endif #include #include #include @@ -33,6 +30,7 @@ #include #include "spa/utils/ansi.h" +#include "spa/utils/cleanup.h" #include "spa/utils/string.h" #include "spa/utils/defs.h" #include "spa/utils/list.h" @@ -1298,39 +1296,20 @@ static void list_tests(struct pwtest_context *ctx) static bool is_debugger_attached(void) { - bool rc = false; -#ifdef HAVE_LIBCAP - int status; - int pid = fork(); + spa_autofree char *line = NULL; + size_t length = 0; - if (pid == -1) - return 0; + spa_autoptr(FILE) f = fopen("/proc/self/status", "re"); + if (!f) + return false; - if (pid == 0) { - int ppid = getppid(); - cap_t caps = cap_get_pid(ppid); - cap_flag_value_t cap_val; - - if (cap_get_flag(caps, CAP_SYS_PTRACE, CAP_EFFECTIVE, &cap_val) == -1 || - cap_val != CAP_SET) - _exit(false); - - if (ptrace(PTRACE_ATTACH, ppid, NULL, 0) == 0) { - waitpid(ppid, NULL, 0); - ptrace(PTRACE_CONT, ppid, NULL, 0); - ptrace(PTRACE_DETACH, ppid, NULL, 0); - rc = false; - } else { - rc = true; - } - _exit(rc); - } else { - waitpid(pid, &status, 0); - rc = WEXITSTATUS(status); + while (getline(&line, &length, f) >= 0) { + unsigned int tracer_pid; + if (sscanf(line, "TracerPid: %u", &tracer_pid) == 1) + return tracer_pid > 0; } -#endif - return !!rc; + return false; } static void usage(FILE *fp, const char *progname) From 9438df8d30b917c54e2250ef2e53d2ab711f34b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Tue, 15 Jul 2025 19:16:28 +0200 Subject: [PATCH 05/34] Revert "ci: Add an x86 build" This reverts commit 2042a0483bed607c10244fe605b668239203b694. A new approach is added in subsequent commits based on debian for more architectures. --- .gitlab-ci.yml | 63 ++------------------------------------------------ cross-x86.txt | 23 ------------------ 2 files changed, 2 insertions(+), 84 deletions(-) delete mode 100644 cross-x86.txt diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 0c286ccf1..e381b7c64 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -102,36 +102,6 @@ include: # FDO_DISTRIBUTION_EXEC: >- # pip3 install meson -# This is a pruned down container with enough dependencies for a basic i686 -# build to make sure we've not broken anything. This can be extended if we want -# to cover more of the code. -.fedora_x86: - variables: - # Update this tag when you want to trigger a rebuild - FDO_DISTRIBUTION_TAG: '2025-05-29.1' - FDO_DISTRIBUTION_VERSION: '42' - FDO_DISTRIBUTION_PACKAGES: >- - git - gcc - gcc-c++ - meson - glibc-devel.i686 - systemd-devel.i686 - dbus-devel.i686 - alsa-lib-devel.i686 - bluez-libs-devel.i686 - libffi-devel.i686 - pcre2-devel.i686 - sysprof-devel.i686 - zlib-ng-compat-devel.i686 - libblkid-devel.i686 - libmount-devel.i686 - libselinux-devel.i686 - glib2-devel.i686 - alsa-lib-devel - avahi-devel - bluez-libs-devel - .ubuntu: variables: # Update this tag when you want to trigger a rebuild @@ -243,14 +213,8 @@ include: - echo "Building with meson options $MESON_OPTIONS" - meson setup "$BUILD_DIR" --prefix="$PREFIX" $MESON_OPTIONS - meson compile -C "$BUILD_DIR" $COMPILE_ARGS - - | - if [ -z "$MESON_SKIP_TEST" ]; then - meson test -C "$BUILD_DIR" --no-rebuild - fi - - | - if [ -z "$MESON_SKIP_INSTALL" ]; then - meson install -C "$BUILD_DIR" --no-rebuild - fi + - meson test -C "$BUILD_DIR" --no-rebuild + - meson install -C "$BUILD_DIR" --no-rebuild artifacts: name: pipewire-$CI_COMMIT_SHA when: always @@ -273,14 +237,6 @@ container_fedora: variables: GIT_STRATEGY: none # no need to pull the whole tree for rebuilding the image -container_fedora_x86: - extends: - - .fedora_x86 - - .fdo.container-build@fedora - stage: container - variables: - GIT_STRATEGY: none # no need to pull the whole tree for rebuilding the image - container_alpine: extends: - .alpine @@ -406,21 +362,6 @@ build_on_fedora_html_docs: rules: - !reference [pages, rules] -build_on_fedora_x86: - extends: - - .fedora_x86 - - .not_coverity - - .fdo.distribution-image@fedora - - .build - stage: build - needs: - - job: container_fedora_x86 - artifacts: false - variables: - MESON_OPTIONS: "--cross-file=cross-x86.txt" - MESON_SKIP_TEST: "true" - MESON_SKIP_INSTALL: "true" - build_on_alpine: extends: - .alpine diff --git a/cross-x86.txt b/cross-x86.txt deleted file mode 100644 index 8ddd5ecc5..000000000 --- a/cross-x86.txt +++ /dev/null @@ -1,23 +0,0 @@ -[binaries] -c = 'gcc' -cpp = 'g++' -ld = 'ld' -cmake = 'cmake' -strip = 'strip' -pkg-config = 'pkg-config' - -[properties] -pkg_config_libdir = '/usr/lib/pkgconfig' -ld_args = '-m elf_i386' - -[built-in options] -c_args = '-m32 -msse' -c_link_args = '-m32 -msse' -cpp_args = '-m32 -msse' -cpp_link_args = '-m32 -msse' - -[host_machine] -system = 'linux' -cpu_family = 'x86' -cpu = 'i686' -endian = 'little' From 0fcabdbd0c618825bf2bd95758d440cc8262b9d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Sat, 10 May 2025 12:49:06 +0200 Subject: [PATCH 06/34] ci: cross compile on debian 13 to multiple architectures Debian supports many architectures, and it is relatively easy to work with multiarch packages, and finally `meson env2mfile` supports generating cross files with the `--debarch` option. Previously only native builds were done in the CI, so use debian to build pipewire for multiple architectures. Some packages are unfortunately not multiarch compatible, so a separate container is built for every architecture. --- .gitlab-ci.yml | 77 ++++++++++++++++++++++ .gitlab/ci/setup-debian-cross-container.sh | 63 ++++++++++++++++++ 2 files changed, 140 insertions(+) create mode 100755 .gitlab/ci/setup-debian-cross-container.sh diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e381b7c64..b8a703ab6 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -31,6 +31,9 @@ include: - project: 'freedesktop/ci-templates' ref: *templates_sha file: '/templates/alpine.yml' + - project: 'freedesktop/ci-templates' + ref: *templates_sha + file: '/templates/debian.yml' .fedora: variables: @@ -141,6 +144,30 @@ include: # FDO_DISTRIBUTION_EXEC: >- # pip3 install meson +.debian: + variables: + # Update this tag when you want to trigger a rebuild + BASE_TAG: '2025-08-10.0' + FDO_DISTRIBUTION_VERSION: 'trixie' + FDO_DISTRIBUTION_PACKAGES: >- + build-essential + dpkg-dev + findutils + git + meson + +.debian-archictectures: + parallel: + matrix: + - ARCH: + - amd64 + - arm64 + - armhf + - i386 + - ppc64el + - riscv64 + - s390x + .alpine: variables: # Update this tag when you want to trigger a rebuild @@ -229,6 +256,18 @@ container_ubuntu: variables: GIT_STRATEGY: none # no need to pull the whole tree for rebuilding the image +container_debian: + extends: + - .debian + - .debian-archictectures + - .fdo.container-build@debian + stage: container + variables: + GIT_STRATEGY: none # no need to pull the whole tree for rebuilding the image + FDO_DISTRIBUTION_TAG: "$BASE_TAG-$ARCH" + FDO_DISTRIBUTION_EXEC: >- + ./.gitlab/ci/setup-debian-cross-container.sh "$ARCH" + container_fedora: extends: - .fedora @@ -267,6 +306,44 @@ build_on_ubuntu: variables: MESON_OPTIONS: "-Dsession-managers=[] -Dsnap=enabled" +build_on_debian: + extends: + - .debian + - .debian-archictectures + - .not_coverity + - .fdo.distribution-image@debian + - .build + stage: build + needs: + - job: container_debian + artifacts: false + # ideally + # parallel: + # matrix: + # - ARCH: "$ARCH" + # however https://gitlab.com/gitlab-org/gitlab/-/issues/423553 + # ("Expand variables in `needs:parallel:matrix`") + variables: + FDO_DISTRIBUTION_TAG: "$BASE_TAG-$ARCH" + # see /.gitlab/ci/setup-debian-cross-container.sh for installed packages + MESON_OPTIONS: >- + --cross-file /opt/meson-$ARCH.cross + -D c_args=['-UFASTPATH'] + -D cpp_args=['-UFASTPATH'] + -D auto_features=enabled + -D session-managers=[] + -D bluez5-backend-native-mm=enabled + -D bluez5-codec-lc3plus=disabled + -D bluez5-codec-ldac=disabled + -D bluez5-codec-ldac-dec=disabled + -D libcamera=disabled + -D roc=disabled + -D snap=disabled + -D systemd-user-service=disabled + -D systemd-system-service=disabled + -D onnxruntime=disabled + -D vulkan=enabled + .build_on_fedora: extends: - .fedora diff --git a/.gitlab/ci/setup-debian-cross-container.sh b/.gitlab/ci/setup-debian-cross-container.sh new file mode 100755 index 000000000..616971803 --- /dev/null +++ b/.gitlab/ci/setup-debian-cross-container.sh @@ -0,0 +1,63 @@ +#!/usr/bin/env bash + +set -ex + +packages=( + # libapparmor-dev + libasound2-dev + libavahi-client-dev + libavcodec-dev + libavfilter-dev + libavformat-dev + libbluetooth-dev + libcanberra-dev + libdbus-1-dev + libebur128-dev + libfdk-aac-dev + libffado-dev + libfftw3-dev + libfreeaptx-dev + libglib2.0-dev + libgstreamer1.0-dev + libgstreamer-plugins-base1.0-dev + libjack-jackd2-dev + liblc3-dev + liblilv-dev + libmysofa-dev + libopus-dev + libpulse-dev + libreadline-dev + libsbc-dev + libsdl2-dev + # libsnapd-glib-dev + libsndfile1-dev + libspandsp-dev + libssl-dev + libsystemd-dev + libudev-dev + libusb-1.0-0-dev + libvulkan-dev + libwebrtc-audio-processing-dev + libx11-dev + modemmanager-dev +) + +arch="$1" + +export DEBIAN_FRONTEND=noninteractive + +sed -i \ + 's/^Components:.*$/Components: main contrib non-free non-free-firmware/' \ + /etc/apt/sources.list.d/debian.sources + +dpkg --add-architecture "$arch" +apt update -y + +pkgs=( "crossbuild-essential-$arch" ) +for pkg in "${packages[@]}"; do + pkgs+=( "$pkg:$arch" ) +done + +apt install -y "${pkgs[@]}" + +meson env2mfile --cross --debarch "$arch" -o "/opt/meson-$arch.cross" From 3914d0cab311b9a2b0ec5f9950e848b4546340be Mon Sep 17 00:00:00 2001 From: Sertonix Date: Tue, 12 Aug 2025 12:11:35 +0000 Subject: [PATCH 07/34] module-jack: fix name of LIBJACK_PATH environment variable Fixes 0629647cb5de module-jack: load libjack.so.0 with dlopen --- src/modules/module-jack-tunnel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/module-jack-tunnel.c b/src/modules/module-jack-tunnel.c index 098563833..e64535938 100644 --- a/src/modules/module-jack-tunnel.c +++ b/src/modules/module-jack-tunnel.c @@ -49,7 +49,7 @@ * ## Module Options * * - `jack.library`: the libjack to load, by default libjack.so.0 is searched in - * JACK_PATH directories and then some standard library paths. + * LIBJACK_PATH directories and then some standard library paths. * Can be an absolute path. * - `jack.server`: the name of the JACK server to tunnel to. * - `jack.client-name`: the name of the JACK client. From 860e4554401742074a837489b05928b9940d8bbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Thu, 24 Jul 2025 12:39:44 +0200 Subject: [PATCH 08/34] spa: libcamera: source: support `libcamera::formats::R8` Map `libcamera::formats::R8` to `SPA_VIDEO_FORMAT_GRAY8`. --- spa/plugins/libcamera/libcamera-source.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 4bdafebaa..8ecc32c80 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -371,6 +371,7 @@ struct format_info { #define MAKE_FMT(pix,fmt,mt,mst) { pix, SPA_VIDEO_FORMAT_ ##fmt, SPA_MEDIA_TYPE_ ##mt, SPA_MEDIA_SUBTYPE_ ##mst } const struct format_info format_info[] = { /* RGB formats */ + MAKE_FMT(formats::R8, GRAY8, video, raw), MAKE_FMT(formats::RGB565, RGB16, video, raw), MAKE_FMT(formats::RGB565_BE, RGB16, video, raw), MAKE_FMT(formats::RGB888, BGR, video, raw), From 47ee86938b88021a887f33d164f7d3e1c241473d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Sun, 10 Aug 2025 19:01:34 +0200 Subject: [PATCH 09/34] spa: libcamera: source: port_set_format(): remove goto Remove the `done` label by moving things into the `format != nullptr` branch. --- spa/plugins/libcamera/libcamera-source.cpp | 33 +++++++++++----------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 8ecc32c80..524afef7b 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -1825,9 +1825,6 @@ next: int port_set_format(struct impl *impl, struct port *port, uint32_t flags, const struct spa_pod *format) { - struct spa_video_info info; - int res; - if (format == nullptr) { if (!port->current_format) return 0; @@ -1837,9 +1834,12 @@ int port_set_format(struct impl *impl, struct port *port, port->current_format.reset(); spa_libcamera_close(impl); - goto done; } else { + spa_video_info info; + int res; + spa_zero(info); + if ((res = spa_format_parse(format, &info.media_type, &info.media_subtype)) < 0) return res; @@ -1888,21 +1888,20 @@ int port_set_format(struct impl *impl, struct port *port, default: return -EINVAL; } + + if (port->current_format && !(flags & SPA_NODE_PARAM_FLAG_TEST_ONLY)) { + spa_libcamera_use_buffers(impl, port, nullptr, 0); + port->current_format.reset(); + } + + if (spa_libcamera_set_format(impl, port, &info, flags & SPA_NODE_PARAM_FLAG_TEST_ONLY) < 0) + return -EINVAL; + + if (!(flags & SPA_NODE_PARAM_FLAG_TEST_ONLY)) { + port->current_format = info; + } } - if (port->current_format && !(flags & SPA_NODE_PARAM_FLAG_TEST_ONLY)) { - spa_libcamera_use_buffers(impl, port, nullptr, 0); - port->current_format.reset(); - } - - if (spa_libcamera_set_format(impl, port, &info, flags & SPA_NODE_PARAM_FLAG_TEST_ONLY) < 0) - return -EINVAL; - - if (!(flags & SPA_NODE_PARAM_FLAG_TEST_ONLY)) { - port->current_format = info; - } - - done: impl->info.change_mask |= SPA_NODE_CHANGE_MASK_PARAMS; port->info.change_mask |= SPA_PORT_CHANGE_MASK_PARAMS; if (port->current_format) { From 14e0a8f66ff009c1d486c0532a39737f4e07f017 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Fri, 8 Aug 2025 10:38:25 +0200 Subject: [PATCH 10/34] spa: libcamera: source: propagate error when setting format If `spa_libcamera_set_format()` fails, propagate its return value as is without rewriting it to `EINVAL`. --- spa/plugins/libcamera/libcamera-source.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 524afef7b..4f9816c46 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -1894,8 +1894,9 @@ int port_set_format(struct impl *impl, struct port *port, port->current_format.reset(); } - if (spa_libcamera_set_format(impl, port, &info, flags & SPA_NODE_PARAM_FLAG_TEST_ONLY) < 0) - return -EINVAL; + res = spa_libcamera_set_format(impl, port, &info, flags & SPA_NODE_PARAM_FLAG_TEST_ONLY); + if (res < 0) + return res; if (!(flags & SPA_NODE_PARAM_FLAG_TEST_ONLY)) { port->current_format = info; From 89545946fd95ce01a7ba64d42d0be0d158a84e4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Fri, 8 Aug 2025 12:10:37 +0200 Subject: [PATCH 11/34] spa: libcamera: source: freeBuffers(): split pending request removal `freeBuffers()` should undo exactly what `allocBuffers()` does. However, it currently also clears `impl::pendingRequests`, but that is filled by `spa_libcamera_buffer_recycle()` during `spa_libcamera_alloc_buffers()`. So remove the clearing of `impl::pendingRequests` from `freeBuffers()` and move it directly into `spa_libcamera_clear_buffers()`. --- spa/plugins/libcamera/libcamera-source.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 4f9816c46..87fa2761c 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -323,7 +323,6 @@ int allocBuffers(struct impl *impl, struct port *port, unsigned int count) void freeBuffers(struct impl *impl, struct port *port) { - impl->pendingRequests.clear(); impl->requestPool.clear(); impl->allocator->free(port->streamConfig.stream()); } @@ -354,6 +353,7 @@ int spa_libcamera_clear_buffers(struct impl *impl, struct port *port) d[0].type = SPA_ID_INVALID; } + impl->pendingRequests.clear(); freeBuffers(impl, port); port->n_buffers = 0; port->ring = SPA_RINGBUFFER_INIT(); From b9b7c0ab05a8ec94b4166fa85a4f33cb39d4cd88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Fri, 8 Aug 2025 10:06:59 +0200 Subject: [PATCH 12/34] spa: libcamera: source: freeBuffers(): call when format is unset At the moment the libcamera buffer allocation is completely tied to format negotiation. `freeBuffers()` undoes `allocBuffers()`. And `allocBuffers()` is called as part of `spa_libcamera_set_format()`. Therefore `freeBuffers()` should be called independent of the state of the buffers on any port. Otherwise unsetting the format while there are no buffers on the port will cause the libcamera requests and buffers not to be released correctly. Similarly, removing the buffers from a port would clear the libcamera requests and buffers, making the node unusable without setting a new format. --- spa/plugins/libcamera/libcamera-source.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 87fa2761c..3e5715b2a 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -354,7 +354,6 @@ int spa_libcamera_clear_buffers(struct impl *impl, struct port *port) } impl->pendingRequests.clear(); - freeBuffers(impl, port); port->n_buffers = 0; port->ring = SPA_RINGBUFFER_INIT(); @@ -1831,6 +1830,7 @@ int port_set_format(struct impl *impl, struct port *port, spa_libcamera_stream_off(impl); spa_libcamera_clear_buffers(impl, port); + freeBuffers(impl, port); port->current_format.reset(); spa_libcamera_close(impl); From 348e703be0bee84371a0fe87df6b499987c1c508 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Fri, 8 Aug 2025 09:48:47 +0200 Subject: [PATCH 13/34] spa: libcamera: source: allocBuffers(): restore on failure Undo the allocation and clear the requests if a failure is encountered in order to leave things in a known state. --- spa/plugins/libcamera/libcamera-source.cpp | 61 +++++++++++++--------- 1 file changed, 36 insertions(+), 25 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 3e5715b2a..5588f9828 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -279,18 +279,42 @@ int spa_libcamera_buffer_recycle(struct impl *impl, struct port *port, uint32_t return 0; } +void freeBuffers(struct impl *impl, struct port *port) +{ + impl->requestPool.clear(); + std::ignore = impl->allocator->free(port->streamConfig.stream()); +} + +[[nodiscard]] +std::size_t count_unique_fds(libcamera::Span planes) +{ + std::size_t c = 0; + int fd = -1; + + for (const auto& plane : planes) { + const int current_fd = plane.fd.get(); + if (current_fd >= 0 && current_fd != fd) { + c += 1; + fd = current_fd; + } + } + + return c; +} + int allocBuffers(struct impl *impl, struct port *port, unsigned int count) { + libcamera::Stream *stream = port->streamConfig.stream(); int res; - if ((res = impl->allocator->allocate(port->streamConfig.stream())) < 0) + if ((res = impl->allocator->allocate(stream)) < 0) return res; for (unsigned int i = 0; i < count; i++) { std::unique_ptr request = impl->camera->createRequest(i); if (!request) { - impl->requestPool.clear(); - return -ENOMEM; + res = -ENOMEM; + goto err; } impl->requestPool.push_back(std::move(request)); } @@ -300,33 +324,20 @@ int allocBuffers(struct impl *impl, struct port *port, unsigned int count) * video frame has to be addressed using more than one memory. * address. Therefore, need calculate the number of discontiguous * memory and allocate the specified amount of memory */ - Stream *stream = impl->config->at(0).stream(); - const std::vector> &bufs = - impl->allocator->buffers(stream); - const std::vector &planes = bufs[0]->planes(); - int fd = -1; - uint32_t buffers_blocks = 0; - - for (const FrameBuffer::Plane &plane : planes) { - const int current_fd = plane.fd.get(); - if (current_fd >= 0 && current_fd != fd) { - buffers_blocks += 1; - fd = current_fd; - } + port->buffers_blocks = count_unique_fds(impl->allocator->buffers(stream).front()->planes()); + if (port->buffers_blocks <= 0) { + res = -ENOBUFS; + goto err; } - if (buffers_blocks > 0) { - port->buffers_blocks = buffers_blocks; - } + return 0; + +err: + freeBuffers(impl, port); + return res; } -void freeBuffers(struct impl *impl, struct port *port) -{ - impl->requestPool.clear(); - impl->allocator->free(port->streamConfig.stream()); -} - int spa_libcamera_clear_buffers(struct impl *impl, struct port *port) { uint32_t i; From 29b0c87d717266bf1dd4278b56f152e67ea22ab3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Fri, 8 Aug 2025 09:53:13 +0200 Subject: [PATCH 14/34] spa: libcamera: source: allocBuffers(): more error checking First, check if the request pool is empty, and signal an error if it is not. Second, ensure that the number of allocated buffers matches. --- spa/plugins/libcamera/libcamera-source.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 5588f9828..ae3e9a9b3 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -307,10 +307,19 @@ int allocBuffers(struct impl *impl, struct port *port, unsigned int count) libcamera::Stream *stream = port->streamConfig.stream(); int res; + if (!impl->requestPool.empty()) + return -EBUSY; + if ((res = impl->allocator->allocate(stream)) < 0) return res; - for (unsigned int i = 0; i < count; i++) { + const auto& bufs = impl->allocator->buffers(stream); + if (bufs.empty() || bufs.size() != count) { + res = -ENOBUFS; + goto err; + } + + for (std::size_t i = 0; i < bufs.size(); i++) { std::unique_ptr request = impl->camera->createRequest(i); if (!request) { res = -ENOMEM; @@ -324,7 +333,7 @@ int allocBuffers(struct impl *impl, struct port *port, unsigned int count) * video frame has to be addressed using more than one memory. * address. Therefore, need calculate the number of discontiguous * memory and allocate the specified amount of memory */ - port->buffers_blocks = count_unique_fds(impl->allocator->buffers(stream).front()->planes()); + port->buffers_blocks = count_unique_fds(bufs.front()->planes()); if (port->buffers_blocks <= 0) { res = -ENOBUFS; goto err; From 475665d615eb3f139462080115f03d1484a376e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Thu, 7 Aug 2025 15:22:42 +0200 Subject: [PATCH 15/34] spa: libcamera: source: persistent requests <-> buffer association Currently only a single stream is used. This makes it easy to associate each request with the appropriate frame buffer when it is created. In turn, this makes reusing requests a bit simpler and a bit more efficient. So do that: add buffers to requests when they are allocated in `allocBuffers()`. --- spa/plugins/libcamera/libcamera-source.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index ae3e9a9b3..9feb761ad 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -257,13 +257,6 @@ int spa_libcamera_buffer_recycle(struct impl *impl, struct port *port, uint32_t return -EINVAL; } Request *request = impl->requestPool[buffer_id].get(); - Stream *stream = port->streamConfig.stream(); - FrameBuffer *buffer = impl->allocator->buffers(stream)[buffer_id].get(); - if ((res = request->addBuffer(stream, buffer)) < 0) { - spa_log_warn(impl->log, "can't add buffer %u for request: %s", - buffer_id, spa_strerror(res)); - return -ENOMEM; - } if (!impl->active) { impl->pendingRequests.push_back(request); return 0; @@ -325,6 +318,11 @@ int allocBuffers(struct impl *impl, struct port *port, unsigned int count) res = -ENOMEM; goto err; } + + res = request->addBuffer(stream, bufs[i].get()); + if (res < 0) + goto err; + impl->requestPool.push_back(std::move(request)); } @@ -1252,7 +1250,7 @@ void impl::requestComplete(libcamera::Request *request) if ((request->status() == Request::RequestCancelled)) { spa_log_debug(impl->log, "Request was cancelled"); - request->reuse(); + request->reuse(libcamera::Request::ReuseFlag::ReuseBuffers); SPA_FLAG_SET(b->flags, BUFFER_FLAG_OUTSTANDING); spa_libcamera_buffer_recycle(impl, port, b->id); return; @@ -1299,7 +1297,7 @@ void impl::requestComplete(libcamera::Request *request) b->h->pts = fmd.timestamp; b->h->dts_offset = 0; } - request->reuse(); + request->reuse(libcamera::Request::ReuseFlag::ReuseBuffers); spa_ringbuffer_get_write_index(&port->ring, &index); port->ring_ids[index & MASK_BUFFERS] = buffer_id; @@ -1384,7 +1382,7 @@ int spa_libcamera_stream_off(struct impl *impl) if (!impl->active) { for (std::unique_ptr &req : impl->requestPool) - req->reuse(); + req->reuse(libcamera::Request::ReuseFlag::ReuseBuffers); return 0; } From 68627c5563425579da2800acf423483854126c88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Thu, 7 Aug 2025 15:47:02 +0200 Subject: [PATCH 16/34] spa: libcamera: source: remove `impl::pendingRequests` The handling of `impl::pendingRequests` is a bit problematic because, for example, during startup, if `spa_libcamera_buffer_recycle()` observes that `impl::active == false`, then it will try to append to `impl::pendingRequests`, which is being iterated in the main thread in `spa_libcamera_stream_on()`. That is not allowed on an `std::deque`. So instead remove it altogether, and simply queue all requests when starting. After `libcamera::Camera::stop()` returns, every request is guaranteed not to be used by libcamera, and they can be freely reused, so this is safe to do. This also removes the need for calling `spa_libcamera_buffer_recycle()` when the buffers are set/unset on a port since that function no longer changes anything apart from updating `buffer::flags` but `spa_libcamera_alloc_buffers()` is modified appropriately to take care of that. And in the case of `spa_libcamera_clear_buffers()` clearing the flag is not required because the next `spa_libcamera_alloc_buffers()` call will reset reset the flags. --- spa/plugins/libcamera/libcamera-source.cpp | 29 ++++++---------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 9feb761ad..41fda937b 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -6,7 +6,6 @@ #include #include -#include #include #include #include @@ -157,7 +156,6 @@ struct impl { FrameBufferAllocator *allocator = nullptr; std::vector> requestPool; - std::deque pendingRequests; void requestComplete(libcamera::Request *request); @@ -257,10 +255,7 @@ int spa_libcamera_buffer_recycle(struct impl *impl, struct port *port, uint32_t return -EINVAL; } Request *request = impl->requestPool[buffer_id].get(); - if (!impl->active) { - impl->pendingRequests.push_back(request); - return 0; - } else { + if (impl->active) { request->controls().merge(impl->ctrls); impl->ctrls.clear(); if ((res = impl->camera->queueRequest(request)) < 0) { @@ -359,10 +354,6 @@ int spa_libcamera_clear_buffers(struct impl *impl, struct port *port) b = &port->buffers[i]; d = b->outbuf->datas; - if (SPA_FLAG_IS_SET(b->flags, BUFFER_FLAG_OUTSTANDING)) { - spa_log_debug(impl->log, "queueing outstanding buffer %p", b); - spa_libcamera_buffer_recycle(impl, port, i); - } if (SPA_FLAG_IS_SET(b->flags, BUFFER_FLAG_MAPPED)) { munmap(SPA_PTROFF(b->ptr, -d[0].mapoffset, void), d[0].maxsize - d[0].mapoffset); @@ -371,7 +362,6 @@ int spa_libcamera_clear_buffers(struct impl *impl, struct port *port) d[0].type = SPA_ID_INVALID; } - impl->pendingRequests.clear(); port->n_buffers = 0; port->ring = SPA_RINGBUFFER_INIT(); @@ -1151,7 +1141,7 @@ spa_libcamera_alloc_buffers(struct impl *impl, struct port *port, b = &port->buffers[i]; b->id = i; b->outbuf = buffers[i]; - b->flags = BUFFER_FLAG_OUTSTANDING; + b->flags = 0; b->h = (struct spa_meta_header*)spa_buffer_find_meta_data(buffers[i], SPA_META_Header, sizeof(*b->h)); b->videotransform = (struct spa_meta_videotransform*)spa_buffer_find_meta_data( @@ -1224,8 +1214,6 @@ spa_libcamera_alloc_buffers(struct impl *impl, struct port *port, return -EIO; } } - - spa_libcamera_buffer_recycle(impl, port, i); } port->n_buffers = n_buffers; @@ -1353,11 +1341,12 @@ int spa_libcamera_stream_on(struct impl *impl) impl->camera->requestCompleted.connect(impl, &impl::requestComplete); - for (Request *req : impl->pendingRequests) { - if ((res = impl->camera->queueRequest(req)) < 0) + for (auto& req : impl->requestPool) { + req->reuse(libcamera::Request::ReuseFlag::ReuseBuffers); + + if ((res = impl->camera->queueRequest(req.get())) < 0) goto err_stop_camera; } - impl->pendingRequests.clear(); impl->dll.bw = 0.0; impl->active = true; @@ -1380,15 +1369,11 @@ int spa_libcamera_stream_off(struct impl *impl) struct port *port = &impl->out_ports[0]; int res; - if (!impl->active) { - for (std::unique_ptr &req : impl->requestPool) - req->reuse(libcamera::Request::ReuseFlag::ReuseBuffers); + if (!impl->active) return 0; - } impl->active = false; spa_log_info(impl->log, "stopping camera %s", impl->camera->id().c_str()); - impl->pendingRequests.clear(); if ((res = impl->camera->stop()) < 0) { spa_log_warn(impl->log, "error stopping camera %s: %s", From 099292d63d2851682740b38d5e958abff56dde6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Fri, 25 Jul 2025 19:14:45 +0200 Subject: [PATCH 17/34] spa: libcamera: source: store the request pointer in ring buffer The request will be needed later, so store that directly in the completion ring buffer for easy access. This is fine even though `impl::requestComplete()` calls `Request::reuse()` because only the request cookie is used in `libcamera_on_fd_events()` and that remains constant during the lifetime of a request object. --- spa/plugins/libcamera/libcamera-source.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 41fda937b..f07411370 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -80,7 +80,7 @@ struct port { uint32_t n_buffers = 0; struct spa_list queue; struct spa_ringbuffer ring = SPA_RINGBUFFER_INIT(); - uint32_t ring_ids[MAX_BUFFERS]; + libcamera::Request *ring_ids[MAX_BUFFERS]; static constexpr uint64_t info_all = SPA_PORT_CHANGE_MASK_FLAGS | SPA_PORT_CHANGE_MASK_PROPS | SPA_PORT_CHANGE_MASK_PARAMS; @@ -1044,9 +1044,11 @@ void libcamera_on_fd_events(struct spa_source *source) spa_log_error(impl->log, "nothing is queued"); return; } - buffer_id = port->ring_ids[index & MASK_BUFFERS]; + + auto *request = port->ring_ids[index & MASK_BUFFERS]; spa_ringbuffer_read_update(&port->ring, index + 1); + buffer_id = request->cookie(); b = &port->buffers[buffer_id]; spa_list_append(&port->queue, &b->link); @@ -1288,7 +1290,7 @@ void impl::requestComplete(libcamera::Request *request) request->reuse(libcamera::Request::ReuseFlag::ReuseBuffers); spa_ringbuffer_get_write_index(&port->ring, &index); - port->ring_ids[index & MASK_BUFFERS] = buffer_id; + port->ring_ids[index & MASK_BUFFERS] = request; spa_ringbuffer_write_update(&port->ring, index + 1); if (spa_system_eventfd_write(impl->system, impl->source.fd, 1) < 0) From 72fd462090971f222383e4a8d13dd6530b3f5b32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Fri, 25 Jul 2025 19:20:27 +0200 Subject: [PATCH 18/34] spa: libcamera: source: move request completion data to `impl` A `libcamera::Request` is directly tied to the camera, not any ports ("streams") of it. So move the request completion ring buffer into the `impl` struct. This is a prerequisite for supporting multiple libcamera streams (~ exposing multiple ports on the node) in the future. --- spa/plugins/libcamera/libcamera-source.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index f07411370..de4e425ef 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -79,8 +79,6 @@ struct port { struct buffer buffers[MAX_BUFFERS]; uint32_t n_buffers = 0; struct spa_list queue; - struct spa_ringbuffer ring = SPA_RINGBUFFER_INIT(); - libcamera::Request *ring_ids[MAX_BUFFERS]; static constexpr uint64_t info_all = SPA_PORT_CHANGE_MASK_FLAGS | SPA_PORT_CHANGE_MASK_PROPS | SPA_PORT_CHANGE_MASK_PARAMS; @@ -156,6 +154,8 @@ struct impl { FrameBufferAllocator *allocator = nullptr; std::vector> requestPool; + spa_ringbuffer completed_requests_rb = SPA_RINGBUFFER_INIT(); + std::array completed_requests; void requestComplete(libcamera::Request *request); @@ -363,7 +363,7 @@ int spa_libcamera_clear_buffers(struct impl *impl, struct port *port) } port->n_buffers = 0; - port->ring = SPA_RINGBUFFER_INIT(); + impl->completed_requests_rb = SPA_RINGBUFFER_INIT(); return 0; } @@ -1040,13 +1040,13 @@ void libcamera_on_fd_events(struct spa_source *source) return; } - if (spa_ringbuffer_get_read_index(&port->ring, &index) < 1) { + if (spa_ringbuffer_get_read_index(&impl->completed_requests_rb, &index) < 1) { spa_log_error(impl->log, "nothing is queued"); return; } - auto *request = port->ring_ids[index & MASK_BUFFERS]; - spa_ringbuffer_read_update(&port->ring, index + 1); + auto *request = impl->completed_requests[index & MASK_BUFFERS]; + spa_ringbuffer_read_update(&impl->completed_requests_rb, index + 1); buffer_id = request->cookie(); b = &port->buffers[buffer_id]; @@ -1289,9 +1289,9 @@ void impl::requestComplete(libcamera::Request *request) } request->reuse(libcamera::Request::ReuseFlag::ReuseBuffers); - spa_ringbuffer_get_write_index(&port->ring, &index); - port->ring_ids[index & MASK_BUFFERS] = request; - spa_ringbuffer_write_update(&port->ring, index + 1); + spa_ringbuffer_get_write_index(&impl->completed_requests_rb, &index); + impl->completed_requests[index & MASK_BUFFERS] = request; + spa_ringbuffer_write_update(&impl->completed_requests_rb, index + 1); if (spa_system_eventfd_write(impl->system, impl->source.fd, 1) < 0) spa_log_error(impl->log, "Failed to write on event fd"); From c01a2977a5d531e36e10ac29e7ed69c3dcbfa999 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Thu, 7 Aug 2025 12:32:17 +0200 Subject: [PATCH 19/34] spa: libcamera: source: reset ring buffer when stopping Presently, the ring buffer of completed requests is only cleared when the buffers are removed from the port. This is not entirely correct since pause/start commands do not clear the buffers but they stop the camera. As a consequence, it is possible that some completed requests stay in the ring buffer, causing them to be mistakenly processed when the camera is started next. So reset the ring buffer after the camera is stopped, the same time as the queue of free buffers is cleared. --- spa/plugins/libcamera/libcamera-source.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index de4e425ef..75913ec90 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -363,7 +363,6 @@ int spa_libcamera_clear_buffers(struct impl *impl, struct port *port) } port->n_buffers = 0; - impl->completed_requests_rb = SPA_RINGBUFFER_INIT(); return 0; } @@ -1390,6 +1389,7 @@ int spa_libcamera_stream_off(struct impl *impl) impl->source.fd = -1; } + impl->completed_requests_rb = SPA_RINGBUFFER_INIT(); spa_list_init(&port->queue); return 0; From 22ddb88072656209a36e99635d2f3520a44e4e40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Fri, 25 Jul 2025 20:18:15 +0200 Subject: [PATCH 20/34] spa: libcamera: source: process all requests in the ring buffer It is possible that multiple requests complete before the data loop can run `libcamera_on_fd_events()`. However, previously only the earliest item was processed from the ring buffer, meaning that in those cases request processing could be lagging request completion by multiple requests. Fix that by getting the number of available requests and processing them all. --- spa/plugins/libcamera/libcamera-source.cpp | 74 ++++++++++++---------- 1 file changed, 40 insertions(+), 34 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 75913ec90..d2fe8043f 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -1012,14 +1012,44 @@ int spa_libcamera_apply_controls(struct impl *impl, libcamera::ControlList&& con ); } +void handle_completed_request(struct impl *impl, libcamera::Request *request) +{ + const auto request_id = request->cookie(); + struct port *port = &impl->out_ports[0]; + buffer *b = &port->buffers[request_id]; + + spa_log_trace(impl->log, "%p: request %p[%" PRIu64 "] process status:%u seq:%" PRIu32, + impl, request, request_id, static_cast(request->status()), + request->sequence()); + + spa_list_append(&port->queue, &b->link); + + spa_io_buffers *io = port->io; + if (io == nullptr) { + b = spa_list_first(&port->queue, struct buffer, link); + spa_list_remove(&b->link); + SPA_FLAG_SET(b->flags, BUFFER_FLAG_OUTSTANDING); + spa_libcamera_buffer_recycle(impl, port, b->id); + } else if (io->status != SPA_STATUS_HAVE_DATA) { + if (io->buffer_id < port->n_buffers) + spa_libcamera_buffer_recycle(impl, port, io->buffer_id); + + b = spa_list_first(&port->queue, struct buffer, link); + spa_list_remove(&b->link); + SPA_FLAG_SET(b->flags, BUFFER_FLAG_OUTSTANDING); + + io->buffer_id = b->id; + io->status = SPA_STATUS_HAVE_DATA; + spa_log_trace(impl->log, "%p: now queued %" PRIu32, impl, b->id); + } + + spa_node_call_ready(&impl->callbacks, SPA_STATUS_HAVE_DATA); +} void libcamera_on_fd_events(struct spa_source *source) { struct impl *impl = (struct impl*) source->data; - struct spa_io_buffers *io; - struct port *port = &impl->out_ports[0]; - uint32_t index, buffer_id; - struct buffer *b; + uint32_t index; uint64_t cnt; if (source->rmask & SPA_IO_ERR) { @@ -1039,37 +1069,13 @@ void libcamera_on_fd_events(struct spa_source *source) return; } - if (spa_ringbuffer_get_read_index(&impl->completed_requests_rb, &index) < 1) { - spa_log_error(impl->log, "nothing is queued"); - return; + auto avail = spa_ringbuffer_get_read_index(&impl->completed_requests_rb, &index); + for (; avail > 0; avail--, index++) { + auto *request = impl->completed_requests[index & MASK_BUFFERS]; + + spa_ringbuffer_read_update(&impl->completed_requests_rb, index + 1); + handle_completed_request(impl, request); } - - auto *request = impl->completed_requests[index & MASK_BUFFERS]; - spa_ringbuffer_read_update(&impl->completed_requests_rb, index + 1); - - buffer_id = request->cookie(); - b = &port->buffers[buffer_id]; - spa_list_append(&port->queue, &b->link); - - io = port->io; - if (io == nullptr) { - b = spa_list_first(&port->queue, struct buffer, link); - spa_list_remove(&b->link); - SPA_FLAG_SET(b->flags, BUFFER_FLAG_OUTSTANDING); - spa_libcamera_buffer_recycle(impl, port, b->id); - } else if (io->status != SPA_STATUS_HAVE_DATA) { - if (io->buffer_id < port->n_buffers) - spa_libcamera_buffer_recycle(impl, port, io->buffer_id); - - b = spa_list_first(&port->queue, struct buffer, link); - spa_list_remove(&b->link); - SPA_FLAG_SET(b->flags, BUFFER_FLAG_OUTSTANDING); - - io->buffer_id = b->id; - io->status = SPA_STATUS_HAVE_DATA; - spa_log_trace(impl->log, "libcamera %p: now queued %d", impl, b->id); - } - spa_node_call_ready(&impl->callbacks, SPA_STATUS_HAVE_DATA); } int spa_libcamera_use_buffers(struct impl *impl, struct port *port, From 019a5c130f486994f22caebb66d8e53f691c9a6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Sat, 26 Jul 2025 15:13:26 +0200 Subject: [PATCH 21/34] spa: libcamera: source: process requests on data loop Since `impl::requestComplete()` runs in an internal libcamera thread, extra care would need to be taken to validate all accesses to common data structures. For example, the function might call `spa_libcamera_buffer_recycle()`, which accesses `impl::ctrls`, which would be unsafe because it could read or modified at the same time on the data thread. So move the processing of requests to the data loop. --- spa/plugins/libcamera/libcamera-source.cpp | 121 +++++++++++---------- 1 file changed, 62 insertions(+), 59 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index d2fe8043f..a41f604e5 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -1022,6 +1022,63 @@ void handle_completed_request(struct impl *impl, libcamera::Request *request) impl, request, request_id, static_cast(request->status()), request->sequence()); + if (request->status() == libcamera::Request::Status::RequestCancelled) { + spa_log_trace(impl->log, "%p: request %p[%" PRIu64 "] cancelled", + impl, request, request_id); + request->reuse(libcamera::Request::ReuseFlag::ReuseBuffers); + SPA_FLAG_SET(b->flags, BUFFER_FLAG_OUTSTANDING); + spa_libcamera_buffer_recycle(impl, port, b->id); + return; + } + + const FrameBuffer *buffer = request->findBuffer(port->streamConfig.stream()); + if (buffer == nullptr) { + spa_log_warn(impl->log, "%p: request %p[%" PRIu64 "] has no buffer for stream %p", + impl, request, request_id, port->streamConfig.stream()); + return; + } + + const FrameMetadata &fmd = buffer->metadata(); + + if (impl->clock) { + double target = (double)port->info.rate.num / port->info.rate.denom; + double corr; + + if (impl->dll.bw == 0.0) { + spa_dll_set_bw(&impl->dll, SPA_DLL_BW_MAX, port->info.rate.denom, port->info.rate.denom); + impl->clock->next_nsec = fmd.timestamp; + corr = 1.0; + } else { + double diff = ((double)impl->clock->next_nsec - (double)fmd.timestamp) / SPA_NSEC_PER_SEC; + double error = port->info.rate.denom * (diff - target); + corr = spa_dll_update(&impl->dll, SPA_CLAMPD(error, -128., 128.)); + } + /* FIXME, we should follow the driver clock and target_ values. + * for now we ignore and use our own. */ + impl->clock->target_rate = port->rate; + impl->clock->target_duration = 1; + + impl->clock->nsec = fmd.timestamp; + impl->clock->rate = port->rate; + impl->clock->position = fmd.sequence; + impl->clock->duration = 1; + impl->clock->delay = 0; + impl->clock->rate_diff = corr; + impl->clock->next_nsec += (uint64_t) (target * SPA_NSEC_PER_SEC * corr); + } + + if (b->h) { + b->h->flags = 0; + if (fmd.status != libcamera::FrameMetadata::Status::FrameSuccess) + b->h->flags |= SPA_META_HEADER_FLAG_CORRUPTED; + b->h->offset = 0; + b->h->seq = fmd.sequence; + b->h->pts = fmd.timestamp; + b->h->dts_offset = 0; + } + + request->reuse(libcamera::Request::ReuseFlag::ReuseBuffers); + spa_list_append(&port->queue, &b->link); spa_io_buffers *io = port->io; @@ -1233,66 +1290,12 @@ spa_libcamera_alloc_buffers(struct impl *impl, struct port *port, void impl::requestComplete(libcamera::Request *request) { struct impl *impl = this; - struct port *port = &impl->out_ports[0]; - Stream *stream = port->streamConfig.stream(); - uint32_t index, buffer_id; - struct buffer *b; + uint32_t index; - spa_log_debug(impl->log, "request complete"); - - buffer_id = request->cookie(); - b = &port->buffers[buffer_id]; - - if ((request->status() == Request::RequestCancelled)) { - spa_log_debug(impl->log, "Request was cancelled"); - request->reuse(libcamera::Request::ReuseFlag::ReuseBuffers); - SPA_FLAG_SET(b->flags, BUFFER_FLAG_OUTSTANDING); - spa_libcamera_buffer_recycle(impl, port, b->id); - return; - } - FrameBuffer *buffer = request->findBuffer(stream); - if (buffer == nullptr) { - spa_log_warn(impl->log, "unknown buffer"); - return; - } - const FrameMetadata &fmd = buffer->metadata(); - - if (impl->clock) { - double target = (double)port->info.rate.num / port->info.rate.denom; - double corr; - - if (impl->dll.bw == 0.0) { - spa_dll_set_bw(&impl->dll, SPA_DLL_BW_MAX, port->info.rate.denom, port->info.rate.denom); - impl->clock->next_nsec = fmd.timestamp; - corr = 1.0; - } else { - double diff = ((double)impl->clock->next_nsec - (double)fmd.timestamp) / SPA_NSEC_PER_SEC; - double error = port->info.rate.denom * (diff - target); - corr = spa_dll_update(&impl->dll, SPA_CLAMPD(error, -128., 128.)); - } - /* FIXME, we should follow the driver clock and target_ values. - * for now we ignore and use our own. */ - impl->clock->target_rate = port->rate; - impl->clock->target_duration = 1; - - impl->clock->nsec = fmd.timestamp; - impl->clock->rate = port->rate; - impl->clock->position = fmd.sequence; - impl->clock->duration = 1; - impl->clock->delay = 0; - impl->clock->rate_diff = corr; - impl->clock->next_nsec += (uint64_t) (target * SPA_NSEC_PER_SEC * corr); - } - if (b->h) { - b->h->flags = 0; - if (fmd.status != FrameMetadata::Status::FrameSuccess) - b->h->flags |= SPA_META_HEADER_FLAG_CORRUPTED; - b->h->offset = 0; - b->h->seq = fmd.sequence; - b->h->pts = fmd.timestamp; - b->h->dts_offset = 0; - } - request->reuse(libcamera::Request::ReuseFlag::ReuseBuffers); + spa_log_trace(impl->log, "%p: request %p[%" PRIu64 "] completed status:%u seq:%" PRIu32, + impl, request, request->cookie(), + static_cast(request->status()), + request->sequence()); spa_ringbuffer_get_write_index(&impl->completed_requests_rb, &index); impl->completed_requests[index & MASK_BUFFERS] = request; From 25075bb3d7190e84606dce934814e27f986a56de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Sat, 9 Aug 2025 01:38:18 +0200 Subject: [PATCH 22/34] spa: libcamera: source: set chunk flags on error Set `SPA_CHUNK_FLAG_CORRUPTED` if the frame buffer metadata signals anything other than success. --- spa/plugins/libcamera/libcamera-source.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index a41f604e5..b37fb1e8d 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -1077,6 +1077,15 @@ void handle_completed_request(struct impl *impl, libcamera::Request *request) b->h->dts_offset = 0; } + for (std::size_t i = 0; i < b->outbuf->n_datas; i++) { + auto *d = &b->outbuf->datas[i]; + + d->chunk->flags = 0; + + if (fmd.status != libcamera::FrameMetadata::Status::FrameSuccess) + d->chunk->flags |= SPA_CHUNK_FLAG_CORRUPTED; + } + request->reuse(libcamera::Request::ReuseFlag::ReuseBuffers); spa_list_append(&port->queue, &b->link); From 05a9e52cafe3579a4386d394cee74b8038ed2cad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Thu, 7 Aug 2025 18:07:02 +0200 Subject: [PATCH 23/34] spa: libcamera: source: remove format config shortcut Remove the code that is supposed to compare the current and the to-be-set format for returning early if they match. This is removed: * v4l2 also does not have it either; * it needs more consideration (there are not properly handled fields); * it would make later changes somewhat more complicated. --- spa/plugins/libcamera/libcamera-source.cpp | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index b37fb1e8d..1299606d6 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -1878,35 +1878,16 @@ int port_set_format(struct impl *impl, struct port *port, return -EINVAL; } - if (port->current_format && info.media_type == port->current_format->media_type && - info.media_subtype == port->current_format->media_subtype && - info.info.raw.format == port->current_format->info.raw.format && - info.info.raw.size.width == port->current_format->info.raw.size.width && - info.info.raw.size.height == port->current_format->info.raw.size.height && - info.info.raw.flags == port->current_format->info.raw.flags && - (!(info.info.raw.flags & SPA_VIDEO_FLAG_MODIFIER) || - (info.info.raw.modifier == port->current_format->info.raw.modifier))) - return 0; break; case SPA_MEDIA_SUBTYPE_mjpg: if (spa_format_video_mjpg_parse(format, &info.info.mjpg) < 0) return -EINVAL; - if (port->current_format && info.media_type == port->current_format->media_type && - info.media_subtype == port->current_format->media_subtype && - info.info.mjpg.size.width == port->current_format->info.mjpg.size.width && - info.info.mjpg.size.height == port->current_format->info.mjpg.size.height) - return 0; break; case SPA_MEDIA_SUBTYPE_h264: if (spa_format_video_h264_parse(format, &info.info.h264) < 0) return -EINVAL; - if (port->current_format && info.media_type == port->current_format->media_type && - info.media_subtype == port->current_format->media_subtype && - info.info.h264.size.width == port->current_format->info.h264.size.width && - info.info.h264.size.height == port->current_format->info.h264.size.height) - return 0; break; default: return -EINVAL; From dac9e40be65ea27e4f6b32cac190e4bca0527cfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Thu, 7 Aug 2025 18:12:58 +0200 Subject: [PATCH 24/34] spa: libcamera: source: extract presence of `SPA_NODE_PARAM_FLAG_TEST_ONLY` Use a boolean named `try_only` instead of checking `flags` each time. --- spa/plugins/libcamera/libcamera-source.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 1299606d6..9a912a005 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -1847,6 +1847,8 @@ next: int port_set_format(struct impl *impl, struct port *port, uint32_t flags, const struct spa_pod *format) { + const bool try_only = SPA_FLAG_IS_SET(flags, SPA_NODE_PARAM_FLAG_TEST_ONLY); + if (format == nullptr) { if (!port->current_format) return 0; @@ -1893,18 +1895,17 @@ int port_set_format(struct impl *impl, struct port *port, return -EINVAL; } - if (port->current_format && !(flags & SPA_NODE_PARAM_FLAG_TEST_ONLY)) { + if (port->current_format && !try_only) { spa_libcamera_use_buffers(impl, port, nullptr, 0); port->current_format.reset(); } - res = spa_libcamera_set_format(impl, port, &info, flags & SPA_NODE_PARAM_FLAG_TEST_ONLY); + res = spa_libcamera_set_format(impl, port, &info, try_only); if (res < 0) return res; - if (!(flags & SPA_NODE_PARAM_FLAG_TEST_ONLY)) { + if (!try_only) port->current_format = info; - } } impl->info.change_mask |= SPA_NODE_CHANGE_MASK_PARAMS; From a8a60832cd9baa99a974a3572f35816a606ba494 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Thu, 7 Aug 2025 18:14:07 +0200 Subject: [PATCH 25/34] spa: libcamera: source: do not emit param change if try-only If `SPA_NODE_PARAM_FLAG_TEST_ONLY`, then the format does not change on the node, it is only tested. So do not emit the param change events. --- spa/plugins/libcamera/libcamera-source.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 9a912a005..791d0c417 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -1908,6 +1908,9 @@ int port_set_format(struct impl *impl, struct port *port, port->current_format = info; } + if (try_only) + return 0; + impl->info.change_mask |= SPA_NODE_CHANGE_MASK_PARAMS; port->info.change_mask |= SPA_PORT_CHANGE_MASK_PARAMS; if (port->current_format) { From 31176120f512349eb59585b0b4f368b07149a05c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Mon, 11 Aug 2025 17:44:48 +0200 Subject: [PATCH 26/34] spa: libcamera: source: handle try-only format unset Do nothing if `format == nullptr` and `SPA_NODE_PARAM_FLAG_TEST_ONLY` is present. --- spa/plugins/libcamera/libcamera-source.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 791d0c417..3e65cc2d3 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -1850,7 +1850,7 @@ int port_set_format(struct impl *impl, struct port *port, const bool try_only = SPA_FLAG_IS_SET(flags, SPA_NODE_PARAM_FLAG_TEST_ONLY); if (format == nullptr) { - if (!port->current_format) + if (try_only || !port->current_format) return 0; spa_libcamera_stream_off(impl); From c517e712ed3a06d7c0bbf0a7ab11a659317f9a0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Thu, 7 Aug 2025 18:15:48 +0200 Subject: [PATCH 27/34] spa: libcamera: source: clear buffers when format is changed pipewire assumes that the buffers are removed from a port when its format is changed even without an explicit call to `port_use_buffer()`. So if the format is not just tested, clear the buffers, as well as the libcamera requests and buffers because they are also recreated when a new format is set. This matches the behaviour of the v4l2 plugin. Furthermore, this change also removes the call to `spa_libcamera_use_buffers()` because that function does nothing. And finally this change necessitates that the current format is always reset (when not testing), not just before reaching `spa_libcamera_set_format()`. --- spa/plugins/libcamera/libcamera-source.cpp | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 3e65cc2d3..485bbfb05 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -1849,16 +1849,16 @@ int port_set_format(struct impl *impl, struct port *port, { const bool try_only = SPA_FLAG_IS_SET(flags, SPA_NODE_PARAM_FLAG_TEST_ONLY); - if (format == nullptr) { - if (try_only || !port->current_format) - return 0; - + if (!try_only) { spa_libcamera_stream_off(impl); spa_libcamera_clear_buffers(impl, port); freeBuffers(impl, port); port->current_format.reset(); + } - spa_libcamera_close(impl); + if (format == nullptr) { + if (!try_only) + spa_libcamera_close(impl); } else { spa_video_info info; int res; @@ -1895,11 +1895,6 @@ int port_set_format(struct impl *impl, struct port *port, return -EINVAL; } - if (port->current_format && !try_only) { - spa_libcamera_use_buffers(impl, port, nullptr, 0); - port->current_format.reset(); - } - res = spa_libcamera_set_format(impl, port, &info, try_only); if (res < 0) return res; From 1f60cd291fe7882cefb2ba75f2dd045f03c737d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Fri, 8 Aug 2025 10:44:55 +0200 Subject: [PATCH 28/34] spa: libcamera: source: keep `libcamera::FrameBufferAllocator` Instantiate it once and keep it instead of always dynamically allocating it when the camera is acquired. --- spa/plugins/libcamera/libcamera-source.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 485bbfb05..19cd0f8ff 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -152,7 +152,7 @@ struct impl { std::shared_ptr camera; const std::unique_ptr config; - FrameBufferAllocator *allocator = nullptr; + FrameBufferAllocator allocator; std::vector> requestPool; spa_ringbuffer completed_requests_rb = SPA_RINGBUFFER_INIT(); std::array completed_requests; @@ -212,7 +212,7 @@ int spa_libcamera_open(struct impl *impl) if (int res = impl->camera->acquire(); res < 0) return res; - impl->allocator = new FrameBufferAllocator(impl->camera); + spa_assert(!impl->allocator.allocated()); const ControlInfoMap &controls = impl->camera->controls(); setup_initial_controls(controls, impl->initial_controls); @@ -230,8 +230,8 @@ int spa_libcamera_close(struct impl *impl) return 0; spa_log_info(impl->log, "close camera %s", impl->camera->id().c_str()); - delete impl->allocator; - impl->allocator = nullptr; + + spa_assert(!impl->allocator.allocated()); impl->camera->release(); @@ -270,7 +270,7 @@ int spa_libcamera_buffer_recycle(struct impl *impl, struct port *port, uint32_t void freeBuffers(struct impl *impl, struct port *port) { impl->requestPool.clear(); - std::ignore = impl->allocator->free(port->streamConfig.stream()); + std::ignore = impl->allocator.free(port->streamConfig.stream()); } [[nodiscard]] @@ -298,10 +298,10 @@ int allocBuffers(struct impl *impl, struct port *port, unsigned int count) if (!impl->requestPool.empty()) return -EBUSY; - if ((res = impl->allocator->allocate(stream)) < 0) + if ((res = impl->allocator.allocate(stream)) < 0) return res; - const auto& bufs = impl->allocator->buffers(stream); + const auto& bufs = impl->allocator.buffers(stream); if (bufs.empty() || bufs.size() != count) { res = -ENOBUFS; goto err; @@ -1183,7 +1183,7 @@ spa_libcamera_alloc_buffers(struct impl *impl, struct port *port, Stream *stream = impl->config->at(0).stream(); const std::vector> &bufs = - impl->allocator->buffers(stream); + impl->allocator.buffers(stream); if (n_buffers > 0) { if (bufs.size() != n_buffers) @@ -2171,7 +2171,8 @@ impl::impl(spa_log *log, spa_loop *data_loop, spa_system *system, out_ports{{this}}, manager(std::move(manager)), camera(std::move(camera)), - config(std::move(config)) + config(std::move(config)), + allocator(this->camera) { libcamera_log_topic_init(log); From 507688e6c99979b82fcffa23b3a03ebf198b2443 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Sun, 10 Aug 2025 22:45:44 +0200 Subject: [PATCH 29/34] spa: libcamera: source: add eventfd to loop while locked While concurrent `spa_loop_add_source()` invocations work with the current main epoll-based implementation, this is not guaranteed, so lock the loop. Similarly to how it is done elsewhere and for removal already. --- spa/plugins/libcamera/libcamera-source.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 19cd0f8ff..d4da10fd7 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -1350,7 +1350,16 @@ int spa_libcamera_stream_on(struct impl *impl) impl->source.data = impl; impl->source.mask = SPA_IO_IN | SPA_IO_ERR; impl->source.rmask = 0; - res = spa_loop_add_source(impl->data_loop, &impl->source); + + res = spa_loop_locked( + impl->data_loop, + [](spa_loop *, bool, uint32_t, const void *, size_t, void *user_data) + { + auto *impl = static_cast(user_data); + return spa_loop_add_source(impl->data_loop, &impl->source); + }, + 0, nullptr, 0, impl + ); if (res < 0) goto err_close_source; From 3e28f3e8594efadfbd9b3d9d13d7a6ea42120e54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Sun, 10 Aug 2025 22:40:12 +0200 Subject: [PATCH 30/34] spa: libcamera: source: rework startup sequence After e0e8bf083 ("spa: libcamera: source: create eventfd before starting camera"), things are still not entirely correct. This change ensures that if starting the camera fails, then the runtime state, most importantly the ring buffer of completed requests is restored to its initial state. Furthermore, it is also ensured that `impl::active` can never be observed by the data thread while it is being changed, which is achieved by setting it before/after adding/removing the event source. --- spa/plugins/libcamera/libcamera-source.cpp | 112 ++++++++++----------- 1 file changed, 54 insertions(+), 58 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index d4da10fd7..e931ca230 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -171,6 +171,38 @@ struct impl { std::unique_ptr config); struct spa_dll dll; + + void stop() + { + spa_loop_locked( + data_loop, + [](spa_loop *, bool, uint32_t, const void *, size_t, void *user_data) { + auto *self = static_cast(user_data); + + if (self->source.loop) + spa_loop_remove_source(self->data_loop, &self->source); + + return 0; + }, + 0, nullptr, 0, this + ); + + if (source.fd >= 0) + spa_system_close(system, std::exchange(source.fd, -1)); + + camera->requestCompleted.disconnect(this, &impl::requestComplete); + + if (int res = camera->stop(); res < 0) { + spa_log_warn(log, "failed to stop camera %s: %s", + camera->id().c_str(), spa_strerror(res)); + } + + completed_requests_rb = SPA_RINGBUFFER_INIT(); + active = false; + + for (auto& p : out_ports) + spa_list_init(&p.queue); + } }; #define CHECK_PORT(impl,direction,port_id) ((direction) == SPA_DIRECTION_OUTPUT && (port_id) == 0) @@ -1315,19 +1347,6 @@ void impl::requestComplete(libcamera::Request *request) } -int do_remove_source(struct spa_loop *loop, - bool async, - uint32_t seq, - const void *data, - size_t size, - void *user_data) -{ - auto *impl = static_cast(user_data); - if (impl->source.loop) - spa_loop_remove_source(loop, &impl->source); - return 0; -} - int spa_libcamera_stream_on(struct impl *impl) { struct port *port = &impl->out_ports[0]; @@ -1341,9 +1360,15 @@ int spa_libcamera_stream_on(struct impl *impl) if (impl->active) return 0; + spa_log_info(impl->log, "starting camera %s", impl->camera->id().c_str()); + if ((res = impl->camera->start(&impl->initial_controls)) < 0) + return res == -EACCES ? -EBUSY : res; + + impl->camera->requestCompleted.connect(impl, &impl::requestComplete); + res = spa_system_eventfd_create(impl->system, SPA_FD_CLOEXEC | SPA_FD_NONBLOCK); if (res < 0) - return res; + goto err_stop; impl->source.fd = res; impl->source.func = libcamera_on_fd_events; @@ -1351,6 +1376,16 @@ int spa_libcamera_stream_on(struct impl *impl) impl->source.mask = SPA_IO_IN | SPA_IO_ERR; impl->source.rmask = 0; + for (auto& req : impl->requestPool) { + req->reuse(libcamera::Request::ReuseFlag::ReuseBuffers); + + if ((res = impl->camera->queueRequest(req.get())) < 0) + goto err_stop; + } + + impl->dll.bw = 0.0; + impl->active = true; + res = spa_loop_locked( impl->data_loop, [](spa_loop *, bool, uint32_t, const void *, size_t, void *user_data) @@ -1361,63 +1396,24 @@ int spa_libcamera_stream_on(struct impl *impl) 0, nullptr, 0, impl ); if (res < 0) - goto err_close_source; - - spa_log_info(impl->log, "starting camera %s", impl->camera->id().c_str()); - if ((res = impl->camera->start(&impl->initial_controls)) < 0) - goto err_remove_source; - - impl->camera->requestCompleted.connect(impl, &impl::requestComplete); - - for (auto& req : impl->requestPool) { - req->reuse(libcamera::Request::ReuseFlag::ReuseBuffers); - - if ((res = impl->camera->queueRequest(req.get())) < 0) - goto err_stop_camera; - } - - impl->dll.bw = 0.0; - impl->active = true; + goto err_stop; return 0; -err_stop_camera: - impl->camera->stop(); - impl->camera->requestCompleted.disconnect(impl, &impl::requestComplete); -err_remove_source: - spa_loop_locked(impl->data_loop, do_remove_source, 0, nullptr, 0, impl); -err_close_source: - spa_system_close(impl->system, std::exchange(impl->source.fd, -1)); +err_stop: + impl->stop(); - return res == -EACCES ? -EBUSY : res; + return res; } int spa_libcamera_stream_off(struct impl *impl) { - struct port *port = &impl->out_ports[0]; - int res; - if (!impl->active) return 0; - impl->active = false; spa_log_info(impl->log, "stopping camera %s", impl->camera->id().c_str()); - if ((res = impl->camera->stop()) < 0) { - spa_log_warn(impl->log, "error stopping camera %s: %s", - impl->camera->id().c_str(), spa_strerror(res)); - } - - impl->camera->requestCompleted.disconnect(impl, &impl::requestComplete); - - spa_loop_locked(impl->data_loop, do_remove_source, 0, nullptr, 0, impl); - if (impl->source.fd >= 0) { - spa_system_close(impl->system, impl->source.fd); - impl->source.fd = -1; - } - - impl->completed_requests_rb = SPA_RINGBUFFER_INIT(); - spa_list_init(&port->queue); + impl->stop(); return 0; } From b948ffdb2509c664b25374767006914dc133a033 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Tue, 12 Aug 2025 23:36:49 +0200 Subject: [PATCH 31/34] spa: libcamera: source: remove `SPA_DATA_MemPtr` support The current handling of `SPA_DATA_MemPtr` is not entirely correct because its handling of multi-planar buffers is not appropriate: it leaks memory mappings because it overwrites `buffer::ptr` for each plane. Since this data type should not really be in use in normal deployments, let's remove it for now. --- spa/plugins/libcamera/libcamera-source.cpp | 25 +--------------------- 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index e931ca230..a16ce7971 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -53,7 +53,6 @@ namespace { #define MASK_BUFFERS 31 #define BUFFER_FLAG_OUTSTANDING (1<<0) -#define BUFFER_FLAG_MAPPED (1<<1) struct buffer { uint32_t id; @@ -386,11 +385,6 @@ int spa_libcamera_clear_buffers(struct impl *impl, struct port *port) b = &port->buffers[i]; d = b->outbuf->datas; - if (SPA_FLAG_IS_SET(b->flags, BUFFER_FLAG_MAPPED)) { - munmap(SPA_PTROFF(b->ptr, -d[0].mapoffset, void), - d[0].maxsize - d[0].mapoffset); - } - d[0].type = SPA_ID_INVALID; } @@ -1225,10 +1219,8 @@ spa_libcamera_alloc_buffers(struct impl *impl, struct port *port, if (d[0].type != SPA_ID_INVALID && d[0].type & (1u << SPA_DATA_DmaBuf)) { port->memtype = SPA_DATA_DmaBuf; - } else if (d[0].type != SPA_ID_INVALID && d[0].type & (1u << SPA_DATA_MemFd)) { + } else if (d[0].type & (1u << SPA_DATA_MemFd)) { port->memtype = SPA_DATA_MemFd; - } else if (d[0].type & (1u << SPA_DATA_MemPtr)) { - port->memtype = SPA_DATA_MemPtr; } else { spa_log_error(impl->log, "can't use buffers of type %d", d[0].type); return -EINVAL; @@ -1299,21 +1291,6 @@ spa_libcamera_alloc_buffers(struct impl *impl, struct port *port, d[j].fd = bufs[i]->planes()[j].fd.get(); spa_log_debug(impl->log, "Got fd = %" PRId64 " for buffer: #%d", d[j].fd, i); d[j].data = nullptr; - } - else if (port->memtype == SPA_DATA_MemPtr) { - d[j].fd = -1; - d[j].data = mmap(nullptr, - d[j].maxsize + d[j].mapoffset, - PROT_READ, MAP_SHARED, - bufs[i]->planes()[j].fd.get(), - 0); - if (d[j].data == MAP_FAILED) { - spa_log_error(impl->log, "mmap: %m"); - continue; - } - b->ptr = d[j].data; - SPA_FLAG_SET(b->flags, BUFFER_FLAG_MAPPED); - spa_log_debug(impl->log, "mmap ptr:%p", d[j].data); } else { spa_log_error(impl->log, "invalid buffer type"); return -EIO; From bf327d3dfbbee6642d5a3486c874c9e15d0b25ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Wed, 13 Aug 2025 15:35:04 +0200 Subject: [PATCH 32/34] spa: libcamera: source: simplify `spa_libcamera_clear_buffers()` Remove the unused `impl` parameter and the unnecessary early return. --- spa/plugins/libcamera/libcamera-source.cpp | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index a16ce7971..18cca35ea 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -371,14 +371,9 @@ err: return res; } -int spa_libcamera_clear_buffers(struct impl *impl, struct port *port) +int spa_libcamera_clear_buffers(struct port *port) { - uint32_t i; - - if (port->n_buffers == 0) - return 0; - - for (i = 0; i < port->n_buffers; i++) { + for (std::size_t i = 0; i < port->n_buffers; i++) { struct buffer *b; struct spa_data *d; @@ -1833,7 +1828,7 @@ int port_set_format(struct impl *impl, struct port *port, if (!try_only) { spa_libcamera_stream_off(impl); - spa_libcamera_clear_buffers(impl, port); + spa_libcamera_clear_buffers(port); freeBuffers(impl, port); port->current_format.reset(); } @@ -1947,7 +1942,7 @@ int impl_node_port_use_buffers(void *object, if (port->n_buffers) { spa_libcamera_stream_off(impl); - if ((res = spa_libcamera_clear_buffers(impl, port)) < 0) + if ((res = spa_libcamera_clear_buffers(port)) < 0) return res; } if (n_buffers > 0 && !port->current_format) From b82160c2e71648b1bfc2be1991aeb1ef1e37a716 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Wed, 13 Aug 2025 15:37:51 +0200 Subject: [PATCH 33/34] spa: libcamera: source: remove stale data from buffers When clearing the buffers, remove the stale pointers and file descriptors as accidental reuse of those is problematic and potentially difficult to diagnose. Do this for every data plane. Also clear the node's `buffer` structures to remove any references to the provided `spa_buffer` objects and related metadata. --- spa/plugins/libcamera/libcamera-source.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index 18cca35ea..a469ed377 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -374,13 +374,18 @@ err: int spa_libcamera_clear_buffers(struct port *port) { for (std::size_t i = 0; i < port->n_buffers; i++) { - struct buffer *b; - struct spa_data *d; + buffer *b = &port->buffers[i]; + spa_buffer *sb = b->outbuf; - b = &port->buffers[i]; - d = b->outbuf->datas; + for (std::size_t j = 0; j < sb->n_datas; j++) { + auto *d = &sb->datas[j]; - d[0].type = SPA_ID_INVALID; + d->type = SPA_ID_INVALID; + d->data = nullptr; + d->fd = -1; + } + + *b = {}; } port->n_buffers = 0; From e8ae244b2b656b32eeac257ed4c2e87e43435c44 Mon Sep 17 00:00:00 2001 From: Robert Mader Date: Wed, 13 Aug 2025 13:20:31 +0200 Subject: [PATCH 34/34] gst: src: Promote 'set format' log to info Follow various other elements like glupload and gtk4paintablesink and print the negotiated caps at a higher priority than debug. A small quality of life improvement to facilitate debugging. --- src/gst/gstpipewiresrc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gst/gstpipewiresrc.c b/src/gst/gstpipewiresrc.c index e6f7ff9d0..e5c0bc7ee 100644 --- a/src/gst/gstpipewiresrc.c +++ b/src/gst/gstpipewiresrc.c @@ -1206,7 +1206,7 @@ gst_pipewire_src_negotiate (GstBaseSrc * basesrc) gst_pipewire_clock_reset (GST_PIPEWIRE_CLOCK (pwsrc->stream->clock), 0); - GST_DEBUG_OBJECT (pwsrc, "set format %" GST_PTR_FORMAT, negotiated_caps); + GST_INFO_OBJECT (pwsrc, "set format %" GST_PTR_FORMAT, negotiated_caps); result = gst_base_src_set_caps (GST_BASE_SRC (pwsrc), negotiated_caps); if (!result) goto no_caps;