Skip to content

fix(record_ios): Prevent M4A silent frame drops when encoder buffer is full - #8

Draft
raulmabe-labhouse wants to merge 1 commit into
masterfrom
fix/bug-1-m4a-silent-frame-drops
Draft

fix(record_ios): Prevent M4A silent frame drops when encoder buffer is full#8
raulmabe-labhouse wants to merge 1 commit into
masterfrom
fix/bug-1-m4a-silent-frame-drops

Conversation

@raulmabe-labhouse

Copy link
Copy Markdown

Summary

Fixes silent audio frame drops in the M4A writer when the AAC encoder buffer fills under CPU/memory pressure. Previously, any buffer passed while AVAssetWriterInput.isReadyForMoreMediaData was false was silently discarded, producing M4A files shorter than the actual recording duration.

Problem

Original code (line 84):

guard input.isReadyForMoreMediaData else { return }

When the AAC encoder cannot keep up with incoming PCM data (e.g. Low Power Mode, background processing throttling, CPU-intensive background tasks), isReadyForMoreMediaData returns false. The write() method returned early with no error and no log. Audio frames were permanently lost.

Impact:

  • M4A files end up shorter than the WAV file and shorter than elapsed time
  • No recorder_pause event fires; users have no indication anything went wrong
  • Mixpanel data confirmed severe truncation cases (e.g. 66h expected vs 4.2h actual on iOS)

Solution

Implement a queue-and-drain pattern, following Apple recommended approach for real-time encoding:

  1. Always convert AVAudioPCMBufferCMSampleBuffer immediately (copies data; safe for async handling)
  2. Queue sample buffers when the encoder is not ready
  3. Drain asynchronously on a serial queue when isReadyForMoreMediaData becomes true
  4. On stop(), flush any remaining queued buffers before calling markAsFinished()

Changes

  • Added drainQueue (serial DispatchQueue) for thread-safe drain operations
  • Added pendingSampleBuffers to hold buffers when encoder is busy
  • write(): convert to CMSampleBuffer, enqueue, and dispatch async drain
  • New enqueueAndDrain() and drainPendingBuffers() for the drain loop
  • stop(): runs on drainQueue to ensure full flush before finalization
  • release(): clears queue under lock to avoid leaks

Testing

Recommended reproduction (before fix):

  1. Enable Low Power Mode on iOS device
  2. Start a recording session
  3. Put app in background for 30+ minutes; use other apps
  4. Bring app to foreground and stop

Before: M4A shorter than WAV and elapsed time
After: M4A and WAV should match

Related

  • Investigation report: doc/investigations/recording-duration-bug-2026-02.md (audioapp repo)

Made with Cursor

Avoid silent frame drops when AVAssetWriterInput.isReadyForMoreMediaData
is false under CPU/memory pressure (Low Power Mode, background throttling).

Previously, frames were dropped with no error. Now buffers are queued
and drained asynchronously when the encoder becomes ready.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copilot AI review requested due to automatic review settings February 24, 2026 14:40

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses truncated M4A recordings on iOS by avoiding silent frame drops when AVAssetWriterInput.isReadyForMoreMediaData is false, introducing a queue-and-drain approach for AAC encoding backpressure.

Changes:

  • Queue CMSampleBuffers when the encoder input isn’t ready instead of dropping them.
  • Add a serial drainQueue and drain loop to append queued buffers when possible.
  • Update stop()/release() to coordinate with the drain queue and clear pending buffers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 85 to 86
let input = input,
let writer = writer,

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

In write(...), input and writer are unwrapped but never used anymore (they’re only used for existence checks). This will produce unused-variable warnings; consider changing the guard to boolean nil checks or remove these bindings and rely on the drainQueue-side guard.

Suggested change
let input = input,
let writer = writer,
input != nil,
writer != nil,

Copilot uses AI. Check for mistakes.
Comment on lines +103 to +109
private func enqueueAndDrain(sampleBuffer: CMSampleBuffer) {
guard errorMessage == nil,
let input = input,
let writer = writer else { return }

pendingSampleBuffers.append(sampleBuffer)
drainPendingBuffers()

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

pendingSampleBuffers can grow without bound if the encoder stays backpressured for an extended period (e.g., prolonged background throttling). That can lead to unbounded memory growth and app termination; consider adding a maximum queue size / duration and setting an explicit error (or a defined dropping policy) once exceeded.

Copilot uses AI. Check for mistakes.
Comment on lines +117 to 128
while !pendingSampleBuffers.isEmpty && input.isReadyForMoreMediaData && writer.status != .failed {
let sampleBuffer = pendingSampleBuffers.removeFirst()
let success = input.append(sampleBuffer)
if !success {
if writer.status == .failed {
errorMessage = writer.error?.localizedDescription ?? "Writer failed"
} else if errorMessage == nil {
errorMessage = "Failed to append sample buffer"
}
pendingSampleBuffers.insert(sampleBuffer, at: 0)
return
}

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

Using Array.removeFirst() (and insert(..., at: 0) on failure) is O(n) per buffer and can become a hot path when the queue is large. Consider a ring buffer / deque approach (head index) to make draining and re-queueing O(1).

Copilot uses AI. Check for mistakes.
Comment on lines +146 to +148
self.drainPendingBuffers()
input.markAsFinished()
writer.finishWriting { [weak self] in

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

stop() calls drainPendingBuffers() only once and then immediately markAsFinished() / finishWriting(). If input.isReadyForMoreMediaData is false at that moment, queued buffers remain undrained but the writer is still finalized, so audio can still be truncated. Consider waiting/draining until the pending queue is empty (e.g., via requestMediaDataWhenReady on drainQueue, or a readiness loop with a timeout) before marking the input finished.

Copilot uses AI. Check for mistakes.
Comment on lines 83 to +100
func write(buffer: AVAudioPCMBuffer, framePosition: Int64) {
guard errorMessage == nil,
let input = input,
let writer = writer,
let pcmFormat = pcmFormat else { return }

guard input.isReadyForMoreMediaData else { return }


let pts = CMTimeMake(value: framePosition, timescale: Int32(pcmFormat.sampleRate))

guard let sampleBuffer = buffer.toCMSampleBuffer(presentationTime: pts) else {
if errorMessage == nil {
errorMessage = "Failed to create CMSampleBuffer"
}
return
}

let success = input.append(sampleBuffer)
if !success {
if writer.status == .failed {
errorMessage = writer.error?.localizedDescription ?? "Writer failed"
} else if errorMessage == nil {
errorMessage = "Failed to append sample buffer"

drainQueue.async { [weak self] in
self?.enqueueAndDrain(sampleBuffer: sampleBuffer)
}

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

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

errorMessage is read/written from multiple threads/queues (audio callback thread in write, drainQueue during draining, and the finishWriting completion queue). This introduces a data race and can lead to inconsistent state; consider confining all state mutations/reads (including errorMessage, writer, input) to drainQueue (or protect with a lock/atomic) and make getError/getOutputPath read from the same synchronization context.

Copilot uses AI. Check for mistakes.
@raulmabe-labhouse
raulmabe-labhouse marked this pull request as draft February 25, 2026 13:57
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