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

Potentially incorrect source_get_offset() when sound buffers vary in length #18

Open
ivan-mogilko opened this issue Jul 29, 2022 · 4 comments

Comments

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Jul 29, 2022

In source_get_offset() , when the source's type is AL_STREAMING, the mojoAL is summing up all the queued and processed buffers lengths to the current buffer's offset (as per OpenAL's specs):

mojoAL/mojoal.c

Lines 4205 to 4213 in 102126d

if (src->type == AL_STREAMING) {
/* streaming: the offset counts from the first processed buffer in the queue. */
BufferQueueItem *item = src->buffer_queue.head;
if (item) {
framesize = (int) (item->buffer->channels * sizeof (float));
freq = (int) (item->buffer->frequency);
int proc_buf = SDL_AtomicGet(&src->buffer_queue_processed.num_items);
offset = (proc_buf * item->buffer->len + src->offset);
}

As may be seen from the code, this is done simply by multiplying the current buffer's len by the number of processed buffers (buffer_queue_processed.num_items); this assumes that all the buffers have the same length.

This however may not be the case. For instance, in the practical case I had, I've been looping a sound file, and the ending piece of much smaller size could be followed by a larger buffer with data from another beginning. In such case the reported offset was larger than expected, as the already processed smaller piece was assumed to be as large as the currently playing one.

Overall I have not found any mention that the buffers must all be of same size when queued into the source, neither in documentation nor the code. Could be I'm missing a rule here? One comment in mojoAL even states:
you can legally queue or set a NULL buffer.
assuming it also allows for the 0 length buffer to be queued.

I suppose, that if if having varied buffers is legal, then fixing this may be a matter of tracking a processed length, which is added to when another buffer is processed, and subtracted from when a processed buffer is unqueued.

@adahlkvist-feral
Copy link

Queueing multiple buffers of varying length is not allowed by the openAL spec.

https://www.openal.org/documentation/openal-1.1-specification.pdf

"All buffers in a queue must have the same format and attributes, with the exception of the
NULL buffer (i.e., 0) which can always be queued. An attempt to mix formats or other
buffer attributes will result in a failure and an AL_INVALID_VALUE error will be
thrown. If the queue operation fails, the source queue will remain unchanged (even if
some of the buffers could have been queued)"

The size of a buffer counts as an attribute.

@icculus
Copy link
Owner

icculus commented Apr 12, 2023

I think every AL implementation has allowed different-sized buffers to be queued, fwiw.

@ivan-mogilko
Copy link
Contributor Author

From my perspective, it's essential to know what to expect from the library. If the library demands buffers to be same size all the time, then I will adjust my code. If not, then I might keep it and wait for the issue to be fixed; perhaps will even have a opportunity to make a pr for this.

@adahlkvist-feral
Copy link

If you want to maintain portability to other openAL implementations, I think it would be best to follow the spec.

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

No branches or pull requests

3 participants