stb icon indicating copy to clipboard operation
stb copied to clipboard

Improve the stb_vorbis_get_sample_offset() accuracy

Open Wohlstand opened this issue 2 years ago • 5 comments

Is your feature request related to a problem? Please describe. Current "stb_vorbis_get_sample_offset()" returns the sample position at the frame edge. This is not enough for operations that require the sample-accurate position knowing. For example, having to play looped OGG Vorbis files (which contains Vorbis-comments-based loop tags).

Describe the solution you'd like At my side, I added the separated location variable that takes the position update from stb_vorbis_get_samples_*() and stb_vorbis_seek*() calls. Unlike the current_loc that gets updated after decoding, the current_playback_loc gets updated always you request any sample chunks, so, it shows the sample-accurate position. It's the sample as Xiph's Vorbis returns.

Describe alternatives you've considered In theory, it's possible to don't add the second variable: do return the sum of current_loc and f->channel_buffer_start to tell the sample-accurate position, however, I didn't test this behavior yet and I am not sure will it work properly or not.

Additional context The sample-accurate tell return is required for various cases like loop tags support implementations, and possibly, the proper SF3 support at FluidLite (currently, there is a bug against STB-decoded chunks).

P.S. At me, I do use the modified version where I had to:

  • Replace all custom-made macros with SDL2 side things
  • Added SDL RWops usage as an alternative API (as there was no ability to define the custom I/O overlay yet)

Wohlstand avatar Feb 18 '22 12:02 Wohlstand

Okay, I tested the sum of current_loc and channel_buffer_start, that doesn't help, and the separated loc variable is the only current solution I do know.

So, the little question: should I replace the current stb_vorbis_get_sample_offset() with an updated method, or I need to give the separated stb_vorbis_get_playback_sample_offset() as I did on my end?

I gonna compose the pull-request, and before I should know, how I could make this better

Wohlstand avatar Feb 18 '22 16:02 Wohlstand

Making it a separate stb_vorbis_get_playback_sample_offset() function seems better, mainly so it doesn't change the existing behavior in case someone relies on that. Thanks!

nothings avatar Feb 18 '22 18:02 nothings

Btw, the last question, is this description fine? (including grammar/spelling?)

/*  this function returns the count of returned samples from the beginning of the */
/*  file. Functions "stb_vorbis_get_samples_*", "stb_vorbis_seek_*()" will
 *  affect the returned value. Use this call to get the accurate sample position
 *  during playback. */

I want to have it proofreaded before I will sent PR

Wohlstand avatar Feb 18 '22 18:02 Wohlstand

// btw: At me, I had replaced all single-line comments with classic /* */ because they breaks C90 compatibility which is required for some compilers

Wohlstand avatar Feb 18 '22 18:02 Wohlstand

looks fine, other than the comment delimiters, yeah

nothings avatar Feb 18 '22 19:02 nothings