hyochan / react-native-audio-recorder-player

react-native native module for audio recorder and player.
MIT License
681 stars 203 forks source link

Assign default file extension based on audio format #615

Closed jbaudanza closed 2 weeks ago

jbaudanza commented 3 weeks ago

I am using this library to record ogg/opus files, however, the default file name was always "sound.mp4" or "sound.m4a", regardless of the audio format.

To fix this, I tried passing in a custom path to startRecord, with the correct file extension, however a native extension is required to get the proper path to the cache directory on Android.

Since a native modification is required anyway, I decided to modify this module to just pick a reasonable default file extension for the giving audio format.

I also added the proper OGG / OPUS types for Android to index.ts. These types are supported as of apiVersion 29.

jbaudanza commented 2 weeks ago
  1. Default Value Return: The current implementation returns kAudioFormatAppleLossless when the encoding value is nil. It might be better to determine the default value externally or pass it as a parameter to this function.

If the encoding value is unrecognized, I think the library should raise an error, rather than falling back to a default. This behavior would be more expected and less surprising to a consumer of this module.

btw, I think kAudioFormatMPEG4AAC would be a better choice of default format, rather than kAudioFormatAppleLossless, but I didn't want to address that in this PR.

  1. Error Handling: The function currently returns nil for unmatched strings. Adding exception handling to provide clear error messages would be beneficial.

The calling function startRecorder will indeed return a rejected promise to the caller if avFormat returns nil.

  1. Error Handling: For unsupported AudioFormatID, the current implementation returns "audio" as a default value. It would be better to provide clear error messages.

These formats are not necessarily "unsupported", but are traditionally not written to a file. If, for some reason, the caller wants to write these a file, I think .audio is a reasonable choice for a file extension.

  1. Documentation: Adding comments explaining each AudioFormatID in the code would make it easier to understand.

I think the constant names are descriptive enough without adding more comments. More details are available in the Apple documentation: https://developer.apple.com/documentation/coreaudiotypes/1572096-audio_format_identifiers