Adds IRFFT Op to Signal Library #2137
Conversation
|
|
||
| auto* params = static_cast<TfLiteAudioFrontendIrfftParams*>( | ||
| context->AllocatePersistentBuffer( | ||
| context, sizeof(TfLiteAudioFrontendIrfftParams))); |
There was a problem hiding this comment.
Can we add a check here to ensure params is not nullptr? Elsewhere, we found this just looked like:
if (params == nullptr) {
return nullptr;
}
There was a problem hiding this comment.
Done. Is there any preference over this vs TFLITE_DCHECK()?
There was a problem hiding this comment.
It matches existing kernel Init functions. They should have an ENSURE and/or a DCHECK done later.
signal/micro/kernels/irfft.cc
Outdated
| void* Init(TfLiteContext* context, const char* buffer, size_t length) { | ||
| TFLITE_DCHECK(context->AllocatePersistentBuffer != nullptr); | ||
|
|
||
| const uint8_t* buffer_t = reinterpret_cast<const uint8_t*>(buffer); |
There was a problem hiding this comment.
buffer_t reads as if it is a type, since _t is a common type extension. Since this is only used once down at line 62, can we just remove this variable and put the reinterpret_cast in the call to the FlexbufferWrapper ctor?
|
|
||
| size_t state_size = (*get_needed_memory_func)(params->fft_length); | ||
| params->state = reinterpret_cast<int8_t*>( | ||
| context->AllocatePersistentBuffer(context, state_size * sizeof(int8_t))); |
There was a problem hiding this comment.
We should do a nullptr check here too.
signal/src/irfft_float.cc
Outdated
| } | ||
|
|
||
| void* IrfftFloatInit(int32_t fft_length, void* state, size_t state_size) { | ||
| IrfftFloatState* irfft_float_state = (IrfftFloatState*)state; |
There was a problem hiding this comment.
We should switch this to a static_cast
signal/src/irfft_float.cc
Outdated
| void* IrfftFloatInit(int32_t fft_length, void* state, size_t state_size) { | ||
| IrfftFloatState* irfft_float_state = (IrfftFloatState*)state; | ||
| irfft_float_state->cfg = | ||
| (kiss_fft_float::kiss_fftr_cfg)(irfft_float_state + 1); |
There was a problem hiding this comment.
And this one to a reinterpret_cast
signal/src/irfft_float.cc
Outdated
| } | ||
|
|
||
| void IrfftFloatApply(void* state, const Complex<float>* input, float* output) { | ||
| IrfftFloatState* irfft_float_state = (IrfftFloatState*)state; |
|
Ready for another look. |
|
|
||
| auto* params = static_cast<TfLiteAudioFrontendIrfftParams*>( | ||
| context->AllocatePersistentBuffer( | ||
| context, sizeof(TfLiteAudioFrontendIrfftParams))); |
There was a problem hiding this comment.
It matches existing kernel Init functions. They should have an ENSURE and/or a DCHECK done later.
3 participants
Inverse-RFFT as part of Signal library ops.
Testing via current FFT Op tests.
BUG=287346710