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

stb_vorbis: Replace plain int with int64_t at offset/length related values #1329

Open
Wohlstand opened this issue May 21, 2022 · 10 comments · May be fixed by #1331
Open

stb_vorbis: Replace plain int with int64_t at offset/length related values #1329

Wohlstand opened this issue May 21, 2022 · 10 comments · May be fixed by #1331

Comments

@Wohlstand
Copy link

Is your feature request related to a problem? Please describe.
Having the plain signed 32-bit integer causes problems at OGG files with 198 kHz sample rate and over than 3 hours length (examples: several concert footages with the 5+ hours length), because signed 32-bit integer allows addressing ~3 hours of audio with 198 kHz sample rate. I highly recommend replacing it with the 64-bit integer to allow huge files to be played normally. Official Xiph Vorbis uses the int64_t to address sample offsets.

Describe the solution you'd like
Offset related values should use int64_t to extend the maximum range of sample addressing. As this library is never gets compiled as a shared library, there should no ABI breakages with such changes.

@nothings
Copy link
Owner

libsdl probably should be using libvorbis, not stb_vorbis

@Wohlstand
Copy link
Author

libsdl

Looks like you confuses, look:

  • SDL2 by itself gives the raw audio output only, it doesn't use any codecs
  • and SDL2_mixer is the audio library that works over SDL Audio, it allows to load SFX and play music, and it does use of codecs.

Why there is a talk about adding of stb_vorbis? To simplify deployment to some platforms without the necessary to build dependent libraries and minify the library size when there is no need to use all of supported codecs. There are some people from various sides suggested to add use of stb_vorbis for these purposes.

@Wohlstand
Copy link
Author

P.S. Recently I made the test, I opened the OGG file with the 5:49:43 length and 198 kHz sample rate, so:

  • It plays normally
  • Once it reaches 3:05 hours, the tell reports an invalid value and there is no way to seek far than this time value.

@nothings
Copy link
Owner

yes, stb_vorbis was not designed to handle 3-hour files at 198 KHz or 6-hour files at 96Khz or 12-hour files at 48Khz. this is a known limitation. fixing it is non-trivial, since seeking would also need to be fixed.

@Wohlstand
Copy link
Author

The change is not hard, I already made it on my side, and the thing worked fine. I could send the pull request that supplies the type change for the sample position.

@nothings
Copy link
Owner

you tested seeking to 64-bit positions?

@Wohlstand
Copy link
Author

Wohlstand commented May 23, 2022

Yes, I tested, when 64-bit values are used (at my modded version), the seek works fine, and tell() calls return a valid position. When using the current build with 32bit integers, seek will work until 4 hours, but when an overflow happens, it will seek into the wrong position. Tell returns the wrong position because of the overflow that happen earlier during to signed 32bit type.

@nothings
Copy link
Owner

then you should definitely submit a pull request

@Wohlstand
Copy link
Author

Wohlstand commented May 23, 2022

Gonna prepare it... I will post as soon as I will backport my changes into the mainstream version.

@Wohlstand
Copy link
Author

btw, @nothings, could you review and merge #1295? That will simplify my work to don't redo that commit again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants