AprilAndFriends / theoraplayer

A multi-threaded C++ library that plays video files supporting multiple codecs across platforms. Easy to use, fast, responsive, abstract interface and minimal dependencies, you'll soon be wondering how you lived without it! ;) Audio and Video interfaces are completely abstracted so the library can be used anywhere, regardless of what you use to display video frames and play audio samples (eg. OpenGL / OpenAL, Direct3D / DirectSound?, SDL / SDL_mixer, X11 / alsa ...) The library can pre-cache video frames and decoded audio samples for maximum efficiency and smooth playback, even on single-cpu systems. Currently, the library supports Theora on Windows, Mac, iOS, Linux, Android, WinRT and Windows Phone. H.264 is supported on Mac and iOS.
BSD 3-Clause "New" or "Revised" License
84 stars 43 forks source link

Demo_av Not sending last 0.1 second to soundbuffer #19

Open Celestria opened 7 years ago

Celestria commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

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 https://github.com/AprilAndFriends/theoraplayer/issues/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 commented 7 years ago

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 commented 7 years ago

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.