-
Notifications
You must be signed in to change notification settings - Fork 757
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
Help: SoundFileView: Clarify zooming/scrolling, add RangeSlider example. #3587
Help: SoundFileView: Clarify zooming/scrolling, add RangeSlider example. #3587
Conversation
22b9fc7
to
d725382
Compare
returns:: A Float, in seconds of audio displayed. | ||
|
||
METHOD:: yZoom | ||
Vertical scaling. code::yZoom = 1:: sets 0 dBFS to the top and bottom of the view. 0.5 is half as tall. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0.5 is half as tall.
This could be improved. What dBFS do yo get when setting yZoom
to 0.5?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If dBFS are too confusing, it's probably better to remove the reference and say +/-1 instead.
If the audio file is properly engineered, you won't have positive dBFS at all (and, it's literally impossible to have positive dBFS in an integer-format audio file, because the maximum possible integer value for the bit depth maps onto 0 dBFS -- you might have values outside +/-1.0 in a floating-point file but it's not recommended to rely on that). So the question here doesn't entirely make sense.
Or should I use yZoom = 2 instead? Avoid the whole topic of out-of-range values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should stick with +/-1 instead of dBFS, and say that yZoom=2
will give you +/-0.5.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That works for me.
As an editorial aside, I find this last sort of comment -- a concrete suggestion for improvement -- to be very helpful. Otherwise, the PR author is left to guess what the desired improvement is, which only wastes everybody's time. (TBH I didn't know exactly what to do with the question about dBFS.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. I'll try to offer more concrete suggestions in the future.
Old text: "Gets the display data... It is not possible to get the data."
OK, I've addressed Patrick's suggestion. I also took the liberty of fixing this howler.
🤦♂️ 🤦♀️ |
🤦♂️ 🤦♀️
|
No description provided.