Rework of CEF integration to support broker-mode and accelerated copy

[RyeMutt]

Description

Related Issues

• Please link to a relevant GitHub issue for additional context.
  • Bug Fix: Link to an issue that includes reproduction steps and testing guidance.
  • Feature/Enhancement: Link to an issue with a write-up, rationale, and requirements.

Issue Link:

---

Checklist

Please ensure the following before requesting review:

• I have provided a clear title and detailed description for this pull request.
• If useful, I have included media such as screenshots and video to show off my changes.
• I have tested the changes locally and verified they work as intended.
• All new and existing tests pass.
• Code follows the project's style guidelines.
• Documentation has been updated if needed.
• Any dependent changes have been merged and published in downstream modules
• I have reviewed the contributing guidelines.

---

Additional Notes

[coderabbitai]

Warning

Review limit reached

@RyeMutt, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 43 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ccf6b86b-2ed5-40c3-8405-be57d142eb8f

📥 Commits

Reviewing files that changed from the base of the PR and between 9f830d9 and 8b90472.

📒 Files selected for processing (1)

• indra/media_plugins/cef/CMakeLists.txt

📝 Walkthrough

Walkthrough

Replaces the single SLPlugin dlopen-based launcher with per-plugin statically linked host executables. Adds a shared CEF daemon mode serving multiple browser tabs from one process with bounded crash recovery. Introduces a zero-copy GPU accelerated paint path using D3D11/WGL keyed-mutex on Windows, IOSurface/Mach ports on macOS, and dma-buf/EGL on Linux.

Changes

Static plugin loading and per-plugin host executables

Layer / File(s)	Summary
LLDir plugin path API indra/llfilesystem/lldir.h, indra/llfilesystem/lldir_linux.cpp, indra/llfilesystem/lldir_linux.h, indra/llfilesystem/lldir_mac.h, indra/llfilesystem/lldir_mac.mm, indra/llfilesystem/lldir_win32.cpp, indra/llfilesystem/lldir_win32.h, indra/llfilesystem/tests/lldir_test.cpp, indra/newview/tests/lldir_stub.cpp	Removes getLLPluginLauncher() from all platforms and test stubs; updates getLLPluginFilename() to return a direct per-plugin host executable path (.exe, .app bundle binary, or bare name).
LLPluginInstance static init indra/llplugin/llplugininstance.h, indra/llplugin/llplugininstance.cpp	Removes Boost dll/mDSOHandle and PLUGIN_INIT_FUNCTION_NAME. Adds sStaticInitFunction/setStaticInitFunction(). load() invokes the registered init function directly instead of dlopen-ing a shared library.
slplugin host driver indra/llplugin/slplugin/slplugin.cpp, indra/llplugin/slplugin/slplugin_static.cpp, indra/llplugin/slplugin/CMakeLists.txt	slplugin.cpp adds --daemon arg parsing and delegates to ll_run_slplugin_host(). slplugin_static.cpp provides ll_get_static_plugin_init() and a pass-through ll_run_slplugin_host() for non-CEF executables. SLPlugin CMakeLists is emptied.
Media plugin CMake and packaging indra/media_plugins/cef/CMakeLists.txt, indra/media_plugins/example/CMakeLists.txt, indra/media_plugins/gstreamer10/CMakeLists.txt, indra/media_plugins/libvlc/CMakeLists.txt, indra/newview/CMakeLists.txt, indra/newview/viewer_manifest.py, indra/dullahan, indra/vcpkg.json	All media plugins switch from shared libraries to statically linked host executables. Viewer CMakeLists removes SLPlugin dependency and adds frameworks on macOS. viewer_manifest.py packages per-plugin host executables on all platforms.

CEF shared daemon mode

Layer / File(s)	Summary
LLPluginProcessChild daemon mode indra/llplugin/llpluginprocesschild.h, indra/llplugin/llpluginprocesschild.cpp	Adds setDaemonMode(bool) and mDaemonMode. In daemon mode, destructor deletes mInstance directly instead of exit(0). Socket-error/pipe-close transitions go to STATE_SHUTDOWNREQ instead of STATE_ERROR.
LLPluginProcessParent daemon discover-or-spawn indra/llplugin/llpluginprocessparent.h, indra/llplugin/llpluginprocessparent.cpp	Adds setUseDaemon()/getUseDaemon() and daemon coordination helpers (daemonDiscoverOrRegister, readDaemonControlPort, registerWithDaemon, acquireSpawnLock). STATE_LISTENING discovers a running daemon or spawns one with --daemon. STATE_EXITING and pluginLockedUpOrQuit() are updated for absent mProcess in daemon mode.
slplugin_daemon.cpp: multi-tab daemon loop indra/llplugin/slplugin/slplugin_daemon.cpp	Implements slplugin_daemon_run(): TCP control listener, rendezvous file publishing, initial tab spawn, accept/idle/cleanup loop with DAEMON_MAX_TABS ceiling and DAEMON_IDLE_TIMEOUT exit.
Viewer daemon routing and crash recovery indra/llplugin/llpluginclassmedia.h, indra/llplugin/llpluginclassmedia.cpp, indra/newview/llviewermedia.h, indra/newview/llviewermedia.cpp, indra/newview/app_settings/settings_alchemy.xml	LLPluginClassMedia gains setUseDaemon() and setDisplayServer(); sends daemon/display fields in init. LLViewerMediaImpl wires setUseDaemon() from ALCefDaemonEnabled, adds rendezvous file cleanup on shutdown, and implements bounded exponential backoff recovery on MEDIA_EVENT_PLUGIN_FAILED.

CEF accelerated zero-copy paint

Layer / File(s)	Summary
GL/EGL/D3D11 interop extension loading indra/llrender/llglheaders.h, indra/llrender/llgl.cpp, indra/llwindow/lldxhardware.h, indra/llwindow/lldxhardware.cpp, indra/llwindow/llwindowwin32.cpp	Declares WGL_NV_DX_interop/interop2 and EGL_KHR_image/GL_OES_EGL_image function pointers. Loads them in initWGL()/initEGL(). Adds initGLDXInterop()/cleanupGLDXInterop() to LLDXHardware and calls them in Win32 window close/switchContext.
Display server reporting for Linux Ozone indra/llwindow/llwindow.h, indra/llwindow/llwindowsdl.h, indra/llwindow/llwindowsdl.cpp	Adds virtual getDisplayServer() to LLWindow. LLWindowSDL implements it returning "wayland" or "x11" from mServerProtocol.
media_plugin_cef accelerated paint producers indra/media_plugins/cef/media_plugin_cef.cpp, indra/llplugin/llpluginclassmedia.cpp, indra/llplugin/llpluginclassmedia.h	Adds CefAccelProducer (Windows D3D11 keyed-mutex + NT handle dup), CefMacSurfaceSender (IOSurface Mach port), CefLinuxSurfaceSender (dma-buf SCM_RIGHTS). LLPluginClassMedia adds accel id assignment, sends accelerated paint fields in init, and handles incoming "accelerated_paint" messages with dirty flag/MEDIA_EVENT_CONTENT_UPDATED.
LLCEFSurfaceReceiver: side-channel receiver indra/newview/llcefsurfacereceiver.h, indra/newview/llcefsurfacereceiver.cpp	Singleton that drains macOS Mach port messages to retrieve IOSurfaceRef per accel_id (takeLatest), or Linux UNIX datagram messages with SCM_RIGHTS to retrieve DmabufFrame per accel_id (takeLatestDmabuf). Other platforms: stubs.
LLCEFAccelInterop: GL texture import and blit indra/newview/llcefaccelinterop.h, indra/newview/llcefaccelinterop.cpp	Windows: opens shared D3D11 texture via WGL_NV_DX_interop, copies under keyed mutex, Y-flip blits. macOS: binds IOSurfaceRef to GL_TEXTURE_RECTANGLE_ARB, blits. Linux: creates EGLImageKHR from dma-buf planes, blits. Other platforms: stubs.
LLViewerMediaImpl accelerated paint consumer indra/newview/llviewermedia.h, indra/newview/llviewermedia.cpp	update() adds an early-return accelerated path calling updateAcceleratedTexture(). The new method lazily inits LLCEFAccelInterop, imports the newest platform surface, and blits it to the media GL texture. Destructor and destroyMediaSource() tear down interop state.
CEF host entry and Windows bootstrap indra/media_plugins/cef/slplugin_cef.cpp, indra/media_plugins/cef/slplugin_cef_bootstrap.cpp, indra/media_plugins/cef/SLPluginCEF-Info.plist.in, indra/llplugin/slplugin/slplugin_info.plist	slplugin_cef.cpp sets persistent CEF runtime and dispatches to single-tab or daemon loop. slplugin_cef_bootstrap.cpp implements RunWinMain for Windows CEF subprocess dispatch and sandbox control. macOS plist templates added/updated.

