Closed kekkokk closed 6 years ago
fippo
commented
6 years ago Edge will only do NACK with RTX, not without (I don't have a reference for that unfortunately). But adapter (or rather the shim) shouldn't consider RTX usable when its not negotiated...
fippo
commented
6 years ago are the packets resent in plain or wrapped in rtx (pt=96)?
What does transceiver.sendEncodingParameters look like on this line: https://github.com/otalk/rtcpeerconnection-shim/blob/master/rtcpeerconnection.js#L314
kekkokk
commented
6 years ago It's wrapped in rtx pt=96 as you can see
2018-09-07 16:54:28,906 INPUT <--------
RTP PACKET
Header:
version: 2
has padding: false
has extension: true
cc: 0
marker: false
type: 100
sequence number: 13520
timestamp: 451096313
synchronization source: 3003
Extention:
code: 48862
length: 1
Extentions:
id: 1 data: 10011001 11001000 10000111
2018-09-07 16:54:28,907 OUTPUT -------->
RTCP PACKET
Header:
version: 2
padding: false
count: 1
type: 201 - RTCP Receiver Report
length: 7
Ssrc:
ssrc: 3003
Report block [ 1 / 1 ] SSRC: 3003
fraction lost: 268435456
cumulative number of packets lost: 256
seqnumcycles: 0
highestseqnum: 13520
interarrival jitter: 1465 (16.2778ms)
last SR (LSR): 262529409
delay since last SR (DLSR): 37027
RTCP PACKET
Header:
version: 2
padding: false
count: 1
type: 205 - RTCP Transport Layer Feedback Packet
length: 3
Ssrc:
ssrc: 3003
Feedback message type:
fmt: Generic NACK [1]
SSRC of media source:
ssrc:3003
NACK block:
pid: 13519
blp: 0000000000000000
2018-09-07 16:54:28,922 INPUT <--------
RTP PACKET
Header:
version: 2
has padding: false
has extention: true
cc: 0
marker: true
type: 96
sequence number: 26710
timestamp: 451090555
synchronization source: 3004
Extention:
code: 48862
length: 1
Extentions:
id: 1 data: 10011001 11011000 10011001
original seq number: 13519
ah... createAnswer has this code:
// Calculate intersection of capabilities.
var commonCapabilities = getCommonCapabilities(
transceiver.localCapabilities,
transceiver.remoteCapabilities);
var hasRtx = commonCapabilities.codecs.filter(function(c) {
return c.name.toLowerCase() === 'rtx';
}).length;
if (!hasRtx && transceiver.sendEncodingParameters[0].rtx) {
delete transceiver.sendEncodingParameters[0].rtx;
}but setRemoteDescription does not (yet). I should have a fix shortly...
fippo
commented
6 years ago Tentative fix + test here: https://github.com/otalk/rtcpeerconnection-shim/pull/159 If you could give it a try that would be great
kekkokk
commented
6 years ago I'll let you know asap, probably on monday
kekkokk
commented
6 years ago I confirm that Edge, with this fix, stop sending wrong rtx in 96. Have a nice day @fippo !
fippo
commented
6 years ago ok... making a patch the release of the shim and will close this issue once done. Thanks for finding that one!
fippo
commented
6 years ago fixed with 1.2.14 of https://www.npmjs.com/package/rtcpeerconnection-shim
kekkokk
commented
6 years ago Hi @fippo, sorry for bothering you but we are dealing with this "RTX" thread again, and found another bug (maybe), but it's not very correlated to adapter. Seems that Edge, instead of Chrome, is sending in the RTX channel the padding for the BWE estimation (I think). It's strange because I'm getting packets with strange calculated length and we have reasons to believe that it's a bug in the Edge webrtc stack.
Take for example this packet:
LENGHT: 272
***************
RTP PACKET
Header:
version: 2
has padding: true
has extention: true
cc: 0
marker: true
type: 96
sequence number: 29031
timestamp: 201402033
synchronization source: 3004
Extention:
code: 0XBEDE
length: 1
Extentions:
id: 1 data: 00101100 11001010 00110110
RTX_OSN: 43845
Padding:
bytes: 252As you can see, the packet has a length of 272, the padding is 252 And the remaining bytes are 20; But, we have 12 bytes of header, 8 (4 extension header + 4 of extension (abs-send-time in this case)). This seems correct, but as RFC says, if it's an RTX packet it MUST contain the OSN field, that is another 2 bytes. So, we think that padding should be 250 and not 252. Seems that Edge is counting OSN as padding. But what if it's an RTX packet with a vp8 payload + some padding? This will obviously result in a compromised vp8 data.
So, what do you think about this argument? are we missing something from RFC or do you think we might forward the issue to Bernard? In this case, how can I contact him?
aboba
commented
6 years ago Best to file a bug on the Edge site, then send email to me (bernarda@microsoft.com).
fippo
commented
6 years ago could reproduce, wireshark interpretes the whole thing as padding :-/ But maybe the rtx with padding is just for bandwidth probing?
kekkokk
commented
6 years ago Yeah I believe that is only for probing, since they are copies of already sent packets (their own seqnum increases but the OSN is the same at every burst) in the main SSRC. They are not consecutives, but seems to follow a pattern, so yeah, I think it's only for probing. Anyway, the length in the last byte shouldn't count the OSN field (I really haven't found any RFC yet about this). I don't know how to react serverside to this. 'Cause if one day edge (I do not see why would happen but, anyway) would send an rtx with both vp8 and padding attached, and serverside we rely on what is written in the last byte, the image would be corrupted since the last 2 bytes of the payload will be stripped.
There are not other implementations in other browser to compare, since firefox and chrome don't send padding if not simulcasting.
Would be also nice to implement what was the main topic of this thread, sending nacks responses in the main channel if rtx is not supported.
Thanks guys for the attention, have a nice day. PS. Will open an issue tomorrow.
Hi @fippo, I'm experiencing some issue that I don't know if they are related directly to EDGE implementation or adapter.
Scenario: Edge publisher (sendonly) ---> SFU (Licode) receiving a VP8 + opus streams:
These are the Sdps that EDGE offer to SFU:
And this is the SFU answer:
as you can notice, EDGE try to signal in the offer:
but the SFU answer without these lines. This is telling edge we don't support multiple ssrcs for the same video. this works fine in chrome and firefox, but if we start to generate packet loss and then sending NACKS, Edge will respond to these nacks in the secondary SSRC for video (3004) instead of sending them in the main SSRC (3003). So basically it ignores our answer.
Chrome instead, is sending nacks in the second FID ssrc only if we respond with an sdp that support rtx/9000 with the corresponding fmtp, otherwise works as expected and send everything in the main video ssrc.
What do you think? Thanks!
ps. using adapter 6.3.2