Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion .claude/skills/audio-nodes/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ classDiagram
AudioNode <|-- WorkletNode
AudioNode <|-- AnalyserNode
AudioNode <|-- AudioDestinationNode
AudioNode <|-- AudioRecorder

AudioScheduledSourceNode <|-- AudioBufferBaseSourceNode
AudioScheduledSourceNode <|-- OscillatorNode
Expand All @@ -152,6 +151,25 @@ classDiagram
AudioBufferBaseSourceNode <|-- AudioBufferQueueSourceNode
```

### AudioRecorder (not an AudioNode)

`core/inputs/AudioRecorder` is a standalone base class, not part of the `AudioNode` hierarchy — it
feeds the graph through a `RecorderAdapterNode` instead of being processed by it.

The split between it and `IOSAudioRecorder` / `AndroidAudioRecorder` is: the base owns everything
that happens to recorded frames (file writer, JS callback, adapter node — `enableFileOutput`,
`setupFileWriter`, `setOnAudioReadyCallback`, `connect`, `detachSideEffects`/`finalizeSideEffects`), the
subclasses own only the platform input stream. The one thing the base needs from the platform is
`resolveStreamFormat()`, returning sample rate, channel count and max frames per buffer; iOS reads
it from `NativeAudioRecorder` on every call (a route change invalidates it), Android returns values
cached when the Oboe stream opened. Add shared recorder behavior to the base, not to one platform.

Pitfall: never redeclare a base member (`deinterleavingBuffer_`, `streamSampleRate_`,
`recordingSegmentPaths_`) in a platform recorder. The shadowing copy compiles fine, but the base's
audio-thread fan-out reads its own member and silently drops that output.

---

### AudioScheduledSourceNode (internal only — not exposed to JS directly)

Base class for source nodes that have a scheduled start and stop time. **Not instantiated directly.**
Expand Down
5 changes: 5 additions & 0 deletions .claude/skills/build-compilation-dependencies/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ react-native-audio-api/
│ │ └── CMakeLists.txt # Actual Android C++ build target
│ ├── common/cpp/audioapi/ # Shared C++ (used by all platforms)
│ │ ├── decoding/ # Decoder factory, backends, SeekDecoderDaemon, AudioDecoding, AudioFileConcatenator
│ │ ├── encoding/ # AudioEncoder interface, EncoderCapabilities, OS encoder/remux selector headers
│ │ ├── libs/ # Third-party wrappers (FFmpeg, miniaudio, pffft, …)
│ │ └── external/ # Prebuilt binaries per platform
│ │ ├── android/ # .a static libs (Opus, Ogg, Vorbis, OpenSSL)
Expand Down Expand Up @@ -315,6 +316,7 @@ CI runs a parallel `cpp-coverage` job via `.github/workflows/cpp-coverage-job.ym
- Compile definitions: `RN_AUDIO_API_ENABLE_WORKLETS=0`, `RN_AUDIO_API_TEST=1`, `RN_AUDIO_API_FFMPEG_DISABLED=1`
- Google Test auto-fetched via `FetchContent` if not installed locally
- New test files in `test/src/**/*.cpp` are picked up automatically by glob — no CMakeLists edit needed
- `jsi.cpp` is compiled into the static lib so library members that reference JSI symbols (e.g. `AudioFileProperties::CreateFromJSIValue`) link when a test first pulls them in; a static-lib member costs nothing unless demanded. If a new test triggers `Undefined symbols: facebook::jsi::...`, the referenced runtime source is missing from the lib — add it there rather than stubbing the symbol

For `MockAudioEventHandlerRegistry`, `TestableXxx` pattern, and full CMakeLists analysis see [build-details.md](build-details.md#c-test-build--commoncpptestcmakeliststxt--detailed-analysis).

Expand Down Expand Up @@ -353,6 +355,8 @@ Resolution pitfalls learned the hard way (both handled inside `package-root.js`)
| `HAVE_ACCELERATE` | Not set | `GCC_PREPROCESSOR_DEFINITIONS` | Not set |
| `RN_AUDIO_API_TEST` | Not set | Not set | Always set to 1 |

**OS-API selector headers** (`decoding/OSDecoding.h`, `encoding/OSEncoding.h`, `encoding/OSRemux.h`, `encoding/OSFilePath.h`): common code reaches platform implementations through `#if defined(__ANDROID__)` / `#elif defined(__APPLE__) && !defined(RN_AUDIO_API_TEST) && !defined(RN_AUDIO_API_NODE)` dispatch. The Apple branch must exclude **both** desktop defines: the gtest build (`RN_AUDIO_API_TEST`) and the WPT node addon (`RN_AUDIO_API_NODE`) run on macOS (where `__APPLE__` is defined) but do not compile or link the `ios/` ObjC++ sources. The node build cannot borrow `RN_AUDIO_API_TEST` instead — that flag also switches on gtest-only code (`gtest_prod.h` includes, test `ArrayBuffer` shims). When adding a new OS-selector header, copy the full three-clause guard; an incremental `wpt_tests/build` dir can mask a missing clause for a long time, so verify with a clean `yarn node:build`. Platform glue selected this way lives in `android/src/main/cpp/audioapi/android/` (e.g. `AndroidDecoding`, `AndroidEncoder`, `AndroidRemux`) and `ios/audioapi/ios/core/utils/` (e.g. `IOSDecoding`, `IOSEncoder`, `IOSRemux`) — both picked up automatically by the CMake glob / podspec glob, no build-file edits needed.

---

## Common Build Failure Patterns
Expand All @@ -367,6 +371,7 @@ Resolution pitfalls learned the hard way (both handled inside `package-root.js`)
| New `.cpp` not compiled in tests | Glob picks it up automatically — may need cmake reconfigure | Delete `test/build/` and re-run |
| iOS compile error `unknown type 'id'` | C++ file included ObjC-only header | Compile that file as ObjC++ (separate subspec with `-x objective-c++`) |
| `RCT_NEW_ARCH_ENABLED` undefined on Android | Old RN gradle plugin | Ensure `newArchEnabled=true` in app's `gradle.properties` |
| iOS: `'to_chars' is unavailable: introduced in iOS 16.3` from `formatter_floating_point.h`, instantiated by `std::format<...>` | `std::format` in code compiled for iOS. libc++ availability-gates the whole `<format>` library to iOS 16.3; the podspec minimum is `ios_min_version = '14.0'`. The desktop C++ test build and Android NDK have no such gate, so `yarn test:cpp` passes and only the iOS build fails. | Use `std::string` concatenation / `std::to_string` in `common/cpp` and `ios/`. Zero-pad by hand (`insert(0, n, '0')`). Android-only files (`android/src/main/cpp`) may keep `std::format`. |

---

Expand Down
4 changes: 2 additions & 2 deletions .claude/skills/post-work-checks/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,13 @@ yarn test # from monorepo root — runs test:js + test:cpp

**When**: after any change to C++ files or TypeScript files in `src/`. Prefer this for a quick local test loop covering both TS and C++ logic; run `yarn validate:fast` before opening a PR.

### AudioEvent enum sync check
### Enum sync check

```bash
yarn check-audio-enum-sync
```

**When**: only when you modify the `AudioEvent` enum or any file that maps event names across C++/Kotlin/TypeScript. Skip this step if you already ran `validate:fast` (it includes enum sync).
**When**: when you modify `AudioEvent`, `FileFormat` / `AudioFileProperties::Format`, or other JSI-crossing recorder enums (`FileDirectory`, `BitDepth`, `IOSAudioQuality`). Skip if you already ran `validate:fast` (it includes enum sync).

---

Expand Down
2 changes: 1 addition & 1 deletion .claude/skills/post-work-checks/maintenance.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ Review this skill when `pre-push-update` reports changes in:
| `packages/react-native-audio-api/package.json` scripts | Package-level command changes (including per-language lint/format) |
| `lefthook.yml` | Pre-commit / commit-msg hook changes |
| `scripts/validate.sh` | Tier behavior (`--fast` / `--cpp-extended` / `--android` / `--ios` / `--full`), skip rules |
| `scripts/check-audio-enum-sync*` or `packages/react-native-audio-api/scripts/check-audio-events-sync.sh` | Enum sync check details |
| `scripts/check-audio-enum-sync*` or `packages/react-native-audio-api/scripts/check-*-enum-sync.sh` / `check-enum-sync.sh` | Enum sync check details (AudioEvent + AudioFileProperties) |
| `.github/workflows/ci.yml`, `tests.yml`, `cpp-job.yml` | What CI covers vs local validation tiers |
9 changes: 8 additions & 1 deletion .claude/skills/thread-safety-itc/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,13 @@ offloader.scheduleTask(std::move(workItem));

See the `utilities` skill for full API.

**Pitfall — file writer / recorder shutdown:** `TaskOffloader::shutdown()` drains the SPSC queue before joining the worker. Call it (or destroy the offloader) only after `isFileOpen_` is cleared so the audio thread stops enqueueing. Otherwise rotated or closed M4A segments lose seconds of buffered audio. Types with a `.slot` member use `slot == size_t max` as the shutdown sentinel.
**Pitfall — file writer / recorder shutdown:** `TaskOffloader::shutdown()` drains the SPSC queue before joining the worker. Call it (or destroy the offloader) only after `isFileOpen_` is cleared so the audio thread stops enqueueing. Otherwise rotated or closed M4A segments lose seconds of buffered audio. Types with a `.slot` member use `slot == size_t max` as the shutdown sentinel. And never hold a lock the worker takes while joining it: `closeFile()` clears the flag, drains and joins, and only then takes its lock to read totals — otherwise the join deadlocks against whatever the worker is doing under that lock.

**Pattern — reacting to audio without blocking on it.** When a decision depends on data the audio thread produced but costs blocking work to make (rotating a recording once its file outgrows a cap), do not make it in `writeAudioData()`. Measuring a file can be a `stat`, and acting on it closes and opens an encoder — both forbidden on the audio thread. Give the offloading writer a listener it calls **on its own worker thread** after each item is processed (`EncodedAudioFileWriter::setOnBufferEncodedCallback`), and let the policy owner decide from there. Keep that pair off the shared `AudioFileWriter` base, which is what the recorder consumes: a writer producing one file per session would only be forced into meaningless stubs. `RotatingFileWriter` reaches them by one `dynamic_cast` in its constructor, stored as a non-owning pointer — a deliberate deviation from the interface-type rule, recorded as such in the code. That cast is what decides the test seam: the concrete writer must stay non-`final` and `switchToFile` must stay `virtual`, or nothing can stand in for it — `resolveOsFilePath` and `createOsEncoder` both fail in the desktop test build, so a real one cannot open a file there. Test doubles derive from it and override every platform step, driving rotation through the protected `notifyBufferEncoded()`. The policy class then needs its own mutex for the state the worker advances, and it must declare the segment writer **last** so the worker is joined before the members it calls into are destroyed. Invoke the listener **outside** the writer's own lock, so it can call straight back in (`getFileSizeBytes()`, then `switchToFile()`) without re-entering a non-recursive mutex; the join-versus-lock ordering is the shutdown pitfall above.

Swapping the output file must not restart the worker: `switchToFile()` exchanges the encoder underneath a live offloader, so a rotating recording costs one thread per session, not one per segment.

**Pitfall — the task type cannot be a nested struct.** `TaskOffloader<T>` constrains `T` with `std::default_initializable`. A task struct carrying default member initializers (which the `.slot` sentinel requires) does *not* satisfy that constraint while its enclosing class is still incomplete, so `using Offloader = TaskOffloader<NestedTask, …>;` inside the class fails to compile with "constraints not satisfied". Making the struct `public` does not help — it is not an access problem. Declare the task type at **namespace scope** instead (`PendingFileWrite`, `PendingCallbackFrames`). Dropping the initializers to satisfy the constraint is worse: `T{}` would then produce `slot == 0`, a valid slot index, making the shutdown sentinel indistinguishable from real work.

---

Expand Down Expand Up @@ -207,6 +213,7 @@ back-to-back).
- **Copying `shared_ptr` inside `processNode()`** — increments atomic refcount; capture before entering hot path.
- **Locking `initialize()` or graph factory methods** — `initialize()` runs synchronously during HostObject construction on the JS thread; node factories and `createMediaElementSource()` are synchronous JS calls. Only lifecycle methods that touch the driver or offline render thread need `driverMutex_`.
- **Locking only `AudioContext`** — iOS recorder, session, and interruption paths mutate the shared `AVAudioEngine` outside `AudioContext`; keep the `AudioEngine` mutex on those entry points. Offline render uses the same `driverMutex_` on `BaseAudioContext`.
- **Duplicating recorder fan-out in platform code** — `AudioRecorder::onAudioFrames(interleavedFrames, numFrames)` (base class, `common/cpp/audioapi/core/inputs/`) is the single audio-thread fan-out to file writer, JS callback, and adapter node, using tryLock-and-drop per consumer. Platform recorders (e.g. `IOSAudioRecorder`) only normalize the platform buffer to interleaved float32 and call it — adding per-consumer writes in the platform receiver block double-writes every buffer. The interleave config (`inputChannelCount_`, scratch buffer) is read unlocked by the audio thread, so it may only be mutated while the input is disarmed (start/stop/input-format-change paths).
- **Re-entering `driverMutex_` or the `AudioEngine` mutex on the same thread** — call `tryStartDriver()` directly from `resume()` instead of `start()`; use lock-free `isStreamRunning()` from `isDriverRunning()`. `AudioContext::start()` does not acquire `driverMutex_`; it asserts the lock is already held when the driver is not initialized (via `scheduleAudioEvent` synchronous path). When already initialized, `start()` is a lock-free no-op so `source.start()` on the audio thread does not take the mutex.