Sequence Diagram

sequenceDiagram
  rect rgba(100, 149, 237, 0.5)
    Note over Viewer,Daemon: Daemon Discovery/Spawn
  end
  participant Viewer as LLViewerMediaImpl
  participant Parent as LLPluginProcessParent
  participant Daemon as media_plugin_cef (daemon)
  participant Child as LLPluginProcessChild

  Viewer->>Parent: setUseDaemon(true, rendezvous_path)
  Parent->>Parent: daemonDiscoverOrRegister()
  alt daemon not running
    Parent->>Daemon: spawn --daemon rendezvous_path port
    Daemon->>Daemon: write port to rendezvous file
  else daemon running
    Parent->>Daemon: TCP register(parent_port)
  end
  Daemon->>Child: spawn LLPluginProcessChild(daemon_mode)

  rect rgba(144, 238, 144, 0.5)
    Note over Viewer,Child: Accelerated Paint Frame
  end
  Child->>Child: CEF onPaint → CefAccelProducer/Sender
  Child->>Viewer: side channel (Mach port / NT handle / SCM_RIGHTS)
  Child->>Parent: accelerated_paint message (accel_id, handle, dims)
  Parent->>Viewer: MEDIA_EVENT_CONTENT_UPDATED
  Viewer->>Viewer: updateAcceleratedTexture()
  Viewer->>Viewer: LLCEFAccelInterop::blitTo(dst_tex)

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Poem

🐇 Hop hop, no more dlopen spell,
Each plugin now its own brave shell!
The daemon shares one CEF brain,
While GPU frames skip the CPU lane.
Zero copies, fast as light—
The rabbit ships shared textures right! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is just the template and contains no actual PR summary, issue link, checklist status, or additional notes.	Replace the template placeholders with a concrete change summary, linked issue, completed checklist items, and any relevant notes.
Docstring Coverage	⚠️ Warning	Docstring coverage is 42.11% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the main changes by calling out CEF integration work and accelerated paths, though 'broker-mode' is slightly imprecise.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 16

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

indra/newview/llviewermedia.cpp (1)

1787-1793: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Tear down the accelerated binding when the media source is destroyed.

Resetting only mAccelBoundHandle leaves mAccelInterop holding the previous source’s platform texture; the next accelerated dirty event can blit stale/invalid content before a fresh bind arrives.

Proposed fix

     // The plugin's shared texture is going away; force a fresh interop bind when
     // the next media source delivers a handle (even if the value is reused).
     mAccelBoundHandle = 0;
+    if (mAccelInterop)
+    {
+        mAccelInterop->shutdown();
+        delete mAccelInterop;
+        mAccelInterop = nullptr;
+    }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@indra/newview/llviewermedia.cpp` around lines 1787 - 1793, The
destroyMediaSource teardown in LLViewerMediaImpl::destroyMediaSource currently
clears mAccelBoundHandle but leaves mAccelInterop pointing at the old plugin
texture, which can allow stale accelerated content to be used. Update the
destroyMediaSource cleanup to fully reset the accelerated binding state by
clearing mAccelInterop as well as the existing handle reset, so the next dirty
event forces a clean rebind to the new media source.

🧹 Nitpick comments (3)

indra/media_plugins/cef/media_plugin_cef.cpp (1)

425-435: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Move the side-channel frame ABI into one shared definition.

The Mach and dma-buf payload structs are duplicated here and again in indra/newview/llcefsurfacereceiver.cpp. Because this is a cross-process ABI, a future field-order or packing drift will fail only at runtime. A shared header plus static_assert(sizeof(...)) would make this contract much safer.

Also applies to: 515-525

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@indra/media_plugins/cef/media_plugin_cef.cpp` around lines 425 - 435, Move
the side-channel frame ABI into a single shared definition so the sender and
receiver stay in lockstep. The duplicated Mach/dma-buf payload structs in
media_plugin_cef.cpp and LLCEFSurfaceReceiver should be replaced by a common
header or shared type used by both CefSurfaceSendMsg and the receiver-side
equivalents, and add size/packing checks with static_assert to guard the
cross-process contract. Keep the existing wire-format field order and reference
the shared ABI from both send/receive paths rather than maintaining two copies.

indra/newview/llcefaccelinterop.h (1)

5-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Update the public contract comments to match the implemented platforms.

The implementation expects a +1 IOSurfaceRef on macOS and already-open SCM_RIGHTS dma-buf fds on Linux; the header still documents IOSurfaceID, /proc/<pid>/fd, Windows-only support, and caller-side swizzling.

Proposed documentation fix

- * Opens the plugin's keyed-mutex shared texture (delivered once per size as a
- * duplicated NT handle), copies each frame into a viewer-owned texture under the
- * mutex, and blits that into a media GL texture - no CPU round-trip. Windows
- * only (D3D11 + WGL_NV_DX_interop2); a stub elsewhere.
+ * Opens the plugin's platform shared surface, copies/imports it into a
+ * viewer-owned GL texture, and blits that into a media GL texture - no CPU
+ * round-trip. Implemented on Windows (D3D11 + WGL_NV_DX_interop2), macOS
+ * (IOSurface), and Linux (dma-buf/EGL); stubbed elsewhere.
@@
-    // handle on Windows (NT handle) / macOS (IOSurfaceID). On Linux the frame is a
-    // dma-buf: `handle` is plane 0's fd number in process `src_pid` (opened via
-    // /proc/<src_pid>/fd/<fd>) and the layout is given by format / modifier plus
+    // handle on Windows (NT handle). On macOS, `handle` is a +1-retained
+    // IOSurfaceRef delivered out-of-band by LLCEFSurfaceReceiver. On Linux the
+    // frame is a dma-buf whose plane fds are already open in the viewer process
+    // via SCM_RIGHTS; `src_pid` is ignored and the layout is given by format / modifier plus
@@
-    // Copy the latest plugin frame into dst_tex (a GL_RGBA8 texture). The data is
-    // BGRA-ordered, so the caller should sample dst_tex with an R<->B swizzle.
+    // Copy/import the latest plugin frame into dst_tex (a GL_RGBA8 texture).
+    // The implementation handles channel order and Y-flip during the blit.

