pion / webrtc

Pure Go implementation of the WebRTC API
https://pion.ly
MIT License
13.78k stars 1.66k forks source link

ontrack never called in pion-pion comms #989

Closed chrbsg closed 4 years ago

chrbsg commented 4 years ago

Your environment.

What did you do?

Check code.zip $ cd server && go build && ./server $ cd client && go build && ./client copy/paste offer from client to server copy/paste reply from server to client

server output: ... 2020/01/28 17:18:39 server: ICE state changed: connected 2020/01/28 17:18:39 sendPCM(0xc0001ee000,0xc00016c000,0xc00013e900)

client output: ... 2020/01/28 17:18:39 client: ICE state changed: checking 2020/01/28 17:18:39 client: ICE state changed: connected 2020/01/28 17:18:39 ondatachannel

What did you expect?

OnTrack should be called in client, should print "ontrack":

pc.OnTrack(func(rxtrack *webrtc.Track, receiver *webrtc.RTPReceiver) {
        log.Println("ontrack")

It's possible that the code is doing something wrong but the connection is up and ondatachannel is being called, so it seems like ontrack should also be called.

What happened?

Client peer does not seem to call ontrack.

https://github.com/pion/webrtc/issues/600 was similar but in this case I am calling pc.AddTransceiver(webrtc.RTPCodecTypeAudio) in the client so it should be working. code.zip

Sean-Der commented 4 years ago

Hey @chrbsg, sorry about the bug! Would you mind uploading another zip, I think it filtered your code out.

seaduboi@38f9d359441f:~/Downloads$ unzip -l code.zip
Archive:  code.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
        0  01-28-2020 09:24   client/
        0  01-28-2020 09:18   server/
---------                     -------
        0                     2 files

thanks!

chrbsg commented 4 years ago

Sorry my mistake with the zip. New file attached.

code.zip

Sean-Der commented 4 years ago

Hey @chrbsg,

In server/main.go:69 sendPCM is commented out. When I uncommented out it I get received packet printed!

I believe everything is working, but would you mind double checking?

chrbsg commented 4 years ago

That's odd, when I uncomment sendPCM I get:

2020/01/30 10:44:08 client: ICE state changed: checking
2020/01/30 10:44:08 client: ICE state changed: connected
2020/01/30 10:44:08 ondatachannel
pc ERROR: 2020/01/30 10:44:08 Incoming unhandled RTP ssrc(4039455774)
2020/01/30 10:44:44 client: ICE state changed: disconnected
2020/01/30 10:44:58 client: ICE state changed: failed

I had guessed the problem here was that ontrack hadn't been called, so the track wasn't set up, so when the client gets an RTP packet for a track that hasn't been properly setup yet it complains.

(apart from the issue above, I don't understand why calling txtrack.WriteSample would result in onTrack being called on the peer - is the call to onTrack deferred until the peer actually sends some data?)

Sean-Der commented 4 years ago

yea OnTrack is only called when it gets the first audio/video packet! We don't know what codec the other side is going to send till we get the first RTP packet, so we have to wait until then.

Incoming unhandled RTP ssrc(4039455774) that usually happens if the sender is using an SSRC they didn't declare in the SDP. When you get that message I would see if the SSRC is actually in the SDP.

Are you running the client/server on the same host? Might be nice to rule out any networking issues.

chrbsg commented 4 years ago

Client and server are running on same host.

The a=ssrc:2596996162 line appears to be the same in both offer and answer?

Offer:

v=0
o=- 90510573 1580469840 IN IP4 0.0.0.0
s=-
t=0 0
a=fingerprint:sha-256 60:B6:73:3D:A4:25:9B:6C:AA:9C:23:EE:47:08:20:75:36:A0:1C:BC:F7:9C:9E:84:8F:96:71:98:B9:59:87:D1
a=group:BUNDLE 0 1
m=audio 9 UDP/TLS/RTP/SAVPF 111
c=IN IP4 0.0.0.0
a=setup:actpass
a=mid:0
a=ice-ufrag:fVvWRvJjuyYlVNPO
a=ice-pwd:ctkAOTYmdTUkRhFwDBnfMKyiCpwSrUma
a=rtcp-mux
a=rtcp-rsize
a=rtpmap:111 opus/48000/2
a=fmtp:111 minptime=10;useinbandfec=1
a=ssrc:2596996162 cname:nWUeekvZwvzKSIGJ
a=ssrc:2596996162 msid:nWUeekvZwvzKSIGJ agtrwZryiEGISIFT
a=ssrc:2596996162 mslabel:nWUeekvZwvzKSIGJ
a=ssrc:2596996162 label:agtrwZryiEGISIFT
a=msid:nWUeekvZwvzKSIGJ agtrwZryiEGISIFT
a=sendrecv
a=candidate:foundation 1 udp 2130706431 172.17.0.1 59601 typ host generation 0
a=candidate:foundation 2 udp 2130706431 172.17.0.1 59601 typ host generation 0
a=candidate:foundation 1 udp 2130706431 192.168.31.147 60143 typ host generation 0
a=candidate:foundation 2 udp 2130706431 192.168.31.147 60143 typ host generation 0
a=end-of-candidates
m=application 9 DTLS/SCTP 5000
c=IN IP4 0.0.0.0
a=setup:actpass
a=mid:1
a=sendrecv
a=sctpmap:5000 webrtc-datachannel 1024
a=ice-ufrag:fVvWRvJjuyYlVNPO
a=ice-pwd:ctkAOTYmdTUkRhFwDBnfMKyiCpwSrUma
a=candidate:foundation 1 udp 2130706431 172.17.0.1 59601 typ host generation 0
a=candidate:foundation 2 udp 2130706431 172.17.0.1 59601 typ host generation 0
a=candidate:foundation 1 udp 2130706431 192.168.31.147 60143 typ host generation 0
a=candidate:foundation 2 udp 2130706431 192.168.31.147 60143 typ host generation 0
a=end-of-candidates

Answer:

v=0
o=- 50593247 1580469849 IN IP4 0.0.0.0
s=-
t=0 0
a=fingerprint:sha-256 D8:B2:71:98:EE:50:84:5F:CA:00:7D:05:B7:E9:A2:B0:0D:C8:06:D4:B2:A2:5A:54:E1:E6:A8:10:DE:95:1E:B2
a=group:BUNDLE 0 1
m=audio 9 UDP/TLS/RTP/SAVPF 111
c=IN IP4 0.0.0.0
a=setup:passive
a=mid:0
a=ice-ufrag:YIwcfmNWfWXDZkKU
a=ice-pwd:LDrllUWopiHDNMfXhWlDDyZLrOibAaSp
a=rtcp-mux
a=rtcp-rsize
a=rtpmap:111 opus/48000/2
a=fmtp:111 minptime=10;useinbandfec=1
a=ssrc:2596996162 cname:NalVGndvknFKaciE
a=ssrc:2596996162 msid:NalVGndvknFKaciE zXtEfCfvjOyOXqdW
a=ssrc:2596996162 mslabel:NalVGndvknFKaciE
a=ssrc:2596996162 label:zXtEfCfvjOyOXqdW
a=msid:NalVGndvknFKaciE zXtEfCfvjOyOXqdW
a=sendrecv
a=candidate:foundation 1 udp 2130706431 172.17.0.1 37997 typ host generation 0
a=candidate:foundation 2 udp 2130706431 172.17.0.1 37997 typ host generation 0
a=candidate:foundation 1 udp 2130706431 192.168.31.147 42572 typ host generation 0
a=candidate:foundation 2 udp 2130706431 192.168.31.147 42572 typ host generation 0
a=end-of-candidates
m=application 9 DTLS/SCTP 5000
c=IN IP4 0.0.0.0
a=setup:passive
a=mid:1
a=sendrecv
a=sctpmap:5000 webrtc-datachannel 1024
a=ice-ufrag:YIwcfmNWfWXDZkKU
a=ice-pwd:LDrllUWopiHDNMfXhWlDDyZLrOibAaSp
a=candidate:foundation 1 udp 2130706431 172.17.0.1 37997 typ host generation 0
a=candidate:foundation 2 udp 2130706431 172.17.0.1 37997 typ host generation 0
a=candidate:foundation 1 udp 2130706431 192.168.31.147 42572 typ host generation 0
a=candidate:foundation 2 udp 2130706431 192.168.31.147 42572 typ host generation 0
a=end-of-candidates

Error:

pc ERROR: 2020/01/31 11:24:13 Incoming unhandled RTP ssrc(4039455774)

But it may be that this is caused by a bug in the server/main.go code! The server was doing:

        pc, err := api.NewPeerConnection(webrtc.Configuration{})
        check(err)
        _, err = pc.AddTransceiver(webrtc.RTPCodecTypeAudio)
        check(err)

So server was adding a transceiver (which has to be done to support incoming audio) - but client was not creating/sending an audio track. If I comment out the AddTransceiver, then everything works as expected, and onTrack is called on the client.

I'm not sure if this is a bug in pion or an unsupported use case? Suppose the server has an optional incoming audio track (for client->server audio), so AddTransceiver is called to enable that on the server (remember that we don't know if the client will use this or not) - is it now mandatory for the client to actually add a track otherwise the server->client audio won't work due to bad/duplicate SSRCs? I have an idea from reading the spec that SSRC collisions are not allowed, so this shouldn't happen, but don't know enough about this to know whether it is supposed to work or not.

chrbsg commented 4 years ago

I had an idea that maybe AddTransceiver interferes with AddTrack. In server/main.go, this fails with "Incoming unhandled RTP":

        _, err = pc.AddTransceiver(webrtc.RTPCodecTypeAudio)
        check(err)
        _, err = pc.AddTrack(txtrack)
        check(err)
        _, err = pc.CreateDataChannel("data", nil)
        check(err)

But this works:

        _, err = pc.AddTrack(txtrack)
        check(err)
        _, err = pc.CreateDataChannel("data", nil)
        check(err)
        _, err = pc.AddTransceiver(webrtc.RTPCodecTypeAudio)
        check(err)
Sean-Der commented 4 years ago

@chrbsg I think SSRC collision is exactly the issue! rand.Uint32() needs to be seeded first I believe?

I will update all the examples in the Pion repo to seed first :/ completely missed that

chrbsg commented 4 years ago

I added PRNG seeding to client/main.go and server/main.go but still see "Incoming unhandled RTP ssrc" when server calls pc.AddTransceiver before pc.AddTrack.

Two other odd things, possibly related:

Code (see index.html for web): code2.zip

Sean-Der commented 4 years ago

Sorry missed this @chrbsg!

The Media API is currently pretty annoying :( You need to instantiate a MediaEngine and then parse the PayloadTypes the other side is using when answering. For now I would generates offers, then the other side has to match Pion.

You can see when answering what you have to do here I am going to fix this with v3 though! I will start making these changes aggressively in the coming weeks, but wanted to stabilize with better stats and re-negotiation before breaking people.

thanks!

Sean-Der commented 4 years ago

I am not sure about the ordering of AddTrack and AddTransceiver causing issues. They both should just be creating a sendrecv transceiver. Since this happens pion-to-pion I will whip up a test and get this fixed!

chrbsg commented 4 years ago

I opened #1030 to track the Firefox bug, since it now appears to have a different root cause to this AddTransceiver/AddTrack ordering issue.

Sean-Der commented 4 years ago

Hey @chrbsg

Another PayloadType juggling issue :( I am tracking all of this with https://github.com/pion/webrtc/projects/17

If you have any other ideas on how to make things better for building media applications I would love to hear!

thanks

chrbsg commented 4 years ago

@Sean-Der One idea... automatically handling RTCP, or a demo of how to do RTCP properly would have been useful. The only demo code I found referring to this was the "Send a PLI on an interval so that the publisher is pushing a keyframe every rtcpPLIInterval". As a developer knowing little about RTCP it would be nice if Pion could do it. I sent audio to web browsers without any RTCP, and it worked, but I think without handling RTCP properly there could be timing/sync issues with dropped packets etc. I never did get around to it, and now we are sending audio over data channels instead, so we can sync against non-webrtc-media data.