There is use of t.stopped in peerconnection.go, but that flag is not set/used in RTPTransceiver at all. This PR sets that when a transceiver is stopped and provides an API to check if a transceiver is stopped.

@Sean-Der @davidzhao @cnderrauber The fact that code does not seem to get tripped makes me think I am missing something, but I noticed the ☝️ (i. e. stopped flag in RTPTransceiver was not set/used) and made this PR to address this. Have tested this change with LiveKit and it seems fine.

Oct 18 '22 12:10 boks1971

Codecov Report

Base: 77.52% // Head: 77.59% // Increases project coverage by +0.07% :tada:

Coverage data is based on head (097d94f) compared to base (42dc0d4). Patch coverage: 91.66% of modified lines in pull request are covered.

:exclamation: Current head 097d94f differs from pull request most recent head 71f4e4b. Consider uploading reports for the commit 71f4e4b to get more accurate results

@@            Coverage Diff             @@
##           master    #2336      +/-   ##
==========================================
+ Coverage   77.52%   77.59%   +0.07%     
==========================================
  Files          87       87              
  Lines        9262     9266       +4     
==========================================
+ Hits         7180     7190      +10     
+ Misses       1648     1644       -4     
+ Partials      434      432       -2     

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

Oct 18 '22 12:10 codecov[bot]

Pion's transceiver don't have a real stopped state now, it call stop method when state go to inactive, so in pion, a transceiver after stop (actually it not set stopped flag, just transit to inactive) can be reused. #2226 also mentioned this. Actually, https://datatracker.ietf.org/doc/html/rfc8829#section-4.2.1 said that a stopped can't be reused, so this PR would comply with the rfc, but changed transceiver reuse behavior, might cause problem if application relied on it. There are #1943 and #1969 do similar things and revert it, don't know why, @Sean-Der might have suggestion about this.

Oct 19 '22 02:10 cnderrauber

Thank you @cnderrauber . Did not realise this path had already been done. Maybe, we should just remove the stopped flag to avoid confusion.

Oct 19 '22 03:10 boks1971

hey @boks1971, do you still want to work together to get this in?

Jun 25 '24 16:06 edaniels

#1969

@edaniels Would be good to clean up that unused flag, but the discussions surrounding it which @cnderrauber pointed out makes me reluctant to change things. There was also a similar commit by @Sean-Der and a revert. Have not checked exactly why it was reverted.

I am still thinking that getting rid of just that field is good as it is has only one value, i. e. false. The long discussion in #2226 is more around the expected behaviour of stop vs inactive and not around that field value per se.

Getting rid of that field looks straightforward except this one place which is conditioned on if t.stopped which is never true now. Unclear whether that if block should be completely removed if stopped field is removed OR the condition needs to be different - https://github.com/pion/webrtc/blob/fc3521e85c997a6f04f59e49470b52eb7afc06d4/peerconnection.go#L409

Jun 25 '24 18:06 boks1971

I think it's safe and good to remove the always false bool then. Separately, I think we need to get the behavior of stopped, ended, and friends in line with the W3 spec. I may try to tackle that soon.

Jun 25 '24 18:06 edaniels