fix(frontend): show media details for Remote Dev / Gateway files#100
Conversation
maybeSendMediaInfo short-circuited on source.isRemote, so no MediaInfo payload ever reached the player for a remote file. Without a payload hasMediaInfo stays false and the details disclosure chevron is never rendered. The probe runs host-side and resolves the file by id, exactly like the spectrogram, which already omits the guard, so remote files can be probed too. Gate on a new EditorMediaSource.canProbeMediaInfo that excludes only raw headerless codecs.
📝 WalkthroughWalkthroughAdds a ChangesMedia probing eligibility
Estimated code review effort: 1 (Trivial) | ~5 minutes Sequence Diagram(s)sequenceDiagram
participant MediaLoader
participant EditorMediaSource
MediaLoader->>EditorMediaSource: check canProbeMediaInfo
EditorMediaSource-->>MediaLoader: true/false
MediaLoader->>MediaLoader: proceed or skip media-info probe
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR enables media details (MediaInfo payload and the UI disclosure chevron) for Remote Dev / Gateway files by removing the isRemote short-circuit and relying on a dedicated eligibility flag on EditorMediaSource, while still excluding headerless raw audio codecs from probing.
Changes:
- Add
EditorMediaSource.canProbeMediaInfoto centralize MediaInfo-probing eligibility (remote files allowed; raw audio excluded). - Update
MediaLoader.maybeSendMediaInfo()to gate solely onsource.canProbeMediaInfo. - Extend
EditorMediaSourceTestto cover remote-likeLightVirtualFileeligibility and raw audio ineligibility.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| frontend/src/test/kotlin/dev/twango/jetplay/media/EditorMediaSourceTest.kt | Adds tests for Remote Dev/Gateway-like files and raw audio MediaInfo probing eligibility. |
| frontend/src/main/kotlin/dev/twango/jetplay/media/EditorMediaSource.kt | Introduces canProbeMediaInfo to allow host-side probing for remote files while excluding raw codecs. |
| frontend/src/main/kotlin/dev/twango/jetplay/editor/MediaLoader.kt | Removes the remote/raw-audio short-circuit from maybeSendMediaInfo in favor of canProbeMediaInfo. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/main/kotlin/dev/twango/jetplay/editor/MediaLoader.kt (1)
136-142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuard looks correct; consider reusing
canProbeMediaInfoinrequestSpectrogramtoo.The new gate correctly mirrors the "host-side, resolve-by-id, raw-audio excluded" pattern already used for spectrograms. However,
requestSpectrogram()(Line 116) still inlines its ownsource.extension.lowercase() in MediaClassification.rawAudioExtensionscheck instead of the newcanProbeMediaInfoproperty, which now expresses the exact same rule. Keeping two independent expressions of the same eligibility rule risks drift if the raw-audio set logic changes.♻️ Proposed consolidation
private fun requestSpectrogram() { if (!spectrogramRequested.compareAndSet(false, true)) return - // Headerless raw codecs lack the demuxer hints to decode cleanly, so skip them outright. - if (source.extension.lowercase() in MediaClassification.rawAudioExtensions) { + // Headerless raw codecs lack the demuxer hints to decode cleanly, so skip them outright. + if (!source.canProbeMediaInfo) { if (!bridge.disposed) bridge.sendSpectrogram(null) return }🤖 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 `@frontend/src/main/kotlin/dev/twango/jetplay/editor/MediaLoader.kt` around lines 136 - 142, `requestSpectrogram()` is still duplicating the media-info eligibility rule instead of using the new `canProbeMediaInfo` property. Update `MediaLoader.requestSpectrogram` to check `source.canProbeMediaInfo` the same way `maybeSendMediaInfo()` does, so the host-side/raw-audio exclusion logic lives in one place and stays consistent if `MediaClassification.rawAudioExtensions` changes.
🤖 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.
Nitpick comments:
In `@frontend/src/main/kotlin/dev/twango/jetplay/editor/MediaLoader.kt`:
- Around line 136-142: `requestSpectrogram()` is still duplicating the
media-info eligibility rule instead of using the new `canProbeMediaInfo`
property. Update `MediaLoader.requestSpectrogram` to check
`source.canProbeMediaInfo` the same way `maybeSendMediaInfo()` does, so the
host-side/raw-audio exclusion logic lives in one place and stays consistent if
`MediaClassification.rawAudioExtensions` changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a55e2c7f-d16d-4de2-a3d1-accdec68489a
📒 Files selected for processing (3)
frontend/src/main/kotlin/dev/twango/jetplay/editor/MediaLoader.ktfrontend/src/main/kotlin/dev/twango/jetplay/media/EditorMediaSource.ktfrontend/src/test/kotlin/dev/twango/jetplay/media/EditorMediaSourceTest.kt
Qodana for JVMIt seems all right 👌 No new problems were found according to the checks applied 💡 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.3
with:
upload-result: trueContact Qodana teamContact us at qodana-support@jetbrains.com
|
Drops the
isRemoteshort-circuit inmaybeSendMediaInfoso a MediaInfo payload (and thus the details disclosure chevron) reaches the player for Remote Dev / Gateway files, which probe host-side by file id exactly like the spectrogram already does.Summary by CodeRabbit