-
-
Notifications
You must be signed in to change notification settings - Fork 33
AudioRecorder - recording to file #752
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?
Conversation
|
big if true 🤘🏼 |
| std::string filePath_{""}; | ||
| std::atomic<size_t> framesWritten_{0}; | ||
|
|
||
| int32_t streamSampleRate_{0}; |
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.
should be a float/double like everywhere else
| emitAudioData(); | ||
| } | ||
|
|
||
| void AndroidRecorderCallback::deinterleaveAndWriteAudioData( |
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.
NIT
| void AndroidRecorderCallback::deinterleaveAndWriteAudioData( | |
| void AndroidRecorderCallback::deinterleaveAndPushAudioData( |
| } | ||
|
|
||
| void AndroidRecorderCallback::sendRemainingData() { | ||
| auto numberOfFrames = circularBus_[0]->getNumberOfAvailableFrames(); |
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.
ideally circular bus should be a separate class instead of plain vector of circular arrays
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.
emitAudioData + sendRemainingData could be rewritten to single method with param emitRemaining: bool
| __android_log_print( | ||
| ANDROID_LOG_ERROR, | ||
| "FileOptions", | ||
| "Error creating directory at path: %s, error: %s", | ||
| directoryPath.c_str(), | ||
| ec.message().c_str()); |
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.
remove unnecessary logs
| frame_ = av_unique_ptr<AVFrame>(av_frame_alloc()); | ||
| packet_ = av_unique_ptr<AVPacket>(av_packet_alloc()); | ||
|
|
||
| int contextFrameRatio = 4; |
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.
magic number
| using namespace facebook; | ||
| using namespace react; | ||
|
|
||
| class NativeFileInfo : public jni::JavaClass<NativeFileInfo> { |
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.
better name?
| auto result = audioRecorder_->setOnAudioReadyCallback( | ||
| sampleRate, bufferLength, channelCount, callbackId); | ||
| auto jsResult = jsi::Object(runtime); | ||
|
|
||
| jsResult.setProperty( | ||
| runtime, | ||
| "status", | ||
| jsi::String::createFromUtf8( | ||
| runtime, result.isSuccess() ? "success" : "error")); |
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.
error message is not returned here
| int log_level_offset; | ||
|
|
||
| enum AVMediaTypeFFmpeg codec_type; /* see AVMEDIA_TYPE_xxx */ | ||
| enum AVMediaTypeFFMPEG codec_type; /* see AVMEDIA_TYPE_xxx */ |
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.
unnecessary change
| sampleRate, | ||
| bitRate, | ||
| bitDepth, | ||
| std::max(flacCompressionLevel - 1, 0), |
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.
this might not be necessary anymore - with file templates and defaults we don't need to worry about absent parameter
| template<> | ||
| class ReturnStatus<void> { | ||
| public: | ||
| enum class Status { | ||
| Success = 0, |
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.
dig in, this template spec doesn't seem to be properly picked up by the compiler 🤔
Closes #
AudioRecorderconstructor no longer accepts any optionsonAudioReadynow requires additional properties passed, that define data received by the callback - desired chunk length, channel count, sample rateIntroduced changes
setSessionActivealways resolving to true no mather if it was succesfull or notChecklist