---
Expand Down
1 change: 1 addition & 0 deletions .claude/skills/thread-safety-itc/maintenance.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ Review this skill when `pre-push-update` reports changes in:
| `common/cpp/audioapi/utils/CrossThreadEventScheduler.hpp` | Scheduler API changes — update decision table |
| `common/cpp/audioapi/core/AudioNode.*` | Audio thread contract changes |
| `common/cpp/audioapi/core/utils/AudioGraphManager.*` | Graph mutation queue changes |
| `common/cpp/audioapi/core/utils/EncodedAudioFileWriter.*`, `RotatingFileWriter.*` | The "reacting to audio without blocking on it" pattern: `switchToFile`/`setOnBufferEncodedCallback` contract, the listener-outside-the-lock rule, the never-join-while-holding rule, the `final`/`virtual` test seam |
| Any new cross-thread primitive in `utils/` | Document in the decision table |
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ jobs:
check-audio-enum-sync:
uses: ./.github/workflows/ci-check.yml
with:
name: Check AudioEvent enum sync
name: Check enum sync
run: yarn check-audio-enum-sync

build-audio-api:
Expand Down
27 changes: 22 additions & 5 deletions apps/common-app/src/demos/Record/Record.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,16 @@ import RecordingVisualization from './RecordingVisualization';
import Status from './Status';
import { RecordingState } from './types';

