Bug since v8.0.11 when adding/removing a video stream (WebRTC)

[ChristopheI]

Hi,

I was previously still in v8.0.0.

I tried to migrate to the last version but I notice problems when adding/removing in a current WebRTC communication a Video Track (using same PeerConnection)
It was still OK in v8.0.10 but not in v8.0.11 (and at least in this version I have quickly tested: v8.0.12, v8.0.14 and in v8.0.22).

Some Scenario to reproduce:

A is using Edge/Chromium
B is using SIPSorcery

Case 1:

• A calls B in audio only.
• B answers
• B add Video
• B remove Video => Communication is automatically cut after some delay

Case 2:

• A calls B in audio+video.
• B answers un audio+video
• B remove Video => Communication is automatically cut after some delay

Case 2:

• A calls B in audio+video.
• B answers un audio
• B add Video => Communication is automatically cut after some delay

I will dig to try to find the reason.
I suppose it's related to this commit: eca5d6a#diff-8359dbc428e517d00e65bd6a25b9295ca23b0596b241cd45aeb56c269c4181a8

[ChristopheI]

Based on code tagged as v8.0.11 [aa5fb6a], I notice these problems below: (by order of criticity)

@sipsorcery and @xBasov Could you please comment ? I can't test text as MediaType on my side.

I also have troubles about IsStarted, IsAudioStarted, IsVideoStarted, IsTextStarted properties. Comments indicates it's related to receive RTP packets. but they are set to true when localTrack are not Inactive

Thx

1. in RemoveLocalTrack()
   VideoStream or TextStream are totally closed if local track is closed. It's incorrect.
   I removed this code

2. in GetNextVideoStreamByLocalTrack()
   Bad test used

3. in SendRtcpTWCCFeedback()
   Missing code to handle text MediaType

4. in SendRtpRaw()
   Missing code to handle text MediaType

5. in SetRemoteDescription
   Use same kind of test for coherency

5. in GetSessionDescription()
   I removed code redundancy

[ChristopheI]

My previous code suggestions are still correct / working for commit perf(audio-stream): reduce memory allocation on send audio frame (#1411) [ff41b1f] (just one commit before 8.0.20-pre)

Then the big commit Improvements so audio media can be handled without having to deal with raw RTP packets (#1424) [ba66613] is breaking support of several video streams in same time ...
=> it was a bug on my side

[sipsorcery]

@ChristopheI thanks for the in depth diagnosis. In my case I don't have any scenarios that I replace streams with. If PR #1438 solves the issue you are experiencing I'd say we merge it.

[ChristopheI]

So I merge PR #1440 .
I'm interested to have a new release including it.
Thx

[ChristopheI]

Notice that I will have soon a way to tests multi audio streams in same time with the possibility to add/remove them.
I suppose I will have to update SIPSorcery to handle this like it's done with video streams

[sipsorcery]

So I merge PR #1440 . I'm interested to have a new release including it. Thx

v8.0.23 just published.