-
Notifications
You must be signed in to change notification settings - Fork 231
Add support for audio recording interruptions with segmentation #648
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add support for audio recording interruptions with segmentation #648
Conversation
@hyochan thoughts? |
I don't think all this is needed, simply removing |
I've tested this fix and observed that the stopRecorder function's promise takes around 20 seconds to resolve when there are 3–4 call interruptions during a 30-minute recording. This delay is quite significant. It would be great if the performance of this function could be optimized, especially for scenarios involving long recordings with interruptions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Enhance recording resilience by segmenting audio files around interruptions and merging them on stop.
- Introduce two native methods: one with an explicit
returnSegments
flag and one without options to preserve backwards compatibility in JS bridging. - Initialize TS-side recorder/player state defaults and nullable subscriptions/callbacks.
- Update
stopRecorder
in TS to route calls to the correct native method based on platform and optional flag.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
ios/RNAudioRecorderPlayer.m | Add stopRecorderWithNoOptions and overload stopRecorder |
index.ts | Initialize fields, make subscriptions nullable, refine stopRecorder , update play/pause returns |
Comments suppressed due to low confidence (4)
ios/RNAudioRecorderPlayer.m:23
- This selector exposes a
BOOL
flag to JS; ensure your TypeScript definitions forRNAudioRecorderPlayer
are updated to include this overload, otherwise calls from JS/TS may fail to resolve.
RCT_EXTERN_METHOD(stopRecorder:(BOOL)returnSegments
index.ts:296
- The JSDoc notes iOS-only behavior but doesn’t clarify Android’s default behavior. Add a remark on Android returning the merged file path or adjust the docs to reflect cross-platform behavior.
* @param {boolean} returnSegments - If true, return comma-separated list of segment file paths (iOS only)
index.ts:299
- New behavior around segmented recordings and branching by platform isn’t covered by existing tests. Consider adding unit or integration tests that simulate interruptions to verify both merge and segment-return flows.
stopRecorder = async (returnSegments?: boolean): Promise<string> => {
index.ts:299
- [nitpick] The optional
returnSegments
flag name could be misinterpreted—perhaps rename it to something more descriptive likeshouldReturnSegmentPaths
for clarity.
stopRecorder = async (returnSegments?: boolean): Promise<string> => {
@@ -17,7 +17,11 @@ @interface RCT_EXTERN_MODULE(RNAudioRecorderPlayer, RCTEventEmitter) | |||
resolve:(RCTPromiseResolveBlock)resolve | |||
reject:(RCTPromiseRejectBlock)reject); | |||
|
|||
RCT_EXTERN_METHOD(stopRecorder:(RCTPromiseResolveBlock)resolve | |||
RCT_EXTERN_METHOD(stopRecorderWithNoOptions:(RCTPromiseResolveBlock)resolve |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rejecter:
label deviates from the standard reject:
used elsewhere in your extern declarations. Consider renaming it to reject:
to maintain consistency and avoid bridging issues.
Copilot uses AI. Check for mistakes.
I would love to see a proper fix for this at this layer; we are using a pretty jank approach though with server side stitching so somebody else would probably have to take the lead on a real client fix |
@lokeshwebdev007 for short recordings it stitches on the client very fast. For longer recordings you will need to accept the delay or stitch server side. |
This pull request enhances the
react-native-audio-recorder-player
library by adding support for handling audio recording interruptions, such as incoming calls, in a seamless way. Previously, when an interruption occurred, resuming the recording would overwrite the existing audio file becauseAVAudioRecorder
’srecord()
method implicitly callsprepareToRecord()
, creating a new file at the specified URL. This made it impossible to truly "resume" a recording with the existing API.To address this issue, we’ve introduced a new approach:
AVMutableComposition
, ensuring no audio is lost.This solution ensures that recordings remain intact across interruptions, providing a smoother and more reliable experience for users. The changes are implemented in
RNAudioRecorderPlayer.swift
, with updates to key methods likestartRecorder
,resumeRecorder
, andstopRecorder
, along with new functions for segment management and merging.