Skip to content

Implement storage download export flow and update database schema#990

Open
agupta07505 wants to merge 25 commits into
rukamori:devfrom
agupta07505:feature/Save-to-localStorage
Open

Implement storage download export flow and update database schema#990
agupta07505 wants to merge 25 commits into
rukamori:devfrom
agupta07505:feature/Save-to-localStorage

Conversation

@agupta07505

Copy link
Copy Markdown

Pull Request

Summary

This pull request introduces the ability for users to select a custom local folder to export downloaded songs using Android's Storage Access Framework (SAF). When a download completes, the DownloadedSongExporter automatically retrieves metadata from the local database, retrieves high-resolution artwork via Coil, packages the cached audio spans, embeds appropriate metadata tags (ID3/MP4 Atoms) directly in the file, and scans the file using Android's Media Scanner. The change adds custom location preferences to the Storage settings UI and upgrades the internal database schema to version 31.

Linked Work

  • Closes:
  • Related:

Change Type

  • Feature
  • Bug fix
  • UI / UX
  • Performance / memory
  • Playback / Media3
  • Lyrics / provider integration
  • Search / YouTube / network data
  • Local library / Room database
  • Widgets / notification / shortcuts
  • Settings / preferences / DataStore
  • Discord / Last.fm / ListenBrainz / external integration
  • Localization / strings / fastlane metadata
  • Build / Gradle / CI / release packaging
  • Dependency update
  • Documentation only

Affected Surfaces

  • :app
  • :core
  • :lyrics
  • :lastfm
  • :canvas
  • :shazamkit
  • :spotifycore
  • Other:

Screenshots / Recordings

Before After
N/A N/A

Behavior Notes

  • Fallback to Default: If no custom storage location is chosen by the user, the exporter defaults to the internal application-managed directory.
  • Permission Retention: When choosing a new folder, persistable read/write URI permissions are acquired, and any previously active permissions are released.
  • Export Triggering: Runs on a background scope whenever a download reaches Download.STATE_COMPLETED.
  • Metadata Embedding: Supports writing ID3 metadata tags (for mp3/aac) and recursively parses/adjusts MP4 chunk offsets (stco/co64 atoms) for m4a containers to ensure files remain playable after embedding metadata and artwork.
  • Error Resilience: Exception handling is implemented for cases of revoked SAF folder permission or metadata loading delay, showing a local warning snackbar instead of crashing the application.

Architecture Checklist

  • The change preserves UDF flow: UI -> ViewModel -> UseCase/domain -> Repository/data.
  • Screen state is represented structurally with Loading, Success, Empty, and Error where this PR introduces or changes screen state.
  • Composables receive immutable, UI-specific domain models instead of raw Room, network, or service entities.
  • Business work is not triggered directly from composition.
  • Exceptions are surfaced through explicit state or result types instead of being swallowed.
  • No runBlocking is introduced in app execution paths.

Compose / Material Checklist

  • UI state is hoisted out of composable layout code.
  • Reactive state is collected with collectAsStateWithLifecycle().
  • New UI models are annotated with @Immutable or @Stable.
  • Lazy layouts use stable key values and explicit contentType.
  • Non-primitive constants, structural lambdas, and allocation-heavy objects in hot paths are remembered.
  • Rapidly changing inputs such as scroll, gesture, animation, or playback progress use derivedStateOf where appropriate.
  • UI strings come from stringResource() and duplicated visible strings on the same screen are avoided.
  • Interactive elements keep a minimum 48dp touch target.
  • Material 3 / Material 3 Expressive tokens are used for color, typography, shape, and motion instead of hardcoded UI values.
  • Edge-to-edge layouts handle and consume WindowInsets correctly when this PR changes screen layout.

Concurrency / Performance Checklist

  • Business coroutines are scoped to viewModelScope or an existing lifecycle-owned application/service scope.
  • Disk, database, network, parsing, and heavy mapping work is dispatched to Dispatchers.IO or Dispatchers.Default.
  • Cancellation is handled explicitly where long-running work, playback, downloads, sync, lyrics fetching, or recognition is involved.
  • The change avoids blocking the main thread.
  • The change avoids unnecessary allocations in recomposition loops, playback loops, polling loops, and provider parsing paths.
  • Large lists, images, lyrics payloads, and network responses are bounded, streamed, cached, paged, or mapped off the main thread as appropriate.

