Document how volume is stored internally for `(get|set)_volume` functions and methods

[Matiiss]

Fixes https://github.com/pygame-community/pygame-ce/issues/3090

This essentially solves the 129-step precision that SDL uses for sounds, in that a user of the .(set|get)_volume() API does not have to worry about it or wonder why

channel.set_volume(0.1)
print(channel.get_volume())  # prints 0.09375

[Starbuck5]

I don't think we should lie to the users about what their volume actually is.

[Matiiss]

I don't think we should lie to the users about what their volume actually is.

I'm unsure how we would be lying here. Obviously the sound is not going to have infinite resolution anyway, so at some point, tiny changes won't actually affect the volume regardless of whether the resolution is a 129 steps or a 1000. This is what I would call "close enough", at no point is the returned volume going to differ from the "actual" volume by more than 0.008.

That is a tiny maximum difference and in my opinion the practicality of returning a more accurate value (as in, closer to what it was set to) outweighs any such discrepancies. For example, suppose you want to increase the volume over a long period of time, you might be tempted to do something like this to say, increase the volume from 0% to 100% over 10 seconds:

sound.set_volume(sound.get_volume() + dt * 0.1)

Well, this simply won't work the way those values are handled currently.

That aside, this is (at least almost) like saying we're lying to the users about what their actual Rect's x value is when they set it to 2.9 and then they get 2 back and now you have to keep track of that position separately if you want to use floats. Keeping the differences in mind, here the maximum difference is 1, that is 125 times more than what the maximum difference for .get_volume is currently. Now, that was sort of solved by porting FRects, but I don't think we want to have FSound and FChannel and fmusic.

The current behavior is also somewhat less intuitive in my opinion. Sure, we can make a note in the docs about this, but that still violates the intuition of how one might expect it to behave and it is less practical.

I see no harm in making it behave this way, in fact, I see a couple benefits.

If none of the above is convincing, here's an alternative, we add the note about this behavior in the docs and add a volume property to Sound and Channel that will behave as this PR intends to make (get|set)_volume behave. Except then we can't quite do the same with mixer.music.

[damusss]