// concatAudioFiles supports WAV, M4A, and FLAC — the formats recordable on
// both iOS and Android.
const RECORDING_EXTENSION = FileFormat.M4A;
const ROTATING_SIZE = 250_000;

const RECORDING_EXTENSION_NAME_MAP = {
[FileFormat.Wav]: 'wav',
[FileFormat.M4A]: 'm4a',
[FileFormat.Flac]: 'flac',
};
const Record: FC = () => {
const [state, setState] = useState<RecordingState>(RecordingState.Idle);
const [hasPermissions, setHasPermissions] = useState<boolean>(false);
Expand Down Expand Up @@ -90,9 +100,7 @@ const Record: FC = () => {
return;
}

const result = await Recorder.start({
fileNameOverride: `overridden_name_${Date.now()}`,
});
const result = await Recorder.start();

setupNotification(false);

Expand Down Expand Up @@ -130,9 +138,14 @@ const Record: FC = () => {
return;
}

const outputPath = info.paths[0].replace(/[^/]+$/, 'recording.m4a');
const extension = RECORDING_EXTENSION_NAME_MAP[RECORDING_EXTENSION];
const outputPath = info.paths[0].replace(
/[^/]+$/,
`recording.${extension}`
);

const finalPath = await concatAudioFiles(info.paths, outputPath);
// const finalPath = info.paths[0];
const audioBuffer = await audioContext.decodeAudioData(finalPath);
setRecordedBuffer(audioBuffer);

Expand Down Expand Up @@ -262,7 +275,11 @@ const Record: FC = () => {
}, [onPauseRecording, onResumeRecording]);

