search

Rantanen / node-mumble

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

Fixes #56. Do not skip voice packages if user is not talking. #80

Closed Prior99 closed 8 years ago

Prior99 commented 8 years ago

We cannot skip incoming packages when the user is not currently talking. Otherwise the first packet of each stream will get lost.

Rantanen commented 8 years ago

I'm still a bit unsure of this change.

If it's the only way to fix the issue, I'll be fine merging it (or something like it) - but I'm mainly worried about the needless jitterbuffer processing. Also with this change the "missedFrames" value will increase needlessly while the users aren't talking.

Could you try whether replacing the continue; with user.buffer.tick(); continue; would fix the issue? This way the jitter buffer would know that time is passing at least.

Rantanen commented 8 years ago

With the proposed change, do you know if the packets.push is being called repeatedly even if voiceActive == false? Or does the jitterbuffer return something that takes the execution to the else-block which doesn't append any frames to the packets array.

Prior99 commented 8 years ago

frame.data is undefined in that case and the else-branch is used, so the packets.push is not called repeatedly.

if( !user.voiceActive ) {
    user.buffer.tick();
    continue;
}

does not solve the problem either.

I am not sure whether there are any other implications or problems with this fix.

Rantanen commented 8 years ago

Okay. Will go with this one then!