Repository navigation
fix(local): repair metadata decoding and song restoration - #545
Conversation
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. |
|
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:
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🔵 Low · up to
|
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments: 1
modules/playback/runtime/src/main/java/moe/ouom/neriplayer/core/player/persistence/PlayerManagerMetadataBaseline.kt: 1
47-82: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win在 SAF 权限失效时跳过原始歌词扫描
inspectLyricsFast会重新抛出 SAF 读取产生的SecurityException。preserveLocalSongMetadataOriginals在本地歌词、翻译歌词和自定义信息写入前调用它。runSongMetadataMutation和公开编辑入口都不捕获该异常,因此编辑会在写入同步状态或歌曲状态前失败。手动匹配虽然捕获异常,但也只会显示失败提示。原始歌词扫描是基线读取,不应阻止编辑。权限失效时应跳过该扫描并继续使用已有歌曲数据。
Suggested fix
- LocalMediaSupport.inspectLyricsFast( - context = application, - song = localSong, - includeStoredFallback = false, - forceRefresh = true - ) + try { + LocalMediaSupport.inspectLyricsFast( + context = application, + song = localSong, + includeStoredFallback = false, + forceRefresh = true + ) + } catch (_: SecurityException) { + null + }
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: cwuom/NeriPlayer/.coderabbit.yaml
- Review profile: QUIET
- Plan: Advanced
- Run ID:
833d3afe-48fe-45ff-9615-0a1b192ba796
⛔ Files ignored due to path filters: 1
modules/local/README.mdis excluded by!**/*.md
📒 Files selected for processing: 33
app/src/androidTest/java/moe/ouom/neriplayer/data/local/media/DownloadedM4aStagingRegressionTest.ktapp/src/androidTest/java/moe/ouom/neriplayer/data/local/media/LocalMediaSupportSafLyricsTest.ktapp/src/main/java/moe/ouom/neriplayer/ui/screen/nowplaying/edit/NowPlayingSongEditPolicy.ktapp/src/main/java/moe/ouom/neriplayer/ui/screen/nowplaying/edit/NowPlayingSongEditRestoreSources.ktapp/src/test/java/moe/ouom/neriplayer/ui/screen/NowPlayingManagedRestoreBaselineTest.ktapp/src/test/java/moe/ouom/neriplayer/ui/screen/NowPlayingSongEditRestoreSourcesTest.ktmodules/local/src/main/java/moe/ouom/neriplayer/data/local/audioimport/LocalAudioImportManager.ktmodules/local/src/main/java/moe/ouom/neriplayer/data/local/audioimport/LocalAudioImportManagerScan.ktmodules/local/src/main/java/moe/ouom/neriplayer/data/local/media/LocalMediaMetadataText.ktmodules/local/src/main/java/moe/ouom/neriplayer/data/local/media/LocalMediaSupport.ktmodules/local/src/main/java/moe/ouom/neriplayer/data/local/media/LocalMediaSupportAssets.ktmodules/local/src/main/java/moe/ouom/neriplayer/data/local/media/LocalMediaSupportContracts.ktmodules/local/src/main/java/moe/ouom/neriplayer/data/local/media/LocalMediaSupportFacadeInspect.ktmodules/local/src/main/java/moe/ouom/neriplayer/data/local/media/LocalMediaSupportFacadeScan.ktmodules/local/src/main/java/moe/ouom/neriplayer/data/local/media/LocalMediaSupportSaf.ktmodules/local/src/main/java/moe/ouom/neriplayer/data/local/media/metadata/LocalMediaSupportMetadata.ktmodules/local/src/main/java/moe/ouom/neriplayer/data/local/playlist/LocalPlaylistRepositoryMetadata.ktmodules/local/src/test/java/moe/ouom/neriplayer/data/local/audioimport/LocalAudioImportMetadataEncodingTest.ktmodules/local/src/test/java/moe/ouom/neriplayer/data/local/audioimport/LocalAudioImportOriginalMetadataTest.ktmodules/local/src/test/java/moe/ouom/neriplayer/data/local/media/LocalMediaDocumentLyricReferenceEnsureTest.ktmodules/local/src/test/java/moe/ouom/neriplayer/data/local/media/LocalMediaLyricSidecarRepairTest.ktmodules/local/src/test/java/moe/ouom/neriplayer/data/local/media/LocalMediaLyricTransactionTest.ktmodules/local/src/test/java/moe/ouom/neriplayer/data/local/media/LocalMediaMetadataEncodingTest.ktmodules/local/src/test/java/moe/ouom/neriplayer/data/local/media/LocalMediaSupportTest.ktmodules/local/src/test/java/moe/ouom/neriplayer/data/local/media/metadata/LocalMediaLyricSidecarMutationTest.ktmodules/local/src/test/java/moe/ouom/neriplayer/data/local/playlist/LocalPlaylistRepositoryTest.ktmodules/playback/runtime/src/main/java/moe/ouom/neriplayer/core/player/metadata/PlayerMetadataHelper.ktmodules/playback/runtime/src/main/java/moe/ouom/neriplayer/core/player/persistence/PlayerManagerMetadataBaseline.ktmodules/playback/runtime/src/main/java/moe/ouom/neriplayer/core/player/persistence/PlayerManagerPersistenceExtensions.ktmodules/playback/runtime/src/test/java/moe/ouom/neriplayer/core/player/metadata/PlayerManagerCustomMetadataNormalizationTest.ktmodules/playback/runtime/src/test/java/moe/ouom/neriplayer/core/player/metadata/PlayerManagerSearchMetadataTest.ktmodules/playback/runtime/src/test/java/moe/ouom/neriplayer/core/player/metadata/PlayerMetadataHelperCoverTest.ktmodules/playback/runtime/src/test/java/moe/ouom/neriplayer/core/player/persistence/PlayerManagerMetadataBaselineTest.kt
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b209618df0
ℹ️ 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".
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments: 1
modules/local/src/main/java/moe/ouom/neriplayer/data/local/media/LocalMediaMetadataText.kt: 1
36-36: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win不要仅凭两个高字节判定 GB18030。
例如,合法的 Latin-1 文本
"éè"只有一个长度为 2 的高字节片段。该片段也可按 GB18030 解为汉字并通过重编码检查,因此decodeContainerText()会替换原文。请在转换这种有歧义的短文本前要求额外的编码证据;否则保留原文。
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: cwuom/NeriPlayer/.coderabbit.yaml
- Review profile: QUIET
- Plan: Advanced
- Run ID:
5f80d0dd-6af1-4b0d-a34d-5da1474df97b
📒 Files selected for processing: 9
modules/local/src/main/java/moe/ouom/neriplayer/data/local/media/LocalMediaMetadataText.ktmodules/local/src/main/java/moe/ouom/neriplayer/data/local/media/LocalMediaSupport.ktmodules/local/src/main/java/moe/ouom/neriplayer/data/local/media/LocalMediaSupportFacadeInspect.ktmodules/local/src/test/java/moe/ouom/neriplayer/data/local/media/LocalMediaEmbeddedLyricsCancellationTest.ktmodules/local/src/test/java/moe/ouom/neriplayer/data/local/media/LocalMediaLyricTransactionTest.ktmodules/local/src/test/java/moe/ouom/neriplayer/data/local/media/LocalMediaMetadataEncodingTest.ktmodules/playback/runtime/src/main/java/moe/ouom/neriplayer/core/player/persistence/PlayerManagerMetadataBaseline.ktmodules/playback/runtime/src/main/java/moe/ouom/neriplayer/core/player/persistence/PlayerManagerPersistenceExtensions.ktmodules/playback/runtime/src/test/java/moe/ouom/neriplayer/core/player/persistence/PlayerManagerLocalMetadataEditRegressionTest.kt
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97c5980514
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcd78efb97
ℹ️ 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".
描述 / Description
Local music could show corrupted title or artist text, retain incorrectly matched lyrics and artwork after Restore, and create duplicate or empty lyric sidecars.
类型 / Type
修复或解决的问题 / Issues Fixed or Closed by This PR
Fixes #544
Also fixes modified lyrics or incorrect artwork surviving Restore, duplicate sidecars, and partial companion writes after failure or cancellation.
清单 / Checklist
其他信息 / Additional Information
Local validation used JDK 21:
LocalAudioImportSafLyricsTest,DownloadCorePublicationInstrumentedTest,CommentLandscapeLayoutTest,DownloadedM4aStagingRegressionTest,LocalMediaCompanionRollbackTest,LocalMediaSupportSafLyricsTest,LocalCoverBaselineInspectorInstrumentedTest, andIssue339MetadataCreationContractTeston the task-owned API 35 arm64 emulator. Native M4A regression tests verified unchanged audio sample hashes and duration. Display orientation policy was restored after the landscape tests.git diff --checkpassed. No compiler or lint warnings were introduced in the modified files.Remote CI for
e6fe4be168ac0952a3786f6eb6f006d1d7d9835fcompleted successfully: NeriPlayer Android CI. All seven intended jobs passed, including both Android test shards, the External Lyrics host, Gradle quality gates, Listen Together, Worker checks, and the unsigned Release build with its Native USB host tests. RemotetestDebugUnitTestreports match the local 13,539-case totals above; the 38 targeted raw-tag, scanned-persistence, and preview-policy regressions all ran without skips.The latest Codex review for this head completed with no unresolved findings, and all seven existing review threads are resolved.
Coverage limits: the original reported MP3 files were unavailable. Encoding and MediaStore corruption cases use generated ID3 fixtures, including stale metadata snapshots, partial tags, large leading artwork, and content-only providers. Non-seekable streams retain a bounded read budget and may keep the queried fallback when later identity frames cannot be reached. No physical-device or vivo OEM-provider testing was performed. The local emulator uses API 35 arm64 with phone geometry; remote CI uses its configured API 35 x86_64 images. Local builds preserve the pre-existing unrelated miuix checkout; remote CI validates the committed submodule revision.
The UI checklist concerns test-fixture validation in this change; both local landscape captures were inspected. Existing unrelated warnings remain outside this change.