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

Demo_av Not sending last 0.1 second to soundbuffer #19

Open
Celestria opened this issue Jan 18, 2017 · 8 comments
Open

Demo_av Not sending last 0.1 second to soundbuffer #19

Celestria opened this issue Jan 18, 2017 · 8 comments

Comments

@Celestria
Copy link

See the following snippet of code:

void OpenAL_AudioInterface::insertData(float* data, int samplesCount)
...
for (int i = 0; i < samplesCount; ++i)
{
if (this->buffSize < this->maxBuffSize)
{
this->tempBuffer[this->buffSize] = float2short(data[i]);
++this->buffSize;
}
if (this->buffSize == this->frequency * this->channelsCount / 10)
{
...
this->bufferQueue.push(buff);

So insertData is called whenever theoraplayer thinks it should queue some sound.
The sound is locally stored in tempBuffer.
When tempbuffer is full enough (0,1 sec) "if (this->buffSize == this->frequency * this->channelsCount / 10)" An openAL buffer is created and the tempbuffer is "emptied".
If the decoder has reached the end and insertData is called for the last time it is possible that there is some sound in tempBuffer but not enough so that it is added to the bufferQueue. So up to almost 0.1 seconds of sound are missing from the playback.

I understand that this seems like a small issue because usually at the end of a movie the sound is muted anyway. But one of the first things I wanted to try is to see if I could get a movie (that has a looping sound from beginning to end) to autorestart and see if this was possible without noticing any audio/video glitches.

@borisblizzard
Copy link
Collaborator

@kspes It might be a good idea to quasi-port our code from XAL to fix this reliably rather than hunting down bugs in this implementation.

@kspes
Copy link
Collaborator

kspes commented Jan 21, 2017

@borisblizzard the OpenAL implementation is there as an Example reference. One of the main ideas behind theoraplayer is that it's simple and has minimal dependencies. So I'd rather not have to introduce another complex dependency to it.

OpenAL audio interface being an example must be as simple and readable as possible, in order not to scare people away :)

@Celestria So, you think the problem might be in the small buffer size? Maybe we should use 1-2 second buffers.
Sorry I can't seem to find free time currently to investigate this, work deadlines usually cripple my open source activities :)

@Celestria
Copy link
Author

@kspes no it is not the small queue buffer size. It is the fact that the leftover sound in tempBuffer is not added to the ALaudio when the movie reaches its end.

@blloyd75
Copy link
Contributor

@kspes, this is actually a failure of the API. The only function called is insertData(). At some point there is an end of file reached, but there is no notification of that to the sound driver. If insertData efficiently batches data, there is no indicator to it that it should send any remaining data.

When EOF is reached, either an extra call of insertData() with no data should be made, or else a function to indicate EOF is reached needs called. Right now, there is no indicator to the consumer that all data has arrived.

Probably the most elegant fix would be to add a new function virtual void streamComplete(), to the AudioInterface. It can have an empty default implementation. But it would give a method for flushing a partial buffer when the stream ends.

The sample could be extended to show how to use this function, and the sample would still be very simple.

Since I'm not sure how easy it would be to get that function called when end of stream happens, the other option would be to adjust the sample so that it always flushes whatever it has in the buffer at the end of the call to OpenAL. That way there is never a partial buffer waiting on future calls to insertData to get written out. Since at end of stream that next call never happens.

@kspes
Copy link
Collaborator

kspes commented Feb 1, 2017

@blloyd75 you're right, we'll add onAudioStreamComplete() in the audio interface. will keep this isssue open until I find the time to do it.

@Celestria
Copy link
Author

There is another issue with using eof. The way autorestart (true) is implemented eof will never occur. So take caution when taking this route. It should work fine aslong as the decoder can keep up the frame buffer and audio buffer near the end of the file and has already started decoding from the start again.
See #18

Since I'm using directsound which does not have buffers like openal I had to do it differently anyway. I just chose to buffer everything coming from insertdata myself and then feed my audio buffer in the update function.

@kspes
Copy link
Collaborator

kspes commented Feb 1, 2017

ok, good points, will see what to do. but in general, it's just a notification that you can easily ignore.
Maybe on looped videos it shouldn't trigger though, will see when I dive into it.

@blloyd75
Copy link
Contributor

blloyd75 commented Feb 2, 2017

For looping video, you wouldn't want eof to fire. It's to finish flushing the buffer when no more data is coming. The end goal for looping is for there to be no apparent seam from end to beginning, so there is no logical EOF. So for a looping video, when problems there are fixed the audio doesn't see the transition from ending to beginning, it just sees a continual uninterrupted audio stream.

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

4 participants