Skip to content
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

pion/webrtc v2.2.26 wrong transceivers reuse, cannot re-add removed tracks on receiver side. #1843

Open
marco-sacchi opened this issue Jun 23, 2021 · 15 comments

Comments

@marco-sacchi
Copy link

marco-sacchi commented Jun 23, 2021

Your environment.

  • Version: Ubuntu 20.04.2 LTS 64-bit - GNOME Version 3.36.8
  • Browser: Google Chrome Version 91.0.4472.77 (Official Build) (64-bit)
  • Used pion versions:
github.com/pion/dtls/v2 v2.0.4 // indirect
github.com/pion/quic v0.1.4 // indirect
github.com/pion/rtcp v1.2.6
github.com/pion/sctp v1.7.11 // indirect
github.com/pion/srtp v1.5.2 // indirect
github.com/pion/stun v0.3.5
github.com/pion/transport v0.12.0 // indirect
github.com/pion/turn/v2 v2.0.5
github.com/pion/webrtc/v2 v2.2.26

What did you do?

First of all, thank you for your product and support!

We are using pion to implement our own SFU.

We are trying to figure out why, although Chrome's webrtc-internal stats display packet reception, negotiation, and transceiver status change are correct, the audio/video stream doesn't show when tracks are removed and then re-added.

Steps

The panelist initiates the call with only the webcam video track (to simplify debugging) and publishes it, attendees joins the call with only the microphone audio track.

The panelist's audio/video stream is then added to the attendees subscriptions and correctly displayed.

The broadcast of the panelist's audio/video stream is stopped and the attendees subscriptions are stopped.

Then the panelist's audio/video stream is added again and the subscriptions restored.

I am attaching the log of the negotiations on both the panelist and the attendant side and the stats.

We don't think this is your issue, but it would be great if we could get support to try and understand why the stream isn't being received again.

EDIT 2021-06-24

Note that:

  • both negotiations logs are copy/pasted from chrome://webrtc-internals and reflect the javascript side of the client.
  • you can also see the SDP generated by your go implementation of webrtc on calls to setRemoteDescription of logs
  • the add and removal of screen-sharing works correctly even when done multiple times
  • the attendee never remove the initial stream with one track coming from the mic
  • you can also see that the id of the tracks remains the same at the next addition which to my knowledge is not normal behavior
  • we have verified that the track.stop() method is not called for any remote track.

Obviously any information and suggestions will be welcome.

We regret not being able to share more information, we are still investigating, and will keep you updated in case we manage to isolate or solve the problem on our side or on your side.

What did you expect?

The audio/video stream should be visible again.

What happened?

The audio/video stream is not restored.

panelist-negotiations.txt
attendee-negotiations.txt

panelist-outbound-rtpvideo-stats
attendee-inbound-rtpvideo-stats

@marco-sacchi
Copy link
Author

After investigating we saw that there is a difference between the behavior of the browser and that of the pion webrtc library in handling transceivers and related m-sections in the SDP.

Browser
Offerer side: new tracks or tracks added again always increase the number of m-sections in the SDP

Aswerer side: transceivers are reused whenever possible.

PION WebRTC package
Offerer side: the transceivers and relative m-sections are reused even when the transceiver has already assumed a sendonly or sendrecv state in the past.

Aswerer side (browser): same as previous.

Following the W3C specification on adding tracks (and related modification of senders and transceivers), a transceiver cannot be reused if it previously had the currentDirection state set to sendonly or sendrecv. (https://www.w3.org/TR/webrtc/#dom-rtcpeerconnection-addtrack)

Commenting out the code that iterates over the transceivers in the PeerConnection that attempt reuse of it, we were able to achieve the same behavior found during a webrtc p2p session between two browsers, with the exception that both parties always generate new transceivers and m-sections (file peerconnection.go, AddTrack function, github.com/pion/webrtc/v2 v2.2.26)

You can see the p2p behavior on this fiddle https://jsfiddle.net/yt7m5xvj/ by following these steps:

  • open it in two tabs
  • click offer on one and paste it on the other, then hit enter
  • copy the answer displayed on the first tab
  • click on "AddTrack" and then on "Remove + Add" to remove the webcam track and add it again.

Currently we cannot switch to the new release of the library and we have seen that even in the latest release currentDirection is not implemented in the RTPTransceiver structure.

Can you give us some advice on how to modify the code to get the behavior from W3C specs instead of always avoiding reusing transceivers?

@marco-sacchi marco-sacchi changed the title Cannot re-add removed tracks on receiver side. pion/webrtc v2.2.26 wrong transceivers reuse, cannot re-add removed tracks on receiver side. Jun 25, 2021
EdoaLive added a commit to modulo-srl/webrtc-pion that referenced this issue Jun 30, 2021
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.
@Sean-Der Sean-Der added this to the 3.1.0 milestone Aug 1, 2021
@Sean-Der
Copy link
Member

Sean-Der commented Aug 4, 2021

I am so sorry @marco-sacchi @EdoaLive that I dropped the ball on this. I really appreciate your deep investigation on this and we are going to fix this :) I am trying to get a v3.1.0 together and this will be in it.

