Audio: Change playback position type from float to double #105545
+244
−35
Conversation
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
The absolute time is calculated based on the total mixed frames and the mix rate. This means it only updates when a mix step happens. Specific play_scheduled behaviors: - If a sound is playing, play_scheduled() will stop that sound. This matches the behavior of play(). - If a sound is scheduled, then paused, then resumed before the schedule happens, the sound still plays at the correct scheduled time. - If a playing sound is paused, then play_scheduled() is called, the sound will restart from the beginning. This matches the behavior of play(). - With a higher max_polyphony, multiple sounds can be scheduled. - play_scheduled is unaffected by pitch scale - play_scheduled does not support samples Scheduled stop is not implemented due to limited use cases.
PizzaLovers007
force-pushed
the
fix-playback-position-types
branch
from
327b188 to
f81aa49
Compare
Two reasons to change this: * At a default mix rate of 44100, the playback position in seconds can experience rounding errors with a 32-bit type if the value is high enough. 44100 requires 15 free bits in the mantissa to be within 1/2 an audio frame, so the cutoff is 512 seconds before rounding issues occur (512=2^9, and 9+15 > 23 mantissa bits in 32-bit float). * Internally, AudioStreamPlayback and AudioStream use a 64-bit type for playback position. See this discussion: godotengine#105510 (comment)
PizzaLovers007
force-pushed
the
fix-playback-position-types
branch
2 times, most recently
from
f3dd43f to
fd9f6f0
Compare
AThousandShips
changed the title
|
These still need compatibility binds I'd say, the methods are still used by GDExtension etc. |
|
I actually tried making compatibility binds, but the C# build failed because it defined the same method twice: https://github.com/PizzaLovers007/godot/actions/runs/14546387351/job/40812407480 I'm less familiar with GDExtension API generation, but based on the exception added in #94243 (int32_t to int64_t), I would guess it generates |
|
That's strange, but since it generates the error of changing the return type it should be needed, but problematic with the clash with C#, I'd suggest testing building godot-cpp and seeing what the templates are there |
PizzaLovers007
force-pushed
the
fix-playback-position-types
branch
from
fd9f6f0 to
f3dd43f
Compare
|
Here's the generated API with the compatibility methods bound: extension_api.json. The new method entries have a Looking at the Is there somewhere else I should be looking to see the template with the compatibility methods? |
|
The old float versions won't be there when built with this PR, you need to compare without these changes to see, the compatibility methods are used by extensions built with older versions. It's used when looking up the function pointer to use based on the hash |
|
Ok I can confirm there's a duplicate method happening on the C# generated code. Here's [DebuggerBrowsable(DebuggerBrowsableState.Never)]
private static readonly IntPtr MethodBind13 = ClassDB_get_method_with_compatibility(NativeName, MethodName.GetPlaybackPosition, 191475506ul);
/// <summary>
/// <para>Returns the position in the <see cref="Godot.AudioStream"/> of the latest sound, in seconds. Returns <c>0.0</c> if no sounds are playing.</para>
/// <para><b>Note:</b> The position is not always accurate, as the <see cref="Godot.AudioServer"/> does not mix audio every processed frame. To get more accurate results, add <see cref="Godot.AudioServer.GetTimeSinceLastMix()"/> to the returned position.</para>
/// <para><b>Note:</b> This method always returns <c>0.0</c> if the <see cref="Godot.AudioStreamPlayer.Stream"/> is an <see cref="Godot.AudioStreamInteractive"/>, since it can have multiple clips playing at once.</para>
/// </summary>
public double GetPlaybackPosition()
{
return NativeCalls.godot_icall_0_144(MethodBind13, GodotObject.GetPtr(this));
}
...
[DebuggerBrowsable(DebuggerBrowsableState.Never)]
private static readonly IntPtr MethodBind31 = ClassDB_get_method_with_compatibility(NativeName, MethodName.GetPlaybackPosition, 191475506ul);
/// <summary>
/// <para>Returns the position in the <see cref="Godot.AudioStream"/> of the latest sound, in seconds. Returns <c>0.0</c> if no sounds are playing.</para>
/// <para><b>Note:</b> The position is not always accurate, as the <see cref="Godot.AudioServer"/> does not mix audio every processed frame. To get more accurate results, add <see cref="Godot.AudioServer.GetTimeSinceLastMix()"/> to the returned position.</para>
/// <para><b>Note:</b> This method always returns <c>0.0</c> if the <see cref="Godot.AudioStreamPlayer.Stream"/> is an <see cref="Godot.AudioStreamInteractive"/>, since it can have multiple clips playing at once.</para>
/// </summary>
public double GetPlaybackPosition()
{
return NativeCalls.godot_icall_0_144(MethodBind31, GodotObject.GetPtr(this));
}I assume the 2nd one is the "compatibility" version given that it's next to the other As for godot-cpp, they are indeed generating with |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Two reasons to change this:
See this discussion: #105510 (comment)