Also applies to: 47-65

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@indra/newview/llcefaccelinterop.h` around lines 5 - 8, The public contract
comment in llcefaccelinterop.h is stale and should be updated to match the
actual supported platforms and ownership rules. Revise the documentation around
the relevant interface symbols to describe the implemented macOS path as taking
a +1 IOSurfaceRef, the Linux path as consuming already-open SCM_RIGHTS dma-buf
file descriptors, and remove references to IOSurfaceID, /proc/<pid>/fd,
Windows-only support, and caller-side swizzling. Keep the comment aligned with
the current behavior of the related interop entry points so the header
accurately describes what callers must provide.

indra/newview/llcefaccelinterop.cpp (1)

229-244: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Check framebuffer completeness before reporting a successful blit.

All three platform paths return true after glBlitFramebuffer without verifying the read/draw FBOs are complete. Add glCheckFramebufferStatus checks so accelerated paint can fall back instead of silently presenting stale/blank media.

Also applies to: 367-380, 603-616

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@indra/newview/llcefaccelinterop.cpp` around lines 229 - 244, The accelerated
blit paths in llcefaccelinterop.cpp return success from the framebuffer copy
without verifying the FBOs are complete. In the code around the
glBlitFramebuffer usage (including the related platform-specific paths mentioned
in the comment), add glCheckFramebufferStatus checks for the read and draw
framebuffer setup before treating the blit as successful, and make the function
fall back instead of returning true when completeness fails. Use the existing
blit helpers and platform-path logic in this file to keep the check consistent
across all three paths.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@indra/llplugin/llpluginprocessparent.cpp`:
- Around line 1390-1417: The daemon registration path in
LLPluginProcessParent::registerWithDaemon accepts a U32 control_port but casts
it directly to apr_port_t, which can wrap if the rendezvous file contains an
out-of-range value. Add a bounds check before apr_sockaddr_info_get() so only
valid TCP ports (1-65535) are accepted, and return false for invalid values.
Keep the validation in registerWithDaemon so the bad port is rejected before any
socket connection attempt.
- Around line 539-548: The daemon launch path in LLPluginProcessParent::setState
handling ignores the return from LLProcess::create(params), so failures leave
the parent waiting indefinitely and holding the spawn lock. Update this launch
block to check the create result, and on failure immediately reset the launch
state/flags and perform the same cleanup/error handling used by the other
process creation paths in LLPluginProcessParent.

In `@indra/llplugin/slplugin/slplugin_daemon.cpp`:
- Around line 129-140: The registration read in slplugin_daemon.cpp is parsing
whatever bytes arrive from apr_socket_recv() once, which can be only a partial
TCP segment; update the socket-read logic in the daemon’s registration handling
to keep reading into an accumulator until a full newline-terminated line is
received or the timeout expires, then trim and pass that complete string to
LLStringUtil::convertToU32(). Keep the fix localized around the incoming socket
receive block so the port is only accepted from a complete registration line.
- Around line 190-197: The rendezvous publication in slplugin_daemon.cpp should
be verified before the spawn lock is removed. In the code that writes the
control port via llofstream rv and then calls
LLFile::remove(rendezvous_lock_path), add a success check for opening/writing
the rendezvous file and only release the lock after confirming the publish
succeeded; if it fails, log an error and keep the lock behavior consistent so
other parents do not spawn a duplicate daemon. Use the existing rendezvous_path,
rendezvous_lock_path, and control_port flow to locate the fix.

In `@indra/llrender/llgl.cpp`:
- Around line 1106-1116: Guard the eglQueryString function pointer before using
it in the EGL init path: the lookup via SDL_EGL_GetProcAddress("eglQueryString")
can fail, and the first RenderInit log calls use it immediately. Update the
llgl.cpp initialization around eglQueryString so it checks for a valid pointer
before calling it, and fall back to the same optional/skip behavior used by
reloadExtensionsString() when the symbol is unavailable.

In `@indra/llwindow/lldxhardware.cpp`:
- Around line 611-615: The interop capability check is too permissive and can
report success even when the accelerated-paint path’s required callback is
missing. Update hasGLDXInterop() to gate on the same WGL DX interop callbacks
that LLCEFAccelInterop::blitTo() actually uses, not just wglDXOpenDeviceNV from
initGLDXInterop(). Make sure the check fails early when wglDXUnlockObjectsNV
(and any other required callbacks) is unavailable so the path is disabled up
front instead of failing later.

In `@indra/media_plugins/cef/CMakeLists.txt`:
- Around line 94-95: Fix the macOS bundle identifier typo in the CEF plugin
CMake configuration: both MACOSX_BUNDLE_GUI_IDENTIFIER and
XCODE_ATTRIBUTE_PRODUCT_BUNDLE_IDENTIFIER currently use org.achemyviewer...,
which should be corrected to the proper alchemyviewer spelling so the generated
Info.plist and bundle identity stay consistent. Update the identifiers in the
CMakeLists entry for the CEF bundle settings.

In `@indra/media_plugins/cef/media_plugin_cef.cpp`:
- Around line 138-145: Treat a failed keyed-mutex copy as a dropped frame in
onAcceleratedPaintCallback(): currently the code returns true even when mMutex
is null or AcquireSync() fails, which can emit accelerated_paint for an
uninitialized recreated texture. Update the copy path so success is returned
only when mContext->CopyResource and the mutex handoff both complete; otherwise
release cef and return false, and apply the same logic to the duplicated block
mentioned in the review.

In `@indra/media_plugins/cef/slplugin_cef_bootstrap.cpp`:
- Around line 103-114: Preserve Unicode in the lpCmdLine handling inside
slplugin_cef_bootstrap.cpp: the current loop in the RunWinMain command-line
parsing path truncates non-ASCII characters when building cmd, which breaks
localized rendezvous paths. Replace the manual wide-to-char cast with a proper
UTF-16 conversion using WideCharToMultiByte, or keep the command line wide
through the parsing logic, and make sure the resulting value is still trimmed
and passed through the existing command-line handling flow unchanged otherwise.

In `@indra/newview/app_settings/settings_alchemy.xml`:
- Around line 8-14: The CEF feature settings are inconsistent because the
comments say “off by default” while the Boolean defaults in the
ALCefAcceleratedPaint, ALCefDaemonEnabled, ALCefDedicatedHost, and ALCefSandbox
entries are enabled. Update the settings in settings_alchemy.xml so the default
Value matches the comments by setting these feature defaults to off, or revise
the comments if the intended rollout is enabled by default.

In `@indra/newview/llcefaccelinterop.cpp`:
- Around line 546-568: The accelerated paint import path in
llcefaccelinterop::releaseImage/eglCreateImageKHR currently destroys the
existing Linux EGLImage before confirming the new dma-buf import succeeds, so a
failed frame can leave blitTo() with nothing usable. Change the import flow to
create the new EGLImage in a temporary handle first, and only call
releaseImage() or replace l->image after eglCreateImageKHR succeeds; if it
fails, keep the previous image intact and return false while preserving the last
good binding.
- Around line 195-199: The AcquireSync call in llcefaccelinterop.cpp currently
waits up to 1000 ms inside the viewer GL update path, which can stall rendering
when the producer is busy. Update the logic in the relevant sync/acquire section
around w->mutex->AcquireSync so it uses a non-blocking or very short timeout and
immediately returns false to retry on the next frame, preserving the existing
“try again next” behavior without blocking the viewer thread.
- Around line 134-135: The Windows shared-texture binding path in
llcefaccelinterop should fail fast when the texture does not expose
IDXGIKeyedMutex instead of silently continuing. Update the binding logic around
QueryInterface on the stable texture (and any caller that proceeds with
CopyResource) so it checks the keyed-mutex result, returns an error/false, and
aborts the bind when w->mutex is unavailable; keep the normal keyed-mutex path
unchanged.
- Around line 328-339: The macOS IOSurface binding in blitTo() is replacing
m->surface before confirming CGLTexImageIOSurface2D succeeded, which can leave a
stale surface/texture state on failure. Update llcefaccelinterop.cpp’s surface
update path so the new surf is only assigned to m->surface after the bind call
returns kCGLNoError, and keep the previous surface/metadata intact if binding
fails. Make sure any cleanup or release of the old surface happens only after a
successful bind, using the existing m->surface, m->width, and m->height members
as the state to preserve.

In `@indra/newview/llviewermedia.cpp`:
- Around line 3069-3082: The accelerated paint path in llviewermedia.cpp can
leave media blank when LLCEFAccelInterop::init() fails because update() returns
early after updateAcceleratedTexture() without falling back to CPU upload.
Update the logic around mMediaSource->getUseAcceleratedPaint(),
updateAcceleratedTexture(), and the later CPU paint path so that failed interop
either disables/recreates accelerated mode and allows CPU painting, or only
enables accelerated paint after a successful capability preflight; apply the
same fix in both affected update branches.
- Around line 3265-3317: The macOS/Linux paths in llviewermedia.cpp are ignoring
setStableTexture() import failures and still proceeding to blitTo(), which can
leave stale content visible. Update the platform-specific binding logic in the
media update path around LLCEFSurfaceReceiver::instance().takeLatest() /
takeLatestDmabuf() so that a failed mAccelInterop->setStableTexture() causes an
immediate return false, matching the Windows behavior and preventing fallback to
the previous texture binding.

---

Outside diff comments:
In `@indra/newview/llviewermedia.cpp`:
- Around line 1787-1793: The destroyMediaSource teardown in
LLViewerMediaImpl::destroyMediaSource currently clears mAccelBoundHandle but
leaves mAccelInterop pointing at the old plugin texture, which can allow stale
accelerated content to be used. Update the destroyMediaSource cleanup to fully
reset the accelerated binding state by clearing mAccelInterop as well as the
existing handle reset, so the next dirty event forces a clean rebind to the new
media source.

---

Nitpick comments:
In `@indra/media_plugins/cef/media_plugin_cef.cpp`:
- Around line 425-435: Move the side-channel frame ABI into a single shared
definition so the sender and receiver stay in lockstep. The duplicated
Mach/dma-buf payload structs in media_plugin_cef.cpp and LLCEFSurfaceReceiver
should be replaced by a common header or shared type used by both
CefSurfaceSendMsg and the receiver-side equivalents, and add size/packing checks
with static_assert to guard the cross-process contract. Keep the existing
wire-format field order and reference the shared ABI from both send/receive
paths rather than maintaining two copies.

In `@indra/newview/llcefaccelinterop.cpp`:
- Around line 229-244: The accelerated blit paths in llcefaccelinterop.cpp
return success from the framebuffer copy without verifying the FBOs are
complete. In the code around the glBlitFramebuffer usage (including the related
platform-specific paths mentioned in the comment), add glCheckFramebufferStatus
checks for the read and draw framebuffer setup before treating the blit as
successful, and make the function fall back instead of returning true when
completeness fails. Use the existing blit helpers and platform-path logic in
this file to keep the check consistent across all three paths.

In `@indra/newview/llcefaccelinterop.h`:
- Around line 5-8: The public contract comment in llcefaccelinterop.h is stale
and should be updated to match the actual supported platforms and ownership
rules. Revise the documentation around the relevant interface symbols to
describe the implemented macOS path as taking a +1 IOSurfaceRef, the Linux path
as consuming already-open SCM_RIGHTS dma-buf file descriptors, and remove
references to IOSurfaceID, /proc/<pid>/fd, Windows-only support, and caller-side
swizzling. Keep the comment aligned with the current behavior of the related
interop entry points so the header accurately describes what callers must
provide.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 47362874-2ea6-4681-aa58-ad9ab399d3d6

📥 Commits

Reviewing files that changed from the base of the PR and between cee877a and 9f830d9.

📒 Files selected for processing (50)

• indra/dullahan
• indra/llfilesystem/lldir.h
• indra/llfilesystem/lldir_linux.cpp
• indra/llfilesystem/lldir_linux.h
• indra/llfilesystem/lldir_mac.h
• indra/llfilesystem/lldir_mac.mm
• indra/llfilesystem/lldir_win32.cpp
• indra/llfilesystem/lldir_win32.h
• indra/llfilesystem/tests/lldir_test.cpp
• indra/llplugin/llpluginclassmedia.cpp
• indra/llplugin/llpluginclassmedia.h
• indra/llplugin/llplugininstance.cpp
• indra/llplugin/llplugininstance.h
• indra/llplugin/llpluginprocesschild.cpp
• indra/llplugin/llpluginprocesschild.h
• indra/llplugin/llpluginprocessparent.cpp
• indra/llplugin/llpluginprocessparent.h
• indra/llplugin/slplugin/CMakeLists.txt
• indra/llplugin/slplugin/slplugin.cpp
• indra/llplugin/slplugin/slplugin_daemon.cpp
• indra/llplugin/slplugin/slplugin_info.plist
• indra/llplugin/slplugin/slplugin_static.cpp
• indra/llrender/llgl.cpp
• indra/llrender/llglheaders.h
• indra/llwindow/lldxhardware.cpp
• indra/llwindow/lldxhardware.h
• indra/llwindow/llwindow.h
• indra/llwindow/llwindowsdl.cpp
• indra/llwindow/llwindowsdl.h
• indra/llwindow/llwindowwin32.cpp
• indra/media_plugins/cef/CMakeLists.txt
• indra/media_plugins/cef/SLPluginCEF-Info.plist.in
• indra/media_plugins/cef/media_plugin_cef.cpp
• indra/media_plugins/cef/slplugin_cef.cpp
• indra/media_plugins/cef/slplugin_cef_bootstrap.cpp
• indra/media_plugins/example/CMakeLists.txt
• indra/media_plugins/gstreamer10/CMakeLists.txt
• indra/media_plugins/libvlc/CMakeLists.txt
• indra/newview/CMakeLists.txt
• indra/newview/app_settings/settings_alchemy.xml
• indra/newview/llcefaccelinterop.cpp
• indra/newview/llcefaccelinterop.h
• indra/newview/llcefsurfacereceiver.cpp
• indra/newview/llcefsurfacereceiver.h
• indra/newview/llsyntaxid.cpp
• indra/newview/llviewermedia.cpp
• indra/newview/llviewermedia.h
• indra/newview/tests/lldir_stub.cpp
• indra/newview/viewer_manifest.py
• indra/vcpkg.json

💤 Files with no reviewable changes (5)

• indra/llfilesystem/lldir_mac.h
• indra/llfilesystem/lldir_linux.h
• indra/llfilesystem/lldir_win32.h
• indra/newview/tests/lldir_stub.cpp
• indra/llfilesystem/tests/lldir_test.cpp

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Handle daemon launch failures immediately.

LLProcess::create(params) can fail, as the nearby launch paths already handle. Ignoring the return leaves this parent waiting for a daemon callback that will never arrive and keeps the spawn lock until it becomes stale.

Proposed fix

-                        LLProcess::create(params);   // fire and forget; keep no mProcess
+                        LLProcessPtr daemon_process = LLProcess::create(params);
+                        if (!daemon_process)
+                        {
+                            LL_WARNS("Plugin") << "failed to launch CEF daemon" << LL_ENDL;
+                            LLFile::remove(daemonRendezvousPath() + ".lock");
+                            mProcessCreationRequested = false;
+                            errorState();
+                            break;
+                        }
+                        // fire and forget; params.attached=false means dropping
+                        // this handle does not terminate the daemon.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	LLProcess::Params params = mProcessParams;
	params.args.add(stringize(mBoundPort));
	params.args.add("--daemon");
	params.args.add(daemonRendezvousPath());
	params.attached = false;
	LLProcess::create(params); // fire and forget; keep no mProcess
	mProcessCreationRequested = true;
	mHeartbeat.start();
	mHeartbeat.setTimerExpirySec(mPluginLaunchTimeout);
	setState(STATE_LAUNCHED);
	LLProcess::Params params = mProcessParams;
	params.args.add(stringize(mBoundPort));
	params.args.add("--daemon");
	params.args.add(daemonRendezvousPath());
	params.attached = false;
	LLProcessPtr daemon_process = LLProcess::create(params);
	if (!daemon_process)
	{
	LL_WARNS("Plugin") << "failed to launch CEF daemon" << LL_ENDL;
	LLFile::remove(daemonRendezvousPath() + ".lock");
	mProcessCreationRequested = false;
	errorState();
	break;
	}
	// fire and forget; params.attached=false means dropping
	// this handle does not terminate the daemon.
	mProcessCreationRequested = true;
	mHeartbeat.start();
	mHeartbeat.setTimerExpirySec(mPluginLaunchTimeout);
	setState(STATE_LAUNCHED);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@indra/llplugin/llpluginprocessparent.cpp` around lines 539 - 548, The daemon
