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

cairo error->exit(1) and replace Kaiser window with Hanning #26

Closed
wants to merge 2 commits into from
Closed

cairo error->exit(1) and replace Kaiser window with Hanning #26

wants to merge 2 commits into from

Conversation

martinwguy
Copy link
Contributor

The most significant of these is to fix issue #15 by changing window function.
An alternative would be an option to select windowing function, but Hanning seems to work best.

Previously, an error in cairo initialization, for example because the
output file would be >32767 pixels wide, caused sndfile-spectrogram
to exit with 0 exit status, which makes detecting sndfile-spectrogram
failure difficult in shell scripts.

This change makes it exit(1) instead.
sndfile-spectrogram's output was visibly noisy for pure tones and
less visibly noisy for other signals. This change improves the
output quality by replacing the Kaiser window function that it was using
by a Hanning window, which gives the best output of the twelve functions
tried (Kaiser, Nuttall, Rectangular, Bartlett, Hamming, Hanning, Blackman,
Blackman-Harris, Welch and first- second- and third-order Gaussian).
@erikd
Copy link
Member

erikd commented Dec 6, 2015

Can we separate out the Cairo error handling from the windowing changes?

And for the windowing, how about providing an option to choose between windows so that I (and others) can compare and contrast more easily?

@martinwguy martinwguy closed this Dec 6, 2015
@martinwguy
Copy link
Contributor Author

OK, I've made a new pull request for the error-handling change.
Now that I've made the windowing function selectable, if I submit a second pull request, it includes both commits though. How should I proceed to suggest several independent changes simultaneously?
Can you cherry-pick it from 87f7843 ?

@erikd
Copy link
Member

erikd commented Dec 6, 2015

Got it. Thanks.

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.

None yet

2 participants