Reuse closed m= sections in Firefox#149
Conversation
|
Thanks. AFAIR there is a reason for me to not do this that way in the past, but I cannot recall which one now. I have to take a deeper look to this. IMHO if this works it must be done for all handlers and not just for Firefox. |
| if (!this._firstMid) | ||
| this._firstMid = newMediaSection.mid; | ||
|
|
||
| // Append new section to the existing vector and the SDP object |
| this._mediaSections.push(newMediaSection); | ||
| this._sdpObject.media.push(newMediaSection.getObject()); | ||
|
|
||
| // Add it to the map |
There was a problem hiding this comment.
Same for all the added comments.
| // Recreate map | ||
| this._midToIndex.clear(); | ||
| for (idx = 0; idx < this._mediaSections.length; ++idx) | ||
| this._midToIndex.set(this._mediaSections[idx].mid, idx); |
|
I will come back to this PR next week. |
ibc
left a comment
ibc
left a comment
There was a problem hiding this comment.
Can you please explain in detail what this PR does and what it fixes? And why does it allow closing the first media section? That will break/close the ICE connection globally.
…recreating the internal sections array.
|
It does not allow closing the first media section, since it uses regular The purpose of this PR was to allow the Firefox handler to reuse closed media section on sending, as other handlers do. Currently, when some producers are closed, and new ones created later, it just keep adding new media sections to SDP, so in case a lot of producers are created and closed in the lifetime of the connection, SDP can grow quite a lot. The problem with Firefox is that when adding new transceiver, it uses not first available closed section, but the one with media type (audio/video) matching the type of the new transceiver. Thus, the method of finding next media section in the internal array (which seaches for the first closed section) does not work with Firefox. This is why closing any sections was disabled the Firefox handler. I used the fork from which the PR is created in the real system for quite a while without any problems. But recently Firefox started to fail on the mismatching media types in the receiving connection. I took a look at the commits in the upstream, saw the one that fixes that problem, merged the changes into the fork, and the PR was bumped up. After all these years, I almost forgot about it. :) |
|
Thanks. I will check in next week. |
3 participants
This PR handles m= sections reuse in Firefox by synchronizing RemoteSdp with the local description regardless of where the new transceiver's section is placed by the browser.