Skip to content
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

Replace FFTW library with the PFFFT library #126

Merged
merged 33 commits into from Nov 11, 2021

Conversation

melissascode
Copy link
Contributor

@melissascode melissascode commented Aug 28, 2021

The FFTW library was utilized in two Hobbits plugins, the Spectrogram and the Width Framer. Due to licensing conflicts, the FFTW library could no longer be used in Hobbits. After researching different libraries, @hello-adam and I decided to use the PFFFT library.

I have refactored the code for both the Spectrogram and the Width Framer. Also, I implemented a QComboBox that allows users to change the FFT Size within the Spectrogram. Prior to adding this, FFT Size was a QSpinBox that allowed users to manually enter the FFT size or use up and down arrows to increment. The QComboBox allows users to choose FFT sizes in powers of two, ranging from 2^5 to 2^14 (or, 32 to 16,384).

melissascode and others added 18 commits July 11, 2021 19:24
…the code associated with the FFTW library. Added in new code that utilizes the PFFFT library in order to perform Fast Fourier Transforms needed. I updated CMakeLists.txt to include the PFFFT library.
…o perform the Fast Fourier Transform. Changed the first parameter of the fillSamples function to be a float* buffer instead of fftw_complex* buffer.
…parameter from fftw_complex* buffer to float* buffer, as the PFFFT library makes use of float buffers.
…size given is not divisible by 16, the FFT size will be increased to the next number divisble by 16.
@hello-adam
Copy link
Member

I need to update the CentOS7 docker container to have pffft (that's why it's failing in azure). You might need to make changes to the CMake stuff after I do that in order to get that behaving correctly.

I'm not sure why the Ubuntu build failed.

@hello-adam
Copy link
Member

looks like there are still references to CONAN_PKG::fftw that need to be removed

@melissascode
Copy link
Contributor Author

melissascode commented Sep 6, 2021

looks like there are still references to CONAN_PKG::fftw that need to be removed

I removed them now and it looks like the checks were all successful. I will message you about how I should get other mentions of FFTW taken out other files I've seen it mentioned in.

src/hobbits-gui/main.cpp Outdated Show resolved Hide resolved
Copy link
Member

@hello-adam hello-adam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if you get sick of any of the this "last mile" stuff - I'd be happy to merge this in and clean it up myself, I just want to give you the opportunity to do it if you want to.

@@ -17,7 +17,7 @@ file(GLOB RCFILES "${CMAKE_CURRENT_SOURCE_DIR}/*.qrc" "${CMAKE_CURRENT_SOURCE_DI


if (BUILDING_WITH_CONAN)
set(ALL_LINK_LIBS hobbits-python hobbits-widgets hobbits-core CONAN_PKG::qt CONAN_PKG::fftw)
set(ALL_LINK_LIBS hobbits-python hobbits-widgets hobbits-core CONAN_PKG::qt CONAN_PKG::pffft)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that hobbits-gui and hobbits-runner actually no longer need to link to the fft library because they don't need to do that "make planner thread safe" call.

src/hobbits-gui/main.cpp Outdated Show resolved Hide resolved
@@ -17,7 +17,7 @@ file(GLOB RCFILES "${CMAKE_CURRENT_SOURCE_DIR}/*.qrc" "${CMAKE_CURRENT_SOURCE_DI
add_executable("hobbits-runner" "${SRCFILES}" "${HDRFILES}" "${RCFILES}")

if (BUILDING_WITH_CONAN)
set(ALL_LINK_LIBS hobbits-python hobbits-widgets hobbits-core CONAN_PKG::qt CONAN_PKG::fftw)
set(ALL_LINK_LIBS hobbits-python hobbits-widgets hobbits-core CONAN_PKG::qt CONAN_PKG::pffft)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the fft lib link is no longer necessary in the hobbits-runner

@melissascode melissascode reopened this Oct 7, 2021
@hello-adam hello-adam merged commit d23d63c into Mahlet-Inc:develop Nov 11, 2021
@hello-adam
Copy link
Member

🎉 This PR is included in version 0.53.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

None yet

2 participants