I am curious how ReplaceTrack would work? Something different must be happening on that path to make it all work. I would prefer if we could make transceiver reuse work though. If I can't figure out something you are right that disabling it completely is the best way to go.

@Sean-Der
Copy link
Member

Hi @marco-sacchi @EdoaLive

I am not able to reproduce this. Would you mind cloning this repro and tell me what is wrong/different about my reproduce. I will continue to investigate this! https://github.com/Sean-Der/pion-webrtc-issue-1843

@Sean-Der
Copy link
Member

Sean-Der commented Aug 22, 2021

Also I used Version 94.0.4606.12 (Official Build) dev (64-bit)

@Antonito if you have the bandwidth would you mind trying my repro/checking if it matches the summary in #1843 (comment) if you don't totally understand I can find someone else :) you are just great at detail/debugging stuff!

I am going to get an old version of Chromium to try also.

@Antonito
Copy link
Member

Hi,

@Sean-Der from what I understood, your example matches the scenario described – the only difference I spot is the audio stuff, but it should not impact the results.

I ran the example a few times on (MacOS):

  • Chrome Version 92.0.4515.159 (Official Build) (x86_64)
  • Safari Version 15.0 (17612.1.26.1.3)
  • Firefox 91.0.1 (64-bit)

I didn’t detect any issue – the video played / stopped / continued as expected 🤔

Would you be able to test with a more recent Chrome version or a different browser @marco-sacchi ?
And eventually confirm the issue occurs on your end with Sean's example?

My best guess right now would be an issue with libwebrtc: might be a bug in an old version, or maybe Pion doesn't do something correctly and newer libwebrtc is less strict about it, or may be arch related (Linux vs MacOS, as I guess @Sean-Der also ran the example on a Mac?)

@Sean-Der
Copy link
Member

Just tried with Version 91.0.4472.101 (Official Build, ungoogled-chromium) (64-bit) on Debian and was not able to reproduce.

I really appreciate this bug report @marco-sacchi and @EdoaLive I just am not able to reproduce. I don't want to disable Transceiver reuse unless we have to. It will really bloat signaling. Those costs really start to add up for some users.

I will leave this open for a few more days, then will resolve.

thanks!

@marco-sacchi
Copy link
Author

Sorry @Sean-Der and @Antonito, I will be watching carefully during the next week.

Using your repro I have no problems: I have now tested with Chrome Version 92.0.4515.159 (Official Build) (64-bit) on Ubuntu 20.04.3 LTS x64 with GNOME Version 3.36.8.

But the difference I noticed right away, apart from the different version of the webrtc module (github.com/pion/webrtc/v3 v3.0.32 instead of github.com/pion/webrtc/v2 v2.2.26 - which is what we're using ), is that the javascript client is completely passive.

We had problems using a stream coming from the webcam (or from the screen sharing) so the negotiation started in the browser both for the start of the broadcast and for the stop (and subsequent reactivation). This means that in your repro the SDP with the offer is always generated by the Pion webrtc module, while in our case it is always generated by the browser.

These problems were present in my system, but also in the MacBook Pro with macOS Mojave Version 10.14.6 (where I tested Safari and Chrome) and in @EdoaLive's Windows 10 system with Chrome (whose version I don't remember).

I will try to integrate your test using the video track coming from the webcam.

@Sean-Der
Copy link
Member

Sean-Der commented Aug 28, 2021

Ah ok nice catch @marco-sacchi, sorry about the behavior differences. How should I modify the example to reproduce the behavior?

  • The browser should public one webcam feed,
  • The browser should always generate the offers
  • Everyone 10 seconds publish a new offer
  • Pion should add/remove the track every other time

If that is right I can update my repo today!

@Sean-Der
Copy link
Member

Also no worry about the delay. You reported this and I was really slow, I moved into a new house June 20th so my life was a blur for a good month :)

@marco-sacchi
Copy link
Author

You have to loop like this:

  • get the webcam stream with getUserMedia
  • generate the offer with the video track and send it to Pion (the webrtc layer of the browser will generate an negotiationneeded event in the PeerConnection object)
  • Pion has to add the track on the server side and send the answer to the browser
  • wait 5 seconds on javascript then
  • remove the stream from the <video> element and from the javascript PeerConnection object with removeTrack without stopping the track with stop(), otherwise you will not be able to make it start again at the next loop
  • the webrtc layer will trigger an negotiationneeded event to generate an offer to send to Pion
  • Pion receives the offer, removes the previously added track and sends the answer to the browser
  • wait 5 seconds on javascript, loop

Note that you must create the PeerConnection object at the beginning and that it must be destroyed only at the end of the N loops you decide to do.

I wrote it from memory, so I hope I haven't been wrong somewhere.

@marco-sacchi
Copy link
Author

Hi @Sean-Der , I created a pull-request following the steps I indicated above and I was able to reproduce the behavior indicated in the first post of the issue..

@Sean-Der
Copy link
Member

Sean-Der commented Sep 4, 2021

