meetecho / janus-gateway

Janus WebRTC Server
https://janus.conf.meetecho.com
GNU General Public License v3.0
8.25k stars 2.48k forks source link

[1.x] AudioBridge: after network interrupt there's no `stopped-talking` event until timeout #3304

Closed Frenzie closed 10 months ago

Frenzie commented 11 months ago

What version of Janus is this happening on? v1.2.1

Have you tested a more recent version of Janus too? No. (Which more recent version? :-)

Was this working before? No

Additional context When you start talking, you receive a talking event along these lines:

{
   "janus": "event",
   "session_id": 7115030027245058,
   "sender": 3472346807042796,
   "plugindata": {
      "plugin": "janus.plugin.audiobridge",
      "data": {
         "audiobridge": "talking",
         "room": "563861637229053",
         "id": "my-network-will-be-interrupted"
      }
   }
}

When you stop talking, you'll receive a stopped-talking event in turn. This all works very well. But if you cut the connection, there will never be a stopped-talking event.

In essence this seems to be because all relevant logic takes place in janus_audiobridge_incoming_rtp(), but in this scenario there is no incoming RTP packet.

https://github.com/meetecho/janus-gateway/blame/6f6ac5eaf679bc2dc4d5de39051c22c233c38a74/src/plugins/janus_audiobridge.c#L5853

Ideally it would send a stopped-talking event long before the eventual timeout, but it's not immediately clear to me if that's relatively simple (no RTP packets for a couple of seconds = stopped talking) or very hard to achieve within the current architecture.

lminiero commented 11 months ago

Good catch, and yes, the problem is that right now the trigger is incoming_rtp, so only a subsequent RTP packet can change the state again. Considering we moved all the decoding stuff to the participant's thread, with the new jitter buffer, I guess we could move the talking detection there as well, before decoding the packet: since that thread monitors queues and is basically never idle, that's where a period of inactivity could indeed translate to a stopped-talking event.

That said, the problem would still remain there when using the VideoRoom plugin, since in that case packets are relayed and not decoded. No solution for that there, unless some sort of loop or "watchdog" is implemented to track inactivity (which I'd rather not add for now though).

lminiero commented 11 months ago

@Frenzie please test the PR above.

Frenzie commented 11 months ago

I'm afraid I won't be able to for the next two weeks, but I'll be happy to in the new year. :+1:

lminiero commented 11 months ago

I've made a few tests by blocking traffic and it seems to be working for me.

Frenzie commented 10 months ago

It seems to work very well! :+1:

lminiero commented 10 months ago

Ack, thanks :+1: