Conversation
michalsek
left a comment
michalsek
left a comment
There was a problem hiding this comment.
I think it would be good to rething start/stop methods when rotating is enabled, returning filepath on start, and only signle path on stop (with metadata) would be weird.
We could consider slightly improving the results by having:
- Only status return when start/stop is called without file output
- Single/path meta for normal file output
- Parent directory(?) for the rotating files or only status at start, list of files (or parent directory) on stop for batched files
Although not a strong opinion on the approach
| export interface AudioRecorderFileOptions { | ||
| channelCount?: number; | ||
| batchDurationSeconds?: number; | ||
| rotateIntervalBytes?: number; |
There was a problem hiding this comment.
make this name more explanatory f.e. byteTresholdForFileRotation
| return 0; | ||
| } | ||
|
|
||
| if (formatCtx_ && formatCtx_->pb) { |
There was a problem hiding this comment.
a lot of implicit bool casts in this file
| if (createWriter(properties) == nullptr) { | ||
| return Result<std::string, std::string>::Err( | ||
| "FFmpeg backend is disabled. Cannot create file writer for the requested format. Use " | ||
| "WAV " | ||
| "format instead."); | ||
| } |
There was a problem hiding this comment.
you can just check compilation flag, there is no need to manually call this function and check if it returns nullptr
| }; | ||
|
|
||
| if (fileProperties_->rotateIntervalBytes > 0) { | ||
| if (createWriter(fileProperties_) == nullptr) { |
| auto createWriter = | ||
| [this]( | ||
| const std::shared_ptr<AudioFileProperties> &props) -> std::shared_ptr<AudioFileWriter> { | ||
| if (props->format == AudioFileProperties::Format::WAV) { | ||
| return std::make_shared<MiniAudioFileWriter>( | ||
| audioEventHandlerRegistry_, | ||
| props, | ||
| streamSampleRate_, | ||
| streamChannelCount_, | ||
| streamMaxBufferSizeInFrames_); | ||
| } else { | ||
| #if !RN_AUDIO_API_FFMPEG_DISABLED | ||
| fileWriter_ = std::make_shared<android::ffmpeg::FFmpegAudioFileWriter>( | ||
| audioEventHandlerRegistry_, properties); | ||
| return std::make_shared<android::ffmpeg::FFmpegAudioFileWriter>( | ||
| audioEventHandlerRegistry_, | ||
| props, | ||
| streamSampleRate_, | ||
| streamChannelCount_, | ||
| streamMaxBufferSizeInFrames_); | ||
| #else | ||
| return Result<std::string, std::string>::Err( | ||
| "FFmpeg backend is disabled. Cannot create file writer for the requested format. Use WAV format instead."); | ||
| return nullptr; | ||
| #endif |
There was a problem hiding this comment.
code repetition with lines 127-149
| if (currentWriter_) { | ||
| return currentWriter_->getFileSizeBytes(); | ||
| } | ||
| return 0; |
There was a problem hiding this comment.
not very against it, proabably need to agreed on something, but a lot of "dull" code can be made more consise
| if (currentWriter_) { | |
| return currentWriter_->getFileSizeBytes(); | |
| } | |
| return 0; | |
| return currentWriter_ ? currentWriter_->getFileSizeBytes() : 0; |
There was a problem hiding this comment.
and similarly some functions inside it also
| std::shared_ptr<AudioFileProperties> fileProperties_; | ||
| std::shared_ptr<AudioEventHandlerRegistry> audioEventHandlerRegistry_; | ||
|
|
||
| friend class RotatingFileWriter; |
There was a problem hiding this comment.
RotatingFileWriter inherits from AudioFileWriter so it automatically has access to all protected methods, there is no need for friend class
| bufferFormat_ = bufferFormat; | ||
| converterInputBufferSize_ = maxInputBufferLength; |
| size_t bufferSize = [nativeRecorder_ getBufferSize]; | ||
| auto createWriter = | ||
| [this, bufferSize]( | ||
| const std::shared_ptr<AudioFileProperties> &props) -> std::shared_ptr<AudioFileWriter> { | ||
| return std::make_shared<IOSFileWriter>( | ||
| audioEventHandlerRegistry_, props, [nativeRecorder_ getInputFormat], bufferSize); | ||
| }; | ||
|
|
||
| if (properties->rotateIntervalBytes > 0) { | ||
| fileWriter_ = std::make_shared<RotatingFileWriter>( | ||
| audioEventHandlerRegistry_, properties, properties->rotateIntervalBytes, createWriter); | ||
| } else { | ||
| fileWriter_ = createWriter(properties); | ||
| } |
There was a problem hiding this comment.
code repetition with 106-128
3 participants
Closes RNAA-384
Introduced changes
Checklist