Repository navigation
Terminate VAAPI display after capability detection - #1497
alan-george-lk wants to merge 6 commits into
Conversation
…o alan/bugfix-remote-room-closure
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Newer findings are available below. Devin Review posted a newer report on this PR, in addition to the findings presented here.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| } | ||
|
|
||
| delete[] entrypoints; | ||
| vaTerminate(va_display); |
There was a problem hiding this comment.
🔴 Encoder receives a terminated VAAPI display
On successful capability detection, vaTerminate invalidates the display that va_open_display_drm returns. init_va then initializes that freed display, so VAAPI encoding can crash or fail.
Learn more
A DRM display is opened by vaGetDisplayDRM and passed through capability detection before VaapiDisplayDrm::Open exposes it to callers. Terminating it here tears down the driver and frees the display context, but va_open_display_drm still returns the same pointer on success. Initialize subsequently passes that pointer to init_va, which calls vaInitialize again; Destroy also uses and terminates it. Those calls operate on an invalid display rather than a fresh one.
Example: With an available H.264 VAAPI render device, factory detection succeeds and terminates display A. Creating and initializing a VAAPI encoder returns display A again and calls vaInitialize(A); the encoder needs a valid, live display instead.
Recommended fix: Keep capability detection's temporary display separate from the encoder's live display. Do not return a terminated handle from va_open_display_drm; arrange for encoder Open() to return a live display and balance each successful initialization with exactly one termination. Add a repeated factory-detection and encode teardown test.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Stack-head lifecycle validation completed on Linux/x64 with an RTX 5090.
|
0a4acda to
6b1b88b
Compare
575274a to
0166c9d
Compare
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Closing to recreate this branch directly on #1473 after removing the NVIDIA PR from the room-lifecycle stack. |
Changeset ✓This PR includes a changeset covering all affected packages:
|
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| self.inner.rtc_engine.publisher_negotiation_needed(); | ||
| } | ||
|
|
||
| remove_result?; |
There was a problem hiding this comment.
🟡 Unpublish error contradicts track removal event
When remove_track fails, unpublish_track emits LocalTrackUnpublished but returns an error. The FFI unpublish callback reports failure while the event reports success.
Learn more
unpublish_track removes the publication before calling the RTC engine. If sender removal fails, it now detaches the track and dispatches LocalTrackUnpublished before returning that error. The FFI unpublish handler waits for this event only on a successful return; an error callback can therefore follow or precede an event that says the track was unpublished. The failure is expected when the peer connection has already closed.
Example: An FFI client requests unpublish just after the server deletes its room. Sender removal fails because the connection is closed, but the SDK emits LocalTrackUnpublished and the request callback returns an error.
Recommended fix: Make the return value and event describe the same outcome. If detaching after a closed connection counts as successful unpublish, return the publication and let the FFI handler preserve event-before-callback ordering. Preserve errors for cases where the sender remains active, and distinguish those cases from an already closed connection.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Stacked directly on #1473. This is the head of the room-deletion lifecycle fix stack.
Summary
vaTerminateafter VAAPI H.264 capability detection succeeds.Validation
MALLOC_ARENA_MAX=1: idle threads remained at 29; RSS was flat from cycles 80-100.