Rantanen / node-mumble

Mumble client in Node.js
MIT License
155 stars 48 forks source link

There is a racecondition where the first voicepacket is lost #56

Closed Prior99 closed 8 years ago

Prior99 commented 9 years ago

When a user starts talking it can occure that the first voice packet gets lost. This does not happen when

if( !user.voiceActive ) {
    continue;
}

in MumbleConnection.prototype._dequeueFrames is removed. I guess this is a racecondition happening when the udp packets come in to fast and overwrite the user.buffer before it was read.

Prior99 commented 9 years ago

I had this running since a day now without the 3 lines and I did not notice any negative impact. I do not understand enough from the code to understand the impacts.

Wouldn't the next both ifs just be skipped and the missed frames be increased?

Maybe you should have a look at this when you have the time, @Rantanen, you understand way more from the code than me.

Rantanen commented 9 years ago

I'm not following what's the issue. The voiceActive vs jitter buffer contents shouldn't have a race condition looking at the code.

Can you describe a test case how I could reproduce this? I wrote some mocha tests (and integrated these with travis-ci) but none of them seemed to reproduce this.

(Also yay we got base for regression tests now \o/)

Prior99 commented 9 years ago

Reproduce it like this: Record some audio with the bot into a pcm-file. Connect the bot and yourself into a room and say something. Stop the bot and import the audiofile in audacity or any similiar program. You will notice, that the first part of the audio is missing.

If you say for example "Cheesecake" you will only hear "secake". Everyone else connected to the server can hear you fine.

If you remove those three lines, everything works fine.

I am not exactly sure whether it's a race-condition or not, or what is causing this issue, but I can definitly experience and reproduce it on 3 machines.

Prior99 commented 9 years ago

I will see if I can get a minimal example to reproduce this.