pipewire: hold the thread-loop lock across blocking invokes; buffer size/stride as ranges by jcelerier · Pull Request #227 · celtera/libremidi

jcelerier · 2026-07-10T06:24:58Z

Two fixes for the shared PipeWire context, found while round-trip-testing ossia score's PipeWire video path against a live 1.3.0 daemon.

1. `context::invoke_sync` corrupts the thread-loop mutex

invoke_sync issued pw_loop_invoke(..., block=true, ...) from foreign threads without holding the thread-loop lock. PipeWire's blocking-invoke path unconditionally fires the loop-control hooks around its wait (pw_thread_loop's impl_before/impl_after — i.e. unlock/lock), which exist to release the caller's lock while it blocks; the API contract is that the caller holds it.

Without the lock:

do_unlock underflows the recurse counter ('this->recurse > 0' failed at thread-loop.c:62 on stderr) and the unlock is refused;
impl_after then acquires the mutex on the way out, so the calling thread exits owning the loop mutex forever;
the loop thread blocks in do_lock on its next iteration, and any later pw_thread_loop_stop deadlocks in pthread_join.

Proven by rebuilding libpipewire with an instrumented thread-loop.c (abort on recurse underflow): the guilty stack was invoke_sync ← context::unsubscribe ← subscription dtor. Every consumer that destroys a subscription or uses invoke_sync off-loop hits this eventually.

Fix: wrap the two blocking pw_loop_invoke calls in with_lock — the hooks then release/retake the lock during the wait exactly as designed. invoke_async (block=false) is unaffected.

2. `format_negotiation::build_buffers_param` rejects padded strides

SPA_PARAM_BUFFERS_size/stride were advertised as fixed Ints, so any producer with padded rows (GPU-allocated buffers routinely pad strides) failed the buffers-param intersection and the link died before a single frame. Now advertised as CHOICE_RANGE(tight, tight, INT32_MAX); consumers must read chunk->stride/size per frame anyway.

Both verified with score's new PipewireRoundtrip harness (26/26 format×transport cells pass, including padded-stride producers; the deadlock repro'd 100% before the fix and is gone after).

🤖 Generated with Claude Code

https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE

…aram ranges Two fixes found by hardware/daemon round-trip testing of ossia score's PipeWire video path (PipeWire 1.3.0): 1. context::invoke_sync issued pw_loop_invoke(block=true) from foreign threads WITHOUT holding the thread-loop lock. The blocking-invoke path in spa/plugins/support/loop.c unconditionally fires the loop-control hooks around its wait (pw_thread_loop's impl_before/impl_after = unlock/lock), which exist precisely to release the caller's lock while it blocks — i.e. the API contract is that the caller holds the lock. Without it, do_unlock underflows the recurse counter ("'this->recurse > 0' failed" on stderr) and is refused, and impl_after then ACQUIRES the mutex on the way out — the calling thread exits owning the loop mutex forever. The next loop iteration blocks in do_lock and any subsequent pw_thread_loop_stop deadlocks in pthread_join. Proven with an instrumented thread-loop.c (abort on underflow): the guilty stack was invoke_sync <- context::unsubscribe <- subscription dtor. Fix: wrap both blocking pw_loop_invoke calls in with_lock (the hooks then release/retake the lock during the wait, as designed). invoke_async (block=false) is unaffected. 2. format_negotiation::build_buffers_param advertised SPA_PARAM_BUFFERS_size/stride as fixed Ints, so any producer with padded rows (GPU-allocated buffers routinely pad strides) failed the buffers-param intersection and the link died before a single frame. Advertise CHOICE_RANGE(tight, tight, INT32_MAX) instead; consumers must read chunk->stride/size per-frame anyway. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE

jcelerier · 2026-07-10T17:17:15Z

Superseded by #229, which combines this with #226 and reconciles the two invoke_sync fixes (stacking both locks would deadlock — details in #229).

