Minor scaling factor fixes#645
Merged
corrados merged 1 commit intojamulussoftware:integrate_floatfrom Sep 30, 2020
Merged
Conversation
Contributor
Author
|
Should have said "slightly more negative than -1.0" in (1) above. |
Contributor
|
Thanks |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I suggest a few slight tweaks to the conversion scale factors.
(1) The tweaks are only strictly "necessary" on the 16 bit / float conversions. Prevents max negative integer from converting to a float that's slightly bigger than one. This is in keeping with the way Port Audio does these conversions.
(2) Not strictly necessary for 32 bit / float conversions, but keeps things consistent and is in keeping with the way Port Audio does these conversions.
(3) Added two comments to remind anyone who might change this code down the line that there is a reason double precision is used for the 32 bit / float scale factors. Not strictly necessary for the 16 bit / float conversions, so I didn't add a comment there. (Update: Somehow the comment survived in the 16 bit / float case). But maybe best to keep the double precision scale factors there for consistency's sake.
These tweaks are in keeping with comments I made on the original hselasky pull request.