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

Silence various type conversion warnings #10

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sagamusix
Copy link

@sagamusix sagamusix commented Sep 12, 2024

Hello Geraint! First off, thank you for this fantastic high-quality library. I intend to use this in OpenMPT for offline sample processing, replacing two older libraries we currently use for pitch-shifting and time-stretching.

This PR intends to silence a couple of warnings I got when compiling with MSVC with high warnings levels; similar warnings may be observed with other compilers if the warning levels are set sufficiently high. Here's a few notes on what I did:

  • Most of the warnings revolve around double -> float conversions when instantiating the Stretch template with a float data type rather than double.
  • There are also some size_t -> int conversion warnings in 64-bit builds, because the base FFT classes appear to express their size in terms of size_t, while other classes such as WindowedFFT express their size in teams of int.
  • There was also one variable shadowing warning (in spectral.h) that I silenced.
  • I tried to keep the semantics identical to what they were before, so all fixes are done by adding casts instead of changing the API. It may be better to change all FFT classes to use the same type (so all int or all size_t but not mixing them), but since you are in charge of the library design, I wanted to leave that decision up to you.
  • I realize that the files in dsp are pulled in from another repository, so please let me know if I should open a PR there instead for those changes.

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

Successfully merging this pull request may close these issues.

1 participant