…sweep) Output device (never worked before this): - PipewireProducer::start() was never called: the producer stream was never created and the output published zero frames since inception. Now started at the end of createOutput, after the dmabuf-mode flags. - Vulkan sysmem published every frame upside-down: the readback used InvertYRenderer unconditionally, but only GL's Y-up framebuffer needs the flip. New wire renderer selects flip/copy via isYUpInFramebuffer. - Readback now targets the WIRE format instead of the window render format: fixes rgb10a2/bgr10a2 (was ~5 dB garbage) and rgba16f (frame size mismatch meant no frames at all). - BGRA: GL readback is always R,G,B,A component order; bytes went out labeled SPA BGRA. Render to RGBA8 + CPU swizzle before push. - QRhi RGB10A2 word order (A2B10G10R10) repacked to SPA xRGB_210LE. - EGL/GBM DRM fourcc mapping was inverted (RGBA memory is ABGR8888 per DRM spec); dmabuf mode restricted to format=rgba (no GPU swizzle), other tags fall back to readback with a warning. - format= tags with no faithful QRhi representation (nv12, yuyv, ...) are rejected with a warning + rgba fallback instead of advertising a format the pushed bytes don't match. - Rational framerate on the wire (29.97 stayed 29.97, was truncated). - Vulkan DMA-BUF chunk stride from vkGetImageSubresourceLayout rowPitch instead of width*bpp (compiled on Qt >= 6.6; unverified at runtime). Input device: - 10-bit packed copy used av_get_bits_per_pixel (30 for X2RGB10LE) so every row lost its right quarter; use padded bits per pixel. - RGBA16F wire is half-float but the AVFrame is RGBA64LE (u16): added per-sample half->u16 conversion (NaN->0, clamped). - YUYV/UYVY bytesPerPixel was 4 (is 2): the over-advertised buffer size killed negotiation entirely and the size validation dropped frames. - YV12 support: new Tag::YV12, negotiated as SPA YV12, U/V plane swap on copy; input negotiates by Tag (the Tag->AVPixelFormat mapping is lossy and collapsed YV12 into I420). - target.object set alongside deprecated node.target (WirePlumber 0.5+ only honors the former). Note: WirePlumber still does not auto-link stream->stream; a patchbay or explicit link is required. - sysmem AVFrame pts from spa_meta_header (was chunk->offset, i.e. 0). - Headless factories makePipewireOutput/makePipewireCapture exported for test harnesses (mirrors videoio's makeAJACapture). Teardown: stream destruction stays under with_lock — correct per the loop-control-hook contract; the deadlock observed there was caused by libremidi's invoke_sync corrupting the loop mutex (fixed in celtera/libremidi#227, pulled via the libossia bump). All fixes verified cell-by-cell with tests/PipewireRoundtrip: 26/26 PASS on Qt 6.4.2 dynamic and Qt 6.12 static, OpenGL and Vulkan. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE

tests/PipewireRoundtrip (SCORE_PIPEWIRE_TESTER=ON, needs a live daemon and DISPLAY): exhaustive round-trip validation of the PipeWire video devices, mirroring AJARoundtrip's index-band + PSNR methodology. - score -> score: TexgenNode -> PipewireOutputNode -> daemon -> InputStream, on OpenGL and Vulkan, sysmem SHM and EGL/GBM DMA-BUF. - raw libpipewire reference producer (video-src.c pattern, with SPA_PARAM_Buffers announcement) painting the test signal in 12 SPA formats: RGBA/BGRA/xRGB210/xBGR210/RGBA_F16/RGB24/I420/YV12/NV12/ P010/YUY2/UYVY, plus padded-stride robustness cells, validating every input copy path and the negotiation fallback ladder. - Verification via swscale to RGBA (DRM-PRIME frames via dmabuf mmap); per-cell PSNR/continuity/latency matrix, exit 1 on FAIL. - Explicit pw-link-style link creation (WirePlumber's policy does not link stream->stream video and target.object alone does not either). libossia bump pulls the libremidi PipeWire context fixes (celtera/libremidi#227): thread-loop lock held across blocking invokes (teardown deadlock) and buffers size/stride ranges (padded strides). Current matrix: 26/26 PASS on Qt 6.4.2 dynamic + Qt 6.12 static. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE

…sweep) Output device (never worked before this): - PipewireProducer::start() was never called: the producer stream was never created and the output published zero frames since inception. Now started at the end of createOutput, after the dmabuf-mode flags. - Vulkan sysmem published every frame upside-down: the readback used InvertYRenderer unconditionally, but only GL's Y-up framebuffer needs the flip. New wire renderer selects flip/copy via isYUpInFramebuffer. - Readback now targets the WIRE format instead of the window render format: fixes rgb10a2/bgr10a2 (was ~5 dB garbage) and rgba16f (frame size mismatch meant no frames at all). - BGRA: GL readback is always R,G,B,A component order; bytes went out labeled SPA BGRA. Render to RGBA8 + CPU swizzle before push. - QRhi RGB10A2 word order (A2B10G10R10) repacked to SPA xRGB_210LE. - EGL/GBM DRM fourcc mapping was inverted (RGBA memory is ABGR8888 per DRM spec); dmabuf mode restricted to format=rgba (no GPU swizzle), other tags fall back to readback with a warning. - format= tags with no faithful QRhi representation (nv12, yuyv, ...) are rejected with a warning + rgba fallback instead of advertising a format the pushed bytes don't match. - Rational framerate on the wire (29.97 stayed 29.97, was truncated). - Vulkan DMA-BUF chunk stride from vkGetImageSubresourceLayout rowPitch instead of width*bpp (compiled on Qt >= 6.6; unverified at runtime). Input device: - 10-bit packed copy used av_get_bits_per_pixel (30 for X2RGB10LE) so every row lost its right quarter; use padded bits per pixel. - RGBA16F wire is half-float but the AVFrame is RGBA64LE (u16): added per-sample half->u16 conversion (NaN->0, clamped). - YUYV/UYVY bytesPerPixel was 4 (is 2): the over-advertised buffer size killed negotiation entirely and the size validation dropped frames. - YV12 support: new Tag::YV12, negotiated as SPA YV12, U/V plane swap on copy; input negotiates by Tag (the Tag->AVPixelFormat mapping is lossy and collapsed YV12 into I420). - target.object set alongside deprecated node.target (WirePlumber 0.5+ only honors the former). Note: WirePlumber still does not auto-link stream->stream; a patchbay or explicit link is required. - sysmem AVFrame pts from spa_meta_header (was chunk->offset, i.e. 0). - Headless factories makePipewireOutput/makePipewireCapture exported for test harnesses (mirrors videoio's makeAJACapture). Teardown: stream destruction stays under with_lock — correct per the loop-control-hook contract; the deadlock observed there was caused by libremidi's invoke_sync corrupting the loop mutex (fixed in celtera/libremidi#227, pulled via the libossia bump). All fixes verified cell-by-cell with tests/PipewireRoundtrip: 26/26 PASS on Qt 6.4.2 dynamic and Qt 6.12 static, OpenGL and Vulkan. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE

tests/PipewireRoundtrip (SCORE_PIPEWIRE_TESTER=ON, needs a live daemon and DISPLAY): exhaustive round-trip validation of the PipeWire video devices, mirroring AJARoundtrip's index-band + PSNR methodology. - score -> score: TexgenNode -> PipewireOutputNode -> daemon -> InputStream, on OpenGL and Vulkan, sysmem SHM and EGL/GBM DMA-BUF. - raw libpipewire reference producer (video-src.c pattern, with SPA_PARAM_Buffers announcement) painting the test signal in 12 SPA formats: RGBA/BGRA/xRGB210/xBGR210/RGBA_F16/RGB24/I420/YV12/NV12/ P010/YUY2/UYVY, plus padded-stride robustness cells, validating every input copy path and the negotiation fallback ladder. - Verification via swscale to RGBA (DRM-PRIME frames via dmabuf mmap); per-cell PSNR/continuity/latency matrix, exit 1 on FAIL. - Explicit pw-link-style link creation (WirePlumber's policy does not link stream->stream video and target.object alone does not either). libossia bump pulls the libremidi PipeWire context fixes (celtera/libremidi#227): thread-loop lock held across blocking invokes (teardown deadlock) and buffers size/stride ranges (padded strides). Current matrix: 26/26 PASS on Qt 6.4.2 dynamic + Qt 6.12 static. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014rZgzE8JjWvHDtaVUhxpLE

jcelerier mentioned this pull request Jul 10, 2026

pipewire: context hardening — combines #226 + #227, reconciled #229

Open

jcelerier closed this Jul 10, 2026

jcelerier mentioned this pull request Jul 10, 2026

pipewire: fix thread-loop lifetime & recursive-lock crashes in the shared context #226

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

pipewire: hold the thread-loop lock across blocking invokes; buffer size/stride as ranges#227

pipewire: hold the thread-loop lock across blocking invokes; buffer size/stride as ranges#227
jcelerier wants to merge 1 commit into
masterfrom
fix/pipewire-blocking-invoke-lock

jcelerier commented Jul 10, 2026

Uh oh!

jcelerier commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

jcelerier commented Jul 10, 2026

1. context::invoke_sync corrupts the thread-loop mutex

2. format_negotiation::build_buffers_param rejects padded strides

Uh oh!

jcelerier commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

1. `context::invoke_sync` corrupts the thread-loop mutex

2. `format_negotiation::build_buffers_param` rejects padded strides