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

Bug Report: Chart Editor Has A Maximum Song Length of 25 Minutes and 21.742 Seconds #3070

Open
2 tasks done
Shaunyman opened this issue Jul 27, 2024 · 5 comments · May be fixed by FunkinCrew/openfl#1
Open
2 tasks done
Labels
status: pending triage The bug or PR has not been reviewed yet. type: minor bug Involves a minor bug or issue.

Comments

@Shaunyman
Copy link

Shaunyman commented Jul 27, 2024

Issue Checklist

  • I have properly named the issue
  • I have checked the issues/discussions pages to see if the issue has been previously reported

What platform are you using?

Itch.io (Downloadable Build) - Windows

If you are playing on a browser, which one are you using?

None

Version

0.4.1

Context (Provide images, videos, etc.)

The chart editor will not allow a song longer than 25 minutes and 21.742 seconds. Even one millisecond longer and the chart editor becomes very small (less than one measure) and you cannot add notes for the whole song. Attached are two photos, one of them is a song that is 25 minutes and 21.742 seconds. It looks like a normal chart would. The other is of a song that is 25 minutes and 21.743 seconds (only one millisecond longer). It does not look like a normal chart would. Also, the times on the bottom right and left of the chart are both negative and only a couple of minutes instead of the whole length.
25m 21 742s
25m 21 743s

Steps to reproduce (or crash logs, errors, etc.)

Go into chart editor. Select File -> New Chart. In "Create New" select any of the 3 options (the issue occurs regardless of which difficulty settings you choose). Click "Continue". Select a song that is longer than 25 minutes and 21.742 seconds and the chart editor will break. Select a song that is 25 minutes and 21.742 seconds or less and the chart editor will work as intended.

@Shaunyman Shaunyman added status: pending triage The bug or PR has not been reviewed yet. type: minor bug Involves a minor bug or issue. labels Jul 27, 2024
@Hundrec
Copy link
Contributor

Hundrec commented Jul 27, 2024

Very interesting bug and precise boundary!

Does changing the BPM (or any other song properties) affect this boundary?

@Shaunyman
Copy link
Author

After applying the following changes, nothing changed with the boundary: setting the BPM to 50, 200, and 150, testing all of the available stages, and changing the note style to pixel notes. Something to be noted is that I did not attempt changing the 3 character options to see if those would cause a change in the boundary. So, while unlikely, it is possible that changing any of those things could cause the boundary to increase, resulting in the ability to make a chart longer than 25 minutes and 21.742 seconds.

@NotHyper-474
Copy link
Contributor

NotHyper-474 commented Jul 28, 2024

I tried to make a song with that same length and it glitched either way. Tried exporting with a different sample rate and it worked.
I'm thinking it's an integer limit issue somewhere.

@NotHyper-474
Copy link
Contributor

NotHyper-474 commented Jul 29, 2024

After a bit of debugging I found out that the issue traces all the way around to these lines in OpenFL's sound backend
https://github.com/FunkinCrew/openfl/blob/228c1b5063911e2ad75cef6e3168ef0a4b9f9134/src/openfl/media/Sound.hx#L675-L679

And as I imagined it has to do with integer limitations. Just look at how the math goes

$$\begin{align*} \text{samples} &= \frac{268435084 \times 8}{2 \times 16} \\\ \text{samples} &= \frac{2147480672}{32} \\\ \text{samples} &= 67108771 \\\ \text{lengthMs} &= \frac{67108771}{\text{sampleRate} \ (44,100)} \times 1000 \approx 1521740.84 \ \text{ms} \end{align*}$$

2.147.480.672 is pretty close to the 32-bit integer positive limit (2.147.483.647, just 2475 of difference) but still below it, so that's why it still works.

Now I debugged the same song with a sample rate of 48,000 Hz
samples = (292174384 * 8) / 32, this is where we have a problem. 292,174,384 times 8 is 2,337,395,072, way bigger than that limit. So the integer overflows and wraps around to negative and screws everything up.
In the end we get this:

$$\begin{align*} \text{samples} &= \frac{292174384 \times 8}{32} \\\ \text{samples} &= \frac{-1957572224}{32} \\\ \text{samples} &= -61174132 \\\ \text{lengthMs} &= \frac{-61174132}{48000} \times 1000 \\\ \text{lengthMs} &\approx -1274461.08 \, \text{ms} \end{align*}$$

@Hundrec
Copy link
Contributor

Hundrec commented Jul 29, 2024

Amazing detective work, Hyper!

That pesky integer limit messes with everything!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: pending triage The bug or PR has not been reviewed yet. type: minor bug Involves a minor bug or issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants