stb_vorbis: Add I/O callback support

rygorous commented 7 years ago

This has been requested.

nothings commented 7 years ago

See https://github.com/nothings/stb/pull/456

rygorous commented 7 years ago

I added this as a new issue because while the feature request is definitely reasonable, I feel like that particular implementation isn't up to snuff. (See my comments on the pull request.)

ghost commented 4 years ago

I just ran into this because it appears this is essential for fully featured streaming from custom locations - because yes, there is pushdata, but that one seems useless for users who need sample-exact seeking since it only seems to support approximates. (Or is there a way to do exact seeks with pushdata which I missed?) So that makes pushdata unusable for use cases where such a guess isn't good enough, and stb_vorbis_decode_memory doesn't support streaming.

Therefore, it seems like custom callbacks are required for feature completion here, and I can report I had a great experience using them with the dr-mp3 & dr-flac libs while the pushdata API just looks weird to me. Is there any advantage to it at all over custom callbacks? Maybe it could even be considered (after intense discussion of course!!) for a potential removal later.

ghost commented 4 years ago

Here a few problems custom callbacks (and possibly abandoning pushdata) would solve:

pushdata requires me to guess input. If I assume it could need any arbitrary size up to the entire file (and there are no documented guaranteed bounds, so if I write defensive code this assumption seems unavoidable) I need to basically make a loop to guess. With custom I/O callbacks, stb_vorbis will tell me what it needs at any time, greatly simplifying this code on my side.
pushdata only uses arbitrary amounts of actually provided input with no caching. This means it is a bad match for being mapped to file-like APIs which is what almost everyone else provides, because I constantly need to seek backwards again to make up for this possibility. With custom I/O callbacks, stb_vorbis could do seeking if necessary and I wouldn't need to bother with this, like I don't need to with all other libraries (dr-flac/dr-mp3/...) out there. It would make usage from actual fully loaded memory areas slightly less convenient, but then again isn't there stb_vorbis_open_memory for that?
pushdata requires me to cache unneeded output. All APIs written by anyone else I've seen (including my own) allow requesting n-or-less samples, pushdata apparently doesn't: any whatever amount falls out and if I wanted less I need to cache it. Since my own code gets called indirectly for exactly n samples, I have no choice but to do this. With custom I/O I could use a standardized give-me-n-samples API as provided by everyone else, and all this extra caching logic on my side would also be unnecessary.
pushdata duplicates parts of the API, and therefore either doesn't offer things or has separate functions where the same would do. This applies e.g. to requesting samples in int format, which simply doesn't seem to be possible with pushdata.

On the flip side, I haven't found a reason yet why pushdata is better or solves anything custom callbacks don't. But I'd be curious to know if there is anything.

nothings commented 4 years ago

pushdata exists because the company that paid for me to write stb_vorbis wanted that api. so there is no value in critiquing it.

ghost commented 4 years ago

Oh, interesting! Fair enough, I hope it's still useful info for why custom callbacks might be a better match for many and possibly worth adding.

(Also some time in the future, if you wanted to reduce the maintenance effort maybe the company could be asked if they want to switch their usage to custom callbacks, too? Feel free to link anyone curious to my comment above)

nothings / stb

stb_vorbis: Add I/O callback support #478