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

Merged
merged 4 commits into from Mar 22, 2018

Conversation

Projects
None yet
3 participants
@jamshark70
Copy link
Contributor

jamshark70 commented Mar 20, 2018

No description provided.

@jamshark70 jamshark70 force-pushed the jamshark70:topic/SoundFileViewScrolling branch from 22b9fc7 to d725382 Mar 20, 2018

@patrickdupuis patrickdupuis added this to the 3.9.2 milestone Mar 20, 2018

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.

This comment has been minimized.

@patrickdupuis

patrickdupuis Mar 20, 2018

Contributor

0.5 is half as tall.

This could be improved. What dBFS do yo get when setting yZoom to 0.5?

This comment has been minimized.

@jamshark70

jamshark70 Mar 20, 2018

Contributor

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.

This comment has been minimized.

@patrickdupuis

patrickdupuis Mar 21, 2018

Contributor

I think you should stick with +/-1 instead of dBFS, and say that yZoom=2 will give you +/-0.5.

This comment has been minimized.

@jamshark70

jamshark70 Mar 22, 2018

Contributor

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.)

This comment has been minimized.

@patrickdupuis

patrickdupuis Mar 22, 2018

Contributor

You're right. I'll try to offer more concrete suggestions in the future.

jamshark70 added some commits Mar 22, 2018

Help: SoundFileView: Fix self-contradictory 'data' method description
Old text: "Gets the display data... It is not possible to get the data."
@jamshark70

This comment has been minimized.

Copy link
Contributor

jamshark70 commented Mar 22, 2018

OK, I've addressed Patrick's suggestion. I also took the liberty of fixing this howler.

METHOD:: data

    Gets the display data, or sets custom data instead of a sound file.

    It is not possible to get the data.

🤦‍♂️ 🤦‍♀️

@patrickdupuis

This comment has been minimized.

Copy link
Contributor

patrickdupuis commented Mar 22, 2018

🤦‍♂️ 🤦‍♀️

I only see male and female. Does GitHub still need to get woke? Oh! the above does not appear the same on my mobile as it does here in a proper browser. I was seeing [black square] male sign [black square] female sign.

@patrickdupuis patrickdupuis merged commit 0f66199 into supercollider:3.9 Mar 22, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment