search

icculus / mojoAL

An SDL2-based implementation of OpenAL in a single C file.
https://icculus.org/mojoAL/
zlib License
154 stars 20 forks source link

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

Open ivan-mogilko opened 2 years ago

ivan-mogilko commented 2 years ago

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):

https://github.com/icculus/mojoAL/blob/102126d3ed3a979728c4dde170bf242ad9d9daa6/mojoal.c#L4205-L4213

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 commented 1 year ago

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 commented 1 year ago

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

ivan-mogilko commented 1 year ago

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 commented 1 year ago

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