feat: delete rejected transceiver

[o-u-p]

when port = 0 and direction = inactive indicates that the media stream is not wanted, so we need delete transceiver, sync the logic with c++ implement https://www.ietf.org/rfc/rfc3264.txt#:~:text=A%20port%20number%20of%20zero,rejected%20stream%20(Section%206).

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 80.39216% with 10 lines in your changes missing coverage. Please review.

Project coverage is 79.66%. Comparing base (49b555b) to head (ea155fa). Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
peerconnection.go	76.74%	7 Missing and 3 partials :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3007      +/-   ##
==========================================
+ Coverage   77.96%   79.66%   +1.70%     
==========================================
  Files          89       78      -11     
  Lines       10578     9728     -850     
==========================================
- Hits         8247     7750     -497     
+ Misses       1840     1533     -307     
+ Partials      491      445      -46     

Flag	Coverage Δ
go	79.66% <80.39%> (+0.12%)	:arrow_up:
wasm	?

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[Sean-Der]

Thank you so much @o-u-p

Would you be able to write a test for this? If you could have a static answer and then just show that disabling works.

[JoTurk]

Correct me if I'm wrong, but shouldn't this be handled in setLocalDescription rather than in createAnswer?

[Sean-Der]

@joeturki you are right! Actual changes/committing sound only happen in set*Description

[KevinVison]

Correct me if I'm wrong, but shouldn't this be handled in setLocalDescription rather than in createAnswer?

you're right, I have update the code and add example page. the unit test may not be easy, since the signal state will be have-remote-offer->SetRemote(offer)->have-remote-offer

[o-u-p]

I'm not sure about the side effects of automatically deleting transceivers like this, Also I'm not sure if this follows the spec on this, It would help us to have few tests for this behavior:

If any RtpTransceiver has been added, and there exists an m= section with a zero port in the current local description or the current remote description, that m= section MUST be recycled by generating an m= section for the added RtpTransceiver as if the m= section were being added to the session description (including a new MID value), and placing it at the same index as the m= section with a zero port.

If an RtpTransceiver is stopped and is not associated with an m= section, an m= section MUST NOT be generated for it. This prevents adding back RtpTransceivers whose m= sections were recycled and used for a new RtpTransceiver in a previous offer/ answer exchange, as described above.

If an RtpTransceiver has been stopped and is associated with an m= section, and the m= section is not being recycled as described above, an m= section MUST be generated for it with the port set to zero and all "a=msid" lines removed.

https://rtcweb-wg.github.io/jsep/#sec.subsequent-offers

Thank you for your work, let me know if you need help writing tests :)

(Sorry for the long review)

see SdpOfferAnswerHandler::RemoveStoppedTransceivers in c++ ` void SdpOfferAnswerHandler::RemoveStoppedTransceivers() { TRACE_EVENT0("webrtc", "SdpOfferAnswerHandler::RemoveStoppedTransceivers");

RTC_DCHECK_RUN_ON(signaling_thread()); // 3.2.10.1: For each transceiver in the connection's set of transceivers // run the following steps: if (!IsUnifiedPlan()) return; if (!ConfiguredForMedia()) { return; } // Traverse a copy of the transceiver list. auto transceiver_list = transceivers()->List(); for (auto transceiver : transceiver_list) { // 3.2.10.1.1: If transceiver is stopped, associated with an m= section // and the associated m= section is rejected in // connection.[[CurrentLocalDescription]] or // connection.[[CurrentRemoteDescription]], remove the // transceiver from the connection's set of transceivers. if (!transceiver->stopped()) { continue; } const ContentInfo* local_content = FindMediaSectionForTransceiver( transceiver->internal(), local_description()); const ContentInfo* remote_content = FindMediaSectionForTransceiver( transceiver->internal(), remote_description()); if ((local_content && local_content->rejected) || (remote_content && remote_content->rejected)) { RTC_LOG(LS_INFO) << "Dissociating transceiver" " since the media section is being recycled."; transceiver->internal()->set_mid(std::nullopt); transceiver->internal()->set_mline_index(std::nullopt); } else if (!local_content && !remote_content) { // TODO(bugs.webrtc.org/11973): Consider if this should be removed already // See https://github.com/w3c/webrtc-pc/issues/2576 RTC_LOG(LS_INFO) << "Dropping stopped transceiver that was never associated"; } transceivers()->Remove(transceiver); } } `

[JoTurk]

I'm talking about side effects from deleting transceivers this way in Pion, we should make sure that.

1. Recycling of m= sections: Unused m= sections should be available for other transceivers.
2. Pion should support recycling m= sections for transceivers associated from other clients.
3. If a transceiver is not recycled and is still associated with a mid section, we should generate a m= section for it with the port set to zero and all a=msid lines removed.

I believe the snippet you referenced is from libwebrtc, and I'm confident libwebrtc implements behavior.

[KevinVison]

I'm talking about side effects from deleting transceivers this way in Pion, we should make sure that.

1. Recycling of m= sections: Unused m= sections should be available for other transceivers.
2. Pion should support recycling m= sections for transceivers associated from other clients.
3. If a transceiver is not recycled and is still associated with a mid section, we should generate a m= section for it with the port set to zero and all a=msid lines removed.

I believe the snippet you referenced is from libwebrtc, and I'm confident libwebrtc implements behavior.

1. I think it's the original logic? and I just delete the transceiver that is not wanted exactly, if there are unused transceiver,it will still use the reuse track logic as before.
2. I think so, but I think my implement is standard and should be suitable for other clients.
3. I think the original logic will do this, correct me if I'm wrong