feat(ui): use sv11-ui components in the media player#34
Merged
Conversation
Initialize shadcn-svelte in ui/ as the foundation for adopting sv11-ui components in the JCEF player. No player behavior changes: App.svelte is untouched and the new primitives are unused — this only lays groundwork. - Add $lib path alias to tsconfig.json, tsconfig.app.json and vite.config.ts (the Vite project had no $lib; the shadcn-svelte CLI requires it) - shadcn-svelte init: components.json (Nova preset, Lucide + Geist to match sv11-ui's stack), cn util, theme tokens - Reconcile theme with jetplay's IDE-driven theming: shadcn's dark palette is the :root default (the player opens dark like the IDE) and the light palette lives under @media(prefers-color-scheme: light); jetplay uses no .dark class. Define --radius in the dark default so rounding isn't undefined in dark mode. - Pull button + dropdown-menu primitives (default shadcn registry) - Ignore the whole generated src/main/resources/player/ dir (vite outDir with emptyOutDir); previously only index.html was ignored, leaking ~4.7MB of rebuilt demo assets into git status Deferred to Phase 1: the sv11 audio-player composite — its registry items land at a doubled path (target-path bug in sv11 registry.json) and it still needs waveform-sampler plus local-media wiring. Verified: ./gradlew test green (48/48); npm run build green (single-file, 203kB / 108kB gzip). npm run check shows only 2 pre-existing App.svelte $state errors that also fail on clean main (not introduced here).
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Shadcn-Svelte UI library and Tailwind theming, configures module resolution, implements button and dropdown component systems, and introduces a full audio-player + waveform subsystem (core, UI controls, waveform renderers, sampling, scratchable scrubbing hook, JVM waveform extraction, MediaServer loopback, bridge wiring, and tests). ChangesShadcn-Svelte Component Library Setup
Sequence Diagram: audio waveform extraction and delivery flow sequenceDiagram
participant MediaFileEditor as MediaFileEditor.kt
participant WaveformExtractor as WaveformExtractor
participant EditorBridge as PlayerBridge.sendWaveform
participant BrowserWindow as window.jetplayWaveform
participant AppSvelte as ui/src/App.svelte
participant AudioPlayer as AudioPlayer.Root
participant AudioGraph as AudioGraph
MediaFileEditor->>WaveformExtractor: extract(localFile)
WaveformExtractor-->>MediaFileEditor: bars[]
MediaFileEditor->>EditorBridge: bridge.sendWaveform(bars)
EditorBridge->>BrowserWindow: executeJs("window.jetplayWaveform(...)")
BrowserWindow->>AppSvelte: jetplayWaveform(bars)
AppSvelte->>AudioPlayer: set waveform prop
AudioPlayer->>AudioGraph: ensureAnalyser(audioElement) / use waveform for scrub visuals
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR scaffolds a shadcn-svelte foundation inside ui/ for the upcoming player UI refresh (Phase 0), adding core primitives, build aliases, and theme/CSS integration without changing player behavior.
Changes:
- Add
$libalias support across Vite + TypeScript config to match shadcn-svelte conventions. - Introduce shadcn-style UI primitives (Button + Dropdown Menu) and shared
cn()/type utilities. - Integrate shadcn-svelte + tw-animate + Geist font and reconcile theme tokens in
app.css; add shadcn dependencies andcomponents.json.
Reviewed changes
Copilot reviewed 27 out of 29 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/vite.config.ts | Adds $lib alias for Vite builds to support shadcn-style imports. |
| ui/tsconfig.json | Adds baseUrl/paths so editors/TS can resolve $lib imports. |
| ui/tsconfig.app.json | Adds $lib paths for app typechecking / svelte-check. |
| ui/src/lib/utils.ts | Adds cn() helper and shared typing utilities used by UI primitives. |
| ui/src/lib/components/ui/dropdown-menu/* | Adds dropdown-menu wrapper components around bits-ui primitives. |
| ui/src/lib/components/ui/dropdown-menu/index.ts | Exposes dropdown-menu components via a single entrypoint. |
| ui/src/lib/components/ui/button/button.svelte | Adds a shadcn-style Button with tailwind-variants styling. |
| ui/src/lib/components/ui/button/index.ts | Re-exports Button component + types/variants. |
| ui/src/app.css | Imports shadcn/tw-animate/Geist and reconciles theme variables + base styles. |
| ui/package.json | Adds shadcn-svelte/bits-ui and related dependencies. |
| ui/package-lock.json | Locks new dependency graph for the added UI/tooling packages. |
| ui/components.json | Adds shadcn-svelte config (aliases, style, registry, etc.). |
| .gitignore | Ignores generated src/main/resources/player/ build output directory. |
Files not reviewed (1)
- ui/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Qodana Community for JVM18 new problems were found
💡 Qodana analysis was run in the pull request mode: only the changed files were checked View the detailed Qodana reportTo be able to view the detailed Qodana report, you can either:
To get - name: 'Qodana Scan'
uses: JetBrains/qodana-action@v2026.1.0
with:
upload-result: trueContact Qodana teamContact us at qodana-support@jetbrains.com
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/src/app.css (1)
53-127:⚠️ Potential issue | 🟠 Major | ⚡ Quick winScope the new base styles so Phase 0 stays behavior-neutral.
Lines 58 and 121-125 are not inert scaffolding:
* {@applyborder-border ... }plusbody {@applybg-background text-foreground; }immediately restyle the existing player, even though none of the new shadcn primitives are mounted yet. That also conflicts with the earlierhtml, bodypalette on Lines 66-68, so the current app can switch from--color-surface/--color-primaryto the new--background/--foregroundvalues right away. If the goal is “no player behavior changes” in Phase 0, these resets need to be scoped behind a future container or deferred until the new UI is actually adopted.Suggested direction
`@layer` base { * { margin: 0; padding: 0; box-sizing: border-box; - `@apply` border-border outline-ring/50; } @@ - body { - `@apply` bg-background text-foreground; - } - html { - `@apply` font-sans; - } }Or, if you want to keep the shadcn reset ready, apply it under a dedicated root that the refreshed player will mount into in Phase 1 instead of globally.
🤖 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 `@ui/src/app.css` around lines 53 - 127, The global base rules are unintentionally overriding the existing app (the universal selector * with `@apply` border-border/outline-ring and the body { `@apply` bg-background text-foreground })—scope these shadcn primitives behind a dedicated root so Phase 0 remains behavior-neutral: move the global resets inside a new container selector (e.g., .shadcn-root or `#new-ui-root`) instead of applying them at `@layer` base globally, and relocate the background/foreground `@apply` from body into that same scoped root while leaving the existing html, body color variables and layout rules untouched; update selectors in this file (the `@layer` base block, the * rule, and the body rule) to use that dedicated root so the old player style is preserved until the new UI mounts.
🧹 Nitpick comments (1)
ui/src/lib/components/ui/dropdown-menu/dropdown-menu-group-heading.svelte (1)
19-20: ⚡ Quick winConfirm
data-insethandling withinset={false}
Inui/src/lib/components/ui/dropdown-menu/dropdown-menu-group-heading.svelte, Svelte omitsdata-*attributes when the bound value isfalse, soinset={false}won’t leave a presentdata-inset="false"in the DOM. As a result,data-[inset]:ps-8won’t match and the indentation issue described doesn’t apply; keep the current approach (conditional class composition is optional for readability).🤖 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 `@ui/src/lib/components/ui/dropdown-menu/dropdown-menu-group-heading.svelte` around lines 19 - 20, The bound attribute data-inset={inset} will be omitted when inset is false (so data-[inset]:ps-8 won't match), so leave the current implementation as-is or, if you want clearer intent, change the class composition in the cn(...) call to conditionally add the padding class when inset is true (e.g., use inset ? 'ps-8' : ''), referencing the inset prop, cn helper, className and the data-[inset]:ps-8 usage in dropdown-menu-group-heading.svelte.
🤖 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 `@ui/src/lib/components/ui/button/button.svelte`:
- Line 7: The anchor branch in Button.svelte currently never receives a real
disabled attribute so the shared "disabled:*" styles don't apply; update the
anchor rendering (the branch that uses an <a> element) to apply the same
non-interactive treatment when the component's disabled prop is true by: adding
aria-disabled="true" and tabindex="-1" and appending explicit non-interactive
classes (e.g. pointer-events-none and opacity-50 or the equivalent used on the
button branch) to the anchor's class list (the same place that uses the base
string), and ensure any click handlers early-return when disabled; this will
make the anchor match the button's visual and interaction contract.
In `@ui/src/lib/components/ui/dropdown-menu/dropdown-menu-checkbox-item.svelte`:
- Around line 15-17: The component overrides children as a plain Snippet so
consumers don't get {checked, indeterminate}; change the children type to a
Snippet that accepts props (e.g. Snippet<{ checked: boolean; indeterminate:
boolean }>) instead of plain Snippet and, when rendering, call childrenProp with
those values (invoke childrenProp?.({ checked, indeterminate })) in
dropdown-menu-checkbox-item.svelte; update the prop typing around
WithoutChildrenOrChild<DropdownMenuPrimitive.CheckboxItemProps> and the
children/childrenProp usage to match how dropdown-menu-radio-item.svelte
forwards { checked }.
In `@ui/src/lib/components/ui/dropdown-menu/dropdown-menu-label.svelte`:
- Around line 19-20: Collapse the overlapping inset padding utilities into a
single conditional class: in the DropdownMenuLabel Svelte component where you
set data-inset={inset} and build the class via cn(..., className), remove one of
the redundant entries and keep a single selector that matches when the attribute
exists (e.g., replace both "data-inset:pl-7 data-[inset]:pl-8" with a single
"data-[inset]:pl-8" or "data-inset:pl-7"), so the effective padding is defined
once while still relying on the data-inset attribute controlled by the inset
prop.
---
Outside diff comments:
In `@ui/src/app.css`:
- Around line 53-127: The global base rules are unintentionally overriding the
existing app (the universal selector * with `@apply` border-border/outline-ring
and the body { `@apply` bg-background text-foreground })—scope these shadcn
primitives behind a dedicated root so Phase 0 remains behavior-neutral: move the
global resets inside a new container selector (e.g., .shadcn-root or
`#new-ui-root`) instead of applying them at `@layer` base globally, and relocate the
background/foreground `@apply` from body into that same scoped root while leaving
the existing html, body color variables and layout rules untouched; update
selectors in this file (the `@layer` base block, the * rule, and the body rule) to
use that dedicated root so the old player style is preserved until the new UI
mounts.
---
Nitpick comments:
In `@ui/src/lib/components/ui/dropdown-menu/dropdown-menu-group-heading.svelte`:
- Around line 19-20: The bound attribute data-inset={inset} will be omitted when
inset is false (so data-[inset]:ps-8 won't match), so leave the current
implementation as-is or, if you want clearer intent, change the class
composition in the cn(...) call to conditionally add the padding class when
inset is true (e.g., use inset ? 'ps-8' : ''), referencing the inset prop, cn
helper, className and the data-[inset]:ps-8 usage in
dropdown-menu-group-heading.svelte.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9358c4d2-daff-4c9a-96f6-e16f0123de65
⛔ Files ignored due to path filters (1)
ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (28)
.gitignoreui/components.jsonui/package.jsonui/src/app.cssui/src/lib/components/ui/button/button.svelteui/src/lib/components/ui/button/index.tsui/src/lib/components/ui/dropdown-menu/dropdown-menu-checkbox-group.svelteui/src/lib/components/ui/dropdown-menu/dropdown-menu-checkbox-item.svelteui/src/lib/components/ui/dropdown-menu/dropdown-menu-content.svelteui/src/lib/components/ui/dropdown-menu/dropdown-menu-group-heading.svelteui/src/lib/components/ui/dropdown-menu/dropdown-menu-group.svelteui/src/lib/components/ui/dropdown-menu/dropdown-menu-item.svelteui/src/lib/components/ui/dropdown-menu/dropdown-menu-label.svelteui/src/lib/components/ui/dropdown-menu/dropdown-menu-portal.svelteui/src/lib/components/ui/dropdown-menu/dropdown-menu-radio-group.svelteui/src/lib/components/ui/dropdown-menu/dropdown-menu-radio-item.svelteui/src/lib/components/ui/dropdown-menu/dropdown-menu-separator.svelteui/src/lib/components/ui/dropdown-menu/dropdown-menu-shortcut.svelteui/src/lib/components/ui/dropdown-menu/dropdown-menu-sub-content.svelteui/src/lib/components/ui/dropdown-menu/dropdown-menu-sub-trigger.svelteui/src/lib/components/ui/dropdown-menu/dropdown-menu-sub.svelteui/src/lib/components/ui/dropdown-menu/dropdown-menu-trigger.svelteui/src/lib/components/ui/dropdown-menu/dropdown-menu.svelteui/src/lib/components/ui/dropdown-menu/index.tsui/src/lib/utils.tsui/tsconfig.app.jsonui/tsconfig.jsonui/vite.config.ts
twangodev
changed the title
Replace the hand-rolled audio player with sv11 components: pull the audio-player + waveform registry items and adapt speaker-01's scratchable waveform into a single-file player driven by the IDE bridge. - AudioPlayerBody: sv11 AudioPlayer.Root/Button/Time/Progress/Duration plus a scrolling waveform via speaker-01's use-scratchable-waveform hook (drag to scrub, momentum, keyboard seek). The one opened file is fed as the track. - Hook adapted: playhead pinned at the LEFT edge so the waveform fills from the start; short clips stretch to fit, longer ones scroll. - Waveform decodes at runtime (precomputeWaveform), length-capped. It only mounts once bars exist and slides in, so the resting/loading layout stays compact (no empty reserved box, title not floating above dead space). - Volume kept via jetplay's VolumeControl (sv11 ships no standalone one); skip +/-10s and keyboard retained. - Dropped speaker-01's orbs (three.js weight) and playlist (single-file). - Fix the inert `dark:` variant in app.css so sv11 components theme under prefers-color-scheme (jetplay has no .dark class). App.svelte unchanged. ./gradlew test green (48); npm run build single-file 295 kB / 137 kB gzip; 27 Playwright tests green.
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ui/src/app.css (2)
72-73:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
font-sansis likely not applied to body text.Line 72 sets an explicit
bodyfont-family, so Line 129 (html {@applyfont-sans; }) won’t affect most text nodes. If Geist is intended globally, applyfont-sansonbody(or remove the explicitbodyfont-family).Also applies to: 128-130
🤖 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 `@ui/src/app.css` around lines 72 - 73, The body has an explicit font-family declaration which overrides the Tailwind/Geist utility, so update the CSS so the global Geist font utility is actually applied: either remove the explicit body font-family line (font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", system-ui, sans-serif;) or move/apply the utility by changing html { `@apply` font-sans; } to body { `@apply` font-sans; } (or add body { `@apply` font-sans; } alongside/instead of the explicit rule) so the font-sans utility takes effect for body text.
83-124:⚠️ Potential issue | 🟠 MajorUnlayered
:rootdark defaults override the@layer baselight-mode CSS variables.
ui/src/app.cssdefines the light:rootvariables inside@layer baseunder@media (prefers-color-scheme: light)(lines 57-123), but the default/dark:rootblock is unlayered (lines 140-175). Whenprefers-color-scheme: lightis active, both sets apply and normal unlayered declarations take precedence over layered ones, so the dark variables can win.
- Move the dark/default
:rootinto@layer base(or move the light:rootout of@layer base) so both palettes are in the same cascade layer and the light media query can override correctly.🤖 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 `@ui/src/app.css` around lines 83 - 124, The dark/default :root variables currently outside `@layer` base conflict with the light-mode :root inside `@layer` base because unlayered rules win; fix by placing both palettes in the same cascade layer — either move the unlayered default/dark :root block into the existing `@layer` base block, or move the `@media` (prefers-color-scheme: light) :root block out of `@layer` base so both :root declarations share the same layer; update the :root blocks around the `@layer` base and `@media` (prefers-color-scheme: light) declarations accordingly (look for :root, `@layer` base, and `@media` (prefers-color-scheme: light)).
🧹 Nitpick comments (2)
ui/src/lib/components/ui/waveform/waveform-scrubber.svelte (1)
80-121: 🏗️ Heavy liftSlider lacks keyboard support for accessibility.
The element has
role="slider"andtabindex={0}but no keyboard event handlers. Screen reader users and keyboard-only users cannot operate the scrubber. ARIA sliders require arrow key support per WCAG.Consider adding
onkeydownto handle arrow keys:onkeydown={(e) => { if (!onSeek) return; const step = duration * 0.05; // 5% increments if (e.key === 'ArrowRight') onSeek(Math.min(duration, currentTime + step)); else if (e.key === 'ArrowLeft') onSeek(Math.max(0, currentTime - step)); }}🤖 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 `@ui/src/lib/components/ui/waveform/waveform-scrubber.svelte` around lines 80 - 121, Add keyboard support to the slider container (the div with role="slider" that binds containerEl and uses handlePointerDown) by wiring an onkeydown handler that calls the existing onSeek callback (or a new wrapper) to adjust playback time using arrow keys and other slider keys; use currentTime and duration to compute new positions (e.g., small step = duration*0.05 for ArrowLeft/ArrowRight, larger step or jump to 0/duration for Home/End) and clamp values to [0, duration], then update localProgress/aria-valuenow accordingly so screen-reader and keyboard-only users can operate the scrubber.ui/src/lib/components/ui/waveform/waveform-live-microphone.svelte (1)
83-93: 💤 Low value
decodeAudioDatamay throw on closed AudioContext.
processAudioBlobchecksaudioContextReffor nullity but not its state. If the context was closed (e.g., during cleanup race),decodeAudioDatawill throw. The error is caught, but decoding silently fails.🛡️ Proposed improvement
async function processAudioBlob(blob: Blob) { try { const arrayBuffer = await blob.arrayBuffer(); - if (audioContextRef) { + if (audioContextRef && audioContextRef.state !== "closed") { const audioBuffer = await audioContextRef.decodeAudioData(arrayBuffer); audioBufferRef = audioBuffer; } } catch (error) { console.error("Error processing audio:", error); } }🤖 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 `@ui/src/lib/components/ui/waveform/waveform-live-microphone.svelte` around lines 83 - 93, processAudioBlob currently only checks audioContextRef for non-null but not whether it's usable; guard against a closed AudioContext by checking audioContextRef.state !== 'closed' (or that state is 'running'/'suspended') before calling decodeAudioData, and return early (or log a clear warning) if the context is closed to avoid silent decoding failure; ensure audioBufferRef assignment happens only after successful decodeAudioData and keep the existing try/catch for other errors.
🤖 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 `@ui/src/lib/AudioPlayerBody.svelte`:
- Around line 148-159: The handler handleKeydown currently intercepts Space for
all bubbled events and toggles player.play()/pause(), which hijacks inner
controls; change the Space branch to only run when the chrome container is
focused (same pattern as Arrow keys) by checking e.target === containerEl before
calling e.preventDefault() and toggling using player.play()/player.pause(); keep
existing ArrowLeft/ArrowRight logic and calls to skipBack()/skipForward()
unchanged.
- Around line 60-65: The id used when calling player.setActiveItem({ id:
fileName, src, ... }) is not globally unique and can collide for different files
with the same basename; change the id to a stable unique identifier (for example
use the full src, a concatenation like `${fileName}:${src}`, or a deterministic
hash of src) so setActiveItem sees different items and swaps sources correctly;
update the call in AudioPlayerBody.svelte (the player.setActiveItem invocation)
to use that unique id instead of fileName.
- Around line 68-82: The component-level flag waveformStarted never resets when
src changes and async decode promises can race and overwrite newer results;
change logic so waveformStarted (or equivalent state) is keyed to src (e.g.,
track by src value) and reset whenever src changes, clear precomputedWaveform at
start of a new src, and guard the async precomputeWaveform result by capturing
the current src (or using a local token/AbortController) inside the $effect
callback so you only assign to precomputedWaveform if the src still matches and
the operation wasn't cancelled; keep checks for dur and WAVEFORM_MAX_SECONDS and
use BARS_PER_SECOND when calling precomputeWaveform.
In `@ui/src/lib/components/ui/audio-player/audio-player-progress.svelte`:
- Around line 57-61: The pointerup handler currently sets userInteracting =
false and resumes playback (using wasPlaying and player.play()) but
pointercancel isn’t handled, leaving state incorrect if interaction is canceled;
add an onpointercancel handler that performs the exact same steps as the
onpointerup block (set userInteracting = false, resume playback when wasPlaying
is true via player.play(), and call externalPointerUp with the event) so
userInteracting/wasPlaying resume logic is consistent; update any shared logic
by extracting the handler into a named function (e.g., handlePointerEnd) and
reuse it for both onpointerup and onpointercancel to avoid duplication.
In `@ui/src/lib/components/ui/waveform/waveform-live-microphone.svelte`:
- Around line 162-174: The ResizeObserver callback currently calls
ctx.scale(dpr, dpr) without resetting the canvas transform, causing scale to
accumulate; in the ResizeObserver callback (where container, canvas, dpr and ctx
are referenced) reset the context transform before applying the devicePixelRatio
scale (use ctx.resetTransform() or ctx.setTransform(1,0,0,1,0,0)), then call
ctx.scale(dpr, dpr) so the scale is applied cleanly after resizing.
- Around line 109-112: The code incorrectly treats negative playbackRate as
unsupported; instead add runtime detection for negative-rate support and fall
back if needed: create a helper (e.g., supportsNegativePlaybackRate) that
attempts to set source.playbackRate.value to a negative value (or performs a
small test using an ephemeral AudioBufferSourceNode) and returns true/false,
then keep using rate = direction > 0 ? ... : ... for source.playbackRate.value
when supported, otherwise implement a fallback path that reverses the
AudioBuffer data (or swaps to a reversed buffer) and sets a positive
playbackRate; reference the variables/functions source.playbackRate.value, rate,
direction and use the detection helper to choose between direct negative-rate
assignment and the reversed-buffer fallback.
In `@ui/src/lib/components/ui/waveform/waveform-microphone.svelte`:
- Around line 84-100: The fade-to-idle animation (fadeToIdle) currently uses
requestAnimationFrame recursively without tracking its id, causing updates to
continue after reruns/unmounts; modify the logic to store the returned animation
id (e.g., fadeAnimId) when calling requestAnimationFrame, cancel any existing
animation with cancelAnimationFrame before starting a new fade, and ensure you
cancel the animation in the component cleanup/unmount (onDestroy) or whenever
_active/_processing changes so stale callbacks don't update data on an unmounted
or stale component.
In `@ui/src/lib/components/ui/waveform/waveform-recording.svelte`:
- Around line 59-71: The ResizeObserver handler calls ctx.scale(dpr, dpr)
without resetting prior transforms, which accumulates scale on each resize;
update the handler in the ResizeObserver callback (where you access container,
canvas, and ctx) to reset the canvas transform before applying the
devicePixelRatio scaling — e.g., use ctx.resetTransform() if available or
ctx.setTransform(1,0,0,1,0,0) to clear transforms, then apply the new scale with
ctx.scale(dpr, dpr); keep the null-check for ctx.
In `@ui/src/lib/components/ui/waveform/waveform-scrolling.svelte`:
- Around line 49-61: The ResizeObserver callback is repeatedly calling
ctx.scale(dpr, dpr) without resetting transforms, causing cumulative scaling;
update the callback in waveform-scrolling.svelte (the ResizeObserver that reads
container, canvas, dpr and gets ctx) to reset the transform before scaling—e.g.,
call ctx.setTransform(1,0,0,1,0,0) or ctx.resetTransform() (if available) and
then apply the devicePixelRatio scale (or call
ctx.setTransform(dpr,0,0,dpr,0,0)) so the canvas is correctly scaled on each
resize without accumulation.
In `@ui/src/lib/components/ui/waveform/waveform.svelte`:
- Around line 172-185: The click handler handleClick can divide by zero when
rect.width < (barWidth + barGap); compute barCount = Math.floor(rect.width /
(barWidth + barGap)) and guard it before using it (either return early if
barCount === 0 or use barCount = Math.max(1, barCount)), then compute
barIndex/dataIndex using that safe barCount and ensure dataIndex is a finite
integer before calling onBarClick(dataIndex, data[dataIndex]); reference
symbols: handleClick, canvasEl, barWidth, barGap, rect.width, barCount, data,
onBarClick.
In `@ui/src/lib/use-scratchable-waveform.svelte.ts`:
- Around line 66-90: The BiquadFilterNode created in playScratch is never
disconnected and accumulates; update playScratch/stopScratch to track the filter
node (e.g., store it in a module-level scratchFilter alongside scratchSource),
and when stopping (in stopScratch and on error) call disconnect() on both
scratchSource and scratchFilter (safely in try/catch), then null them so nodes
are released; ensure playScratch assigns scratchFilter = filter and that
stopScratch disconnects and clears scratchSource and scratchFilter.
- Around line 155-190: Momentum RAF loops started in the momentum block (via
requestAnimationFrame(step)) are not cancelled when a new scrub/reset begins,
causing racing seeks; add a cancellation handle: store the requestAnimationFrame
return value in a variable (e.g., momentumRafId) when starting the momentum
loop, cancel any existing loop with cancelAnimationFrame(momentumRafId) and set
isMomentumActive = false before starting a new momentum or on reset(), and
ensure reset() and any start-scratch/scrub entry points call this cancellation
path so stopScratch()/setOffsetAndSeek()/opts.player.play() aren't interleaved
by prior RAF callbacks.
- Around line 20-25: getScratchBuffer currently writes null results into
scratchBufferCache which permanently disables that URL after a transient
failure; change the logic so you do not cache a permanent null on failure —
either (a) only set scratchBufferCache.set(url, promise) and ensure the promise
rejects on decode failure and remove the entry on rejection, or (b) await
fetchScratchBuffer(url, ctx) and only set scratchBufferCache.set(url, buffer)
when the resolved AudioBuffer is non-null. Update getScratchBuffer and its
interactions with fetchScratchBuffer and scratchBufferCache accordingly so
transient decode errors don't leave a null in the cache.
---
Outside diff comments:
In `@ui/src/app.css`:
- Around line 72-73: The body has an explicit font-family declaration which
overrides the Tailwind/Geist utility, so update the CSS so the global Geist font
utility is actually applied: either remove the explicit body font-family line
(font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", system-ui,
sans-serif;) or move/apply the utility by changing html { `@apply` font-sans; } to
body { `@apply` font-sans; } (or add body { `@apply` font-sans; } alongside/instead
of the explicit rule) so the font-sans utility takes effect for body text.
- Around line 83-124: The dark/default :root variables currently outside `@layer`
base conflict with the light-mode :root inside `@layer` base because unlayered
rules win; fix by placing both palettes in the same cascade layer — either move
the unlayered default/dark :root block into the existing `@layer` base block, or
move the `@media` (prefers-color-scheme: light) :root block out of `@layer` base so
both :root declarations share the same layer; update the :root blocks around the
`@layer` base and `@media` (prefers-color-scheme: light) declarations accordingly
(look for :root, `@layer` base, and `@media` (prefers-color-scheme: light)).
---
Nitpick comments:
In `@ui/src/lib/components/ui/waveform/waveform-live-microphone.svelte`:
- Around line 83-93: processAudioBlob currently only checks audioContextRef for
non-null but not whether it's usable; guard against a closed AudioContext by
checking audioContextRef.state !== 'closed' (or that state is
'running'/'suspended') before calling decodeAudioData, and return early (or log
a clear warning) if the context is closed to avoid silent decoding failure;
ensure audioBufferRef assignment happens only after successful decodeAudioData
and keep the existing try/catch for other errors.
In `@ui/src/lib/components/ui/waveform/waveform-scrubber.svelte`:
- Around line 80-121: Add keyboard support to the slider container (the div with
role="slider" that binds containerEl and uses handlePointerDown) by wiring an
onkeydown handler that calls the existing onSeek callback (or a new wrapper) to
adjust playback time using arrow keys and other slider keys; use currentTime and
duration to compute new positions (e.g., small step = duration*0.05 for
ArrowLeft/ArrowRight, larger step or jump to 0/duration for Home/End) and clamp
values to [0, duration], then update localProgress/aria-valuenow accordingly so
screen-reader and keyboard-only users can operate the scrubber.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac4e9751-fc22-46c0-9e36-61cf30aa28d4
📒 Files selected for processing (27)
ui/src/app.cssui/src/lib/AudioPlayer.svelteui/src/lib/AudioPlayerBody.svelteui/src/lib/components/ui/audio-player/audio-graph.svelte.tsui/src/lib/components/ui/audio-player/audio-player-button.svelteui/src/lib/components/ui/audio-player/audio-player-duration.svelteui/src/lib/components/ui/audio-player/audio-player-progress.svelteui/src/lib/components/ui/audio-player/audio-player-speed-button-group.svelteui/src/lib/components/ui/audio-player/audio-player-speed.svelteui/src/lib/components/ui/audio-player/audio-player-time.svelteui/src/lib/components/ui/audio-player/audio-player.svelteui/src/lib/components/ui/audio-player/context.svelte.tsui/src/lib/components/ui/audio-player/example-tracks.tsui/src/lib/components/ui/audio-player/index.tsui/src/lib/components/ui/audio-player/utils.tsui/src/lib/components/ui/audio-player/waveform-sampler.tsui/src/lib/components/ui/waveform/index.tsui/src/lib/components/ui/waveform/utils.tsui/src/lib/components/ui/waveform/waveform-live-microphone.svelteui/src/lib/components/ui/waveform/waveform-microphone.svelteui/src/lib/components/ui/waveform/waveform-recording.svelteui/src/lib/components/ui/waveform/waveform-scrolling.svelteui/src/lib/components/ui/waveform/waveform-scrubber.svelteui/src/lib/components/ui/waveform/waveform-static.svelteui/src/lib/components/ui/waveform/waveform.svelteui/src/lib/use-scratchable-waveform.svelte.tsui/tests/audio-player.spec.ts
✅ Files skipped from review due to trivial changes (5)
- ui/src/lib/components/ui/audio-player/utils.ts
- ui/src/lib/components/ui/audio-player/audio-player-duration.svelte
- ui/src/lib/components/ui/waveform/waveform-static.svelte
- ui/src/lib/components/ui/audio-player/example-tracks.ts
- ui/src/lib/components/ui/waveform/index.ts
The sv11 <audio> shipped crossorigin="anonymous", which forces a CORS check. The IDE loads the player via loadHTML (null origin) and serves media as a file:// URL, so the element never loaded — nothing played, and the waveform (gated on duration) never appeared. Remove it to match the previous working player; sv11 only needs it for its orb visualizers, which jetplay doesn't use. Add a regression test asserting the audio element has no crossorigin, since Playwright can't reproduce JCEF's file:// / null-origin context.
The JCEF browser can play file:// media but can't fetch()/decode its bytes, so the in-browser waveform never appeared in the IDE. Compute it natively instead: - WaveformExtractor: decode audio with the bundled FFmpeg (JavaCV) into normalized [0,1] amplitude bars at 8/sec (matching the UI's sampleWaveform) — streamed, mono/S16, 30-min cap, graceful empty on failure. - MediaFileEditor: for local non-video audio with ffmpeg available, extract on a pooled thread and push via PlayerBridge.sendWaveform -> window.jetplayWaveform. - UI: App feeds the pushed bars to the player, which now prefers them over the in-browser precompute (kept only as the HTTP/test fallback). Tests: WaveformExtractorTest (generated sine WAV) + Playwright push test. gradle test green (49); 29 Playwright; single-file build.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
From the adversarial review of the ffmpeg waveform feature: - Bridge push race: extraction can finish before the page defines window.jetplayWaveform (short files), silently dropping the bars. Stash them on window.__jetplayWaveform and read them on mount. This is why the waveform didn't show for the 5s sintel files. - Unbounded decode: the 30-min cap relied on container duration, which is 0/AV_NOPTS_VALUE for some inputs. Enforce a hard bar ceiling in the sample loop and honor thread interruption. - Cancel the pooled decode on editor dispose (and interrupt the loop). - Skip raw telephony codecs (they need demuxer hints the extractor lacks). - Drop the dead audioChannels<=0 guard; align the browser fallback cap (was 20min) with the 30min native cap. - Tests: pushed bars take precedence over the fallback; a pre-load buffered push is picked up on mount. gradle test green; npm check clean; 31 Playwright; single-file build.
From the UI/UX audit vs sv11/speaker-01: - Right-pinned waveform playhead (sv11): scrolls in from the right, empty at rest then fills as it plays (reverts the earlier left-pin). - Muted gray bars (#a1a1aa / #71717a) instead of full --foreground. - Card panel: bordered, rounded, shadow; compact waveform (h-12 / 32px). - sv11 control styling: outline pill skip buttons, outline play button with a playing-state tint, always-visible volume (4-state icon + slider + %). - Add a playback-speed control (AudioPlayer.Speed). - Token/typography cleanup (text-foreground / text-muted-foreground). 31 Playwright green; npm check clean; single-file build.
The browser can't fetch()/decode file:// bytes, so the scrubber's DJ-scratch sound never worked in the IDE (it builds a scratch buffer by fetch()+decode of the audio). Serve local media through a tiny loopback HTTP server so the page can fetch it; the scratch now works because `src` is a fetchable http URL. This also unlocks in-browser decode and range-seeking for large files. - MediaServer: HttpServer on 127.0.0.1, serves files registered under an unguessable token, with CORS + Range (206); daemon, lazily started. - MediaFileEditor owns serving: serve(file) tracks the URL and releases it on dispose; TranscodeSession hands its output back via callback. resolvePlayableUrl removed (serving centralized). Security/correctness hardening (adversarial review): - Reject non-loopback Host headers (DNS-rebinding defense). - Per-editor token release — closed editors stop being fetchable. - Private-Network header only on the OPTIONS preflight. - Correct suffix ranges (bytes=-N), 416 past EOF, multi-range/zero-length fall back to a full 200. gradle test green (serve/range/suffix/416/host/404/release); 31 Playwright; single-file build.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/kotlin/dev/twango/jetplay/transcode/WaveformExtractor.kt (1)
34-34:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReject non-positive
barsPerSecondat the API boundary.
sampleRate / barsPerSecondwill throw for0and behave nonsensically for negative values. Guard this inextract()so callers get a deterministic argument error instead of failing deep in sampling.Suggested fix
fun extract(file: File, barsPerSecond: Int = DEFAULT_BARS_PER_SECOND): List<Double> { + require(barsPerSecond > 0) { "barsPerSecond must be > 0" } val grabber = FFmpegFrameGrabber(file).apply { audioChannels = 1 // request a mono downmix sampleMode = FrameGrabber.SampleMode.SHORT }🤖 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 `@src/main/kotlin/dev/twango/jetplay/transcode/WaveformExtractor.kt` at line 34, The extract(file: File, barsPerSecond: Int = DEFAULT_BARS_PER_SECOND) API should validate barsPerSecond at the boundary: add a guard at the start of WaveformExtractor.extract that throws an IllegalArgumentException (with a clear message) when barsPerSecond <= 0 so callers receive a deterministic argument error instead of hitting division-by-zero or nonsensical behavior later during sampleRate / barsPerSecond.src/main/kotlin/dev/twango/jetplay/editor/MediaFileEditor.kt (1)
84-88:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRe-check cancellation after waveform extraction.
waveformFuture.cancel(true)only interrupts the worker.WaveformExtractor.extract()can still return a partial non-empty list on interruption, and this block sends it without checkingbridge.disposedor the thread's interrupted state again. That makes disposal race withsendWaveform(...)on a dead browser.Suggested fix
waveformFuture = ApplicationManager.getApplication().executeOnPooledThread { if (bridge.disposed) return@executeOnPooledThread val bars = WaveformExtractor.extract(localFile) - if (bars.isNotEmpty()) bridge.sendWaveform(bars) + if (Thread.currentThread().isInterrupted || bridge.disposed || bars.isEmpty()) { + return@executeOnPooledThread + } + bridge.sendWaveform(bars) }🤖 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 `@src/main/kotlin/dev/twango/jetplay/editor/MediaFileEditor.kt` around lines 84 - 88, The waveform extraction task can return a partial list after cancellation and still call bridge.sendWaveform(...), so after WaveformExtractor.extract(localFile) re-check cancellation/disposal and only send if still valid: verify Thread.currentThread().isInterrupted() is false (or that waveformFuture is not cancelled) and bridge.disposed is false before calling bridge.sendWaveform(bars); keep the existing bars.isNotEmpty() check and perform these additional checks in the same Runnable that runs under ApplicationManager.getApplication().executeOnPooledThread so you avoid sending a waveform to a disposed bridge.
🧹 Nitpick comments (1)
ui/tests/audio-player.spec.ts (1)
45-60: 💤 Low valueConsider using the typed
Windowinterface instead ofanycasts.The test correctly verifies buffered waveform pickup, but the
(window as any)casts on lines 49 and 56 bypass TypeScript's type safety. The global window interface is already declared inui/src/global.d.tswithjetplayand__jetplayWaveformproperties.♻️ Optional type-safe refactor
await page.addInitScript(() => { - ;(window as any).jetplay = { + window.jetplay = { state: 'ready', fileName: 'x.mp3', fileExtension: 'mp3', mediaUrl: '/assets/does-not-exist.mp3', isVideo: false, } - ;(window as any).__jetplayWaveform = [0.4, 0.5, 0.6, 0.7] + window.__jetplayWaveform = [0.4, 0.5, 0.6, 0.7] })🤖 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 `@ui/tests/audio-player.spec.ts` around lines 45 - 60, Replace the unsafe (window as any) casts in the test "a waveform buffered before load is picked up" with a typed Window cast that uses the declared global interface and the two properties; e.g., create a typed alias like const w = window as unknown as Window & { jetplay?: { state: string; fileName: string; fileExtension: string; mediaUrl: string; isVideo: boolean }; __jetplayWaveform?: number[] } and then assign w.jetplay = { ... } and w.__jetplayWaveform = [0.4, 0.5, 0.6, 0.7]; reference the existing jetplay and __jetplayWaveform symbols so the test uses the proper types instead of any.
🤖 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 `@src/test/kotlin/dev/twango/jetplay/media/MediaServerTest.kt`:
- Around line 89-91: The test currently calls open(url).also { it.disconnect()
}.responseCode which disconnects the HttpURLConnection before the request is
forced; change to store the connection in a variable (e.g., val conn =
open(url)), read conn.responseCode into the assertion, then call
conn.disconnect(); do the same pattern for the second check around
MediaServer.release(url) so responseCode is read before calling disconnect() and
avoid lazy-connect flakes.
---
Outside diff comments:
In `@src/main/kotlin/dev/twango/jetplay/editor/MediaFileEditor.kt`:
- Around line 84-88: The waveform extraction task can return a partial list
after cancellation and still call bridge.sendWaveform(...), so after
WaveformExtractor.extract(localFile) re-check cancellation/disposal and only
send if still valid: verify Thread.currentThread().isInterrupted() is false (or
that waveformFuture is not cancelled) and bridge.disposed is false before
calling bridge.sendWaveform(bars); keep the existing bars.isNotEmpty() check and
perform these additional checks in the same Runnable that runs under
ApplicationManager.getApplication().executeOnPooledThread so you avoid sending a
waveform to a disposed bridge.
In `@src/main/kotlin/dev/twango/jetplay/transcode/WaveformExtractor.kt`:
- Line 34: The extract(file: File, barsPerSecond: Int = DEFAULT_BARS_PER_SECOND)
API should validate barsPerSecond at the boundary: add a guard at the start of
WaveformExtractor.extract that throws an IllegalArgumentException (with a clear
message) when barsPerSecond <= 0 so callers receive a deterministic argument
error instead of hitting division-by-zero or nonsensical behavior later during
sampleRate / barsPerSecond.
---
Nitpick comments:
In `@ui/tests/audio-player.spec.ts`:
- Around line 45-60: Replace the unsafe (window as any) casts in the test "a
waveform buffered before load is picked up" with a typed Window cast that uses
the declared global interface and the two properties; e.g., create a typed alias
like const w = window as unknown as Window & { jetplay?: { state: string;
fileName: string; fileExtension: string; mediaUrl: string; isVideo: boolean };
__jetplayWaveform?: number[] } and then assign w.jetplay = { ... } and
w.__jetplayWaveform = [0.4, 0.5, 0.6, 0.7]; reference the existing jetplay and
__jetplayWaveform symbols so the test uses the proper types instead of any.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 49a9afc1-2c0d-444d-b269-bded1c2fe1c0
📒 Files selected for processing (16)
src/main/kotlin/dev/twango/jetplay/browser/PlayerBridge.ktsrc/main/kotlin/dev/twango/jetplay/editor/MediaFileEditor.ktsrc/main/kotlin/dev/twango/jetplay/media/LocalFileMediaSource.ktsrc/main/kotlin/dev/twango/jetplay/media/MediaServer.ktsrc/main/kotlin/dev/twango/jetplay/media/MediaSource.ktsrc/main/kotlin/dev/twango/jetplay/media/RemoteFileMediaSource.ktsrc/main/kotlin/dev/twango/jetplay/transcode/TranscodeSession.ktsrc/main/kotlin/dev/twango/jetplay/transcode/WaveformExtractor.ktsrc/test/kotlin/dev/twango/jetplay/media/MediaServerTest.ktui/src/App.svelteui/src/global.d.tsui/src/lib/AudioPlayerBody.svelteui/src/lib/components/ui/audio-player/audio-player.svelteui/src/lib/use-scratchable-waveform.svelte.tsui/tests/audio-player.spec.tsui/tests/states.spec.ts
💤 Files with no reviewable changes (3)
- src/main/kotlin/dev/twango/jetplay/media/MediaSource.kt
- src/main/kotlin/dev/twango/jetplay/media/RemoteFileMediaSource.kt
- src/main/kotlin/dev/twango/jetplay/media/LocalFileMediaSource.kt
🚧 Files skipped from review as they are similar to previous changes (6)
- ui/src/global.d.ts
- ui/src/App.svelte
- src/main/kotlin/dev/twango/jetplay/browser/PlayerBridge.kt
- ui/src/lib/components/ui/audio-player/audio-player.svelte
- ui/src/lib/AudioPlayerBody.svelte
- ui/src/lib/use-scratchable-waveform.svelte.ts
- Volume control now uses the same bits-ui Slider as AudioPlayerProgress (matching track/range/thumb), instead of a hand-rolled click-only div — so it drags, shows a thumb on hover, and supports keyboard like the scrub bar. - Swap the speed control's gear (Settings) icon for a speedometer (Gauge).
The audio player's metadata header now expands into a technical readout. The IDE probes the opened file's container/codec/stream details with the bundled FFmpeg (header only, off the EDT) and pushes them to the player the same way the waveform is — stashed on window + handler call, so an early push isn't dropped. Collapsed shows the filename plus a glanceable summary line (WAV · 48 kHz · 16-bit · stereo · 1.2 MB); clicking expands a full label/value grid (codec, container, sample rate, channels, bit depth, bitrate, duration, size). With no info pushed the header looks exactly as before — remote / raw-codec / ffmpeg-unavailable paths degrade cleanly. - MediaInfoExtractor: FFmpeg header probe → MediaInfo. RAW sample mode to read the true source sample format; PCM bit depth from the codec name; lossless via sample format; bitrate falls back to size/duration. - PlayerBridge.sendMediaInfo / MediaFileEditor.maybeSendMediaInfo mirror the waveform path (pooled thread, same guards, cancelled on dispose). - Gate the player's Space/Arrow shortcuts to the container so a focused inner control (the new toggle, mute, slider) receives its native key instead of having Space swallowed into play/pause. Tests: MediaInfoExtractorTest (probe a generated WAV; null for non-audio); Playwright for summary/expand, buffered push, and Space-on-toggle.
…badge) The extension badge already shows the format, so leading the summary line with the container repeated it (e.g. "WAV" badge + "WAV · 48 kHz · …"). Remove the container token from the summary; the exact demux container still appears in the expanded grid's Container row.
The codec inspector now also reads the file's embedded metadata. FFmpeg's getMetadata() gives the container tags (title/artist/album/date/genre/…, plus rating when present); they render as a separate Tags group in the expanded panel, above the technical stream grid. Embedded cover art is pulled from the attached-picture stream (the packet data is the encoded image — no decode) and shown as a blurred, dimmed ambient background behind the player card, not a thumbnail. - MediaInfoExtractor: buildTags() maps FFmpeg's normalized keys to ordered labels; extractAlbumArt() reads the first attached_pic stream, sniffs the image type, and base64s it into a data URL (capped at 4 MB). - PlayerBridge: the media-info push is now built as strict JSON (jsonString escapes quotes/backslash/controls and U+2028/2029) since tag text is arbitrary user input. - UI: Tags group + blurred album-art layer (fades in); the expanded panel groups descriptive tags then technical details; aria-controls points at it. Tests: buildTags ordering/skip; mediaInfoJson tag embedding + escaping; Playwright for the tags panel and the album-art background.
There was a problem hiding this comment.
detekt found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
getAudioCodecName() returns the decoder name — "mp3float" for MP3 — which is wrong and confusing in a codec inspector. Derive the codec from its id via avcodec_get_name() instead, giving "mp3", "aac", "flac", "pcm_s16le", … Caught by probing real MP3 files (the WAV unit test couldn't surface it, since the PCM decoder name already matches the codec name). Also gitignore the large local-only test tracks dropped into assets/: that dir is symlinked into ui/public and bundled into the plugin jar, so the multi-MB files must stay out of git history and the shipped artifact.
Five royalty-free MP3s in assets/ with real ID3 tags and embedded album cover art, for exercising the codec inspector — the tags group and the blurred album-art background — in runIde and the Playwright dev server. Music by Kevin MacLeod (incompetech.com), CC BY 4.0; see assets/CREDITS.md. Reverts the gitignore rule that had kept them local-only.
Rewrites the cinema video player with modern overlay controls and extends the codec inspector to video files. UI: - bits-ui Slider for the seek bar and volume (same primitive as the audio player), restyled white for over-video. - Speed (Gauge dropdown), fullscreen, skip ±10s, and frame-step controls. - Top overlay bar with filename + an expandable inspector panel grouping Video (resolution/fps/codec/color/bitrate), Audio, General, and tags. - Auto-hide stays but is suspended while the inspector is open; Space/arrow shortcuts gate to the container so focused controls get their own keys. - Retire the hand-rolled SeekBar/VolumeControl; share the inspector formatters via mediaInfoFormat.ts (audio player refactored onto it). Data: - MediaInfoExtractor reads video streams now (width/height/fps, codec via avcodec_get_name, pixel format via av_get_pix_fmt_name, bitrate) and returns info when either an audio OR video stream exists. ImageMode.RAW so the source pixel format is reported, not the decoder's output. - MediaFileEditor probes video too (drops the isVideo skip); PlayerBridge and global.d.ts carry the video fields. Tests: real assets/sintel.webm probe in MediaInfoExtractorTest; Playwright for the new controls, the speed dropdown, and the video metadata panel.
The seek bar's value tracks currentTime; bits-ui re-snaps that off-grid value onto the step on every timeupdate and re-fires onValueChange, which re-seeked the video every frame. Guard the seek so onValueChange only seeks during a real pointer scrub, and drive keyboard nudges directly. - Clear the scrub guard from a window pointerup/pointercancel (bits-ui releases on document, not the element), so releasing off the thin track can't leave the guard stuck open (which re-armed the stutter on replay) or the video paused. - Intercept arrow/Home/End/Page on the focused seek thumb in the capture phase and seek directly, stopping bits-ui from desyncing the thumb (its onValueChange fires before a Root-level handler could open the guard). - Drop the volume-slider guards: volume is always on the step grid (0..1, 0 on mute) so bits-ui never re-snaps it — the guard was unnecessary and only broke keyboard volume control. - Make playwright.config resilient to a busy port 5173 (JETPLAY_TEST_PORT). Tests: no self-seek during playback, keyboard seek on the focused thumb, and scrub-release-off-track resumes playback; audio mute preserves the volume level.
… to hang Video transcoding re-encodes to VP9 with libvpx, which defaults to a very slow deadline — multiples of real time, so converting a normal HD clip looked like the "Converting for playback…" state was stuck. We only need a watchable preview, so encode with deadline=realtime, cpu-used=8, row-mt, and all cores. Measured: a 5s 854x480 clip dropped 4389ms -> 535ms (~8x); HD now transcodes faster than real time instead of ~5x slower.
Drop the fullscreen button, toggle, F shortcut, fullscreenchange listener, and the Maximize/Minimize icons.
The transcode/download paths deliver mediaReady asynchronously, but htmlLoader.load() and the transcode run concurrently — a fast transcode can call window.jetplayReady before the Svelte app has mounted and defined the handler, so the `?.` call is a silent no-op. The page then mounts showing the initial state:'loading' and sticks on "Converting for playback…" at 0% with no further transition. (The VP9 realtime speedup made this race much easier to lose.) Buffer the latest state push on window (__jetplayReadyUrl / __jetplayError / __jetplayProgress / __jetplayState) and seed the app from it on mount — the same pattern already used for the waveform and codec-inspector pushes.
Extract named constants for HTTP status codes, audio/bitrate magic numbers and image signatures; make bitDepth a single-expression when (<=3 returns); cut loop jump-statements in the waveform and album-art readers; simplify MediaServer's range condition; and suppress the inherent TooManyFunctions on the FileEditor impl. ktlint formatting rules auto-corrected across the touched files. No behavior change.
- app.css: apply Geist (--font-sans) to body so it actually takes effect
- button: real hover on the anchor render path; make disabled links
non-interactive (anchors don't match the disabled: pseudo-class)
- audio scratch: evict failed-decode cache entries, disconnect per-slice
filter nodes, and cancel in-flight momentum on new scrub/reset
- AudioPlayerBody: key the track id and waveform decode to src (guard
stale async results) so re-used instances swap source correctly
- progress slider: handle pointercancel like pointerup
- waveform: guard divide-by-zero on very narrow containers
- dropdown: forward {checked,indeterminate} to checkbox-item children;
drop the dead data-inset utility on the label
- waveform-microphone: store and cancel the fade-to-idle RAF on cleanup
- waveform-recording: reset canvas transform before scaling (defensive)
- TranscodeSession: guard cancel()/onReady with a lock so a late
callback can't run after the editor is disposed
- MediaServerTest: read responseCode before disconnect (JDK flake)
- utils: unknown instead of any (drop dead eslint-disable directives)
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
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.
Refresh the JCEF media player by adopting sv11-ui components.
Summary by CodeRabbit
New Features
UI Components
Tests
Chores