Skip to content

Detached#17

Merged
RedSTARO merged 2 commits intomasterfrom
detached
Mar 10, 2026
Merged

Detached#17
RedSTARO merged 2 commits intomasterfrom
detached

Conversation

@RedSTARO
Copy link
Copy Markdown
Owner

No description provided.

- Renamed `getSongUri` to `getOnlinePlaySongUri` and updated it to construct a direct server URL string instead of using Retrofit.
- Renamed `getSongUris` to `getDownloadBatchSongUris` and updated it to return manually constructed `SongUrlV1Data` objects with direct server URLs.
- Integrated `DataStoreManager` to fetch the server domain dynamically.
- Updated call sites in `DownloadWorker` and `DataSource` to reflect the new method names.
@RedSTARO RedSTARO marked this pull request as ready for review March 10, 2026 13:22
@RedSTARO RedSTARO merged commit 585108a into master Mar 10, 2026
1 check failed
@RedSTARO RedSTARO deleted the detached branch March 10, 2026 13:23
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3001af8d1d

ℹ️ 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".

}
Log.d("RedirectingDataSource", "Redirecting URI: ${dataSpec.uri} to $currentUri")

val redirectedSpec = dataSpec.buildUpon().setUri(currentUri!!).build()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Guard redirected URI before dereferencing it

RedirectingDataSource.open only assigns currentUri when the incoming URI matches the custom redefinencm://playbackPlaceHolder shape, but it unconditionally calls currentUri!! afterward. Any non-placeholder media item URI (for example, a direct http/file/content item from another controller path) will hit this code path and crash playback with an NPE instead of falling back to the original URI.

Useful? React with 👍 / 👎.

return songIds.map { id ->
SongUrlV1Data(
id = id,
url = "${runBlocking { DataStoreManager.getStringItem("server", "http://ncm.tryagain.icu/") }}song/url/v1/302?id=$id&level=${SettingProvider.downloadQuality}",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Return downloadable media URLs instead of /302 endpoint URLs

This now builds download URLs as /song/url/v1/302?..., but DownloadWorker.enqueueDownload derives the saved extension from uri.lastPathSegment; for these URLs the segment is always 302, so files are saved as <songId>.302. That produces non-audio filenames and breaks compatibility with external players/scanners that rely on standard audio extensions.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant