fix(linux/pipewire): restore dummy buffer for software encoders and fix buffer ownership - #5495
Conversation
e9388b2 to
c70c430
Compare
|
Thanks for the review — and agreed on both the destructor ownership and the The reason kmsgrab and wlgrab work with a no-op // kmsgrab.cpp:1702
img->data = new std::uint8_t[height * img->row_pitch];
// wlgrab.cpp:351
img->data = new std::uint8_t[height * img->row_pitch];So when // pipewire.cpp:939
img->data = nullptr;That's why the same no-op dummy breaks software encoding only on PipeWire To keep One trade-off to be transparent about: that means one extra |
Thanks for clarifying the differences. I wasn't aware that both kmsgrab and wlgrab handle the alloc earlier already. In that case I'm good with your PR changes on Also feel free to mark my earlier comments as resolved (for some reason github won't allow me to do that myself due to missing permissions). P.S.: I'll apply this PR to my personal setup build and do some testing UPDATE: No issues on my system running this so far. |
|
@ReenigneArcher This is (at least according to my testing) good to merge and will restore using software encoding for pipewire-based methods |
c70c430 to
c9e11a8
Compare
Bundle ReportBundle size has no change ✅ |
|
Looks like it's segfaulting during unit tests. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5495 +/- ##
==========================================
+ Coverage 31.59% 32.45% +0.85%
==========================================
Files 102 104 +2
Lines 24621 24968 +347
Branches 10827 11084 +257
==========================================
+ Hits 7780 8103 +323
+ Misses 15783 13934 -1849
- Partials 1058 2931 +1873
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 56 files with indirect coverage changes Continue to review full report in Codecov by Harness.
|
Build artifactsArtifacts from workflow run 32147892152:
You must be signed in to GitHub to download workflow artifacts. |
Screenshot ComparisonPR #5495 screenshots vs Matrix:
|
|
On the unit test segfault in CI: I asked my agent to analyze it, and it found something that seems to explain what's happening: a possible double-free in the new test. As for why I didn't see this locally: the test never crashed on my machine, probably because glibc's tcache hides double-frees (the freed chunk stays reusable), while the CI allocators detect them and abort. It's reproducible locally with What I'm considering: not freeing the frame in the test (the device owns it), and using the real class from @ReenigneArcher I'm confident everything is in order now. Could you please trigger the CI pipeline once more? |
957b086 to
670578e
Compare
670578e to
ec06b9e
Compare
…e encoder The software encoder assumed every captured frame is BGR0 (4 bytes/pixel, single plane). KWin screencast / XDG portal PipeWire captures deliver NV12 (1 byte/pixel row pitch, two planes). Detect the real format from img.row_pitch and recreate the sws context once when needed.
…ership
dummy_img() returned an image with data == nullptr, which makes
sws_scale_frame fail with EINVAL in the software encoder during startup
validation ('Couldn't scale frame' -> 'Unable to find display or encoder').
KMS capture works because its alloc_img() allocates a real buffer; allocate
a black new[] buffer here, marked as owned.
Also fix buffer ownership in img_descriptor_t: the memory-buffer capture
path points img->data at the PipeWire staging vector (front_buffer), owned
by pipewire_t. The destructor freed it with delete[], corrupting the heap
(SIGABRT) once the software-encoder path is reachable in real streams.
data_owned now tracks whether the image owns its buffer.
…frames Validates the source-format detection in avcodec_software_encode_device_t: BGR0 (4 bytes per pixel, KMS/DMABUF layout) and NV12 (1 byte per pixel row pitch, PipeWire captures such as KWin screencast / portal).
ec06b9e to
8ec3d60
Compare
|


































































Description
the problem (
software encoder+KWin/portalcrashes at startup with"Couldn't scale frame"; andSIGABRTduring teardown), the cause (regression fromPR #5360, which removed thedummy_imgallocation and added a destructor withdelete[]despite lacking ownership), and what the PR does (restores the dummy buffer withdata_owned; addsNV12support to the software encoder; adds a unit test).Screenshot
Issues Fixed or Closed
Roadmap Issues
Type of Change
Checklist
AI Usage
See our AI usage policy.