[asterisk-bugs] [JIRA] (ASTERISK-26732) res_rtp_asterisk: Implement RTCP Multiplexing - breaking WebRTC in Chrome

Thu Mar 2 12:36:10 CST 2017

    [ https://issues.asterisk.org/jira/browse/ASTERISK-26732?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=235536#comment-235536 ] 

Lorenzo Miniero edited comment on ASTERISK-26732 at 3/2/17 12:34 PM:
---------------------------------------------------------------------

I don't need this, as I always use my Janus in front of Asterisk which takes care of rtcp-mux for me, but thought I'd submit this ugly patch anyway, in case it can help people using Asterisk's built-in WebRTC functionality and are affected by this issue.

This is a VERY ugly patch, which tries to solve the issue in a somewhat simple way, and as it will be clearer only solves half of the problem: the other half would require a refactoring in the way Asterisk handles incoming traffic, and is beyond my expertise. Anyway, with a couple of quick tests on both outgoing and incoming calls, it seems to work on Chrome for me, so I'm sharing it anyway.

The way the patch works is the following:

1. added new rtcpmux boolean in sip_pvt (so chan_sip only, sorry, not familiar with chan_pjsip);
2. added new AST_RTP_PROPERTY_RTCPMUX ast_rtp_property (rtp_engine.h);
3. added new rtcpmux boolean in ast_rtp (res_rtp_asterisk.c);
3. if a=rtcp-mux is found in either process_sdp_a_audio or process_sdp_a_video, the private rtcpmux is set, and we use ast_rtp_instance_set_prop to set the AST_RTP_PROPERTY_RTCPMUX property to true; this in turn sets the rtcpmux in the related ast_rtp instance to true;
4. changed a couple of checks in res_rtp_asterisk.c so that it doesn't complain if no RTCP candidate is involved;
5. if rtcpmux is set, pj_ice_sess_send_data uses the RTP component instead of the RTCP component to send data to the peer;
6. incoming traffic: big TODO (more on this in a minute);
7. negotiation-wise, in all offers and answers originated by Asterisk we add a=rtcpmux for both audio and video (for answers should check if it was offered, but I'm lazy... told you it's ugly); anyway, rtcp-mux is only enabled if the peer added it themselves.

I was saying that incoming traffic is still a TODO, and this is for a simple reason. Normally, with rtcp-mux you parse the incoming packet, check the payload type, and use that to identify whether it's RTP or RTCP: in fact, since the headers don't match, there's a range of payload types you can't use as they'd match RTCP traffic. Once you identify the packet, you then treat it as RTP or RTCP in the code.

In Asterisk, you can't do that in an easy way, as traffic is not push based (network pushing data to your logic), but pull-based, with manual calls to ast_rtp_read that specify whether you want to read an RTP or a RTCP packet. Considering the ambiguity rtcp-mux introduces, you cannot know in advance if the next packet you'll get from the network buffer will be RTP or RTCP. This means that, at the moment, I believe the code is pulling both from the RTP component, and simply dropping the RTCP packets somewhere in the process (e.g., when failing to decrypt SRTP as it's really SRTCP).

Long story short, audio works in both directions, but RTCP is broken. The only way to get this working, I guess, is some kind of refactoring that works like this:

1. ast_rtp_read is called with rtcp=0;
2. data is read;
3. a check is done: if it's RTP, it's handled as usual; if it's RTCP, it's saved for later;
4. ast_rtp_read is called with rtcp=1;
5. the oldest RTCP packet from the queue is returned and removed from the buffer;
6. etc.etc.

Not exactly sure if I'm misunderstanding the way the media processing is supposed to work in Asterisk, and if this solution makes any sense. Anyway, none of that is in this patch, so I'll leave this to further discussion.

was (Author: lminiero):
I don't need this, as I always use my Janus in front of Asterisk which takes care of rtcp-mux for me, but thought I'd submit this ugly patch anyway, in case it can help people using Asterisk's built-in WebRTC functionality and are affected by this issue.

This is a VERY ugly patch, which tries to solve the issue in a somewhat simple way, and as it will be clearer only solves half of the problem: the other half would require a refactoring in the way Asterisk handles incoming traffic, and is beyond my expertise. Anyway, with a couple of quick tests on both outgoing and incoming calls, it seems to work on Chrome for me, so I'm sharing it anyway.

The way the patch works is the following:

1. added new rtcpmux boolean in sip_pvt (so chan_sip only, sorry, not familiar with chan_pjsip);
2. added new AST_RTP_PROPERTY_RTCPMUX ast_rtp_property (rtp_engine.h);
3. added new rtcpmux boolean in ast_rtp (res_rtp_asterisk.c);
3. if a=rtcp-mux is found in either process_sdp_a_audio or process_sdp_a_video, the private rtcpmux is set, and we use ast_rtp_instance_set_prop to set the AST_RTP_PROPERTY_RTCPMUX property to true; this in turn sets the rtcpmux in the related ast_rtp instance to true;
4. changed a couple of checks in res_rtp_asterisk.c so that it doesn't complain if no RTCP candidate is done;
5. if rtcpmux is set, pj_ice_sess_send_data uses the RTP component instead of the RTCP component to send data to the peer;
6. incoming traffic: big TODO (more on this in a minute);
7. negotiation-wise, in all offers and answers originated by Asterisk we add a=rtcpmux for both audio and video (for answers should check if it was offered, but I'm lazy... told you it's ugly); anyway, rtcp-mux is only enabled if the peer added it themselves.

I was saying that incoming traffic is still a TODO, and this is for a simple reason. Normally, with rtcp-mux you parse the incoming packet, check the payload type, and use that to identify whether it's RTP or RTCP: in fact, since the headers don't match, there's a range of payload types you can't use as they'd match RTCP traffic. Once you identify the packet, you then treat it as RTP or RTCP in the code.

In Asterisk, you can't do that in an easy way, as traffic is not push based (network pushing data to your logic), but pull-based, with manual calls to ast_rtp_read that specify whether you want to read an RTP or a RTCP packet. Considering the ambiguity rtcp-mux introduces, you cannot know in advance if the next packet you'll get from the network buffer will be RTP or RTCP. This means that, at the moment, I believe the code is pulling both from the RTP component, and simply dropping the RTCP packets somewhere in the process (e.g., when failing to decrypt SRTP as it's really SRTCP).

Long story short, audio works in both directions, but RTCP is broken. The only way to get this working, I guess, is some kind of refactoring that works like this:

1. ast_rtp_read is called with rtcp=0;
2. data is read;
3. a check is done: if it's RTP, it's handled as usual; if it's RTCP, it's saved for later;
4. ast_rtp_read is called with rtcp=1;
5. the oldest RTCP packet from the queue is returned and removed from the buffer;
6. etc.etc.

Not exactly sure if I'm misunderstanding the way the media processing is supposed to work in Asterisk, and if this solution makes any change. Anyway, none of that is in this patch, so I'll leave this to further discussion.

> res_rtp_asterisk: Implement RTCP Multiplexing - breaking WebRTC in Chrome
> -------------------------------------------------------------------------
>
>                 Key: ASTERISK-26732
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-26732
>             Project: Asterisk
>          Issue Type: Bug
>      Security Level: None
>          Components: Resources/res_rtp_asterisk
>    Affects Versions: 13.13.1, 14.2.1
>         Environment: Chrome 57 onwards 
>            Reporter: Dan Jenkins
>            Assignee: Mark Michelson
>         Attachments: asterisk-ugly-rtcpmux.diff
>
>
> Chrome 57 has a breaking change when it comes to interop with WebRTC gateways. They've changed their previous "negotiate", to "require" when it comes to rtcp-mux
> Asterisk, as I understand it does not have rtcp multiplexing and so will break when it comes to compatibility with WebRTC across all versions of Asterisk that supports WebRTC (as far as I understand, thats back to 11 I think)
> We have a flag we can enable client side for the time being; and I'm trying to find out how long that flag will be available for - but thats no lomng term solution.
> I wrote on the mailing list about the issue - http://lists.digium.com/pipermail/asterisk-dev/2017-January/076091.html
> For comparison - I've got other systems using WebRTC which use RTPEngine (with Kamailio) and Freeswitch - both of these have enabled me to enable flags etc to get past this issue.
> I don't know what the right move is going forward. I'm just reporting the issue - every single application out there that utilises Asterisk for WebRTC will have to either move platform or enable a flag client side in the hope that Asterisk will enable the feature set required for compatibility with Chrome
> This affects Chrome 57 onwards - We're currently on Chrome 55 mid cycle - which means roughly 6 weeks until this becomes mainstream.
> If you want help reproducing this, please let me know and we can have a conversation about URLs in somewhere less public. 
> The error you'll get will be something along the lines of "setRemoteDescriptionOnFailure
> Failed to set remote answer sdp: Session error code: ERROR_CONTENT. Session error description: Failed to setup RTCP mux filter.."

--
This message was sent by Atlassian JIRA
(v6.2#6252)