[#176] Seekable recording review + m4a remux (merge after PR 158)#178
Draft
fel-cesar wants to merge 63 commits into
Draft
[#176] Seekable recording review + m4a remux (merge after PR 158)#178fel-cesar wants to merge 63 commits into
fel-cesar wants to merge 63 commits into
Conversation
Install expo-audio (~56.0.12) for microphone capture and playback, and expo-crypto (~56.0.4) for RFC 4122 v4 IDs used as recordings.id. Register expo-audio as a config plugin with a Fluent-specific microphone permission string so prebuild injects RECORD_AUDIO and MODIFY_AUDIO_SETTINGS into the Android manifest. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Add Recording / RecordingRow types and RecordingSyncStatus union so the existing recordings schema is typed end-to-end. Introduce repository.insertRecording that atomically demotes the prior is_latest row for a bible_text_id, increments take_number, and inserts the new take, plus deleteRecordingById for the delete flow. Add queries.getBibleTextId to resolve verse coordinates to bible_texts.id and queries.getLatestRecordingForVerse for the Record tab review view. Cover both modules with unit tests over a mocked op-sqlite handle. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Add PausedTakeMarker and get/set/clearPausedTake helpers over the op-sqlite KV store so a paused take (verse, temp file URI, elapsed ms, started-at) survives app backgrounding or crash and can be resumed or recovered on next launch. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the ViewChapter stub with a DraftingPage shell that hosts the selected verse, a disabled ChapterAudioPlayerBar placeholder for the upcoming #47 player, and a DraftingTabBar switching between a Bible placeholder and the new RecordTab. Add useRecorder hook implementing the Idle / Recording / Paused / Review state machine on top of expo-audio: request and track microphone permission (unknown | granted | denied | blocked), capture takes into a temp file, insert them via repository.insertRecording, mint recording IDs with expo-crypto randomUUID, and persist a paused- take marker via the storage KV so pauses survive backgrounding. RecordTab renders verse chevrons, the source-text accordion, per-state controls (start, pause, resume, stop, re-record, delete, playback), a duration counter, a delete confirmation, and a navigate-away prompt. When permission is blocked the tab opens system settings via Linking.openSettings, otherwise it re-requests through the OS dialog. Point AppNavigator.VerseDetail at DraftingPage and delete the legacy ViewChapter screen. Cover the hook and screen with Jest tests using the LoginScreen mocking pattern. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Split the pill-shaped record CTA into an 88px red circle carrying just the target icon, a centered verse-reference label below it, and a disabled play-button placeholder that hints at future draft playback. Add a dedicated recordAccent theme token so the red is not overloaded with destructive semantics. Update the RecordTab smoke test for the new record-start-label and record-play-idle-placeholder test IDs. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Swap the recording controls so the muted stop circle sits on the left and the prominent red pause circle on the right, mirror the same layout for paused (stop + resume), and lift the duration counter above the buttons in muted tabular-nums. Retune the live waveform to a narrower, taller red band with 22 rounded bars and split the bar style into live (red) and static (blue) so the review waveform keeps its own accent. Add per-state coaching tips under the buttons — "Tap pause to study the source, stop to finish." while recording, and "Recording paused — review the source below, then resume." while paused. Rework the review state to mirror the idle affordance: a muted record-done placeholder next to a prominent blue play circle, with Re-record and Delete moved into labeled rounded rectangles (light card fill and destructive-outlined) using a shared placeholder-circle style with the idle play hint. Extend RecordTab.test.tsx to cover the recording tip, the paused-tip wording, and the new review-state controls. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Compose prevDisabled / nextDisabled from the existing edge flags plus a recording-active check so the prev/next chevrons cannot navigate away mid-take. Paused, idle, and review keep their existing guards (paused still prompts Resume/Discard via withPausedGuard). The muted foreground color and reduced-opacity icon style follow the disabled state so the lock is visually obvious. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Recorder previously polled recorder.currentTime which keeps advancing on some devices while the native pipeline is paused, so elapsedMs inflated past a Resume by the length of every prior pause. Replace it with wall-clock timing: baseElapsedRef accumulates completed segments, runningSinceRef marks the start of the current active segment, and each tick reports base + (now - runningSince). Pause commits the segment before native pause, resume opens a new segment, and stop derives the final duration from the same refs so persistence matches the display. Drop the tick interval to 50 ms so the readout is smooth at centisecond precision, and format duration as MM:SS:HH with zero-padded minutes so verses over a minute align in the UI. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Switch the verse-nav row to justifyContent: 'space-around' so the chevrons and reference sit with balanced margins instead of hugging the edges. Delete the two commented-out waveform style lines that were left behind during recording-state polish. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Split the player concern out of useRecorder into useDraftPlayback, which takes only the take's file URI and owns the playback-side audio routing and rewind-on-finish. useRecorder composes it with a one-way dependency and stops playback explicitly before re-record/delete, keeping its public API unchanged. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Hook the review play button up to togglePlayback with a play/pause affordance, and stop playback when navigating between verses. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Recordings need to move out of the evictable cache into the document directory and be read back at absolute paths, which requires the expo-file-system File/Directory/Paths APIs. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Introduce recordingStorage with a user/project/book/chapter/verse key layout rooted at the document directory. Persist only relative keys and resolve absolute uris at read time so paths survive reinstalls, plus best-effort move/delete helpers for takes and per-user cleanup. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Add user_id and chapter_assignment_id columns (with migrations for existing DBs), thread them through the Recording types, inserts and verse queries, and resolve a project id for a project unit to build storage keys. deleteRecordingById now also unlinks the durable file. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Accept optional attribution context (user/project/book/chapter/verse) via a ref, move each committed take out of the cache into its durable key before inserting the row so a record never points at an evictable file, and resolve relative keys to absolute uris for playback. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve the active user and owning project id in DraftingPage and pass them, along with the chapter assignment and book code, through RecordTab into useRecorder so committed takes are stored under the correct key. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Configure useAudioRecorder to write into the document directory instead of the evictable cache so paused or backgrounded partial takes survive a process kill until they are moved or deleted. On discardPaused, read the persisted paused-take marker (recorder.uri is unreliable after a kill) and unlink the durable partial file before clearing state. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Split useRecorder into a use-case-agnostic state machine that owns only recording mechanics (transitions, elapsed ticking, permissions, background auto-pause, playback) and injects persistence, storage layout, and domain identity through a RecorderAdapter. Move all verse-specific SQLite, durable-storage, and paused-marker wiring into a new useVerseRecorder adapter and point RecordTab at it, so the recorder can be reused for other capture use cases without dragging DB concerns along. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Read the committed take's playback uri via adapterRef.current instead of the adapter prop, matching every other adapter call in useRecorder. This keeps the ref indirection consistent so the hook never depends on a fresh adapter object reference when handing a uri to useDraftPlayback. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
The recorder loads its state from the local DB asynchronously, so the tab briefly rendered the idle "Record" button before snapping to review when an existing take resolved. Hold the record/review UI back until the recorder is ready (capped at 100ms so a slow load still surfaces), showing a height-reserved placeholder meanwhile so the layout does not shift. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Split the monolithic Record tab into components, hooks, and utils under drafting/record and colocate useVerseRecorder with its screen. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Reject corrupted paused-take markers unless bibleTextId is a finite number matching the KV lookup key. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Precompute static bar heights in StyleSheet and split live active/paused styles so react-native/no-inline-styles lint passes cleanly. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Install SDK 56-aligned expo-asset and register its config plugin so expo-doctor passes and native prebuild deduplicates expo-asset/constants. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Reset bibleTextId to null synchronously when chapterData or selectedVerse changes so RecordTab cannot act on a stale id while getBibleTextId is in flight. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Wrap discardPaused in try/catch so a failed discard does not leave verse switches or navigation blocked without user feedback. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Extend the Record tab beforeRemove listener to cover recording state, not just paused, so users must discard the take before leaving. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
If moveIntoStore succeeds but the DB insert rejects, remove the moved file before returning null so commit failures stay recoverable without leaving orphaned audio on disk. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Await recorder pause/resume/stop and surface user-visible alerts when native recording operations reject, matching discardPaused error handling. Refs #49 Co-authored-by: Cursor <cursoragent@cursor.com>
Write the paused-take manifest the moment recording starts (and refresh it on pause), not only on pause/background. A hard task-swipe kill can destroy the process before the background auto-pause runs, so a pause-only marker never existed for the very case recovery targets. Capture the verse's navigation context (chapterAssignmentId, verseNumber) on the marker so a recovered take can be surfaced and navigated to without a reverse lookup, and add findPausedTake() to fetch the single outstanding marker for the home prompt. Refs #49
Add getVerseDetailNavByChapterAssignment() so a recovered take, which only carries its chapterAssignmentId, can rebuild the VerseDetail route params (chapter id/name, project name, language) needed to navigate. Refs #49
On the home screen (after sync) surface any recording recovered after a process kill and force a decision: Continue navigates to that verse's Record tab — landing on the recovered verse via a new recoverVerse route param, where the paused take rehydrates — or Discard deletes the partial segments. The prompt is skipped when the take can't be resolved to a verse. Refs #49
Update the kill-resilience doc for the home recovery prompt (findPausedTake, Continue/Discard) and the marker-at-start persistence, and note the timer-vs-audio duration trade-off on kill. Refs #49
The committed durationMs came from the wall-clock timer, which undercounts the real audio length (worst after a hard kill, where a Stop-from-prompt could commit elapsedMs = 0). Parse the merged file's ADTS frames to get the sample-accurate length and use it as the source of truth, falling back to the timer only when the probe yields nothing. Also surface the persisted duration in the Review timer via an optional resolveDurationMs adapter hook. Refs #49
Wrap the merged-file creation and byte-append loop in concatenateAacSegments so a mid-loop bytes()/write() failure best-effort deletes the orphaned temporary merge file before rethrowing, instead of leaking it in the document directory. Refs #49
Update expo, expo-asset, and expo-build-properties to their latest 56.0.x patches (with transitive bumps to @expo/* tooling, expo-modules-core, and iconv-lite) to keep the SDK aligned. Refs #49
Support seeking during review: expose position/duration and a seek() on useDraftPlayback, and make the decorative review waveform tappable and draggable to seek (no new dependency; real amplitude stays deferred to #96). Group the recorder's playback surface into a nested `playback` sub-object so it reads as a concern the recorder only coordinates rather than one it owns. Refs #49
Rename useDraftPlayback to useAudioPlayback (and its API type) so the single-source player reads as reusable for any clip — draft review now, source/original verse audio later — rather than being draft-specific. No behavior change; the recorder still coordinates it via its gated playback sub-object. Refs #49
Drop the duplicate RecorderPlaybackApi interface — it was structurally identical to the hook's type with a single use site — and type recorder.playback as UseAudioPlaybackApi directly. Refs #49
Revert the review waveform to a non-interactive decorative bar: drop the touch-responder seek handlers, position/duration-driven progress fill, and the props that fed them. The underlying useAudioPlayback seek/position/ duration capability stays on the recorder's playback surface for later use. Refs #49
DraftingProvider seeded selectedVerse from initialVerse only on mount, so a later prop change (e.g. recording recovery) would not update the selection if the provider stayed mounted. Sync selectedVerse when initialVerse changes so recovery no longer depends on the loading-gated remount. Add a provider test covering the re-point behavior.
…-playback # Conflicts: # src/app/context/DraftingContext.tsx # src/app/tabs/BibleTab.tsx
Introduce an Expo local module (Android MediaExtractor + MediaMuxer) that repackages a committed ADTS AAC take into a seekable MP4/M4A container without re-encoding, plus a JS bridge that loads it optionally so builds that haven't prebuilt the module degrade gracefully. Refs #176
Add remuxTakeToSeekableContainer and call it in the verse-recorder commit path after the ADTS duration probe, moving the resulting .m4a into durable storage and cleaning up the intermediate segments. Capture stays ADTS for kill-safe resume; the remux falls back to the original .aac when the native module is unavailable or fails, so playback is never lost. Refs #176
Wire position/duration/onSeek through RecordTab into RecordingWaveform and make the review waveform scrubbable now that committed takes are seekable M4A. Fix the scrub flicker by holding the fill at the released seek target until the async player position confirms it, and capturing touches on the bar container (pointerEvents box-only) so locationX is measured against the whole row instead of an individual bar. Refs #176
Add a current-position over total-duration readout above the review waveform so users see where they are scrubbing to and where playback is. Lower the audio status poll to 50ms so the label and fill advance smoothly instead of stepping at the default interval. Refs #176
ExoPlayer leaves playWhenReady true when a track ends naturally, so seeking back to 0 without pausing first resumed playback immediately — finished takes appeared to loop instead of stopping at the end. Pause in the didJustFinish handler before seekTo(0), and align the useAudioPlayback poll-interval test with the hook's actual 30ms updateInterval. Refs #176
5 tasks
fel-cesar
changed the title
fel-cesar
changed the title
fel-cesar
changed the title
Give the useAudioPlayer jest mock a two-arg signature so the mock factory call typechecks (was Expected 0 arguments), and correct the "50ms" references to match the actual 30ms updateInterval. Refs #176
12 tasks
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.
TLDR
Makes committed recording takes seekable so the Review-state waveform can scrub to arbitrary positions — the acceptance criterion #49 waived and deferred to #176. Capture stays kill-safe ADTS AAC; on
stop()the committed take is losslessly remuxed into a seekable M4A (MP4/AAC) via a new Android-only Expo local module, and the Review UI gains an interactive scrub bar plus a position/duration readout. Remux failure falls back to the original ADTS file, so playback (non-seekable) is never lost.Reviewer checklist
Closes #176)[#176]commits; capture/resume format unchangedDetails
Closes #176
Recording captures into ADTS AAC (
.aac) on purpose: an MP4/M4A is only valid once itsmoovatom is written on a cleanstop(), so a process kill mid-take leaves it unplayable and un-appendable, whereas ADTS is self-framing — a killed segment stays playable and segments concatenate by byte-append (what makes pause / resume-after-kill work). The trade-off is that ADTS has no container index, so absolute seeking is unreliable. This PR keeps ADTS for capture and adds a post-commit lossless remux to a seekable M4A for playback: no re-encode, just repackaging the elementary AAC stream into an MP4 container with synthesized per-frame presentation timestamps.Type of change:
Technical changes
modules/aac-remux/— new Expo local native module (AacRemux, Android/Kotlin).remuxToMp4usesMediaExtractor+MediaMuxerto rewrap ADTS AAC into MP4/AAC, synthesizing presentation timestamps from sample rate + frame length (ADTS carries none) and hardening muxer start/stop/release.src/services/aacRemux.ts— JS bridge loading the module viarequireOptionalNativeModule, so a build that hasn't prebuilt the module degrades gracefully (isAacRemuxAvailable).src/services/recordingStorage.ts—remuxTakeToSeekableContainer(): remuxes to a.m4aand returns its URI; on unavailable module or failure logs and returns the original.aac(no data loss).src/app/tabs/drafting/record/hooks/useVerseRecorder.ts— calls the remux in the commit path after the ADTS duration probe (which walks ADTS frame headers), moves the result into durable storage, cleans up intermediate segments.src/hooks/useRecorder.ts— comment documenting why capture stays ADTS and where the remux happens.src/app/tabs/drafting/record/components/RecordingWaveform.tsx+RecordTab.tsx— interactive Review scrub bar (onSeek/positionMs/durationMs) and a current-position over total-duration readout; flicker fixed by holding the fill at the released seek target until the async player confirms it and capturing touches on the bar container (pointerEvents="box-only").src/hooks/useAudioPlayback.ts— status poll lowered to 50ms so the readout/fill advance smoothly.Alternatives considered (remux)
MediaMuxer/MediaExtractorare first-party, zero-dependency, and lossless, so a small local module was preferred.ffmpeg-kit): a-c copycontainer remux is trivial and battle-tested, but ffmpeg-kit is retired/archived (prebuilt binaries pulled), adds significant APK size, and brings LGPL/GPL licensing considerations — disproportionate for a plain container rewrap with no filtering/transcoding.Testing
npm run format:check,npm run lint,npm run typecheck,npm test -- --ci— all green.recordingStorage), commit URI resolution + segment cleanup (useVerseRecorder), scrub seek + flicker-hold + position readout (RecordingWaveform), 50ms poll interval (useAudioPlayback).How to verify
npm run format:check && npm run lint && npm run typecheck && npm test -- --cinpm run prebuild && npm run android(the native module is autolinked via CNG).Expected: capture/resume unchanged; committed take is a seekable
.m4a; scrubbing seeks accurately; on a build without the native module, playback still works from.aac(just not seekable).Follow-ups
AGENTS.md; keep in draft until done.