Repository navigation
fix(desktop): check libVLC candidates without a recursive search - #520
Open
veryCrunchy wants to merge 9 commits into
Open
veryCrunchy wants to merge 9 commits into
veryCrunchy wants to merge 9 commits into
Conversation
vlcj's default discovery searches every candidate directory recursively and follows symbolic links. Without libVLC it walks all of /usr/lib, /usr/local/lib, and every PATH entry. On the Linux CI runner that tree includes the Android SDK, which is the most likely reason the desktop test task stopped producing output until the 45 minute job limit after DesktopLibVlcPlaybackTest was added. The Windows desktop tests also grew by about six minutes. In the app, the same walk ran at startup and hid the external-player fallback until it finished. Keep vlcj's candidate directories, order, and per-OS loading, but check each candidate itself and never search below it.
The ui test tasks log only failures, so a stuck test printed nothing until CI cancelled the job. Log the name of any test still running after 3 minutes and stop a test task after 30 minutes, well inside the CI job limit. The slowest desktop test takes about 12 seconds and the full desktop run about 6 minutes of test time.
obiente-cloud
Bot
temporarily deployed
to
Obiente Preview / PR #520 / NC Native
October 19, 2026 10:57
Destroyed
Obiente previewNC Native · Obiente updates this comment as the preview changes. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0644604abf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
obiente-cloud
Bot
temporarily deployed
to
Obiente Preview / PR #520 / NC Native
October 19, 2026 11:05
Destroyed
The SMTC COM handler's JNA callbacks use the Windows stdcall convention, which JNA cannot create on Linux or macOS, so its class initializer failed there. Production opens the handler only on Windows.
obiente-cloud
Bot
temporarily deployed
to
Obiente Preview / PR #520 / NC Native
October 19, 2026 12:11
Destroyed
vlcj names only the x86_64 and i386 triplet directories, so on arm64 and other architectures the distribution libVLC in /usr/lib/<triplet> was reachable only through the recursive search removed earlier. Check the JVM's own triplet directory before vlcj's first system directory, and the other known triplets after all of vlcj's candidates, because vlcj stops at the first copy it finds even when that copy cannot load. Each directory is still checked directly.
obiente-cloud
Bot
temporarily deployed
to
Obiente Preview / PR #520 / NC Native
October 19, 2026 12:43
Destroyed
Android lint 9.3.1 visits a Kotlin script applied with apply(from = ...) and crashes analysing it (findFirCompiledSymbol only works on compiled declarations), which failed :ui:lintAnalyzeRelease on main as well. The crash occurs whatever the applied script contains and wherever it lives, and disappears without the apply call. The download and staging tasks now live in ui/build.gradle.kts unchanged; the CI path filters and docs point there.
obiente-cloud
Bot
temporarily deployed
to
Obiente Preview / PR #520 / NC Native
October 19, 2026 13:43
Destroyed
This branch was successfully deployed
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
1 participant
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.
Outcome
Fixes the Linux CI
buildjob, which has stopped producing output in:ui:desktopTestuntil the 45 minute job limit since desktop video playback landed (#511). The same root cause also delayed the desktop video fallback at app startup on machines without libVLC.vlcj's default
NativeDiscoverysearches each candidate directory recursively and follows symbolic links, with no depth bound. When libVLC is absent, it walks every tree under/usr/lib,/usr/local/liband every PATH entry. On GitHub's ubuntu-24.04 runner that includes the Android SDK and NDKs under/usr/local/lib/android/sdk.DesktopLibVlcPlaybackTestruns discovery once per test method, before it can skip, and the test task logs only failures, so nothing was printed.In the app,
warmDesktopVideoPlayback()runs the same walk at startup. While discovery was pending, the media viewer hid its "Play video" external-player fallback. Windows and macOS packages find their bundled libVLC first. This behavior is unreleased (changes/unreleased).Changes:
DesktopLibVlcRuntimeusesdesktopLibVlcDiscoveryStrategies(). These wrap vlcj's Linux, macOS and Windows strategies with the same candidate directories, order, and per-OS library loading and plugin path handling. Each candidate is checked directly for both libraries and is never searched below. Trade-off: a libVLC nested inside a subdirectory of a candidate is no longer found. JNA's default library path still applies.:uitest tasks logTest still running after 3 minutes: <class> > <test>and stop after 30 minutes. A future hang then fails with a name, well inside the CI job limit. The slowest desktop test takes about 12 s.Main's Linux CI had stopped at
desktopTestsince #511, which hid two further failures that this PR also fixes:DesktopSystemMediaControlsTest.theWindowsButtonHandlerAnswersComAndDispatchesTheButtonbuilds the Windows SMTC COM handler, whose stdcall JNA callbacks cannot be created on Linux ("Invalid calling convention 63"). The test now runs only on Windows. Production already opens the Windows integration only whenos.nameis Windows.:ui:lintAnalyzeReleasecrashed inside lint ("findFirCompiledSymbolonly works on compiled declarations"). Android lint 9.3.1 crashes while analysing any Kotlin script applied withapply(from = ...), whatever its content or location. Reproduced locally on main. The libVLC packaging tasks fromui/desktop-libvlc.gradle.ktsnow live unchanged inui/build.gradle.kts, with a comment explaining why they must not be split out again. The two path-filter entries for the removed file are dropped fromci.yml. Lint is not disabled, baselined or suppressed.Review follow-up: vlcj 4.12.1 checks only
/usr/lib/x86_64-linux-gnu,/usr/lib64,/usr/local/lib64,/usr/lib/i386-linux-gnu,/usr/liband/usr/local/lib(then PATH), with no default-path fallback. Removing the recursive walk would therefore have hidden Debian/Ubuntu multiarch installs on other architectures. Discovery now adds/usr/lib/<triplet>for the JVM's own architecture before vlcj's directories. The other known triplets (x86_64, aarch64, arm-linux-gnueabihf, i386, riscv64, powerpc64le, s390x) come last, so a foreign-architecture copy is never chosen first. Versioned runtime names (libvlc.so.5,libvlccore.so.9) are accepted, and the plugin path still resolves to<dir>/vlc/plugins.Verification
Every check relevant to the changed scope passes, or each unrun check is listed below with a reason
bash tools/check-repository.shpassesA new
changes/unreleased/fragment records the changeNo credentials, private server data, machine-local paths, or generated output are included
New
DesktopLibVlcDiscoveryTest(4 tests):DesktopLibVlcPlaybackTeststill plays real libVLC on a Windows machine with VLC installed, found through the new discovery.Full
:ui:desktopTest --rerun :androidApp:testDebugUnitTest --rerunon Windows with JDK 21: desktop 4056 tests (70 skipped), Android 1183 tests, 0 failures. The slow-test log never fired. The new log line was confirmed with a temporary 2 s threshold, then reverted.bash tools/check-kotlin-architecture.shandbash tools/check-repository.shpass.Timing evidence: vlcj's recursive search over one Windows machine's 63 PATH entries took 192.5 s per walk, which matches the roughly six minute Windows
desktopTestslowdown after Play Photos videos and add desktop in-app video playback #511. A shell reproduction of the search in Ubuntu 24.04 under WSL followed/usr/bin/X11 -> .40 levels deep.Not verified locally: a full Linux run of the suite. This PR's Linux CI job is the confirmation. If any other test still hangs, the new log names it after 3 minutes.
DesktopLibVlcDiscoveryTest(8 tests, now including a runtime-only aarch64 tree, native-triplet preference, candidate order, and no walking below system directories),DesktopLibVlcPlaybackTest(2, real libVLC),DesktopSystemMediaControlsTest(6; the Windows handler test runs on Windows).:ui:lintAnalyzeRelease :androidApp:verifyReleaseLintGatepasses locally. On main it crashes before the fix.buildpassed in 10m18s, including the lint gate andassembleDebug. LinuxdesktopTestexecuted and passed in about 4m38s at 6099a09, and was restored from the build cache at the final head because only build-script code moved.Compatibility and risk
Desktop only. Packaged builds keep finding their bundled libVLC in the first candidate. Linux and source runs find a system libVLC in the standard and Debian multiarch library directories. Only nested, non-standard layouts are no longer discovered. The packaging tasks are unchanged and only moved into
ui/build.gradle.kts.Visual changes
Not applicable.