Skip to content

Add iFFT function (DSP-160) #104

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

howard0su
Copy link
Contributor

Description

Add reverse FFT support.
[Only fc32 as a PR to discuss]

Generate a reverse twiddle table when init. No need to change other logic as FFT. finally rescale the result by 1/N.

Need discussion on API design

  1. shall we add additional parameter iFFT to dsps_fft2r_init_fc32, or define a new function
  2. Where to put the logic to scale 1/N in iFFT caculation? it can be embedded into dsps_bit_rev_fc32_ansi (also need one more parameter) or a new function

Related

Fix #62

Testing

Add a simple test case to test


Checklist

Before submitting a Pull Request, please ensure the following:

  • 🚨 This PR does not introduce breaking changes.
  • All CI checks (GH Actions) pass.
  • Documentation is updated as needed.
  • Tests are updated or added as necessary.
  • Code is well-commented, especially in complex areas.
  • Git history is clean — commits are squashed to the minimum necessary.

@github-actions github-actions bot changed the title Add iFFT function Add iFFT function (DSP-160) Apr 9, 2025
@howard0su
Copy link
Contributor Author

any suggestions on API before I move forward?

@dmitry1945
Copy link
Collaborator

Hi @howard0su ,
I'm thinking....
Pro: yes, we will not spend additional time to divide the result.

Contra:
The table could be in ROM (on esp32s3),
It's useless for fixed point FFT, and then on fixed point FFT will be different interface,
Compare to the full amount of cycles (for 1024 is ~100k) one multiplication to 1/N will add ~1k cycles, it's about 1 %.
It's possible that people will use FFT and iFFT together, and then, it's not clear, which one is better....

But, from another side, yes, I think, that we can add this parameter as a default one.

Please, let me think up to tomorrow....

And, thank you for your work!

@howard0su
Copy link
Contributor Author

decision?

@dmitry1945
Copy link
Collaborator

Hi @howard0su
I would suggest, to have just additional function to initialize the iFFT table, like dsps_ifft_init_fc32, and this function will initialize r2 and r4 tables (just im=-im for both tables).
I will not change the interface, and it will give simple solution to use iFFT instead of FFT.

What do you think about this?

Dmitry

@howard0su
Copy link
Contributor Author

So the consumer still have to normalize the result by divide N?

I am open for options, but just want to make sure we think through the pros and cons.

@dmitry1945
Copy link
Collaborator

@howard0su It's better to add just additional iFFT function to normalize the result.
One of the problem, that the N, in general, could be different with init function, and in this case it's not possible to apply it to the table.

@howard0su
Copy link
Contributor Author

I didn't find a good way to apply 1/N to the table. It doesn't work if you just scale the table by N during initialization.

Regarding additional iFFT function, yes, we can easily add one additional function, or we can put it into the logic of dsps_bit_rev_fc32_ansi

@dmitry1945
Copy link
Collaborator

Hi @howard0su
the dsps_bit_rev_fc32_ansi is a bad way because new CPUs will support HW acceleration for bit reverse...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support iFFT (DSP-97)
3 participants