@marco-sacchi That example is perfect! I am also surprised on the behavior (and don't know the right answer). If Pion gets an offer that disables a currently disabled track, I wouldn't expect it to be re-enabled automatically.

I need to make a JS Fiddle to see what happens Chrome<->Chrome. If your transceiver is disabled by the remote, can it be re-enabled by the remote?

@marco-sacchi
Copy link
Author

@Sean-Der, you don't need to create a fiddle, I had already attached one in this post (#1843 (comment)) of this issue.

@Sean-Der
Copy link
Member

Sean-Der commented Sep 5, 2021

Hey @marco-sacchi

Can you pull + try the repro repo. It looks like signaling does work in this case. I just had to do the following

  • Keeping the Timestamps + SequenceNumbers contiguous
  • Don't unset the video elements srcObject

@marco-sacchi
Copy link
Author

@Sean-Der, you can't avoid to unset the video element, this way the track continues to exist on the javascript side.

Your changes work because, from the browser's point of view, instead of sending a new track you are just stopping and resuming sending packets to a track that already exists in the peerConnection. Even though Pion has created a new transceiver, the new track is not actually used.

As you can see, on the javascript side, pc.ontrack is triggered only once, while it should fire again at the next renegotiation for the new inbound track:

10:52:46.018 Connection open
10:52:47.545 Camera started.
10:52:47.548 Signal offer to server RTCSessionDescription {type: "offer", sdp: "v=0\r\no=- 8348135068033631080 2 IN IP4 127.0.0.1\r\ns…7961 label:5390b2ac-f010-431a-ad6c-ffa87d871858\r\n"}
10:52:47.551 Answer received.
10:52:47.555 New remote video track: 1db9c085-90c2-4f5d-ae7e-0f3b8cdeb774  <---
10:52:57.265 Camera stopped.
10:52:57.291 Signal offer to server RTCSessionDescription {type: "offer", sdp: "v=0\r\no=- 8348135068033631080 3 IN IP4 127.0.0.1\r\ns…7961 label:5390b2ac-f010-431a-ad6c-ffa87d871858\r\n"}
10:52:57.295 Answer received.
10:52:59.569 Camera started.
10:52:59.574 Signal offer to server RTCSessionDescription {type: "offer", sdp: "v=0\r\no=- 8348135068033631080 4 IN IP4 127.0.0.1\r\ns…2695 label:d37b842a-7134-4eb0-afcf-51d47367afd5\r\n"}
10:52:59.575 Answer received.
10:53:18.191 > pc.getTransceivers()
10:53:18.212 (2) [RTCRtpTransceiver, RTCRtpTransceiver]
   0: RTCRtpTransceiver
   currentDirection: "recvonly"
   direction: "recvonly"
   mid: "0"
   receiver: RTCRtpReceiver
      playoutDelayHint: null
      rtcpTransport: null
      track: MediaStreamTrack {kind: "video", id: "1db9c085-90c2-4f5d-ae7e-0f3b8cdeb774", label: "1db9c085-90c2-4f5d-ae7e-0f3b8cdeb774", enabled: true, muted: false, …}
      transport: RTCDtlsTransport {iceTransport: RTCIceTransport, state: "connected", onstatechange: null, onerror: null}
      [[Prototype]]: RTCRtpReceiver
   sender: RTCRtpSender {track: null, transport: RTCDtlsTransport, rtcpTransport: null, dtmf: null}
   stopped: false
   [[Prototype]]: RTCRtpTransceiver
   
   1: RTCRtpTransceiver
   currentDirection: "sendonly"
   direction: "sendrecv"
   mid: "1"
   receiver: RTCRtpReceiver
      playoutDelayHint: null
      rtcpTransport: null
      track: MediaStreamTrack {kind: "video", id: "f02f9159-c0b4-4d06-8df8-c0f7f3a7ea72", label: "f02f9159-c0b4-4d06-8df8-c0f7f3a7ea72", enabled: true, muted: true, …}
      transport: RTCDtlsTransport {iceTransport: RTCIceTransport, state: "connected", onstatechange: null, onerror: null}
      [[Prototype]]: RTCRtpReceiver
   sender: RTCRtpSender
      dtmf: null
      rtcpTransport: null
      track: MediaStreamTrack {kind: "video", id: "d37b842a-7134-4eb0-afcf-51d47367afd5", label: "Aukey-PC-LM1E Camera (****:****)", enabled: true, muted: false, …}
      transport: RTCDtlsTransport {iceTransport: RTCIceTransport, state: "connected", onstatechange: null, onerror: null}
      [[Prototype]]: RTCRtpSender
      stopped: false
   [[Prototype]]: RTCRtpTransceiver
   length: 2
[[Prototype]]: Array(0)

Note:

I opened this issue, not because Pion doesn't reuse transceivers, but because it reuses them even when it shouldn't, causing a different behavior than the p2p connection between two browsers.

Following the W3C specification on adding tracks (and related modification of senders and transceivers): "a transceiver cannot be reused if it previously had the currentDirection state set to sendonly or sendrecv. (https://www.w3.org/TR/webrtc/#dom-rtcpeerconnection-addtrack)".

Could you please check this behavior?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants