Skip to content
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

Make audio loop import settings consistent by using samples everywhere (instead of seconds) #84

Open
davthedev opened this issue Sep 19, 2019 · 10 comments · May be fixed by godotengine/godot#87554
Labels
breaks compat Proposal will inevitably break compatibility topic:audio
Milestone

Comments

@davthedev
Copy link

Describe the project you are working on:
A game that comprises music and sound in both OGG and WAV formats. Both have looping sections.

Describe how this feature / enhancement will help your project:
Less guesswork of the units used when adjusting audio loops.

In the current situation, there are two different styles for setting audio loops on files. It depends on the format : WAV or OGG. The units used are not consistent.

  • For an OGG file, the loop offset is set during import. The unit seems to correspond to seconds.

  • For a WAV file, the loop offset can be set as both start and end points. The units seems to be samples count i.e. 44100 per one second of audio if it is a 44100hz recording.

In both cases, there is no mention of the unit used next to the input field.

For consistency all over the engine, I suggest choosing a consistent unit for all audio resources in Godot and sticking to it. For instance, define that time units in seconds instead of samples should be the standard and convert the WAV loop settings to it.

Show a mock up screenshots/video or a flow diagram explaining how your proposal will work:

Mentioning the unit used in front of the input field. For instance, a little "sec" or "s" for seconds.

Describe implementation detail for your proposal (in code), if possible:

If this enhancement will not be used often, can it be worked around with a few lines of script?:

No, as it is linked to audio resources settings

Is there a reason why this should be core and not an add-on in the asset library?:

A significant user experience improvement for all audio import / fine-tuning operations. Useful for all games that use audio.

@Calinou Calinou changed the title Consistent & explicit units for audio loop settings Make audio loop import settings consistent by using samples everywhere (instead of seconds) Aug 17, 2020
@Calinou
Copy link
Member

Calinou commented Aug 17, 2020

We should use samples instead of seconds as these are a more precise measurement, with no risk of floating-point precision errors or similar.

@tragicmanner
Copy link

This is further compounded by this issue as I understand it: godotengine/godot#18251

I was talking with someone in the godot discord and this issue was brought up that their loop points were not accurate enough due to being limited to three decimal places. Sample count would help with this as well.

@Calinou Calinou added this to the 4.0 milestone Apr 20, 2022
@Calinou Calinou added the breaks compat Proposal will inevitably break compatibility label May 10, 2022
@aaronfranke aaronfranke modified the milestones: 4.0, 5.0 Feb 24, 2023
@BlueSpeedsterYT
Copy link

I feel like if we will make things consistent in regards to audio, I feel like it would also be best if the loop points system of the WAV import could also be brought over to OGG files, as it could be helpful in regards to storage space.
Is it related to this proposal? No not really.
Will it help a ton of developers? Absolutely.

@Calinou
Copy link
Member

Calinou commented Apr 13, 2023

Looping was redesigned in 4.0 to be BPM-aware for OGG/MP3 files: godotengine/godot#63265

This doesn't apply to WAV files which are generally not used for music due to their file size. It is technically possible to port these changes to WAV, but WAV is also a format that can store full loop point metadata unlike OGG/MP3. You may want to place loop points using your audio production software instead, so that it works across different software.

@lyuma
Copy link

lyuma commented Jan 25, 2024

I would like to note two extremely well written replies to a few questions I posed in the PR here:

godotengine/godot#87554 (comment)
godotengine/godot#87554 (comment)

The comments provide very strong justification for why loop points should be in samples, even in the case of using BPM.

It's unclear to me why the current behavior is so codec-dependent (OGG and WAV ideally would behave the same, IMHO).

Anyway, just wanted to drop this note and point out the active discussion on the PR.

@davthedev
Copy link
Author

I think we should not force the use case based on the file type.
Looped WAV audios make the basis of most sampler synths. Principle that can be extended to make short looped sound effects.
Remember the old MOD/S3M/XM/IT music files? They contain a midi-like score associated with the samples used to render it. There is an optional looping section often used for brass / pad / lead / wind instruments and similar. The loop points are edited inside the music editor directly instead of the wav recording software. Examples: Modplug Tracker, MilkyTracker, Fasttracker II...

Best of both worlds would be the ability to read the embedded loop points of the WAV file and import those values, while enabling editing in Godot directly without modification to the original file.

An identical sample can also be referenced twice with two different sets of loop and start points as well in modern music software. Think about audio sprites, a full recording of a drum kit in which you isolate each individual drum sample for example. But this is a bit off the scope of this proposal; the comment is about the pertinence of letting loop points be edited in Godot in all cases.

@GreatNovaDragon
Copy link

With a breaking change that is introduced in Godot 4.3 (Reverse-Z), shouldn't it now be re-considered, if this cant be brought into a non 5.x version but an 4.x version?

@Calinou
Copy link
Member

Calinou commented Apr 30, 2024

With a breaking change that is introduced in Godot 4.3 (Reverse-Z), shouldn't it now be re-considered, if this cant be brought into a non 5.x version but an 4.x version?

Just because we introduced one breaking change in a minor release doesn't mean we can start introducing all the breaking changes in another minor release 🙂

@GreatNovaDragon
Copy link

Per SemVer, 4.3 IS actually 5.0, so I dont actually get why Reverse-Z was brought over but not others that are delayed for, no reason but the perceived need, where Major Version changes need major changes. (Which is probably why so many software changes to build numbers instead of SemVer, or fully disregards the minor version, and have the next release just be a higher number)

@clayjohn
Copy link
Member

Per SemVer, 4.3 IS actually 5.0, so I dont actually get why Reverse-Z was brought over but not others that are delayed for, no reason but the perceived need, where Major Version changes need major changes. (Which is probably why so many software changes to build numbers instead of SemVer, or fully disregards the minor version, and have the next release just be a higher number)

To add to what Calinou says above. We roughly follow SemVar, but we also allow ourselves to break compatibility in limited ways when doing so is extremely beneficial. The reverse-z changes are exceptional in that the compatibility breakage is extremely limited and will only impact a few shaders that are relying on very advanced features (in which case we expect that the shader author will have no problem adjusting the shader), and the benefit of reverse-z is very widespread (it affects all 3D users) and makes a big difference.

We have to evaluate each breaking change on its merits and balance the pros and cons. Notably, this proposal hasn't been rejected or anything. Its just flagged as "breaks compat" as it should be so we can properly take that fact into consideration when deciding if this is a change we would like to make.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks compat Proposal will inevitably break compatibility topic:audio
Projects
None yet
8 participants