Placeholder-Software / Dissonance

Unity Voice Chat Asset
70 stars 5 forks source link

[Bug] Dissonance failed in playback incoming voice data after unmuted #121

Closed dmarashi closed 5 years ago

dmarashi commented 5 years ago

Context

If a user muted first, tried to speak under muted twice, and then un-muted, the other players cannot hear that user's speaking even though s/he's speaking after un-muted.

Expected Behavior

All the other players should hear the other not-muted players' talking.

Actual Behavior

The other players cannot hear the just-unmuted players' talking. The received voice packet would be considered as for an early speaking session and hence got discarded. (IsPacketFromPreviousSession(_currentChannelSession, metadata.ChannelSession) from line 103 of PeerVoiceReceiver.cs would return true)

Fix

It's caused by if the user activates a new speaking session, the session id would be increase by 1 (and cyclically re-used from 0 to 3) no matter it's muted or not. However, if the user is under muted, no voice data would be sent to other users, and hence there's a room for unsynced speaking session id between the muted user and the others.

Change line 284 in VoiceBroadcastTrigger.cs as

var next = ShouldActivate(!Comms.IsMuted && IsUserActivated());

should fix the problem

Steps to Reproduce

  1. Import Dissonance plugin and integration for UNet_LLAPI. Set up the 'TeamChat' input axis and build the LLAPI Demo scene.
  2. Run the standalone build and connect to Unity Editor (can be connected via local host server)
  3. Mute one side
  4. In the muted side, press the 'TeamChat' button once to create a 'speaking' session, than press the 'TeamChat' again to create another 'speaking' session.
  5. Un-mute, and then try press the 'TeamChat' button to create a 'speaking' session. Note that the receiver side will receive the voice data but the data won't be played (got discarded).

Your Environment

martindevans commented 5 years ago

Wow, thankyou for the comprehensive bug report! I'm impressed that you tracked it right down into the core of the networking system.

I think the fix you've worked out should fix the problem if you're using broadcast triggers. However it's possible to open and close channels from code without using a trigger at all, so I'll have to work out a fix at that lower level.

martindevans commented 5 years ago

I've managed to reproduce this and I've put a fix together for it. If you'd like to try it out email me (martin@placeholder-software.co.uk) your invoice number and I'll send you back a build :)

martindevans commented 5 years ago

I just went through a code review for this fix and we realised it doesn't entirely handle some cases. It should work if you're only using broadcast triggers but there are some complex cases if you're directly opening channels from code which won't currently work. I'll update this issue again once I've resolved this :)

martindevans commented 5 years ago

I've put together another fix for this, it turned out to be an incredibly complex issue which required fixing right at the heart of the network system. Doing that without breaking network protocol compatibility was rather difficult! Email me again if you'd like a new test build :)

martindevans commented 5 years ago

I've just pushed Dissonance 6.2.6 to the store which includes a fix for this. It should be available within a few days once Unity approve it.