matrix-org / waterfall

A cascading stream forwarding unit for scalable, distributed voice and video conferencing over Matrix
Apache License 2.0
98 stars 5 forks source link

remote_track: unified track handling / MTU fix #105

Closed daniel-abramov closed 1 year ago

daniel-abramov commented 1 year ago

While investigating problems with Firefox and audio not being played back, I've noticed that sometimes I get very weird errors in the log that are not coming from our own logging (i.e. they did not follow the format of our logging framework). Those were the errors that Pion logged directly to the stderr (not a good practice btw), but by accident I was lucky (or unlucky) to catch them.

That's what they said:

mux ERROR: 2023/01/19 19:45:01 mux: failed to read from packetio.Buffer short buffer
mux ERROR: 2023/01/19 19:45:01 mux: failed to read from packetio.Buffer short buffer
mux ERROR: 2023/01/19 19:45:01 mux: failed to read from packetio.Buffer short buffer
mux ERROR: 2023/01/19 19:45:01 mux: failed to read from packetio.Buffer short buffer
mux ERROR: 2023/01/19 19:45:01 mux: failed to read from packetio.Buffer short buffer
mux ERROR: 2023/01/19 19:45:01 mux: failed to read from packetio.Buffer short buffer
mux ERROR: 2023/01/19 19:45:02 mux: failed to read from packetio.Buffer short buffer
mux ERROR: 2023/01/19 19:45:02 mux: failed to read from packetio.Buffer short buffer
mux ERROR: 2023/01/19 19:45:02 mux: failed to read from packetio.Buffer short buffer
mux ERROR: 2023/01/19 19:45:03 mux: failed to read from packetio.Buffer short buffer
mux ERROR: 2023/01/19 19:45:03 mux: failed to read from packetio.Buffer short buffer
mux ERROR: 2023/01/19 19:45:04 mux: failed to read from packetio.Buffer short buffer
mux ERROR: 2023/01/19 19:45:04 mux: failed to read from packetio.Buffer short buffer
mux ERROR: 2023/01/19 19:45:04 mux: failed to read from packetio.Buffer short buffer
mux ERROR: 2023/01/19 19:45:04 mux: failed to read from packetio.Buffer short buffer

I'm not sure what these errors are supposed to mean since I don't even know which function call causes them (Pion did not return any errors from the functions that we called, so if I had not monitored the stdout/stderr I would have missed them), so I tried to search where they come from.

I've found 2 discussions where this error has been mentioned, one was related to audio, and the other one to the MTU.

The receiveMTU is a constant in Pion that is equal to 1460. The only constant when reading data is the input buffer size of 1400 that we use in the handling of the audio track. This number (and approach) I've taken from Pion examples that used it in all places when reading and forwarding RTP packets. This value is however lower than the MTU constant. I wonder if this could have caused this error.

Regardless, I thought it would be better to eliminate any potential issues, so I removed the code that used the buffer of a size of 1400 (don't like this magic number there and it seems like @SimonBrandner also would not mind getting rid of it as he asked a question related to this part of code when reviewing the previous simulcast PR), so I think it would be safer to assume that we better get rid of it and replace it with the same approach that we're using for video subscriptions (except that we don't need to do the RTP rewriting and stuff), so and decided to use the same function to handle both audio and video remote tracks so that the code is identical (except the RTP handling logic which is different for audio and video).