Data / Persistence Checklist

  • Room entity, DAO, or database changes include a schema update under app/schemas.
  • Database migrations preserve existing user data and fail clearly when migration is impossible.
  • DataStore or preference-key changes are backward compatible.
  • Network DTOs remain isolated from UI models.
  • Provider responses handle missing, malformed, regional, or rate-limited data without crashing the app.

Playback / Integration Checklist

  • Media3 playback, queue, cache, and service behavior remains stable across foreground, background, notification, and process recreation paths.
  • Audio focus, notification controls, widgets, shortcuts, and media session actions are considered when playback behavior changes.
  • External integrations handle absent credentials, revoked auth, network failure, and API changes.
  • Native, AAR, or ABI-sensitive changes account for mobile/tv and universal, arm64, armeabi, x86, and x86_64 variants.

Localization / Assets Checklist

  • User-facing strings are added to the base resources and translated resources are updated or intentionally left for translation follow-up.
  • Unused string resources, drawables, and metadata are removed when replaced.
  • Image assets have explicit display dimensions and are decoded at the displayed size.
  • Fastlane metadata, screenshots, icons, or release text are updated when user-facing store behavior changes.

Privacy / Security Checklist

  • No secrets, keys, tokens, keystores, signing files, private certificates, or local machine paths are committed.
  • Logs do not expose access tokens, cookies, auth headers, user identifiers, listening history, or local file paths.
  • New network calls are justified by the feature and use existing client, proxy, timeout, and error-handling patterns.
  • User data remains local unless the PR explicitly documents the integration and consent path.

Verification

  • Android Studio sync: Verified successful Gradle sync and resource compiling.
  • Manual device/emulator verification: Verified selecting target directories via SAF picker and confirmed downloaded songs successfully write with complete metadata and artwork.
  • UI screenshot/recording attached: None.
  • Accessibility or touch-target review: Preference location elements utilize standard list entry items with at least 48dp touch height.
  • Regression areas checked: Verified internal storage downloads continue to function normally when custom export directory is not configured.
  • CI expectation: Code builds successfully without any errors.

Reviewer Focus

  • Inspect MP4 atom writing/offset shift implementation (adjustMp4ChunkOffsets, adjustStcoAtom, adjustCo64Atom) inside DownloadedSongExporter.kt to ensure binary data alteration preserves file structure.
  • Verify safe usage of takePersistableUriPermission and lifecycle permission releases within StorageSettings.kt to avoid permission leaks.

Release Notes

  • Added option to select a custom local storage location for auto-exporting downloaded songs with embedded artwork and metadata.

@agupta07505
agupta07505 force-pushed the feature/Save-to-localStorage branch from bb69dd8 to c500b3b Compare July 6, 2026 08:21
@rukamori

Copy link
Copy Markdown
Owner

still isn’t ready to merge. The most critical issue is that the exporter can use player-cache spans and only checks for gaps, not whether the final size matches the completed download, so an incomplete file could be exported as valid. The SAF flow also deletes the previous export before the replacement is fully written, while metadata-writing failures are treated as success, which risks data loss or silently broken/untagged files. Removing a download currently deletes the user’s exported copy too, which should only happen if that behavior is clearly intended. Changing the selected folder also doesn’t migrate existing exports because an old internal export is still counted as already exported. Please make replacement atomic, validate the final file length, preserve exported files by default, re-export existing downloads after changing folders.

@agupta07505

Copy link
Copy Markdown
Author

I have fixed the issues try to verify it...
Now it's working and saving to storage with thumbnails and other metadata....
(Maybe sometimes it takes a little bit of time to reflect in storage but it works perfectly...)

@agupta07505
agupta07505 marked this pull request as draft July 16, 2026 01:29
@agupta07505
agupta07505 force-pushed the feature/Save-to-localStorage branch from 510a077 to 7fd4732 Compare July 18, 2026 18:26
@agupta07505
agupta07505 marked this pull request as ready for review July 19, 2026 12:13
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.

2 participants