fix(linux): recover the DMA-BUF plane size the producer leaves at zero - #319
Conversation
The DMA-BUF import path #299 added has never run: mutter and sway both offer shared memory, so params[0] wins and the memfd branch is taken. Forcing the dmabuf object (new OPENSCREEN_PIPEWIRE_FORCE_DMABUF, see below) shows the import fails on the very first buffer. wlroots hands out planes with maxsize = 0 — legitimate, since the size of a dmabuf belongs to the exporting buffer and not to the SPA descriptor. osc_map_dmabuf refused on that and returned NULL *before calling mmap*, and the failure was reported as "this driver does not allow CPU mapping of the capture buffer". The GPU was blamed for a refusal it was never asked for. Measured on AMD Mendocino / amdgpu, sway 1.9 via xdg-desktop-portal-wlr: [osc-dmabuf] refused before mmap: fd=23 len=0 lseek_size=3686400 [osc-dmabuf] maxsize=0, recovered 3686400 bytes via lseek [osc-dmabuf] mmap fd=23 len=3686400 ok 3686400 is exactly 1280x720x4. The size is recoverable by seeking the fd, which is the documented way, and the mmap then succeeds. Also here: - The resolved length is stored in dmabuf_maps[].len. It used to store the producer's 0, leaving every later bounds check and the munmap on an empty mapping. - The failure reason is carried up instead of assumed, so "no fd", "size unknown" and "driver refused" no longer read as the same thing. - OPENSCREEN_PIPEWIRE_DEBUG traces the negotiated modifier and each mapping. Without it there is no way to tell whether a capture used this path at all. - OPENSCREEN_PIPEWIRE_FORCE_DMABUF drops the shm object so a compositor that offers both is forced onto DMA-BUF. Test-only, and the only way to exercise any of this without the compositor from #287. - Corrects the claim that every Smithay/wlroots compositor lacks a memfd path. sway 1.9 has one and takes it. 56 crate tests pass, enum_format_survives_a_dmabuf_only_producer included. Not verified: whether niri reports maxsize = 0 as wlroots does, so this may not be the whole of #287. And no frames are encoded under nested sway — an empty desktop generates almost no damage — so osc_read_frame's dmabuf arm is still unexercised beyond the mapping itself.
|
Warning Review limit reached
Next review available in: 43 seconds You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Follow-up to #299, whose import half turns out never to have run.
How this was found
#299 fixed the negotiation half — advertising
SPA_FORMAT_VIDEO_modifier— and that half is well covered offline byenum_format_survives_a_dmabuf_only_producer. The import half had no coverage, so I tried to exercise it locally.It cannot be reached by any compositor available here:
uses_dmabuf=0,dataType: MemFd— even after #299uses_dmabuf=0,dataType: MemFdThat is correct behaviour, not a bug: the helper publishes shm first and DMA-BUF second, and
pw_streamtreats the array as a preference list, so any producer able to do shm winsparams[0]. It also means the DMA-BUF branch is unreachable outside the compositor of #287.The bug
Forcing the dmabuf object (
OPENSCREEN_PIPEWIRE_FORCE_DMABUF, added here) gets DMA-BUF negotiated — and the import fails on the first buffer:wlroots reports
maxsize = 0. That is legitimate — the size of a dmabuf is a property of the exporting buffer, not of the SPA descriptor — butosc_map_dmabufrefused on it and returned NULL before calling mmap. The user then saw:The driver was blamed for a refusal it was never asked for. That message is what a bug report from a niri user would have contained, pointing whoever picked it up at the GPU.
The fix, measured
3686400is exactly 1280×720×4 — the size is right there on the fd. Seeking it and retrying:AMD Mendocino / amdgpu, sway 1.9, xdg-desktop-portal-wlr 0.7.1. So CPU mapping does work on this driver; the size was the whole obstacle.
Also in here
dmabuf_maps[].lenused to take the producer's0, leaving every later bounds check and themunmappointed at an empty mapping.OPENSCREEN_PIPEWIRE_DEBUGtraces the negotiated modifier and every mapping. Without it a capture reports identical success whichever path it took, which is why this went unnoticed.OPENSCREEN_PIPEWIRE_FORCE_DMABUFdrops the shm object. Test-only, and the only way to exercise any of this without niri — worth keeping for the same reason it was needed today.What this does not settle
maxsize = 0as wlroots does. If it does not, this is not the whole of [Bug]: Screen recording fails on Niri/Wayland with PipeWire (no more input formats) #287.osc_read_frame's dmabuf arm beyond the mapping: no frames are encoded under nested sway, an empty desktop producing almost no damage. The mapping is proven; the read is not.56 crate tests pass.
🤖 Generated with Claude Code