feat: add api to concatenate audio files#1045
feat: add api to concatenate audio files#1045Traviskn wants to merge 8 commits intosoftware-mansion:mainfrom
Conversation
mdydek
left a comment
mdydek
left a comment
There was a problem hiding this comment.
some changes needed before proper review, but at first glance it looks solid
- remove unneeded AudioFileConcatenator class - move declaration of classes to the .h file - add method comments
|
Updated this PR with the recommended changes! |
mdydek
left a comment
mdydek
left a comment
There was a problem hiding this comment.
impressive work, wonderful contribution! Only few nitpicks from the linter
There was a problem hiding this comment.
unfortunately, have to revert approve, code looks good, but it concats only m4a files (ffmpeg is not setup for wav and flac formats, miniaudio handles them and it fails because it meets unknown data), can you add also concatenating those formats, miniaudio operates on higher level than ffmpeg so it should be fairly simple. Also it would make us less dependent on ffmpeg, which is always better
fixes the error "Unable to install vendored xcframework libavcodec for Pod RNAudioAPI, because it contains both static and dynamic frameworks."
|
Unfortunately miniaudio only supports WAV encoding (mentioned in the features list on their README and verified in code https://github.com/mackron/miniaudio), so while I was able to use it for WAV files I still needed to use ffmpeg for concatenating other file formats. I tested WAV and FLAC and both were working. I also ran into a consistent cocoa pods error when installing into a local test app and included a fix in the download-prebuilt-binaries.sh script |
|
The latest push should address all the comments/requested changes. Let me know if any more changes are needed, happy to make any adjustments! |
|
|
||
| if (!isFFmpegOutputPath(normalizedOutputPath)) { | ||
| return Err( | ||
| "concatAudioFiles supports WAV output with miniaudio and M4A/MP4/CAF output with FFmpeg."); |
There was a problem hiding this comment.
have you tested CAF concatenation? because in my testing it is not working (I think it is not supposed to in the first place), i can manage to get it working only with wav and m4a, if that is also yours case, just update mentions to exclude caf
There was a problem hiding this comment.
i managed to get it working with broader ffmpeg build, so this the last thing to correct
There was a problem hiding this comment.
Ah sorry I must have missed that, I'll update this to not mention CAF. Thanks for testing and catching that!
There was a problem hiding this comment.
I have removed mentions of CAF files and removed an explicit check for CAF file extensions in the latest push
2 participants
Refs #1043
Introduced changes
concatAudioFiles(inputPaths, outputPath): Promise<string>exported from the package root, joining a list of compatible local audio files into a single output file. Pairs naturally with the recorder'srotateIntervalBytesoption to stitch rotated segments back into one file after recording.core/utils/AudioFileConcatenator) using the bundled FFmpeg, exposed to JS through a newAudioFileUtilsHostObjectand wired up inAudioAPIModuleInstalleras thecreateAudioFileUtilsJSI global.tests/mock.test.ts, and a C++ unit test (common/cpp/test/src/utils/AudioFileConcatenatorTest.cpp).packages/audiodocs/docs/utils/file-concatenation.mdxand cross-link it from therotateIntervalBytessection of the audio recorder docs.common/cpp/test/buildfromformat:*:commonandcpplint.shso local C++ test builds don't trip linting/formatting.Checklist