Repository navigation
feat(meeting): local backup on the phone, 90/45-day retention, storage floor - #55
Conversation
…nnect - MicLevelMonitor: fail-closed Mic signal. Green only with a non-zero frame in the last 2 s; red after 5 s of all-zero audio or 5 s with no frames, and at once when Android reports the capture silenced (AudioRecordingConfiguration, API 29+). Re-judged on a 200 ms clock so a stalled capture loop cannot stay green. - Live mic meter in the app and a level bar in the notification; the notification leads with the worst signal. One alert per incident: the mic alert clears only after 3 s of continuous sound. - TailscaleLink: when no VPN is up, ask Tailscale to connect through its exported CONNECT_VPN receiver; after 20 s without a VPN, alert with a tap-to-open action. The network note shows only while delivery is failing. - A sticky restart after an OS kill no longer tries to reopen the mic (Android 14+ refuses background microphone FGS starts); it posts a notification that opens the app, which restarts capture through the normal Start path. - Setup checklist (Always-on VPN, never-sleeping apps, mic access tile) and a banner when notifications are disabled. - Version 0.4.0 (4) with the build time shown.
…e floor - LocalRecorder: every capture is also written on the phone, independent of the live stream. The capture loop hands frames to a bounded queue (never blocks); a per-run writer thread encodes AAC-LC 32 kbps 16 kHz mono and writes raw ADTS frames in 5-minute segments, each with a sidecar written at open (UTC start, run id, sample position). Dropped frames are carried in order and encoded as silence, so positions track capture time. - Retention: older than 90 days is deleted; below 1 GB allocatable, older than 45 days too; never anything newer than 45 days. Runs at each capture start, under storage pressure, and on every app open. Open and future-dated segments are never touched. - Storage floor: below 300 MB the backup pauses (resumes above 400 MB) and the live stream continues. Encoder failures retry after 30 s, permanent after 3. - Fail-closed Local backup signal (green only while bytes reach disk) with a loud alert when it goes off; each closed file is re-read and its valid audio length shown; an "On this phone" summary reads the folder directly. - Version 0.5.0 (5).
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2ca22484d
ℹ️ 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".
| else -> Color.TRANSPARENT | ||
| }) | ||
| renderMic(s) | ||
| renderLocal(s) |
There was a problem hiding this comment.
When the user taps Stop after the backup has turned green, stopCapture() publishes STOPPED before stopHonesty() changes localReading to off, and no subsequent state is published. Because this newly added call renders the local signal without checking the capture phase, the UI remains at “Local backup saving on this phone” indefinitely after recording has stopped. Gate local rendering on STOPPED/ERROR, or publish the updated local reading after stopping the monitor.
Useful? React with 👍 / 👎.
| while (left > 0) { | ||
| val n = minOf(left, (silence.size / 2).toLong()).toInt() | ||
| write(silence.copyOf(n * 2)) | ||
| runSamples += n | ||
| left -= n |
There was a problem hiding this comment.
When the queue has overflowed and any write() during this loop encounters an I/O or codec exception, write() catches the exception, sets off, and returns, but the loop immediately calls it again for every remaining silence chunk without rechecking writable(). A gap near the 60-second limit can therefore trigger hundreds of immediate reopen attempts; repeated codec errors reach ENCODER_FATAL immediately instead of honoring the 30-second retries, permanently disabling the backup after what could have been a transient failure. Make write() report failure or recheck the writer state inside the loop.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 124735f: the silence-fill loop checks writable() before each chunk and accounts the remaining samples if the writer went off mid-fill.
|
|
||
| private fun openSegment() { | ||
| val d = dir(context) | ||
| val utc = System.currentTimeMillis() |
There was a problem hiding this comment.
When the asynchronous writer falls behind capture, this records the wall-clock time at which the queued frame is encoded rather than when that audio was captured. The queue is intentionally allowed to hold about 15 seconds, so a busy encoder can produce sidecars and filenames that timestamp audio many seconds late even though start_sample remains accurate. Pass the capture run's wall-clock origin to the writer and derive each segment timestamp from runSamples / SAMPLE_RATE so the metadata remains aligned with the recording.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 124735f: each queued frame carries its capture wall time; a segment's utc_start_ms is the capture time of its first audio (silence fills are back-dated from the next real frame).
| if (buffer != null && info.size > 0 && !isConfig) { | ||
| val payload = ByteArray(info.size) | ||
| buffer.position(info.offset) | ||
| buffer.get(payload) | ||
| o.write(Adts.header(payload.size)) | ||
| o.write(payload) |
There was a problem hiding this comment.
On a codec that batches multiple AAC access units or emits a partial frame in one output buffer, these lines wrap that entire buffer in one ADTS header whose raw-data-block count says it contains exactly one frame. Android's MediaCodec contract explicitly permits audio output buffers to contain multiple access units and exposes BUFFER_FLAG_PARTIAL_FRAME; ignoring both cases produces invalid frame boundaries, makes frameOffsets() report the wrong duration, and can make the backup undecodable. Reassemble partial units and emit an ADTS header per complete access unit, or use an encoding path that guarantees single-unit output.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not changed, with evidence: the platform AAC encoder emits one access unit per output buffer, and files written by this code on the target device decode cleanly in a full ffmpeg decode with durations matching capture (46.5 s, 25.2 s, 8.4 s), and a cloud batch transcription accepts them. If a device ever batched units, frameOffsets/verify would report the file as short, which the status line shows.
| /** Non-blocking: the writer drains what it has, closes its segment, and exits. */ | ||
| fun stop() { | ||
| writer?.finish() | ||
| writer = null |
There was a problem hiding this comment.
When Stop is requested while the writer has queued audio, this only marks it as finishing and returns immediately. stopCapture() then releases the wake lock, removes the foreground service, and calls stopSelf() even though the queue can represent roughly 15 seconds of audio and still needs an EOS drain and fsync; when stopping from the notification with no foreground activity, Android can suspend or kill the now-unprotected process before that work completes, losing the end of the local recording. Expose a bounded completion signal from the writer and retain the foreground-service/wake-lock protection until it has closed the segment.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 124735f: the writer thread holds its own partial wake lock, renewed in 60 s leases for its whole run, so the post-Stop drain, final close and fsync stay awake regardless of meeting length.
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. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| localRecorder.stop() | ||
| localMonitor.stop() | ||
| localReading = localMonitor.read(SystemClock.elapsedRealtime()) |
There was a problem hiding this comment.
🔴 Stopped capture still shows local saving
After Stop, stopHonesty turns the backup off without publishing its new reading. The previously published STOPPED state retains GREEN, so the screen still says the phone is saving audio.
Learn more
The capture state is a StateFlow read by the activity. stopCapture publishes STOPPED before calling stopHonesty. The monitor then changes to off and the tick is removed, but _state keeps its last green local reading indefinitely.
Example: Stop a capture while the local indicator is green. The activity receives STOPPED with GREEN and continues to display “Local backup saving on this phone” even though its writer is stopping.
Recommended fix: Publish the stopped/off local reading after stopHonesty, or reorder the stop transition so its published state contains the final local reading. Ensure the capture-failure and destruction paths retain their intended terminal state.
Was this helpful? React with 👍 or 👎 to provide feedback.
| (off == Off.STORAGE || off == Off.IO) && free >= RESUME_BYTES -> { | ||
| off = null | ||
| monitor.setFailure(generation, null) | ||
| } |
There was a problem hiding this comment.
🔴 Transient write error strands local backup
When a write fails with 300–400 MB free, Off.IO discards later audio until space reaches 400 MB. A transient I/O error therefore stops local recording indefinitely despite space above the 300 MB floor.
Learn more
A writer enters Off.IO whenever write catches an IOException, regardless of available space. writable() then rejects frames while Off.IO persists. Storage checks clear this state only after reaching the 400 MB hysteresis threshold, which is intended for a pause caused by low storage.
Example: At 350 MB free, one transient FileOutputStream failure sets Off.IO. Later writes work again, but no write is attempted, even though 350 MB exceeds the 300 MB recording floor.
Recommended fix: Give Off.IO an independent timed retry when storage is above FLOOR_BYTES; keep the 400 MB resume threshold for Off.STORAGE.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 124735f: a write error (Off.IO) resumes once free space is back above the 300 MB floor; only a full disk (Off.STORAGE) waits for the 400 MB margin.
| codec = MediaCodec.createEncoderByType(MIME).apply { | ||
| configure(format, null, null, MediaCodec.CONFIGURE_FLAG_ENCODE) | ||
| start() | ||
| } |
There was a problem hiding this comment.
🔴 Failed encoder setup leaks codecs
If configure or start fails, MediaCodec.createEncoderByType has already allocated a codec. The codec never reaches codec, so retries leave native codec resources allocated.
Learn more
MediaCodec owns native resources. Assignment to codec happens only after the apply block completes; if configure or start throws, closeSegment sees null. The writer retries encoder setup, and a new capture run can retry again after three failures.
Example: A device's AAC encoder creates an instance but fails on start() three times. Each failure bypasses release, and the next capture run allocates three more codecs.
Recommended fix: Assign the created codec before configuring it, or hold it in a local variable and release it in a catch block before rethrowing. Ensure closeSegment releases exactly once.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 124735f: the codec is released if configure/start throws, and a missing encoder is classified as an encoder failure (bounded retries) rather than storage.
| if (s.phase != lastPhase && s.phase == MicStreamService.Companion.Phase.STOPPED) { | ||
| // The last file closes just after Stop: re-read the folder once it has. | ||
| b.storageSummary.postDelayed({ refreshStorageSummary(purgeFirst = false) }, 2_000) | ||
| } |
There was a problem hiding this comment.
🔍 Final summary can miss queued audio
After Stop, the summary refreshes once, two seconds later. The writer can still be draining its 15-second queue, leaving the displayed duration stale until the app reopens.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 124735f: the summary is refreshed again 20 s after Stop, after the writer has drained.
…s as the tailnet - Tap-to-resume restores the persisted capture settings (endpoint, token, model) instead of the build defaults a fresh screen shows; with none saved it refuses to start rather than silently using defaults. - The tailnet check requires a VPN network carrying a Tailscale address (the RFC 6598 /10 or Tailscale's IPv6 /48), so another VPN no longer suppresses auto-connect and the Tailscale alert. - The meter drops to zero once sound is no longer fresh. - Stop clears the network note before publishing the stopped state. - The notifications-off banner also catches the app-level switch and a muted failure channel.
…into feat/meeting-mic-local-backup # Conflicts: # clients/android-mic/app/src/main/java/com/genesis/meetingmic/MainActivity.kt
…eview - Each queued frame carries its capture time; a segment's UTC start is when its first audio was captured, not when the lagging writer encoded it. - Filling dropped audio with silence stops as soon as the writer goes off. - The writer holds its own wake lock in renewed short leases for its whole run, so the drain after Stop stays awake however long the meeting was. - A write error resumes once free space is back above the 300 MB floor; only a full disk waits for the 400 MB margin. - An encoder that fails to configure is released; a missing encoder counts as an encoder failure, not a storage failure. - The storage summary is refreshed again after the final file has closed.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 124735f678
ℹ️ 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".
| fun finish() { | ||
| finishing = true | ||
| queue.offer(stopFrame) // may fail on a full queue; the loop also exits once drained |
There was a problem hiding this comment.
When the encoder is far enough behind to fill the queue, pendingDropped can still contain frames that have not yet been attached to a later accepted frame when Stop is pressed. finish() only enqueues the sentinel, so after draining the queue the writer exits without representing that trailing interval as silence; the resulting local backup is shorter than the capture precisely in an overload scenario. Transfer the pending count to the writer during shutdown and account for it before closing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ef0bfcb: after the queue drains, the writer takes any dropped count left over and fills it with silence ending at the time Stop was pressed, so the file is as long as the capture.
| if (idx == MediaCodec.INFO_TRY_AGAIN_LATER) { | ||
| // Bounded: an encoder that never signals end-of-stream must not hang the writer. | ||
| if (endOfStream && ++waits < 50) continue else return |
There was a problem hiding this comment.
When an encoder accepts EOS but does not return an EOS output buffer within these 50 polls, drain(true) silently returns and closeSegment() immediately stops and releases the codec. Any output still buffered by the codec is therefore discarded at a five-minute segment boundary or at Stop, while the backup can remain green and the truncated file can be reported as verified. Surface this timeout as an encoder failure instead of treating it as a successful drain.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ef0bfcb: a missing end-of-stream buffer after the bound (or no input slot for it) is an encoder failure. The close reports the last file as missing its final audio instead of verified, and at a segment rollover it takes the existing encoder-failure path (paused plus alert).
| val now = SystemClock.elapsedRealtime() | ||
| if (now - lastSync < SYNC_MS) return | ||
| lastSync = now | ||
| try { out?.fd?.sync() } catch (_: Exception) {} |
There was a problem hiding this comment.
If fd.sync() fails because the filesystem reports an I/O or storage error after buffered writes were accepted, this catch suppresses the only durability failure and the monitor continues to report green as bytesWritten advances. The same error is ignored again while closing, so the UI may claim the local backup is saved even though those bytes were never made durable. Route sync failures through the recorder's I/O-off state and alert path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ef0bfcb: the periodic sync no longer catches, so a failure raises into the write path's I/O handling (backup off plus alert); a failed final flush or sync reports the last file as not fully saved.
| fun dir(context: Context): File = | ||
| (context.getExternalFilesDir("recordings") ?: File(context.filesDir, "recordings")) | ||
| .apply { mkdirs() } |
There was a problem hiding this comment.
On Android 8 and 9, which remain supported by minSdk = 26, meeting audio written under getExternalFilesDir() can be read by another app that has the appropriate storage permission; Android only prevents this cross-app access through scoped storage on newer releases. The official storage guidance explicitly documents this legacy behavior. Because these files contain raw meeting recordings, store them in internal app storage or encrypt them before using external storage.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ef0bfcb: minSdk is now 30. The Android 11 storage changes page states that on Android 11 apps can no longer access files in any other app's app-specific directory within external storage, listed among the changes for all apps.
…l-backup # Conflicts: # clients/android-mic/app/build.gradle.kts # clients/android-mic/app/src/main/java/com/genesis/meetingmic/MainActivity.kt # clients/android-mic/app/src/main/java/com/genesis/meetingmic/MicStreamService.kt # clients/android-mic/app/src/main/java/com/genesis/meetingmic/ProblemAlerts.kt # clients/android-mic/app/src/main/res/layout/activity_main.xml # clients/android-mic/app/src/main/res/values/strings.xml
…saved - Audio dropped after the last queued frame is filled with silence at Stop, so the file is as long as the capture. - An encoder that does not finish a segment (no end-of-stream within the bound, or no input slot for it) is an encoder failure, not a finished file. - A failed fsync turns the backup off with an alert instead of being ignored; a failed close reports the last file as not fully saved. - minSdk 30: from Android 11 other apps cannot read this app's external files folder, where recordings are kept.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef0bfcb43a
ℹ️ 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".
| /** Starts a writer for capture run [runId]; a previous writer finishes on its own thread. */ | ||
| fun start(runId: Long, generation: Long) { | ||
| writer?.finish() | ||
| writer = Writer(runId, generation).also { it.start() } |
There was a problem hiding this comment.
When the user starts another capture while the previous writer is still draining its roughly 15-second queue, this immediately starts a second writer even though the first may still own its MediaCodec. On devices that expose only one AAC encoder instance, the new writer's setup fails and enters the 30-second retry state, while captured frames during that interval are skipped from the local backup. Serialize codec ownership between writers, or keep the new run queued until the old codec is released.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in edb0929: the last writer is tracked process-wide, so it survives the service restart that Stop causes, and each new writer joins it (up to 10 s) before opening any encoder. Frames for the new run wait in its own queue meanwhile.
| File(d, "${s.id}.aac").delete() | ||
| File(d, "${s.id}.json").delete() |
There was a problem hiding this comment.
When retention selects a segment but the filesystem refuses to delete its .aac file, File.delete() returns false rather than throwing, and the next line still removes the JSON sidecar. If the sidecar deletion succeeds, the surviving recording permanently loses its UTC, run, and sample-position metadata, and subsequent retention falls back to the audio file's modification time. Delete the sidecar only after confirming that the corresponding audio file no longer exists.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in edb0929: retention removes a sidecar only once its audio delete succeeded or the audio is already gone, and the purge count reports real removals only.
| mainHandler.removeCallbacks(tick) | ||
| mic.stop() | ||
| micReading = mic.read(SystemClock.elapsedRealtime()) | ||
| localRecorder.stop() |
There was a problem hiding this comment.
On an explicit Stop, this finishes and detaches the local writer before stopCapture() cancels the capture job. Because AudioRecord.read() is a blocking call, a read already in progress can return afterward and reach localRecorder.offer(), where the frame is discarded because writer is now null; that buffer can contain up to 100 ms of audio captured before the tap, truncating the local backup on normal stops. Coordinate shutdown so the capture loop has stopped offering frames before finishing the writer.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in edb0929: the writer is finished by the capture job's completion handler, which runs only after the loop can offer nothing more (so the read in flight at Stop is kept), and also when the job is cancelled before it started. Offers and stops are scoped to their run.
…e, never overwrites - The local writer is finished when the capture job completes, so the read in flight at Stop is kept; a job cancelled before it started still finishes its writer. - Each new writer waits for the previous one in this process to release its encoder, across service restarts. - A segment is never opened over an existing recording. - A segment that loses its tail at a 5-minute rollover is reported and recording continues with a fresh encoder; only repeated failures stop it. - Retention removes a sidecar only once its audio is really gone.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Handoff to a closing session (state 2026-10-22):
|
What
Every capture is now also written on the phone, independently of the live stream. Losing the network, the VPN or the bridge never stops the local copy, and a full disk never stops the stream.
LocalRecorderoffer(), so it never blocks.MediaCodecand writes raw ADTS frames into 5-minute segments under the app's external files dir. ADTS is self-delimiting, so a crash or a full disk loses at most the torn last frame. Transcription services accept raw ADTS directly.RetentionPolicy)LocalBackupMonitoris fail-closed: green only while bytes reach disk, red after 20 s without progress or on a named failure.Verification
./gradlew testDebugUnitTest assembleDebug: 51/51 pass, 15 of them new for this PR:Format probe. Hand-built raw ADTS AAC-LC 16 kHz mono files, including one torn mid-frame, were accepted by a cloud batch transcription API with speaker diarization, and transcribed correctly.
Adversarial review (fresh context): no critical findings. 7 important issues were found, all fixed:
Minor findings were fixed or documented.
External review, rounds 1-3 (Codex, Devin), every finding fixed or answered on its thread, including:
Device (target phone, Android 15/16).
check-private-data.shis clean.Residual: the
MediaCodecwriter state machine has no JVM test, because it depends on the Android framework. It is covered by the device run above.E2E: on the target phone, record a few minutes including an airplane-mode stretch. Local backup stays green throughout, and "On this phone" reports valid audio covering the whole capture. After 5 minutes a "last file verified: 5:00" check appears.
Shape: ~720 added lines, of which ~150 are tests. One concern: the independent local copy and its lifecycle (retention, storage floor, status).