mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-08 23:54:39 +00:00
fix(desktop,media): address PR #3175 review findings
Correctness fixes in GlobalMediaPlayer.kt
- snapshotFlow { hasMedia } collector for initial seek used `return@collect`
which only exits the lambda; the collector kept running and each
subsequent playVideo() call accumulated a live collector that would
re-fire a stale seekTo() on the wrong media. Replaced with `Flow.first`
which terminates the collection cleanly.
- playVideo()/playAudio() reset the public MediaPlaybackState to
volume=100/isMuted=false on a new URL, but the kdroidFilter player
retains its `volume` across openUri(); muting one track and starting a
new one left the engine silent while the UI showed unmuted. Reset
`player.volume = 1f` to match the public state.
- ensureVideoPlayer()/ensureAudioPlayer() called createVideoPlayerState()
synchronously from the Compose getter; if native init throws (missing
GStreamer on Linux, broken NativeLibraryLoader extraction) the whole
window would crash. Wrapped in runCatching and changed
activeVideoPlayerState to nullable. Consumers in DesktopVideoPlayer
and GlobalFullscreenOverlay handle the null path by rendering the
thumbnail / blank backdrop respectively; playVideo()/playAudio()
surface "Video playback unavailable" through the existing
errorReason -> PlaybackErrorMessage path.
Crash mitigation (kdroidFilter 0.10.0 UAF in MacVideoPlayerSurface)
- NowPlayingBar previously mounted a SECOND VideoPlayerSurface against
the same VideoPlayerState while the feed card was already mounting
one, doubling the draw rate against the shared frame bitmap and
widening the UAF window in MacVideoPlayerSurface's RasterFromBitmap
path. Mini-preview now renders the cached thumbnail (or the music
icon fallback). 0.10.1 contains an upstream fix
("recover video playback after composition removal") but is not yet
on Maven Central — single-surface mounting is the only mitigation
we can ship today.
VideoThumbnailCache.kt
- Truncated-download cache poisoning: when an origin ignored the
Range: header and returned HTTP 200 with the full body, we capped
the copy at MAX_THUMB_BYTES and persisted the truncated file
forever. Subsequent thumbnail attempts hit the broken cache file
and re-failed JCodec/ffmpeg every time. Tag download results with
whether the server actually returned 206; on 200, extract from the
temp file and delete it (no persistent cache hit).
- Tor bypass: replaced the bare OkHttpClient with
DesktopHttpClient.currentClient() so thumbnail fetches respect the
user's Tor preference (fail-closed when Tor is expected but
bootstrapping).
- ffmpeg version probe leaked the process on hang: now drains stdout
to DISCARD and calls destroyForcibly() on timeout.
- Frame-extract ffmpeg subprocess could deadlock on a chatty stderr
pipe: redirectError(DISCARD) so we never wait on stderr; a finally
block destroys the process if anything leaked through the timeout.
CI workflow cleanup
- Removed vlc-setup download cache + pre-fetch steps from
build.yml and smoke-test-desktop.yml. They were targeting an
ir.mahozad.vlc-setup plugin we no longer apply, so they wasted
~minutes of CI time per leg and tied the build to videolan.org
reachability for no reason.
- Trimmed create-release.yml's stale VLC-plugins justification on
the linuxdeploy-vs-appimagetool comment.
.gitignore + missing per-OS ffmpeg READMEs
- The pre-PR rules blanket-ignored desktopApp/src/jvmMain/appResources/{linux,macos,windows}/
so the LGPL FFmpeg drop-in slot READMEs created in 704f4f44e never
reached the commit. Refined the ignore rules to keep stale vlc/ workspace
trees out of git (still ignored) while explicitly tracking the
ffmpeg/README.md drop-in slot under each OS. The READMEs document the
recommended LGPL build source per OS for the bundled-FFmpeg packaging
path.
Verified on macOS arm64:
./gradlew :desktopApp:compileKotlin BUILD SUCCESSFUL
./gradlew :desktopApp:test BUILD SUCCESSFUL
./gradlew :desktopApp:spotlessApply clean
Refs PR #3175 review by @davotoula.
This commit is contained in:
@@ -79,82 +79,6 @@ jobs:
|
||||
with:
|
||||
cache-read-only: ${{ github.ref != 'refs/heads/main' }}
|
||||
|
||||
# Cache vlc-setup plugin downloads (VLC + UPX archives) keyed on the
|
||||
# versions pinned in desktopApp/build.gradle.kts. Each OS gets its own
|
||||
# cache namespace because the plugin downloads platform-specific archives.
|
||||
# On a hit the vlcDownload / upxDownload tasks are up-to-date and we
|
||||
# never touch get.videolan.org; on a miss (version bump or new runner)
|
||||
# we fall back to fetching, which is what the in-build retry budget
|
||||
# exists for.
|
||||
- name: Cache vlc-setup downloads
|
||||
uses: actions/cache@v5
|
||||
with:
|
||||
path: ~/.gradle/vlcSetup
|
||||
key: vlcsetup-${{ runner.os }}-${{ hashFiles('desktopApp/build.gradle.kts') }}
|
||||
restore-keys: |
|
||||
vlcsetup-${{ runner.os }}-
|
||||
|
||||
# Pre-fetch VLC + UPX archives into ~/.gradle/vlcSetup before invoking
|
||||
# Gradle. The vlc-setup plugin (ir.mahozad.vlc-setup 0.1.0) writes its
|
||||
# downloads to ${gradleUserHomeDir}/vlcSetup/ and sets overwrite(false),
|
||||
# so an existing file there makes vlcDownload / upxDownload up-to-date
|
||||
# and Gradle never opens a socket to videolan.org.
|
||||
#
|
||||
# Why curl instead of relying on de.undercouch.gradle.tasks.download:
|
||||
# curl --retry-all-errors with a long --retry-max-time tolerates a
|
||||
# sustained get.videolan.org outage far better than the plugin's inner
|
||||
# retry budget (retries(4) + 5min readTimeout in build.gradle.kts),
|
||||
# which has been hitting SocketTimeoutException on Windows runners.
|
||||
#
|
||||
# Cache hit: the file is already on disk, fetch() short-circuits, this
|
||||
# step takes <1s. Cache miss: curl downloads with aggressive retries,
|
||||
# populating the cache for the next run.
|
||||
#
|
||||
# Versions are pinned to match desktopApp/build.gradle.kts (vlcVersion
|
||||
# = 3.0.20) and the vlc-setup extension default (upxVersion = 4.2.4).
|
||||
# NOTE: vlcVersion lags behind upstream VLC because the Linux plugins on
|
||||
# Maven Central (ir.mahozad:vlc-plugins-linux) are only published for
|
||||
# 3.0.20 / 3.0.20-2. Bump only after the Maven artifact is republished.
|
||||
# macOS does not download UPX — UPX cannot compress .dylib files.
|
||||
- name: Pre-fetch VLC + UPX archives
|
||||
env:
|
||||
VLC_VERSION: "3.0.20"
|
||||
UPX_VERSION: "4.2.4"
|
||||
run: |
|
||||
set -euo pipefail
|
||||
DEST="$HOME/.gradle/vlcSetup"
|
||||
mkdir -p "$DEST"
|
||||
fetch() {
|
||||
local url="$1" out="$2"
|
||||
if [[ -s "$out" ]]; then
|
||||
echo "cached: $out"
|
||||
return 0
|
||||
fi
|
||||
echo "fetching: $url"
|
||||
curl -fL --retry 10 --retry-delay 5 --retry-all-errors \
|
||||
--retry-max-time 900 --connect-timeout 30 \
|
||||
-o "$out.part" "$url"
|
||||
mv "$out.part" "$out"
|
||||
}
|
||||
case "${{ runner.os }}" in
|
||||
Windows)
|
||||
fetch "https://get.videolan.org/vlc/${VLC_VERSION}/win64/vlc-${VLC_VERSION}-win64.zip" \
|
||||
"$DEST/vlc-${VLC_VERSION}.zip"
|
||||
fetch "https://github.com/upx/upx/releases/download/v${UPX_VERSION}/upx-${UPX_VERSION}-win64.zip" \
|
||||
"$DEST/upx-${UPX_VERSION}.zip"
|
||||
;;
|
||||
Linux)
|
||||
fetch "https://repo1.maven.org/maven2/ir/mahozad/vlc-plugins-linux/${VLC_VERSION}/vlc-plugins-linux-${VLC_VERSION}.jar" \
|
||||
"$DEST/vlc-${VLC_VERSION}.jar"
|
||||
fetch "https://github.com/upx/upx/releases/download/v${UPX_VERSION}/upx-${UPX_VERSION}-amd64_linux.tar.xz" \
|
||||
"$DEST/upx-${UPX_VERSION}.tar.xz"
|
||||
;;
|
||||
macOS)
|
||||
fetch "https://get.videolan.org/vlc/${VLC_VERSION}/macosx/vlc-${VLC_VERSION}-universal.dmg" \
|
||||
"$DEST/vlc-${VLC_VERSION}.dmg"
|
||||
;;
|
||||
esac
|
||||
|
||||
# Compose UI smoke test (DesktopLaunchSmokeTest) uses Skiko which needs
|
||||
# a display server on Linux. xvfb provides a virtual framebuffer.
|
||||
- name: Install xvfb (Linux)
|
||||
|
||||
@@ -23,9 +23,9 @@ env:
|
||||
# Single source of truth in scripts/asset-name.sh.
|
||||
# appimagetool pinned release — bump via Dependabot, verify SHA256 via env var below.
|
||||
# We used to use linuxdeploy here, but it auto-walks the AppDir with ldd to
|
||||
# bundle deps — that fights jpackage's self-contained JRE (libjvm.so RPATH
|
||||
# mismatch) and the UPX-compressed VLC plugins. appimagetool only embeds the
|
||||
# AppDir as-is, which is what we actually want.
|
||||
# bundle deps — that fights jpackage's self-contained JRE (libjvm.so has
|
||||
# $ORIGIN RPATH so ldd can't resolve it standalone). appimagetool only
|
||||
# embeds the AppDir as-is, which is what we actually want.
|
||||
APPIMAGETOOL_URL: https://github.com/AppImage/appimagetool/releases/download/1.9.0/appimagetool-x86_64.AppImage
|
||||
APPIMAGETOOL_SHA256: 46fdd785094c7f6e545b61afcfb0f3d98d8eab243f644b4b17698c01d06083d1
|
||||
|
||||
|
||||
@@ -68,36 +68,6 @@ jobs:
|
||||
with:
|
||||
cache-read-only: true
|
||||
|
||||
- name: Cache vlc-setup downloads
|
||||
uses: actions/cache@v5
|
||||
with:
|
||||
path: ~/.gradle/vlcSetup
|
||||
key: vlcsetup-Linux-${{ hashFiles('desktopApp/build.gradle.kts') }}
|
||||
restore-keys: |
|
||||
vlcsetup-Linux-
|
||||
|
||||
- name: Pre-fetch VLC + UPX archives
|
||||
env:
|
||||
VLC_VERSION: "3.0.20"
|
||||
UPX_VERSION: "4.2.4"
|
||||
run: |
|
||||
set -euo pipefail
|
||||
DEST="$HOME/.gradle/vlcSetup"
|
||||
mkdir -p "$DEST"
|
||||
fetch() {
|
||||
local url="$1" out="$2"
|
||||
if [[ -s "$out" ]]; then echo "cached: $out"; return 0; fi
|
||||
echo "fetching: $url"
|
||||
curl -fL --retry 10 --retry-delay 5 --retry-all-errors \
|
||||
--retry-max-time 900 --connect-timeout 30 \
|
||||
-o "$out.part" "$url"
|
||||
mv "$out.part" "$out"
|
||||
}
|
||||
fetch "https://repo1.maven.org/maven2/ir/mahozad/vlc-plugins-linux/${VLC_VERSION}/vlc-plugins-linux-${VLC_VERSION}.jar" \
|
||||
"$DEST/vlc-${VLC_VERSION}.jar"
|
||||
fetch "https://github.com/upx/upx/releases/download/v${UPX_VERSION}/upx-${UPX_VERSION}-amd64_linux.tar.xz" \
|
||||
"$DEST/upx-${UPX_VERSION}.tar.xz"
|
||||
|
||||
- name: Install xvfb + packaging deps
|
||||
run: sudo apt-get update && sudo apt-get install -y xvfb fakeroot
|
||||
|
||||
|
||||
Reference in New Issue
Block a user