Repository navigation
webrtc-sys: VAAPI memory lifecycle and error handling fixes - #1500
Merged
alan-george-lk merged 4 commits intoOct 7, 2026
Merged
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
alan-george-lk
added this pull request to stack #1501
October 5, 2026 20:05
Contributor
Changeset ✓This PR includes a changeset covering all affected packages:
|
alan-george-lk
changed the title
alan-george-lk
commented
Oct 6, 2026
| return false; | ||
| } | ||
|
|
||
| va_status = vaInitialize(va_display, &major_ver, &minor_ver); |
Contributor
Author
There was a problem hiding this comment.
Context: calling vaInitialize here without a free later in the function was a leak. But calling vaTerminate in this function (which is just checking H264 support) could terminate other valid contexts. So this refactor removes the need for init/terminate entirely, and relegates those operations to Create and Destroy singularly
alan-george-lk
force-pushed
the
alan/bugfix-vaterminate
branch
from
October 6, 2026 17:49
d0aa920 to
af77e09
Compare
xianshijing-lk
approved these changes
Oct 6, 2026
alan-george-lk
force-pushed
the
alan/bugfix-vaterminate
branch
from
October 6, 2026 21:44
af77e09 to
9ea57c0
Compare
alan-george-lk
changed the title
alan-george-lk
changed the title
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
alan-george-lk
force-pushed
the
alan/bugfix-vaterminate
branch
from
October 7, 2026 00:25
9ea57c0 to
411d290
Compare
alan-george-lk
added a commit
that referenced
this pull request
Oct 7, 2026
) ## Joint Summary Stacked/works in junction with #1500. Relationship: - #1473: releases local publications and tracks after remote room deletion, allowing their encoders to be destroyed - #1500: terminates VAAPI displays at the correct lifecycle boundaries, releasing the associated driver resources These in combination fix a fairly hefty memory leak reported by a customer. Their use case is remotely killing a room via server API, which kills the client room locally, and joining another room in the same client process. ## This PR Summary Previously, `unpublish_track` called `remove_track(sender)?` first. When a remote room deletion had already closed the connection, that call errored and returned early, leaving the `publication → track → transceiver` graph attached, which could retain the room session. The patch: - Saves the `remove_track` result instead of returning immediately. - Always detaches the transceiver and clears the publication’s track reference. - Requests renegotiation only when sender removal succeeded. - Returns the original removal error only after cleanup. The added reconnection test publishes a local video track, deletes the room remotely, then verifies both that the publication loses its track and that the room session can actually drop. That directly demonstrates the retained-reference lifecycle issue is resolved. ## Testing/Validation A standalone C++ app was created to reproduce the behavior that runs relevant code in cycles (described below). The SDK is initialized once for the entire run with `MALLOC_ARENA_MAX=1` set to tighten memory measurements. The environment was a Linux desktop with a 5090. For each of the 100 cycles: 1. Create a unique room using `lk --dev room create`. 1. Mint a fresh participant token for that room. 1. Connect one participant. 1. Publish three 1280×720 H.264 tracks, explicitly requiring NVENC. 1. Feed synthetic I420 frames to each track at 30 FPS for one second. 1. Confirm every track encoded and sent data using NVIDIA H264 Encoder. 1. Remotely delete the occupied room using `lk --dev room delete <room name>`. 1. Confirm both the RoomDeleted disconnect reason and room EOS event. 1. Destroy/release the room, participant, tracks, sources, and encoders. 1. Sample idle RSS and thread count before starting the next cycle. Note: The first 20 cycles are treated as warmup, growth is measured from cycle 20 through cycle 100. ### Results Pre-PRs Fresh clean build and 100-cycle run completed on Rust `main` commit `535ead0d`. | Cycle | Idle RSS | Delta from cycle 20 | Threads | Thread delta from cycle 20 | |---:|---:|---:|---:|---:| | 1 | 223.207 MiB | — | 45 | — | | 10 | 251.770 MiB | — | 153 | — | | 20 | 288.160 MiB | 0 | 273 | 0 | | 50 | 388.801 MiB | +100.641 MiB | 633 | +360 | | 80 | 501.766 MiB | +213.606 MiB | 993 | +720 | | 90 | 526.793 MiB | +238.633 MiB | 1,113 | +840 | | 100 | 563.527 MiB | +275.367 MiB | 1,233 | +960 | | Result | Value | |---|---:| | Completed cycles | 100/100 | | NVENC tracks | 300/300 | | RoomDeleted + EOS | 100/100 | | RSS growth, cycles 20–100 | +275.367 MiB | | Average RSS growth | +3.442 MiB/cycle | | Thread growth, cycles 20–100 | +960 | | Average thread growth | +12 threads/cycle | | Test failures | 0 | | Remaining rooms | 0 | ### Results Post-PRs Note: this combines this branch + #1500. | Cycle | Idle RSS | Delta from cycle 20 | Threads | Thread delta from cycle 20 | |---:|---:|---:|---:|---:| | 1 | 157.223 MiB | — | 29 | — | | 10 | 163.199 MiB | — | 29 | — | | 20 | 163.215 MiB | 0 | 29 | 0 | | 50 | 163.402 MiB | +0.187 MiB | 29 | 0 | | 80 | 163.402 MiB | +0.187 MiB | 29 | 0 | | 90 | 163.402 MiB | +0.187 MiB | 29 | 0 | | 100 | 163.402 MiB | +0.187 MiB | 29 | 0 | | Result | Value | |---|---:| | Completed cycles | 100/100 | | NVENC tracks | 300/300 | | RoomDeleted + EOS | 100/100 | | RSS growth, cycles 20–100 | +0.188 MiB | | Average RSS growth | ~0.002 MiB/cycle | | Thread growth, cycles 20–100 | 0 | | Average thread growth | 0 threads/cycle | | Test failures | 0 | | Remaining rooms | 0 | ### Customer Validation I cut a specific C++ SDK branch build against this Rust fix and the customer confirmed this resolved the problem.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Joint Summary
Stacked/works in junction with #1473. Relationship:
See #1473 for full write up and test results.
This PR Summary
vaTerminateduringClose/destruction.This resolves the premature-display-termination issue identified in review while retaining cleanup of the VA driver resources and threads created by capability detection.
Validation
Tested on Linux with an AMD VAAPI device and an RTX 5090:
local_videotest-pattern publishing continuously encoded and sent 329/329 frames in 12 seconds usingVAAPI H264 Encoder. The previous implementation stalled after its first frame.