useEffect(() => {
Recorder.enableFileOutput({ rotateIntervalBytes: 1_000_000, format: FileFormat.M4A });
Recorder.enableFileOutput({
rotateIntervalBytes: ROTATING_SIZE,
format: RECORDING_EXTENSION,
fileName: 'my_recording'
});

return () => {
stopPlayback();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,13 +281,11 @@ const AudioPipelineStress: FC = () => {
}
};

const performCleanRecording = async (
fileNameOverride: string
): Promise<RecordingCapture> => {
const performCleanRecording = async (): Promise<RecordingCapture> => {
await activateRecordingSession();

resourcesRef.current.configureRecorderTap();
await resourcesRef.current.startRecording(fileNameOverride);
await resourcesRef.current.startRecording();
await waitForRecordingCallbacks(1);
await sleep(SHORT_RECORDING_MS);

Expand Down Expand Up @@ -327,7 +325,7 @@ const AudioPipelineStress: FC = () => {
};

const performCleanRecordPlaybackCycle = async (label: string) => {
const capture = await performCleanRecording(`${label}-${Date.now()}`);
const capture = await performCleanRecording();
await performCleanPlayback(
capture.decodedBuffer,
Math.min(capture.decodedBuffer.duration, 1.4),
Expand Down Expand Up @@ -474,9 +472,7 @@ const AudioPipelineStress: FC = () => {
'record-and-decode',
'Record briefly, then decode output',
async () => {
const capture = await performCleanRecording(
`record-warmup-${Date.now()}`
);
const capture = await performCleanRecording();
addInfoStep(
steps,
'recorded-file',
Expand All @@ -499,9 +495,7 @@ const AudioPipelineStress: FC = () => {
`cycle-${cycle}`,
`Cycle ${cycle}: record, decode, and play recorded audio`,
async () => {
const capture = await performCleanRecording(
`record-to-playback-${cycle}-${Date.now()}`
);
const capture = await performCleanRecording();

const playbackStats = await performCleanPlayback(
capture.decodedBuffer,
Expand Down Expand Up @@ -551,9 +545,7 @@ const AudioPipelineStress: FC = () => {
`Cycle ${cycle} pre-record playback engine timing`,
formatPlaybackProgressStats(playbackStats)
);
await performCleanRecording(
`playback-to-record-${cycle}-${Date.now()}`
);
await performCleanRecording();
}
);
}
Expand Down Expand Up @@ -691,7 +683,7 @@ const AudioPipelineStress: FC = () => {
'clean-recovery-cycle',
'Run one clean record and decode cycle after recovery',
async () => {
await performCleanRecording(`post-record-recovery-${Date.now()}`);
await performCleanRecording();
}
);
}
Expand All @@ -716,9 +708,7 @@ const AudioPipelineStress: FC = () => {
await AudioManager.setAudioSessionActivity(true);

resourcesRef.current.configureRecorderTap();
const result = await resourcesRef.current.tryStartRecording(
`wrong-category-${Date.now()}`
);
const result = await resourcesRef.current.tryStartRecording();

if (result.status === 'success') {
throw new Error(
Expand All @@ -743,9 +733,7 @@ const AudioPipelineStress: FC = () => {
'clean-recovery-record',
'Switch back to playAndRecord and confirm clean recording works',
async () => {
await performCleanRecording(
`wrong-category-recovery-${Date.now()}`
);
await performCleanRecording();
}
);
}
Expand Down
Loading
Loading