Wear playlist transfer — PR 3: transcodificado y estimación en el teléfono - #9
Merged
PonceGL merged 19 commits intoAug 9, 2026
Merged
Conversation
Nothing in this codebase injects CoroutineDispatchers today — repo-wide convention is Dispatchers.IO/.Main referenced directly, which fails AND-CONC-03 and forces tests to depend on the real dispatcher. Rather than migrate existing code (out of scope, high blast radius), these qualifiers are for new code only: WatchAudioTranscoder is the first consumer, needing an injected Main dispatcher so its Transformer setup (which requires a Looper thread) is swappable in tests.
…h handoff WatchAudioTranscoder decides whether a song needs re-encoding before it goes to the watch, and performs that encode with media3-transformer (already a declared dependency, previously unused anywhere in the codebase). Lossless/high-bitrate sources are transcoded to AAC-LC 128kbps — watches decode AAC in hardware, and 128kbps keeps the Bluetooth transfer time down until a Wi-Fi transport exists (a later phase). Already-lossy sources at or under 256kbps pass through untouched: re-encoding an already-small MP3 only costs CPU and quality for no transfer-time benefit. requiresTranscoding is a pure function, split out from the actual encode specifically so it stays cheap to unit test — the encode path itself needs a real hardware encoder and a Looper thread, so it can only be verified on a device.
Pure size/time heuristics for the send-to-watch confirmation sheet, computed only over songs that aren't already on the watch. Sizes a song by its transcoded target bitrate when it'll be re-encoded, or by its own bitrate when it'll pass through untouched. The Bluetooth throughput assumption is deliberately conservative and needs re-measuring against a real phone+watch pair once device testing resumes — an estimate that undershoots erodes trust in the sheet more than one that's a bit pessimistic.
…timator WatchAudioTranscoderTest covers requiresTranscoding's full branch matrix (lossless, passthrough bitrate boundary, unknown mime/bitrate, case sensitivity) — 7/7 passing. WatchPlaylistTransferEstimatorTest covers passthrough vs transcoded sizing, that only pending songs are summed, and the zero/near-zero edge cases — 5/5 passing. Verified: :app:testDebugUnitTest, full suite, 426 tests, only the same 5 pre-existing failures unrelated to this branch (confirmed by running the identical suite against a clean dev-personal worktree: 414 tests, same 5 failures, same names).
…transcoded file Adds a WatchAudioOverride hook to startTransferToWatch/performTransfer: when present, it substitutes the song's own file resolution and eligibility check entirely, streaming the given file with its own mimeType/bitrate reported in the transfer metadata instead of the original song's. The override file was just written locally by WatchAudioTranscoder, so it's unconditionally eligible regardless of where the source song actually lives (local file, cloud proxy, ...). No behavior change for existing callers — audioOverride defaults to null.
The rest of the wear/ package resolves these via Wearable.getXClient(application) internally, which is fine for production but can't be faked in a JVM unit test — mocking Wearable's static factory methods needs MockK's inline-mocking agent, and its dynamic self-attach hangs indefinitely in this environment's sandboxing (confirmed: the worker process sat at 0% CPU for 5+ minutes). CapabilityClient/MessageClient are non-final abstract classes, so injecting them lets a test construct a coordinator with mocked instances directly, with no agent involved. Only used by PlaylistWatchTransferCoordinator so far; the rest of the package is unchanged.
PhoneWatchBatchTransferState is the aggregate (song counts, current song, overall status) driven by PlaylistWatchTransferCoordinator; PhoneWatchTransferState (existing) keeps tracking the active song's byte-level progress under its own requestId. Same StateFlow-per-map shape and terminal-cleanup pattern as the existing per-song transfers, just keyed by batchId instead of requestId.
Sends a whole playlist to the watch: syncs membership/order first (so the watch can show and start playing it before every song has arrived), then transcodes and transfers pending songs one at a time — never in parallel, to avoid saturating the single Bluetooth channel. Reuses the existing single-song pipeline end to end via PhoneDirectWatchTransferCoordinator's WatchAudioOverride hook. A song missing from the library or that never reaches a terminal state within the timeout is counted as failed rather than silently skipped, so completed+failed always accounts for every song in the batch. Deviates from the usual CoroutineScope(SupervisorJob() + Dispatchers.X) pattern seen elsewhere in this package: takes the injected @AppScope scope instead of constructing its own, so the scope has an owner (GEN-CONC-01). The song-transfer-await timeout is a settable instance property, not a companion var, so tests can shrink it on their own instance without mutating shared state other tests could see.
An active or just-finished batch takes priority over any concurrent lone single-song transfer (e.g. from the song info sheet) in the notification — it's the longer-running, more significant operation, and showing both at once would make a single notification unreadable. Content text reads "N of M songs" rather than a byte count, matching the confirmation-sheet-to-notification UX: the user cares about song progress, not bytes, for a playlist send. The service now stays foreground as long as either transfers or batchTransfers is non-empty, not just transfers.
…ne transfer state store PlaylistWatchTransferCoordinatorTest (9 cases): empty playlist, no reachable watch, playlist order preserved, dedupe of songs already on every reachable watch, one song failing doesn't abort the batch, mid-batch cancellation, timeout on a song that never confirms, fan-out to multiple reachable nodes counted once per song, a missing song counted as failed rather than dropped. PhoneWatchTransferStateStoreTest (20 cases): covers both the batch state this PR adds and the pre-existing per-song transfer state, which had no test coverage at all before this. Doesn't assert on the store's terminal-state cleanup — it runs on an internal, non-injectable Dispatchers.Default scope after a real-time delay, so testing it here would mean either a real sleep (GEN-TEST-04) or refactoring the store's scope handling, out of scope for this change. capabilityClient/messageClient are constructor-injected into the coordinator specifically so they can be faked directly in the test without mocking Wearable's static factory methods, which needs an agent that hangs in this environment. Verified: :app:testDebugUnitTest, full suite, 455 tests. Only the same 5 pre-existing failures unrelated to this branch (confirmed against a clean dev-personal worktree earlier in this feature). The 29 new tests in this PR (9 + 20) are all green.
estimateWatchTransfer, isPlaylistFullyOnWatch, sendPlaylistToWatch, cancelPlaylistTransfer, and activePlaylistBatchTransfer — thin delegation to PlaylistWatchTransferCoordinator/PhoneWatchTransferStateStore/ WearPhoneTransferSender, mirroring the exact pattern SongInfoBottomSheetViewModel already uses for the single-song case. Adds 4 constructor dependencies to an already-1200-line ViewModel. Flagged, not fixed here — splitting it is a separate, unrelated refactor.
…aylist screen New action in the playlist options sheet — labeled "Send to Watch" or "Update on Watch" depending on whether any of its songs are already there. Tapping it refreshes watch availability and opens a confirmation dialog showing pending-song count and the size/time estimate (WatchPlaylistTransferEstimator, already built) before anything is sent. A non-blocking progress banner appears at the top of the songs list once a batch is running for this playlist, with a cancel action — the user can navigate away or leave the app while it continues; the foreground notification (already built) is what tracks it from there.
A playlist batch takes priority over a concurrent lone single-song transfer in the top bar badge and compact-navigation pill — same priority rule as the transfer notification (WatchTransferForegroundService) and the playlist screen's own banner: it's the longer-running, more significant operation, and showing both at once would be unreadable. WatchPlaylistBatchProgressDialog mirrors the existing single-song WatchTransferProgressDialog's look (loading ring + percent, wavy progress bar, cancel button) rather than reusing PlaylistDetailScreen's banner — LibraryScreen already establishes badge-tap-opens-dialog as its own convention for this, and a lone playlist name/song-count doesn't need the full list context a banner implies.
Covers estimateWatchTransfer, isPlaylistFullyOnWatch (empty list,
partial, and fully-on-watch cases), sendPlaylistToWatch,
cancelPlaylistTransfer, activePlaylistBatchTransfer, and
refreshWatchAvailability. The rest of PlaylistViewModel's existing
surface (CRUD, sorting, AI generation, M3U import/export) is untouched
and out of scope — no PlaylistViewModelTest existed before this.
activePlaylistBatchTransfer is a stateIn(WhileSubscribed) flow —
reading .value directly never triggers the upstream collection, so
that test uses Turbine's test{} for a real subscriber instead.
Verified: :app:testDebugUnitTest, full suite, 462 tests. Only the same
5 pre-existing failures unrelated to this branch. The 7 new tests in
this PR are green.
WearDataListenerService now routes the PLAYLIST_SYNC message path to WearTransferRepository.onPlaylistSyncReceived, which upserts the playlist entity and its song cross-refs (order preserved via position) into LocalPlaylistDao in one transaction. Re-syncing an existing playlistId (e.g. after editing it on the phone) replaces membership/order rather than merging with stale cross-refs, and preserves the original createdAt while bumping updatedAt — the DAO's upsertPlaylist already had this transactional behavior from PR2, this just starts calling it. The manifest's MESSAGE_RECEIVED intent filter gets a matching <data> entry for /playlist_sync, mirroring the existing entries for the other message paths. Unlike the two pre-existing branches in the same when-block (TRANSFER_METADATA, FAVORITES_SYNC_STATE), this new branch's catch re-throws CancellationException instead of swallowing it — left the other two alone since fixing them is out of scope here.
Backs the upcoming local-playlists screens. Resolves each playlist song's availability reactively by joining its cross-ref order against LocalSongDao.getAllSongs(), so a song that finishes transferring while the detail screen is open flips from pending to playable without the user backing out and re-entering. playlistIdsReceiving surfaces which playlists currently have an in-flight song transfer, for a receiving indicator on the list screen. playAll/playFrom skip songs still pending transfer.
LocalPlaylistsScreen lists playlists synced from the phone with a
receiving indicator; LocalPlaylistDetailScreen shows a playlist's
songs in sync order, marking pending ones as disabled with a
'waiting to transfer' label instead of hiding them, so the list's
shape matches the phone immediately even before every song has
arrived. Both use androidx.wear.compose.foundation.lazy's items()
overload with an explicit key (playlist.playlistId / item.songId)
instead of the module's usual count-based items(n){} — needed here
because playlists reorder by updatedAt on every sync and songs flip
availability while the screen is open, both of which lose state and
break animations without a stable key.
DownloadsScreen gets a new 'Playlists' entry navigating into the new
screens. Reachable via Downloads → Playlists → a playlist → its
songs.
Persists at most one in-flight playlist batch transfer intent (batchId, playlistId, playlistName, songIds, requestedAtMillis) to the app's shared DataStore<Preferences>, matching the existing *PreferencesRepository convention. Deliberately doesn't persist the rest of PhoneWatchTransferStateStore (per-song byte progress, reachable nodes, ...) — that's UI-only state, cheap to rebuild, and churns too fast to persist sensibly. Only the intent needs to survive a process restart; the coordinator already re-derives everything else when it runs a batch. clearInFlightBatch(batchId) only removes the stored intent if its batchId still matches — if a newer batch already overwrote it (e.g. the user sent another playlist before the first one's cleanup ran), clearing unconditionally would drop that newer intent instead.
PlaylistWatchTransferCoordinator now persists its batch intent when a transfer starts and clears it on every terminal outcome (completed, failed with no reachable watch, cancelled) — so a batch surviving to the next app start is exactly the ones that were cut off by the process dying mid-transfer, not a theoretical case for a transfer that can run tens of minutes over Bluetooth. resumePersistedBatchIfNeeded() re-runs any such orphaned batch: it refreshes the watch-library snapshot first (empty right after a cold start) and waits briefly for it to resolve, so the existing dedup against what's already on the watch is accurate on the first pass instead of relying solely on the watch's own duplicate rejection. Re-running from scratch is safe either way — the watch rejects a transfer for a song it already has. Wired into PixelPlayApplication.onCreate(), alongside the app's other one-shot startup work. Best-effort: a cold start not directly triggered by the user may be too restricted to start the foreground service this resumes into, so failures here are logged and skipped rather than crashing app startup — the persisted intent stays put for the next launch that can.
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
1 participant
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.
Apilado sobre #8 (contrato compartido + Room de playlists locales). Tercer PR del plan: decide y ejecuta el transcodificado que prepara el audio para el reloj, y estima tamaño/tiempo para el sheet de confirmación. Sin lógica de negocio de transferencia todavía (eso es PR4) — solo las piezas puras y el pipeline de encode.
Contenido
WatchAudioTranscoder: decide si una canción necesita re-encode (requiresTranscoding, función pura) y lo ejecuta conmedia3-transformer(dependencia ya declarada en el proyecto, sin usar hasta ahora en ningún sitio del código). Fuentes sin pérdida o de bitrate alto se re-codifican a AAC-LC 128 kbps; fuentes ya lossy a ≤256 kbps se envían tal cual — recodificar un MP3 ya pequeño solo cuesta CPU y calidad sin beneficio de tiempo de transferencia.WatchPlaylistTransferEstimator: heurística de tamaño/tiempo para el sheet de confirmación, calculada solo sobre las canciones pendientes (no toda la playlist).@IoDispatcher/@MainDispatcherendi/Qualifiers.kt+ providers enAppModule.kt: el repo no inyecta dispatchers en ningún sitio hoy (incumpleAND-CONC-03). En vez de migrar código existente (fuera de alcance, alto radio de impacto), son solo para código nuevo —WatchAudioTranscoderes el primer consumidor, porqueTransformernecesita un hilo con Looper y así queda sustituible en tests.Por qué separado de PR4
WatchAudioTranscoder/WatchPlaylistTransferEstimatorson piezas puras y testeables en JVM; el coordinador de lote que los orquesta (PR4) trae su propia complejidad de concurrencia/estado. Mantenerlos separados hace que este PR sea trivialmente revisable.Verificado
:app:testDebugUnitTest→ 426 tests, 5 fallos — los mismos 5 que existen endev-personallimpio sin ninguno de estos cambios (lo confirmé corriendo la misma suite en un worktree aparte: 414 tests, mismos 5 fallos, mismos nombres). Los 12 tests nuevos (WatchAudioTranscoderTest7/7,WatchPlaylistTransferEstimatorTest5/5) están en verde.:app:compileDebugKotlin→ compila limpio, KSP/Hilt resuelve el grafo de DI sin errores.Sin verificar (necesita dispositivo)
La ruta de encode real (
transcodeIfNeeded/runTransform) necesita un encoder de hardware y un hilo con Looper — solorequiresTranscoding(la decisión) tiene test. La calibración deASSUMED_TRANSFER_RATE_BYTES_PER_SEC(40 KB/s) viene de la rama de referencia y necesita remedirse contra un par teléfono+reloj real.Ejecutado en local (JBR de Android Studio como JDK 21).