search

versatica / mediasoup

Cutting Edge WebRTC Video Conferencing
https://mediasoup.org
ISC License
6.22k stars 1.13k forks source link

May need RTP MID extension when consuming #360

Closed ibc closed 4 years ago

ibc commented 4 years ago

From https://bugs.chromium.org/p/webrtc/issues/detail?id=10385#c9

Hmmm..... the a=ssrc:nnnn msid line is a leftover from Plan B, and was included (I think) to offer fallback for responders using Plan B - an offer with only one audio and one video m-line could then be understood both by Plan B responders and Unified Plan responders. It's not supposed to be part of the end-state spec.

The demux spec in BUNDLE says that when MID is not present, you should demux on MID if present, and on payload type if not present, which requires that the payload types are unique per media stream track. This is not true for your SDP - you use 101 and 102 in both sections' a=rtpmap entries. Thus, you can't demux on MID (not present), and can't demux on PT (identical), and can't demux on SSRC (not known).

Parsing a=ssrc:cname is probably a hack put in so that one could demux on SSRC in this case - was part of my proposal at https://tools.ietf.org/html/draft-alvestrand-mmusic-simulcast-ssrc-01, but I didn't pursue that due to lack of compelling feedback saying "we need this".

In short: This is not supposed to work.

We must verify if this is true:

ibc commented 4 years ago

As I answered, I don't think he is right:

https://bugs.chromium.org/p/webrtc/issues/detail?id=10385#c10

ibc commented 4 years ago

So finally it seems that MID exten is mandatory as per spec (and also a=ssrc with CNAME). Well, we should eventually enable MID exten for receivers.

ibc commented 4 years ago

@jmillan can you tale a look?

Adding a packet->Dump() before the packet is encrypted and sent to the remote consumer (at the very beginning of Transport::::OnConsumerSendRtpPacket()), it seems that the MID extension is properly set. Sure it is.

Now, the problem:

Compatibility Issue

In this consumer-mid branch, mediasoup announces support for "sendrecv" MID extension in supported capabilities. In addition, transport.consume() now generates a Consumer whose rtpParameters contain a proper .mid value (incremental string per consumer).

However just mediasoup-client consumer-mid branch is ready for this (see the diff). Other versions of mediasoup-client (or libmediasoupclient) will fail to demux received packets because MID RTP has been negotiated, RTP packets come with MID extension, however the RTP MID value does not match the a=mid of the corresponding recv transceiver.

ibc commented 4 years ago

In addition to this, I've tested by removing all a=ssrc lines in the remote SDP offer given to the PC. Chrome (Canary) should be able to demux received packets by matching the RTP MID value, but it fails. Explained here:

jmillan commented 4 years ago

@jmillan can you tale a look?

https://github.com/versatica/mediasoup/compare/consumer-mid https://github.com/versatica/mediasoup-client/compare/consumer-mid

@ibc I see it fine

ibc commented 4 years ago

And how to deal with the compatibility issue?

ibc commented 4 years ago

Regarding the Issue explained above, the only good solution is require mediasoup-client >= 3.5.0 and the same for libmediasoupclient when released.

ibc commented 4 years ago

So this is ready in consumer-mid branch. Will be merged into v3 once we have libmediasoupclient.

ibc commented 4 years ago

Done in mediasoup 3.5.2.