I don't think it is a lie as matiiss said, but I'm not even sure if anything should be added in the docs, as the SDL3 migration states volume will become a floating point number anyways (at least that's what I grasped by reading it) so it won't be a "lie" anymore saying it's 0-128 will become outdated with SDL3.

[Zimzozaur]

@Starbuck5, don’t you think the getter should return the exact value that the setter sets? In this case, the getter returns a different value due to an implementation detail that isn’t mentioned in the docs. Isn’t the point of an API to hide such internals and provide simple methods to access parts of an object without requiring users to consider what’s beneath the API layer?

[Starbuck5]

@Starbuck5, don’t you think the getter should return the exact value that the setter sets?

I think the function get_volume should return the actual volume.

I don't think it is a lie as matiiss said, but I'm not even sure if anything should be added in the docs, as the SDL3 migration states volume will become a floating point number anyways (at least that's what I grasped by reading it) so it won't be a "lie" anymore saying it's 0-128 will become outdated with SDL3.

This is incorrect. Looking at the SDL_mixer headers on their main branch they still very much use MIX_MAX_VOLUME. What you're reading is an SDL changelog, not an SDL_mixer changelog.

That aside, this is (at least almost) like saying we're lying to the users about what their actual Rect's x value is when they set it to 2.9 and then they get 2 back and now you have to keep track of that position separately if you want to use floats. Keeping the differences in mind, here the maximum difference is 1, that is 125 times more than what the maximum difference for .get_volume is currently. Now, that was sort of solved by porting FRects, but I don't think we want to have FSound and FChannel and fmusic.

I want the Python function to return the actual C state, both here and in Rect. If you applied this PR's logic to Rect each Rect would have a bunch of random stored floats to try to return the value the user "expects" from the Rect instead of the actual value.

I see no harm in making it behave this way, in fact, I see a couple benefits.

Code complexity, a small increase in runtime memory use.

@Matiiss you've brought up music several times in this thread but you haven't changed the behavior of music.get_volume, which has the same behavior as the other get_volumes.

[Zimzozaur]

I want the Python function to return the actual C state

Why not both? Why not having 2 methods like get_C_volume and get_Python_volume (do not take names literally). First of all, docs hide API details and why should remember the multiplayer (that is not written in docs) instead of adding a second return method that makes it easy to test volume for example.

I am asking this for the next time because you didn't respond, and I would like to understand your point of view.

[MyreMylar]

Looking into human decibel perception it seem generally agreed that we can only distinguish in around 3 decibel increments.

AFAICT reasonably expensive sound equipment promises around 130db range which would be about 43 audible increments of volume. Based on that back-of-the-envelope math I don't see much incentive for SDL Mixer to increase from 128 increment precision to floating point in the future.

What is the real purpose of us changing what these functions do/return? It is all different types of 'lying' about what is actually changing. Right now we are 'lying' that you can increment volume by values like 0.01 and have it actually change anything. Which it won't either mathematically in SDL Mixer or audibly to anyone listening.

The get volume returns neither exactly what SDL Mixer is doing (as it returns a float conversion of the integer internal state), and even if it did those increments themselves are inaudible unless you move them up or down in increments of at least 3 ints.

Generally we should probably document the volume functions better with this kind of information so users have an idea of what is happening.

If there is a useful, practical reason to change the current 'lying' behaviour to different 'lying' behaviour then please let us know what it is or where this issue came up in programming. AFAICT this is a fairly uncommon issue, though I found one other person noting it on stack overflow where the user didn't seem to know how to round numbers themselves.

The default presumption has to be to not change behaviour, even seemingly minor changes like this, unless there is a compelling reason to do so - as no doubt somebody, somewhere is relying on the current implementation.

So, please let us know a good 'why?' use case for motivation; beyond that you noticed it and and found it odd or mildly annoying.

[Matiiss]

Currently I can list two reasons:

• Convenience and practicality (is this two reasons?)

  For example, suppose you want to increase the volume over a long period of time, you might be tempted to do something like this to say, increase the volume from 0% to 100% over 10 seconds:

  sound.set_volume(sound.get_volume() + dt * 0.1)

  Well, this simply won't work the way those values are handled currently.

  - https://github.com/pygame-community/pygame-ce/pull/3091#issuecomment-2334668576

• Principle of least surprise (I suppose this is close to having found the behavior "odd or mildly annoying", but it is an important principle either way) It's a lot more intuitive to get back the value you set, not some closest float approximation where a 0-128 range is mapped to 0-1 range.

It's highly unlikely someone actually relies on the current behavior, even if someone does, I can't then imagine a way this could break their code. (Assuming their code is relatively reasonable, of course (in before someone hits me with an obscure example))

Also, as mentioned, we wouldn't hear all the increments anyway, so the volume seemingly not changing after incremented by a tiny bit is on par with the proposed changes, where the returned value would not necessarily reflect the actual volume, but the "actual volume" is not that important because even if SDL did use floats, they would have limited precision regarding the actual volume being outputted anyway. I already mentioned this in my previous comment along other points https://github.com/pygame-community/pygame-ce/pull/3091#issuecomment-2334668576

[bilhox]

@Matiiss I can see you forgot mixer_music.

[Matiiss]

@Matiiss I can see you forgot mixer_music.

I didn't forget it, I was planning on covering it in a separate PR, but at this point the future of this idea is uncertain.

[Matiiss]

Alright everyone, this came to mind the other day when I was thinking about Surface.(set|get)_alpha and how it stores an integer value and if you set it to some float value, it will get truncated anyway. However, I don't think we should intervene in those methods the same way I wanted to intervene here.

Thus for consistency's sake, I think I can agree to simply documenting this "issue" and just leave it at that (for now anyway). I also don't see how it would be that big of an inconvenience for the user to keep track of the float volume themselves.