launch path in LLPluginProcessParent::setState handling ignores the return from
LLProcess::create(params), so failures leave the parent waiting indefinitely and
holding the spawn lock. Update this launch block to check the create result, and
on failure immediately reset the launch state/flags and perform the same
cleanup/error handling used by the other process creation paths in
LLPluginProcessParent.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject invalid daemon control ports before casting.

A corrupt/stale rendezvous file can contain a value above 65535; casting that U32 to apr_port_t can wrap and register with the wrong localhost port.

Proposed fix

     U32 port = 0;
-    f >> port;
+    if (!(f >> port) || port > 65535)
+    {
+        return 0;
+    }
     return port;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	U32 port = 0;
	f >> port;
	return port;
	}
	bool LLPluginProcessParent::registerWithDaemon(U32 control_port)
	{
	if (!control_port)
	{
	return false;
	}
	apr_socket_t* sock = nullptr;
	if (apr_socket_create(&sock, APR_INET, SOCK_STREAM, APR_PROTO_TCP, gAPRPoolp) != APR_SUCCESS)
	{
	return false;
	}
	apr_socket_timeout_set(sock, 2 * APR_USEC_PER_SEC);
	bool ok = false;
	apr_sockaddr_t* addr = nullptr;
	if (apr_sockaddr_info_get(&addr, "127.0.0.1", APR_INET, (apr_port_t)control_port, 0, gAPRPoolp) == APR_SUCCESS &&
	apr_socket_connect(sock, addr) == APR_SUCCESS)
	{
	// Tell the daemon which port to connect back to for this tab.
	std::string msg = stringize(mBoundPort) + "\n";
	apr_size_t len = msg.size();
	ok = (apr_socket_send(sock, msg.data(), &len) == APR_SUCCESS) && (len == msg.size());
	U32 port = 0;
	if (!(f >> port) || port > 65535)
	{
	return 0;
	}
	return port;
	}
	bool LLPluginProcessParent::registerWithDaemon(U32 control_port)
	{
	if (!control_port)
	{
	return false;
	}
	apr_socket_t* sock = nullptr;
	if (apr_socket_create(&sock, APR_INET, SOCK_STREAM, APR_PROTO_TCP, gAPRPoolp) != APR_SUCCESS)
	{
	return false;
	}
	apr_socket_timeout_set(sock, 2 * APR_USEC_PER_SEC);
	bool ok = false;
	apr_sockaddr_t* addr = nullptr;
	if (apr_sockaddr_info_get(&addr, "127.0.0.1", APR_INET, (apr_port_t)control_port, 0, gAPRPoolp) == APR_SUCCESS &&
	apr_socket_connect(sock, addr) == APR_SUCCESS)
	{
	// Tell the daemon which port to connect back to for this tab.
	std::string msg = stringize(mBoundPort) + "\n";
	apr_size_t len = msg.size();
	ok = (apr_socket_send(sock, msg.data(), &len) == APR_SUCCESS) && (len == msg.size());

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@indra/llplugin/llpluginprocessparent.cpp` around lines 1390 - 1417, The
daemon registration path in LLPluginProcessParent::registerWithDaemon accepts a
U32 control_port but casts it directly to apr_port_t, which can wrap if the
rendezvous file contains an out-of-range value. Add a bounds check before
apr_sockaddr_info_get() so only valid TCP ports (1-65535) are accepted, and
return false for invalid values. Keep the validation in registerWithDaemon so
the bad port is rejected before any socket connection attempt.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Read the full newline-terminated registration before parsing.

apr_socket_recv() can return a partial TCP segment; accepting a trimmed partial port can connect the tab to the wrong local port or fail registration. Accumulate until \n or timeout before convertToU32().

Proposed fix

-        char buf[32] = {0};
-        apr_size_t len = sizeof(buf) - 1;
-        if (apr_socket_recv(incoming, buf, &len) == APR_SUCCESS && len > 0)
+        std::string s;
+        char buf[32] = {0};
+        while (s.find('\n') == std::string::npos && s.size() < sizeof(buf) - 1)
         {
-            buf[len] = '\0';
-            std::string s(buf);
+            apr_size_t len = sizeof(buf) - 1 - s.size();
+            if (apr_socket_recv(incoming, buf, &len) != APR_SUCCESS || len == 0)
+            {
+                break;
+            }
+            s.append(buf, len);
+        }
+        if (s.find('\n') != std::string::npos)
+        {
             LLStringUtil::trim(s);
             U32 parent_port = 0;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// Blocking read of the short port line (the registration is tiny).
	apr_socket_timeout_set(incoming, 1000000); // 1s
	char buf[32] = {0};
	apr_size_t len = sizeof(buf) - 1;
	if (apr_socket_recv(incoming, buf, &len) == APR_SUCCESS && len > 0)
	{
	buf[len] = '\0';
	std::string s(buf);
	LLStringUtil::trim(s);
	U32 parent_port = 0;
	if (!s.empty() && LLStringUtil::convertToU32(s, parent_port) && parent_port)
	{
	// Blocking read of the short port line (the registration is tiny).
	apr_socket_timeout_set(incoming, 1000000); // 1s
	std::string s;
	char buf[32] = {0};
	while (s.find('\n') == std::string::npos && s.size() < sizeof(buf) - 1)
	{
	apr_size_t len = sizeof(buf) - 1 - s.size();
	if (apr_socket_recv(incoming, buf, &len) != APR_SUCCESS || len == 0)
	{
	break;
	}
	s.append(buf, len);
	}
	if (s.find('\n') != std::string::npos)
	{
	LLStringUtil::trim(s);
	U32 parent_port = 0;
	if (!s.empty() && LLStringUtil::convertToU32(s, parent_port) && parent_port)
	{

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@indra/llplugin/slplugin/slplugin_daemon.cpp` around lines 129 - 140, The
registration read in slplugin_daemon.cpp is parsing whatever bytes arrive from
apr_socket_recv() once, which can be only a partial TCP segment; update the
socket-read logic in the daemon’s registration handling to keep reading into an
accumulator until a full newline-terminated line is received or the timeout
expires, then trim and pass that complete string to
LLStringUtil::convertToU32(). Keep the fix localized around the incoming socket
receive block so the port is only accepted from a complete registration line.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Verify rendezvous publication before releasing the spawn lock.

If the file open/write fails, the daemon runs undiscoverable after Line 196 removes the lock; later parents can spawn additional daemons instead of registering with this one.

Proposed fix

-    {
-        llofstream rv(rendezvous_path, std::ios::trunc);
-        rv << control_port << std::endl;
-    }
+    bool rendezvous_published = false;
+    {
+        llofstream rv(rendezvous_path, std::ios::trunc);
+        if (rv)
+        {
+            rv << control_port << std::endl;
+            rv.close();
+            rendezvous_published = rv.good();
+        }
+    }
+    if (!rendezvous_published)
+    {
+        LL_WARNS("slplugin") << "daemon: failed to publish rendezvous at "
+                             << rendezvous_path << "; serving a single tab" << LL_ENDL;
+        LLFile::remove(rendezvous_lock_path);
+        apr_socket_close(control);
+        return slplugin_run(first_port);
+    }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	{
	llofstream rv(rendezvous_path, std::ios::trunc);
	rv << control_port << std::endl;
	}
	// The rendezvous is now published, so release the spawn lock the launching
	// parent took - other parents can stop waiting and register.
	LLFile::remove(rendezvous_lock_path);
	LL_INFOS("slplugin") << "daemon: control port " << control_port
	bool rendezvous_published = false;
	{
	llofstream rv(rendezvous_path, std::ios::trunc);
	if (rv)
	{
	rv << control_port << std::endl;
	rv.close();
	rendezvous_published = rv.good();
	}
	}
	if (!rendezvous_published)
	{
	LL_WARNS("slplugin") << "daemon: failed to publish rendezvous at "
	<< rendezvous_path << "; serving a single tab" << LL_ENDL;
	LLFile::remove(rendezvous_lock_path);
	apr_socket_close(control);
	return slplugin_run(first_port);
	}
	// The rendezvous is now published, so release the spawn lock the launching
	// parent took - other parents can stop waiting and register.
	LLFile::remove(rendezvous_lock_path);
	LL_INFOS("slplugin") << "daemon: control port " << control_port

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@indra/llplugin/slplugin/slplugin_daemon.cpp` around lines 190 - 197, The
rendezvous publication in slplugin_daemon.cpp should be verified before the
spawn lock is removed. In the code that writes the control port via llofstream
rv and then calls LLFile::remove(rendezvous_lock_path), add a success check for
opening/writing the rendezvous file and only release the lock after confirming
the publish succeeded; if it fails, log an error and keep the lock behavior
consistent so other parents do not spawn a duplicate daemon. Use the existing
rendezvous_path, rendezvous_lock_path, and control_port flow to locate the fix.

[coderabbitai]

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

For SDL3 on Wayland/EGL, does SDL_EGL_GetProcAddress("eglQueryString") always return a valid pointer for core EGL symbols, or should callers null-check before invoking it?

💡 Result:

You should always null-check the return value of SDL_EGL_GetProcAddress. While SDL_EGL_GetProcAddress is designed to provide access to EGL library functions, it does not guarantee that the returned pointer will be valid in all circumstances [1][2]. Specifically: 1. Dynamic Loading and Backend Support: SDL_EGL_GetProcAddress relies on the underlying EGL implementation and whether an EGL library has been successfully loaded for the current video backend [3][4]. If the video driver is not initialized or if the requested function is unavailable, the function can return NULL [3][4]. 2. EGL Version Constraints: The underlying mechanism used by SDL—often involving eglGetProcAddress—is subject to EGL version constraints [3][5]. For EGL versions earlier than 1.5, eglGetProcAddress is officially specified to only return addresses for extension functions, not core entry points [5]. Although SDL attempts to mitigate this by falling back to dlsym (or the platform-specific equivalent) via SDL_LoadFunction for core symbols, there is no guarantee that every symbol will be resolvable across all platforms and driver configurations [3]. 3. Runtime Safety: Best practice in C/C++ graphics programming—and as evidenced by existing SDL implementations and user code—is to treat any function pointer retrieval as a potentially failing operation [6]. Failing to check for a NULL pointer before dereferencing it will lead to an application crash if the symbol lookup fails [7]. In summary, you should treat the return value as untrusted and verify it is not NULL before use to ensure the robustness of your application [6].

Citations:

• 1: https://wiki.libsdl.org/SDL3/SDL_EGL_GetProcAddress
• 2: https://manpages.ubuntu.com/manpages/questing/man3/SDL_EGL_GetProcAddress.3.html
• 3: https://github.com/libsdl-org/SDL/blob/41f07949/src/video/SDL_egl.c
• 4: libsdl-org/SDL@0644042
• 5: https://registry.khronos.org/EGL/sdk/docs/man/html/eglGetProcAddress.xhtml
• 6: https://gitlab.unistra.fr/adarwich/pa2022/-/blob/58640090311d0a8447f62392e6c76d084b56eda6/SDL/src/video/offscreen/SDL_offscreenopengl.c
• 7: SDL_GL_LoadLibrary / SDL_GL_GetProcAddress fails with KMSDRM backend. libsdl-org/SDL#5399

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- llgl.cpp around initEGL ---'
sed -n '1088,1135p' indra/llrender/llgl.cpp

echo
echo '--- llgl.cpp around reloadExtensionsString ---'
sed -n '1578,1605p' indra/llrender/llgl.cpp

echo
echo '--- SDL_EGL_GetProcAddress usages in repo ---'
rg -n 'SDL_EGL_GetProcAddress\(' indra | sed -n '1,120p'

echo
echo '--- direct eglQueryString call sites in repo ---'
rg -n '\beglQueryString\b' indra | sed -n '1,160p'

Repository: AlchemyViewer/Alchemy

Length of output: 5645

---

Guard the eglQueryString lookup before the first call. SDL_EGL_GetProcAddress("eglQueryString") is used immediately on lines 1115-1116, so a failed lookup turns Wayland/EGL startup into a null-call. reloadExtensionsString() already treats the same symbol as optional.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@indra/llrender/llgl.cpp` around lines 1106 - 1116, Guard the eglQueryString
function pointer before using it in the EGL init path: the lookup via
SDL_EGL_GetProcAddress("eglQueryString") can fail, and the first RenderInit log
calls use it immediately. Update the llgl.cpp initialization around
eglQueryString so it checks for a valid pointer before calling it, and fall back
to the same optional/skip behavior used by reloadExtensionsString() when the
symbol is unavailable.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Avoid a 1-second stall on the viewer GL thread.

AcquireSync(0, 1000) can block the media update/render path for up to a second when the producer is busy. Use a non-blocking or very short timeout and retry next frame.

Proposed fix

-        if (FAILED(w->mutex->AcquireSync(0, 1000)))
+        const HRESULT hr = w->mutex->AcquireSync(0, 0);
+        if (hr == DXGI_ERROR_WAIT_TIMEOUT)
         {
             return false;   // producer busy this frame; try again next
         }
+        if (FAILED(hr))
+        {
+            LL_WARNS("Media") << "accelerated paint: AcquireSync failed hr=0x"
+                              << std::hex << hr << std::dec << LL_ENDL;
+            return false;
+        }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	if (w->mutex)
	{
	if (FAILED(w->mutex->AcquireSync(0, 1000)))
	{
	return false; // producer busy this frame; try again next
	if (w->mutex)
	{
	const HRESULT hr = w->mutex->AcquireSync(0, 0);
	if (hr == DXGI_ERROR_WAIT_TIMEOUT)
	{
	return false; // producer busy this frame; try again next
	}
	if (FAILED(hr))
	{
	LL_WARNS("Media") << "accelerated paint: AcquireSync failed hr=0x"
	<< std::hex << hr << std::dec << LL_ENDL;
	return false;
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@indra/newview/llcefaccelinterop.cpp` around lines 195 - 199, The AcquireSync
call in llcefaccelinterop.cpp currently waits up to 1000 ms inside the viewer GL
update path, which can stall rendering when the producer is busy. Update the
logic in the relevant sync/acquire section around w->mutex->AcquireSync so it
uses a non-blocking or very short timeout and immediately returns false to retry
on the next frame, preserving the existing “try again next” behavior without
blocking the viewer thread.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Keep the previous macOS surface until the new IOSurface bind succeeds.

This releases/replaces m->surface before checking CGLTexImageIOSurface2D; on failure blitTo() can still see a non-null surface and continue with an invalid or stale texture binding.

Proposed fix

-    if (m->surface) { CFRelease(m->surface); }
-    m->surface = surf;
-    m->width = (int)IOSurfaceGetWidth(surf);
-    m->height = (int)IOSurfaceGetHeight(surf);
+    const int new_width = (int)IOSurfaceGetWidth(surf);
+    const int new_height = (int)IOSurfaceGetHeight(surf);
 
     CGLContextObj cgl = CGLGetCurrentContext();
     glBindTexture(GL_TEXTURE_RECTANGLE_ARB, m->tex);
     CGLError err = CGLTexImageIOSurface2D(cgl, GL_TEXTURE_RECTANGLE_ARB, GL_RGBA,
-                                          m->width, m->height, GL_BGRA,
+                                          new_width, new_height, GL_BGRA,
                                           GL_UNSIGNED_INT_8_8_8_8_REV, surf, 0);
     glBindTexture(GL_TEXTURE_RECTANGLE_ARB, 0);
-    return err == kCGLNoError;
+    if (err != kCGLNoError)
+    {
+        CFRelease(surf);
+        return false;
+    }
+
+    if (m->surface) { CFRelease(m->surface); }
+    m->surface = surf;
+    m->width = new_width;
+    m->height = new_height;
+    return true;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	if (m->surface) { CFRelease(m->surface); }
	m->surface = surf;
	m->width = (int)IOSurfaceGetWidth(surf);
	m->height = (int)IOSurfaceGetHeight(surf);
	CGLContextObj cgl = CGLGetCurrentContext();
	glBindTexture(GL_TEXTURE_RECTANGLE_ARB, m->tex);
	CGLError err = CGLTexImageIOSurface2D(cgl, GL_TEXTURE_RECTANGLE_ARB, GL_RGBA,
	m->width, m->height, GL_BGRA,
	GL_UNSIGNED_INT_8_8_8_8_REV, surf, 0);
	glBindTexture(GL_TEXTURE_RECTANGLE_ARB, 0);
	return err == kCGLNoError;
	const int new_width = (int)IOSurfaceGetWidth(surf);
	const int new_height = (int)IOSurfaceGetHeight(surf);
	CGLContextObj cgl = CGLGetCurrentContext();
	glBindTexture(GL_TEXTURE_RECTANGLE_ARB, m->tex);
	CGLError err = CGLTexImageIOSurface2D(cgl, GL_TEXTURE_RECTANGLE_ARB, GL_RGBA,
	new_width, new_height, GL_BGRA,
	GL_UNSIGNED_INT_8_8_8_8_REV, surf, 0);
	glBindTexture(GL_TEXTURE_RECTANGLE_ARB, 0);
	if (err != kCGLNoError)
	{
	CFRelease(surf);
	return false;
	}
	if (m->surface) { CFRelease(m->surface); }
	m->surface = surf;
	m->width = new_width;
	m->height = new_height;
	return true;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@indra/newview/llcefaccelinterop.cpp` around lines 328 - 339, The macOS
IOSurface binding in blitTo() is replacing m->surface before confirming
CGLTexImageIOSurface2D succeeded, which can leave a stale surface/texture state
on failure. Update llcefaccelinterop.cpp’s surface update path so the new surf
is only assigned to m->surface after the bind call returns kCGLNoError, and keep
the previous surface/metadata intact if binding fails. Make sure any cleanup or
release of the old surface happens only after a successful bind, using the
existing m->surface, m->width, and m->height members as the state to preserve.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not discard the last good Linux EGLImage before the replacement imports.

releaseImage() runs before eglCreateImageKHR; one bad dma-buf frame destroys the current binding and leaves blitTo() with no image. Create into a temporary image, then swap only on success.

Proposed fix

-    l->releaseImage();
-    l->image = eglCreateImageKHR(l->display, EGL_NO_CONTEXT, EGL_LINUX_DMA_BUF_EXT, (EGLClientBuffer)0, attrs);
+    EGLImageKHR new_image = eglCreateImageKHR(l->display, EGL_NO_CONTEXT, EGL_LINUX_DMA_BUF_EXT, (EGLClientBuffer)0, attrs);
@@
-        LL_INFOS("Media") << "accelerated paint dma-buf import: " << (l->image != EGL_NO_IMAGE_KHR ? "ok" : "FAILED")
+        LL_INFOS("Media") << "accelerated paint dma-buf import: " << (new_image != EGL_NO_IMAGE_KHR ? "ok" : "FAILED")
@@
-    if (l->image == EGL_NO_IMAGE_KHR)
+    if (new_image == EGL_NO_IMAGE_KHR)
     {
         LL_WARNS("Media") << "accelerated paint: eglCreateImageKHR(dma_buf) failed (planes=" << n
                           << " modifier=0x" << std::hex << modifier << std::dec << ")" << LL_ENDL;
         return false;
     }
 
+    l->releaseImage();
+    l->image = new_image;
     l->width = width;

Also applies to: 570-574

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@indra/newview/llcefaccelinterop.cpp` around lines 546 - 568, The accelerated
paint import path in llcefaccelinterop::releaseImage/eglCreateImageKHR currently
destroys the existing Linux EGLImage before confirming the new dma-buf import
succeeds, so a failed frame can leave blitTo() with nothing usable. Change the
import flow to create the new EGLImage in a temporary handle first, and only
call releaseImage() or replace l->image after eglCreateImageKHR succeeds; if it
fails, keep the previous image intact and return false while preserving the last
good binding.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Add a CPU fallback when accelerated interop is unavailable.

If LLCEFAccelInterop::init() returns false, updateAcceleratedTexture() returns false but update() still skips the CPU upload path, leaving media blank and retrying interop initialization on later dirty frames. Disable/recreate with CPU paint, or only request accelerated paint after a successful capability preflight.

Also applies to: 3247-3255

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@indra/newview/llviewermedia.cpp` around lines 3069 - 3082, The accelerated
paint path in llviewermedia.cpp can leave media blank when
LLCEFAccelInterop::init() fails because update() returns early after
updateAcceleratedTexture() without falling back to CPU upload. Update the logic
around mMediaSource->getUseAcceleratedPaint(), updateAcceleratedTexture(), and
the later CPU paint path so that failed interop either disables/recreates
accelerated mode and allows CPU painting, or only enables accelerated paint
after a successful capability preflight; apply the same fix in both affected
update branches.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Honor setStableTexture() failures on macOS and Linux.

The Windows branch returns on bind failure, but macOS/Linux continue to blitTo() after a failed import, which can display stale content from the previous binding.

Proposed fix

-        mAccelInterop->setStableTexture((unsigned long long)(uintptr_t)surf,
-                                        mMediaSource->getAcceleratedPaintWidth(),
-                                        mMediaSource->getAcceleratedPaintHeight(),
-                                        mMediaSource->getAcceleratedPaintFormat(),
-                                        0, 0, 0, 0);
+        if (!mAccelInterop->setStableTexture((unsigned long long)(uintptr_t)surf,
+                                             mMediaSource->getAcceleratedPaintWidth(),
+                                             mMediaSource->getAcceleratedPaintHeight(),
+                                             mMediaSource->getAcceleratedPaintFormat(),
+                                             0, 0, 0, 0))
+        {
+            return false;
+        }
@@
-        mAccelInterop->setStableTexture(plane_fds[0],
-                                        frame.width, frame.height, frame.format,
-                                        plane_strides[0], plane_offsets[0], frame.modifier,
-                                        0, frame.plane_count, plane_fds, plane_strides, plane_offsets);
+        bool imported = mAccelInterop->setStableTexture(plane_fds[0],
+                                                        frame.width, frame.height, frame.format,
+                                                        plane_strides[0], plane_offsets[0], frame.modifier,
+                                                        0, frame.plane_count, plane_fds, plane_strides, plane_offsets);
         frame.closeFds();
+        if (!imported)
+        {
+            return false;
+        }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	void* surf = LLCEFSurfaceReceiver::instance().takeLatest(mMediaSource->getAccelId());
	if (surf)
	{
	mAccelInterop->setStableTexture((unsigned long long)(uintptr_t)surf,
	mMediaSource->getAcceleratedPaintWidth(),
	mMediaSource->getAcceleratedPaintHeight(),
	mMediaSource->getAcceleratedPaintFormat(),
	0, 0, 0, 0);
	}
	#elif LL_WINDOWS
	// The handle is persistent (re)sent only on (re)create. (Re)bind the interop
	// when it differs from what we have bound; only advance mAccelBoundHandle on a
	// successful bind so a transient failure is retried with the same handle.
	unsigned long long handle = mMediaSource->getAcceleratedPaintHandle();
	if (handle != 0 && handle != mAccelBoundHandle)
	{
	if (mAccelInterop->setStableTexture(handle,
	mMediaSource->getAcceleratedPaintWidth(),
	mMediaSource->getAcceleratedPaintHeight(),
	mMediaSource->getAcceleratedPaintFormat()))
	{
	mAccelBoundHandle = handle;
	}
	else
	{
	return false;
	}
	}
	#else // LL_LINUX
	// CEF's dma-buf fds arrive out-of-band via the SCM_RIGHTS side channel (a
	// dma-buf fd cannot cross the process boundary through the LLSD/TCP channel,
	// nor be reopened via /proc), demuxed by this media's accel id. Take the
	// newest frame, import its planes directly (the fds are open in this process),
	// then close them. Keep the current binding if no new frame arrived this tick.
	LLCEFSurfaceReceiver::DmabufFrame frame;
	if (LLCEFSurfaceReceiver::instance().takeLatestDmabuf(mMediaSource->getAccelId(), frame))
	{
	unsigned long long plane_fds[4] = {};
	unsigned int plane_strides[4] = {};
	unsigned long long plane_offsets[4] = {};
	for (int i = 0; i < frame.plane_count && i < 4; ++i)
	{
	plane_fds[i] = (unsigned long long)frame.fd[i];
	plane_strides[i] = frame.stride[i];
	plane_offsets[i] = frame.offset[i];
	}
	// The interop imports the fds directly (no /proc); we own them and close
	// them right after - EGL keeps its own reference once the image is made.
	mAccelInterop->setStableTexture(plane_fds[0],
	frame.width, frame.height, frame.format,
	plane_strides[0], plane_offsets[0], frame.modifier,
	0, frame.plane_count, plane_fds, plane_strides, plane_offsets);
	frame.closeFds();
	void* surf = LLCEFSurfaceReceiver::instance().takeLatest(mMediaSource->getAccelId());
	if (surf)
	{
	if (!mAccelInterop->setStableTexture((unsigned long long)(uintptr_t)surf,
	mMediaSource->getAcceleratedPaintWidth(),
	mMediaSource->getAcceleratedPaintHeight(),
	mMediaSource->getAcceleratedPaintFormat(),
	0, 0, 0, 0))
	{
	return false;
	}
	}
	`#elif` LL_WINDOWS
	// The handle is persistent (re)sent only on (re)create. (Re)bind the interop
	// when it differs from what we have bound; only advance mAccelBoundHandle on a
	// successful bind so a transient failure is retried with the same handle.
	unsigned long long handle = mMediaSource->getAcceleratedPaintHandle();
	if (handle != 0 && handle != mAccelBoundHandle)
	{
	if (mAccelInterop->setStableTexture(handle,
	mMediaSource->getAcceleratedPaintWidth(),
	mMediaSource->getAcceleratedPaintHeight(),
	mMediaSource->getAcceleratedPaintFormat()))
	{
	mAccelBoundHandle = handle;
	}
	else
	{
	return false;
	}
	}
	`#else` // LL_LINUX
	// CEF's dma-buf fds arrive out-of-band via the SCM_RIGHTS side channel (a
	// dma-buf fd cannot cross the process boundary through the LLSD/TCP channel,
	// nor be reopened via /proc), demuxed by this media's accel id. Take the
	// newest frame, import its planes directly (the fds are open in this process),
	// then close them. Keep the current binding if no new frame arrived this tick.
	LLCEFSurfaceReceiver::DmabufFrame frame;
	if (LLCEFSurfaceReceiver::instance().takeLatestDmabuf(mMediaSource->getAccelId(), frame))
	{
	unsigned long long plane_fds[4] = {};
	unsigned int plane_strides[4] = {};
	unsigned long long plane_offsets[4] = {};
	for (int i = 0; i < frame.plane_count && i < 4; ++i)
	{
	plane_fds[i] = (unsigned long long)frame.fd[i];
	plane_strides[i] = frame.stride[i];
	plane_offsets[i] = frame.offset[i];
	}
	// The interop imports the fds directly (no /proc); we own them and close
	// them right after - EGL keeps its own reference once the image is made.
	bool imported = mAccelInterop->setStableTexture(plane_fds[0],
	frame.width, frame.height, frame.format,
	plane_strides[0], plane_offsets[0], frame.modifier,
	0, frame.plane_count, plane_fds, plane_strides, plane_offsets);
	frame.closeFds();
	if (!imported)
	{
	return false;
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@indra/newview/llviewermedia.cpp` around lines 3265 - 3317, The macOS/Linux
paths in llviewermedia.cpp are ignoring setStableTexture() import failures and
still proceeding to blitTo(), which can leave stale content visible. Update the
platform-specific binding logic in the media update path around
LLCEFSurfaceReceiver::instance().takeLatest() / takeLatestDmabuf() so that a
failed mAccelInterop->setStableTexture() causes an immediate return false,
matching the Windows behavior and preventing fallback to the previous texture
binding.