-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix AddTrack transceiver reuse per W3C specs #1861
base: v2
Are you sure you want to change the base?
Conversation
According to W3C specifications, the transceiver can be reused if "The sender has never been used to send. More precisely, the [[CurrentDirection]] slot of the RTCRtpTransceiver associated with the sender has never had a value of "sendrecv" or "sendonly"". Current implementation does not have CurrentDirection slot, so the flag usedToSend is set on setDirection change. This fixes pion#1843 and maybe other issues.
Thank you so much for the fix @EdoaLive ! I will write a test for it and merge to |
hey @EdoaLive , do you still want to work together to get this in? |
Hi @edaniels, what do you mean by "work together"? Could you be more clear about what has to be done? |
other than merging the PR, this needs some tests to validate the fix since we're dealing with a state machine. If you get those in, I think we'd be good! No rush on that. |
Do you mean automatic (unit?) testing or real-world testing? In the second case, we used the patched version for years with many users, so there's that. Anyway I'll get back tho this when (and if) this issue happens with our new version and with pion v3 implementation. |
I do mean unit testing to validate the new code doesn't break any old code. I get what you're saying with needing something other than just two pion peers. It would be nice if we had higher level tests for that, but that's definitely too much for this PR. |
According to W3C specifications, the transceiver can be reused if "The sender has never been used to send. More precisely, the [[CurrentDirection]] slot of the RTCRtpTransceiver associated with the sender has never had a value of "sendrecv" or "sendonly"".
Current implementation does not have CurrentDirection slot, so the flag usedToSend is set on setDirection change.
This fixes #1843 and maybe other issues (for us it fixes other two issues when reusing transceiver, eg clients going and coming back to a conference).
This has been done on the v2 branch because we currently don't have the resources to switch our software to v3 and test it there. Looking at the source (master) seems this kind of issue is still there.