Rebase and update stb_vorbis.h to SDL_sound's version : #395

sezero · 2022-05-21T11:05:12Z

Keeps the original stb_vorbis as intact as possible so
that future updates from mainstream would be painless.
Reduces STB_VORBIS_MAX_CHANNELS from 16 to 6 (objections?)
Adds several missing libm function overrides,
SDL_mixer now requires SDL >= 2.0.9 because of SDL_exp()
Fixes slow loads and leaks in start_decoder:
stb_vorbis: fix slow loads and leaks in start_decoder. nothings/stb#1174
Fixes submap array out-of-bounds indexing bug:
stb_vorbis: extend Mapping submap_floor and submap_residue. nothings/stb#1312
Replace signed overflow clamps with unsigned overflow:
stb_vorbis: Undefined behavior found by libFuzzer/UBSan. nothings/stb#1168
Replaces alloca() usage with setup_malloc() (from libxmp.)
Fixes '-Wmaybe-uninitialized' warnings in get_seek_page_info:
stb_vorbis: fix -Wmaybe-uninitialized warnings in get_seek_page_info. nothings/stb#1172
A minor UBSan fix and suppression:
stb_vorbis: Undefined behavior found by libFuzzer/UBSan. nothings/stb#1168
Fixes signature of dummy realloc() for STB_VORBIS_NO_CRT:
Fix signature of dummy realloc() for STB_VORBIS_NO_CRT nothings/stb#1198
Renames BUFFER_SIZE macro to STB_BUFFER_SIZE:
rename BUFFER_SIZE macro to STB_BUFFER_SIZE nothings/stb#1078
Pulls in sample-accurate offset patch of Vitaly Novichkov:
(stb_vorbis_get_playback_sample_offset, because it's used
in OGG_Tell and OGG_GetSome):
Improve the stb_vorbis_get_sample_offset() accuracy nothings/stb#1294
stb_vorbis: report the sample-accurate offset nothings/stb#1295
Fixes a few warnings here and there in some environments.
Replaces two dummy '(void) 0' with 'do {} while(0)'

- Keeps the original stb_vorbis as intact as possible so that future updates from mainstream would be painless. - Reduces STB_VORBIS_MAX_CHANNELS from 16 to 6 (objections?) - Adds several missing libm function overrides, - SDL_mixer now requires SDL >= 2.0.9 because of SDL_exp() - Fixes slow loads and leaks in start_decoder: nothings/stb#1174 - Fixes submap array out-of-bounds indexing bug: nothings/stb#1312 - Replace signed overflow clamps with unsigned overflow: nothings/stb#1168 - Replaces alloca() usage with setup_malloc() (from libxmp.) - Fixes '-Wmaybe-uninitialized' warnings in get_seek_page_info: nothings/stb#1172 - A minor UBSan fix and suppression: nothings/stb#1168 - Fixes signature of dummy realloc() for STB_VORBIS_NO_CRT: nothings/stb#1198 - Renames BUFFER_SIZE macro to STB_BUFFER_SIZE: nothings/stb#1078 - Pulls in sample-accurate offset patch of Vitaly Novichkov: (stb_vorbis_get_playback_sample_offset, because it's used in OGG_Tell and OGG_GetSome): nothings/stb#1294 nothings/stb#1295 - Fixes a few warnings here and there in some environments. - Replaces two dummy '(void) 0' with 'do {} while(0)'

Fixes #329

sezero · 2022-05-21T12:53:02Z

One noticeable difference is this changes stb_vorbis_get_sample_offset
back to original plain int, whereas @Wohlstand's version has it as an
Sint64. Also, stb_vorbis_get_playback_sample_offset and its internal
friends are now plain int (just like they are in his PR in mainstream)
instead of Sint64 again.

I don't know the reason Vitaly's Sint64 change: If there is a compelling
reason for it, I can do that change with proper documentation + SDL_mixer
ifdefs.

Wohlstand · 2022-05-21T13:00:39Z

That was my new feature, it's VERY important as original call adds the bug at looping, it requires a sample accurate seek/tell.

I changed to Sint64 to match the 64bit off_t and value at original vorbis.

sezero · 2022-05-21T14:04:57Z

it's VERY important as original call adds the bug at looping,

I don't know what that means??

I changed to Sint64 to match the 64bit off_t and value at original vorbis.

Well, stb_vorbis mainstream doesn't follow that.

@slouken what do you think?

slouken · 2022-05-21T16:36:28Z

I can't imagine having a music file that's larger than 2 billion samples. int should be fine for the range that we're working with. @Wohlstand, do you have a repro for the original bug you were fixing with that change?

Other than that outstanding question, I'm fine with merging this so our stb_vorbis.h is in line with SDL_sound.

sezero · 2022-05-21T16:38:52Z

Other than that outstanding question, I'm fine with merging this so our stb_vorbis.h is in line with SDL_sound.

OK, waiting for @Wohlstand's answer before merging.

Wohlstand · 2022-05-21T16:51:25Z

Also, forgot to tell: plain 32bit signed integer is able to address ~3 hours of 198000 kHz audio (there are some audio footages as an example, they may contain a lot of hours of audio, I do have some files with 5 hours of cocert recording sent by my friend a while ago, and I worry they may easily get overflow and fail).Отправлено с устройства Galaxy -------- Исходное сообщение --------От: Ozkan Sezer ***@***.***> Дата: 21.05.2022 17:05 (GMT+03:00) Кому: libsdl-org/SDL_mixer ***@***.***> Копия: Vitaly Novichkov ***@***.***>, Mention ***@***.***> Тема: Re: [libsdl-org/SDL_mixer] Rebase and update stb_vorbis.h to SDL_sound's version : (PR #395) it's VERY important as original call adds the bug at looping, I don't know what that means?? I changed to Sint64 to match the 64bit off_t and value at original vorbis. Well, stb_vorbis mainstream doesn't follow that. @slouken what do you think? —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

Wohlstand · 2022-05-21T16:54:31Z

do you have a repro for the original bug you were fixing with that change?

One of my users sent me some OGG files that got a click on loop passing because of that bug I explained (that caused me to implement a new call for the sample-accurate seek).

slouken · 2022-05-21T16:56:19Z

Do you still have the looping Ogg file for testing?
How large are the files of concert recordings from your friend?

sezero · 2022-05-21T16:58:58Z

One of my users sent me some OGG files that got a click on loop passing because of that bug I explained (that caused me to implement a new call).

Is that bug related to the Sint64 change, or is it only about implementing the new stb_vorbis_get_playback_sample_offset?

Wohlstand · 2022-05-21T17:16:47Z

Do you still have the looping Ogg file for testing?
How large are the files of concert recordings from your friend?

Looped files:
Lemme send some once I get my computer
concert recordings:
I still have some at my archive (if I didn't removed them), lemme check them out...

Wohlstand · 2022-05-21T17:18:37Z

One of my users sent me some OGG files that got a click on loop passing because of that bug I explained (that caused me to implement a new call).

Is that bug related to the Sint64 change, or is it only about implementing the new stb_vorbis_get_playback_sample_offset?

There are different and independent changes that fixing different cases:

Sint64 fixes playing of 3+ hours 198 KHz files
stb_vorbis_get_playback_sample_offset() fixed the tell accuracy requires for looping music.

sezero · 2022-05-21T17:21:58Z

One of my users sent me some OGG files that got a click on loop passing because of that bug I explained (that caused me to implement a new call).

Is that bug related to the Sint64 change, or is it only about implementing the new stb_vorbis_get_playback_sample_offset?

There are different and independent changes that fixing different cases:
* Sint64 fixes playing of 3+ hours 198 KHz files

* stb_vorbis_get_playback_sample_offset() fixed the tell accuracy requires for looping music.

I wasn't opposing to the new stb_vorbis_get_playback_sample_offset at all:
I included it already as you can see. (And as I understand it, the tell inaccuracy
was with using the original stb_vorbis_get_sample_offset, correct?)

As for Sint64 return: @slouken, what do you say?

slouken · 2022-05-21T17:24:59Z

I'm not opposed to updating that to 64-bit. I think maybe make a PR with use case against upstream and see if the original author thinks it's a good idea?

sezero · 2022-05-21T17:28:02Z

Yes. Merging this now.

Vitaly: Can you create a bug entry here and get the idea of Sean Barrett about Sint64?

Wohlstand · 2022-05-21T17:35:43Z

For the loop bug:
This file was sent to me by a user who reported the problem: Shipwreck_Shore_Underwater.ogg.zip
without my accurate "tell" call, it will always click on loop edge.

Vitaly: Can you create a bug entry here and get the idea of Sean Barrett about Sint64?

Sure, I still seek at my archive for mentioned concert recordings...

sezero · 2022-05-21T17:46:53Z

Ok, here are the ones changed from Sint64 to what it originally was,
for future reference:

stb_vorbis_get_sample_offset, stb_vorbis_get_playback_sample_offset:
they return int instead of Sint64.
stb_vorbis_stream_length_in_samples: returns unsigned int instead
of Sint64.
stb_vorbis_seek: takes unsigned int as sample_number parameter
instead of Sint64.
struct stb_vorbis :: total_samples: uint instead of Sint64.
struct stb_vorbis :: current_playback_loc (from Vitaly's patch):
int32 instead of Sint64. (Do note however, Vitaly didn't change
the current_loc member and kept it as uint32.)

Wohlstand · 2022-05-21T17:54:30Z

P.S. the huge problem is using C++ single-line comments at original stb_vorbis, which caused incompatibility with C90 compilers, that is why I replaced all comments with /* */.

sezero · 2022-05-21T17:56:37Z

P.S. the huge problem is using C++ single-line comments at original stb_vorbis, which caused incompatibility with C90 compilers, that is why I replaced all comments with /* */.

Maybe, but that causes difficulties when we want to apply mainstream changes.
Avoid using -ansi or similar.

Wohlstand · 2022-05-21T17:56:43Z

P.P.S. About the length of these concert footages, they have 10 hours each.

Wohlstand · 2022-05-21T18:01:32Z

Maybe, but that causes difficulties when we want to apply mainstream changes.

BTW, speaking in general, if use single-line comments in Pure-C, it's fine in the scope of project code, but speaking about public headers, they must follow the C90 standard to guarantee any project will be able to use it. Anyway, for this case, I'll post the issue on the mainstream side. (made nothings/stb#1328)

Wohlstand · 2022-05-21T18:16:18Z

About the Sint64, I made the issue: nothings/stb#1329

sezero · 2022-05-21T18:19:53Z

About the Sint64, I made the issue: nothings/stb#1329

OK, I'll watching that ticket.

sezero referenced this pull request May 21, 2022

Added support for playing Ogg files using stb_vorbis

2b495a2

Fixes #329

music_ogg_stb.c: further build fixes.

53a9110

sezero requested a review from slouken May 21, 2022 12:40

sezero merged commit 1be13e4 into libsdl-org:main May 21, 2022

sezero deleted the stbv-1 branch May 21, 2022 17:28

sezero mentioned this pull request May 21, 2022

change stb_vorbis.h to use 64 bit sample numbers and offsets? #396

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rebase and update stb_vorbis.h to SDL_sound's version : #395

Rebase and update stb_vorbis.h to SDL_sound's version : #395

sezero commented May 21, 2022

sezero commented May 21, 2022

Wohlstand commented May 21, 2022 •

edited

Loading

sezero commented May 21, 2022

slouken commented May 21, 2022

sezero commented May 21, 2022

Wohlstand commented May 21, 2022 via email

Wohlstand commented May 21, 2022 •

edited

Loading

slouken commented May 21, 2022

sezero commented May 21, 2022

Wohlstand commented May 21, 2022

Wohlstand commented May 21, 2022

sezero commented May 21, 2022

slouken commented May 21, 2022

sezero commented May 21, 2022

Wohlstand commented May 21, 2022

sezero commented May 21, 2022

Wohlstand commented May 21, 2022

sezero commented May 21, 2022

Wohlstand commented May 21, 2022

Wohlstand commented May 21, 2022 •

edited

Loading

Wohlstand commented May 21, 2022

sezero commented May 21, 2022

Rebase and update stb_vorbis.h to SDL_sound's version : #395

Rebase and update stb_vorbis.h to SDL_sound's version : #395

Conversation

sezero commented May 21, 2022

sezero commented May 21, 2022

Wohlstand commented May 21, 2022 • edited Loading

sezero commented May 21, 2022

slouken commented May 21, 2022

sezero commented May 21, 2022

Wohlstand commented May 21, 2022 via email

Wohlstand commented May 21, 2022 • edited Loading

slouken commented May 21, 2022

sezero commented May 21, 2022

Wohlstand commented May 21, 2022

Wohlstand commented May 21, 2022

sezero commented May 21, 2022

slouken commented May 21, 2022

sezero commented May 21, 2022

Wohlstand commented May 21, 2022

sezero commented May 21, 2022

Wohlstand commented May 21, 2022

sezero commented May 21, 2022

Wohlstand commented May 21, 2022

Wohlstand commented May 21, 2022 • edited Loading

Wohlstand commented May 21, 2022

sezero commented May 21, 2022

Wohlstand commented May 21, 2022 •

edited

Loading

Wohlstand commented May 21, 2022 •

edited

Loading

Wohlstand commented May 21, 2022 •

edited

Loading