From 8d5867d116fbdf26b824976b1748595774d6cbbd Mon Sep 17 00:00:00 2001 From: Ethan Hugg Date: Thu, 28 Feb 2013 13:17:42 -0800 Subject: [PATCH] Bug 837035 - Support DataChannels with SDP Negotiation r=jesup --- .../signaling/src/media/VcmSIPCCBinding.cpp | 43 ++++++++++++- .../src/peerconnection/PeerConnectionImpl.cpp | 15 ++++- .../src/peerconnection/PeerConnectionImpl.h | 3 + .../signaling/src/sipcc/core/gsm/gsm_sdp.c | 64 ++++++++++--------- .../signaling/src/sipcc/core/gsm/h/fsm.h | 8 ++- .../signaling/src/sipcc/core/gsm/h/lsm.h | 2 +- .../webrtc/signaling/src/sipcc/core/gsm/lsm.c | 59 +++++++++-------- .../webrtc/signaling/src/sipcc/core/sdp/sdp.h | 1 + .../signaling/src/sipcc/core/sdp/sdp_access.c | 25 ++++++++ .../src/sipcc/core/sdp/sdp_attr_access.c | 2 + .../webrtc/signaling/src/sipcc/include/vcm.h | 5 +- netwerk/sctp/datachannel/Makefile.in | 1 + netwerk/sctp/src/Makefile.in | 1 + 13 files changed, 162 insertions(+), 67 deletions(-) diff --git a/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp b/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp index 19a4ecc7828a..e149eead9067 100644 --- a/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp +++ b/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp @@ -1046,18 +1046,57 @@ short vcmGetDtlsIdentity(const char *peerconnection, * * @param[in] peerconnection - the peerconnection in use * @param[in] streams - the number of streams for this data channel - * @param[in] sctp_port - the negotiated sctp port + * @param[in] local_datachannel_port - the negotiated sctp port + * @param[in] remote_datachannel_port - the negotiated sctp port * @param[in] protocol - the protocol as a string * * @return 0 success, error failure */ -short vcmSetDataChannelParameters(const char *peerconnection, cc_uint16_t streams, int sctp_port, const char* protocol) +static short vcmInitializeDataChannel_m(const char *peerconnection, cc_uint16_t streams, + int local_datachannel_port, int remote_datachannel_port, const char* protocol) { + nsresult res; + CSFLogDebug( logTag, "%s: PC = %s", __FUNCTION__, peerconnection); + sipcc::PeerConnectionWrapper pc(peerconnection); + ENSURE_PC(pc, VCM_ERROR); + + res = pc.impl()->InitializeDataChannel(local_datachannel_port, remote_datachannel_port, streams); + if (NS_FAILED(res)) { + return VCM_ERROR; + } + return 0; } +/* Set negotiated DataChannel parameters. + * + * @param[in] peerconnection - the peerconnection in use + * @param[in] streams - the number of streams for this data channel + * @param[in] local_datachannel_port - the negotiated sctp port + * @param[in] remote_datachannel_port - the negotiated sctp port + * @param[in] protocol - the protocol as a string + * + * @return 0 success, error failure + */ +short vcmInitializeDataChannel(const char *peerconnection, cc_uint16_t streams, + int local_datachannel_port, int remote_datachannel_port, const char* protocol) +{ + short ret; + + VcmSIPCCBinding::getMainThread()->Dispatch( + WrapRunnableNMRet(&vcmInitializeDataChannel_m, + peerconnection, + streams, + local_datachannel_port, + remote_datachannel_port, + protocol, + &ret), + NS_DISPATCH_SYNC); + + return ret; +} /** diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp index 6091f162002f..291e1e519e5d 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp @@ -610,11 +610,22 @@ PeerConnectionImpl::CreateFakeMediaStream(uint32_t aHint, nsIDOMMediaStream** aR return NS_OK; } +// Stubbing this call out for now. +// We can remove it when we are confident of datachannels being started +// correctly on SDP negotiation +NS_IMETHODIMP +PeerConnectionImpl::ConnectDataConnection(uint16_t aLocalport, + uint16_t aRemoteport, + uint16_t aNumstreams) +{ + return NS_OK; // InitializeDataChannel(aLocalport, aRemoteport, aNumstreams); +} + // Data channels won't work without a window, so in order for the C++ unit // tests to work (it doesn't have a window available) we ifdef the following // two implementations. -NS_IMETHODIMP -PeerConnectionImpl::ConnectDataConnection(uint16_t aLocalport, +nsresult +PeerConnectionImpl::InitializeDataChannel(uint16_t aLocalport, uint16_t aRemoteport, uint16_t aNumstreams) { diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h index 9a81db18b073..347a9c233345 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h @@ -223,6 +223,9 @@ public: NS_IMETHODIMP CreateOffer(MediaConstraints& aConstraints); NS_IMETHODIMP CreateAnswer(MediaConstraints& aConstraints); + nsresult + InitializeDataChannel(uint16_t aLocalport, uint16_t aRemoteport, uint16_t aNumstreams); + // Called whenever something is unrecognized by the parser // May be called more than once and does not necessarily mean // that parsing was stopped, only that something was unrecognized. diff --git a/media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c b/media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c index 829a51ac6cfd..f9bad5e2c6a6 100644 --- a/media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c +++ b/media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c @@ -532,7 +532,12 @@ gsmsdp_init_media (fsmdef_media_t *media) media->video = NULL; media->candidate_ct = 0; media->rtcp_mux = FALSE; - media->protocol = NULL; + + media->local_datachannel_port = 0; + media->remote_datachannel_port = 0; + media->datachannel_streams = WEBRTC_DATACHANNEL_STREAMS_DEFAULT; + sstrncpy(media->datachannel_protocol, WEBRTC_DATA_CHANNEL_PROT, SDP_MAX_STRING_LEN); + media->payloads = NULL; media->num_payloads = 0; } @@ -1454,11 +1459,11 @@ gsmsdp_set_sctp_attributes (void *sdp_p, uint16_t level, fsmdef_media_t *media) } /* Use SCTP port in place of fmtp payload type */ - (void) sdp_attr_set_fmtp_payload_type(sdp_p, level, 0, a_inst, media->sctp_port); + (void) sdp_attr_set_fmtp_payload_type(sdp_p, level, 0, a_inst, media->local_datachannel_port); - sdp_attr_set_fmtp_data_channel_protocol (sdp_p, level, 0, a_inst, WEBRTC_DATA_CHANNEL_PROT); + sdp_attr_set_fmtp_data_channel_protocol (sdp_p, level, 0, a_inst, media->datachannel_protocol); - sdp_attr_set_fmtp_streams (sdp_p, level, 0, a_inst, 16); + sdp_attr_set_fmtp_streams (sdp_p, level, 0, a_inst, media->datachannel_streams); } /* @@ -2444,7 +2449,7 @@ gsmsdp_update_local_sdp_media (fsmdef_dcb_t *dcb_p, cc_sdp_t *cc_sdp_p, (void) sdp_set_media_type(sdp_p, level, media->type); - (void) sdp_set_media_portnum(sdp_p, level, port, media->sctp_port); + (void) sdp_set_media_portnum(sdp_p, level, port, media->local_datachannel_port); /* Set media transport and crypto attributes if it is for SRTP */ gsmsdp_update_local_sdp_media_transport(dcb_p, sdp_p, media, transport, @@ -3303,27 +3308,37 @@ gsmsdp_negotiate_codec (fsmdef_dcb_t *dcb_p, cc_sdp_t *sdp_p, return (RTP_NONE); } +/* + * gsmsdp_negotiate_datachannel_attribs + * + * dcb_p - a pointer to the current dcb + * sdp_p - the sdp we are analyzing + * media - the media info + * offer - Boolean indicating if the remote SDP came in an OFFER. + */ static void -gsmsdp_negotiate_datachannel_attribs(fsmdef_dcb_t* dcb_p, cc_sdp_t* sdp_p, uint16_t level, fsmdef_media_t* media) +gsmsdp_negotiate_datachannel_attribs(fsmdef_dcb_t* dcb_p, cc_sdp_t* sdp_p, uint16_t level, + fsmdef_media_t* media, boolean offer) { uint32 num_streams; char *protocol; sdp_attr_get_fmtp_streams (sdp_p->dest_sdp, level, 0, 1, &num_streams); - media->streams = num_streams; + media->datachannel_streams = num_streams; - if(media->protocol == NULL) { - media->protocol = cpr_malloc(SDP_MAX_STRING_LEN+1); - if (media->protocol == NULL) - return; + sdp_attr_get_fmtp_data_channel_protocol(sdp_p->dest_sdp, level, 0, 1, media->datachannel_protocol); + + media->remote_datachannel_port = sdp_get_media_sctp_port(sdp_p->dest_sdp, level); + + /* + * TODO: remove the following block when SCTP code is updated + * See bug 837035 comment #5 + */ + if (offer) { + /* Increment port for answer SDP */ + media->local_datachannel_port = media->remote_datachannel_port + 1; } - sdp_attr_get_fmtp_data_channel_protocol(sdp_p->dest_sdp, level, 0, 1, media->protocol); - - media->sctp_port = sdp_attr_get_fmtp_payload_type (sdp_p->dest_sdp, level, 0, 1); - - /* Increment port for answer SDP */ - media->sctp_port++; } /* @@ -4516,7 +4531,7 @@ gsmsdp_negotiate_media_lines (fsm_fcb_t *fcb_p, cc_sdp_t *sdp_p, boolean initial break; } } else { - gsmsdp_negotiate_datachannel_attribs(dcb_p, sdp_p, i, media); + gsmsdp_negotiate_datachannel_attribs(dcb_p, sdp_p, i, media, offer); } /* @@ -4635,17 +4650,6 @@ gsmsdp_negotiate_media_lines (fsm_fcb_t *fcb_p, cc_sdp_t *sdp_p, boolean initial MOZ_ASSERT(result); /* TODO(ekr@rtfm.com) add real error checking, but this "can't fail" */ - } else { - /* - * Inform VCM that a Data Channel has been negotiated - */ - int pc_stream_id; /* Set but unused. Provided to - fulfill the API contract - TODO(adam@nostrum.com): - use or remove */ - - lsm_data_channel_negotiated(dcb_p->line, dcb_p->call_id, - media, &pc_stream_id); } } } @@ -5066,7 +5070,7 @@ gsmsdp_add_media_line (fsmdef_dcb_t *dcb_p, const cc_media_cap_t *media_cap, if(media_cap->type == SDP_MEDIA_APPLICATION) { config_get_value(CFGID_SCTP_PORT, &sctp_port, sizeof(sctp_port)); - media->sctp_port = sctp_port; + media->local_datachannel_port = sctp_port; } /* diff --git a/media/webrtc/signaling/src/sipcc/core/gsm/h/fsm.h b/media/webrtc/signaling/src/sipcc/core/gsm/h/fsm.h index b5cd33ba4583..5138f4258b0f 100755 --- a/media/webrtc/signaling/src/sipcc/core/gsm/h/fsm.h +++ b/media/webrtc/signaling/src/sipcc/core/gsm/h/fsm.h @@ -230,13 +230,15 @@ typedef struct fsmdef_media_t_ { /* * port number used in m= data channel line */ - uint16_t sctp_port; + uint16_t local_datachannel_port; + uint16_t remote_datachannel_port; /* * Data Channel properties */ - uint32 streams; - char *protocol; +#define WEBRTC_DATACHANNEL_STREAMS_DEFAULT 16 + uint32 datachannel_streams; + char datachannel_protocol[SDP_MAX_STRING_LEN + 1]; /* * This field contains the number of elements in the payloads field. diff --git a/media/webrtc/signaling/src/sipcc/core/gsm/h/lsm.h b/media/webrtc/signaling/src/sipcc/core/gsm/h/lsm.h index b0df529747af..2b1066e1ef4d 100755 --- a/media/webrtc/signaling/src/sipcc/core/gsm/h/lsm.h +++ b/media/webrtc/signaling/src/sipcc/core/gsm/h/lsm.h @@ -174,7 +174,7 @@ lsm_util_tone_start_with_speaker_as_backup (vcm_tones_t tone, short alert_info, cc_call_handle_t call_handle, groupid_t group_id, streamid_t stream_id, uint16_t direction); -void lsm_data_channel_negotiated (line_t line, callid_t call_id, fsmdef_media_t *media, int *pc_stream_id); +void lsm_initialize_datachannel (fsmdef_dcb_t *dcb, fsmdef_media_t *media); #endif //_LSM_H_ diff --git a/media/webrtc/signaling/src/sipcc/core/gsm/lsm.c b/media/webrtc/signaling/src/sipcc/core/gsm/lsm.c index 70a10c098c93..3e0117ad8d93 100755 --- a/media/webrtc/signaling/src/sipcc/core/gsm/lsm.c +++ b/media/webrtc/signaling/src/sipcc/core/gsm/lsm.c @@ -34,6 +34,8 @@ #include "util_string.h" #include "platform_api.h" +static const char* logTag = "lsm"; + #ifndef NO #define NO (0) #endif @@ -905,7 +907,8 @@ lsm_rx_start (lsm_lcb_t *lcb, const char *fname, fsmdef_media_t *media) * For SRTP, the receive can not be opened if the remote's crypto * parameters are not received yet. */ - if (!gsmsdp_is_crypto_ready(media, TRUE)) { + if (media->type != SDP_MEDIA_APPLICATION && + !gsmsdp_is_crypto_ready(media, TRUE)) { LSM_DEBUG(DEB_L_C_F_PREFIX"%s: Not ready to open receive port (%d)\n", DEB_L_C_F_PREFIX_ARGS(LSM, dcb->line, dcb->call_id, fname1), fname, media->src_port); continue; @@ -1051,6 +1054,13 @@ lsm_rx_start (lsm_lcb_t *lcb, const char *fname, fsmdef_media_t *media) } } } + + if (media->type == SDP_MEDIA_APPLICATION) { + /* Enable datachannels + Datachannels are always two-way so initializing only here in rx_start. + */ + lsm_initialize_datachannel(dcb, media); + } } } @@ -5301,40 +5311,35 @@ void lsm_add_remote_stream (line_t line, callid_t call_id, fsmdef_media_t *media } /* - * lsm_data_channel_negotiated + * lsm_initialize_datachannel * * Description: - * The function informs the API of a negotiated data channel m= line + * The function initializes the datachannel with port and + * protocol info. * * Parameters: - * [in] line - line - * [in] call_id - GSM call ID - * [in] media - media line to add as remote stream - * [out] pc_stream_id + * [in] dcb - pointer to get the peerconnection id + * [in] media - pointer to get the datachannel info * Returns: None */ -void lsm_data_channel_negotiated (line_t line, callid_t call_id, fsmdef_media_t *media, int *pc_stream_id) +void lsm_initialize_datachannel (fsmdef_dcb_t *dcb, fsmdef_media_t *media) { - static const char fname[] = "lsm_data_channel_negotiated"; - fsmdef_dcb_t *dcb; - lsm_lcb_t *lcb; - - lcb = lsm_get_lcb_by_call_id(call_id); - if (lcb) { - dcb = lcb->dcb; - if (dcb == NULL) { - LSM_ERR_MSG(get_debug_string(DEBUG_INPUT_NULL), fname); - return; - } - - /* - * have access to media->streams, media->protocol, media->sctp_port - * vcmSetDataChannelParameters may need renaming TODO: jesup - */ - - vcmSetDataChannelParameters(dcb->peerconnection, media->streams, media->sctp_port, media->protocol); - + if (!dcb) { + CSFLogError(logTag, "%s DCB is NULL", __FUNCTION__); + return; } + + if (!media) { + CSFLogError(logTag, "%s media is NULL", __FUNCTION__); + return; + } + + /* + * have access to media->streams, media->protocol, media->local/remote_datachannel_port + */ + vcmInitializeDataChannel(dcb->peerconnection, media->datachannel_streams, + media->local_datachannel_port, media->remote_datachannel_port, + media->datachannel_protocol); } /** diff --git a/media/webrtc/signaling/src/sipcc/core/sdp/sdp.h b/media/webrtc/signaling/src/sipcc/core/sdp/sdp.h index a7ad11db6e90..22f9943964eb 100644 --- a/media/webrtc/signaling/src/sipcc/core/sdp/sdp.h +++ b/media/webrtc/signaling/src/sipcc/core/sdp/sdp.h @@ -1163,6 +1163,7 @@ extern sdp_result_e sdp_set_media_port_format(void *sdp_ptr, u16 level, sdp_port_format_e port_format); extern sdp_result_e sdp_set_media_portnum(void *sdp_ptr, u16 level, int32 portnum, int32 sctpport); +extern int32 sdp_get_media_sctp_port(void *sdp_ptr, u16 level); extern sdp_result_e sdp_set_media_portcount(void *sdp_ptr, u16 level, int32 num_ports); extern sdp_result_e sdp_set_media_vpi(void *sdp_ptr, u16 level, int32 vpi); diff --git a/media/webrtc/signaling/src/sipcc/core/sdp/sdp_access.c b/media/webrtc/signaling/src/sipcc/core/sdp/sdp_access.c index dac9069407f1..8fae68e76a0d 100644 --- a/media/webrtc/signaling/src/sipcc/core/sdp/sdp_access.c +++ b/media/webrtc/signaling/src/sipcc/core/sdp/sdp_access.c @@ -2016,6 +2016,31 @@ sdp_result_e sdp_set_media_portnum (void *sdp_ptr, u16 level, int32 portnum, int return (SDP_SUCCESS); } +/* Function: sdp_get_media_sctp_port + * Description: Gets the value of the sctp port number parameter for the m= + * media token line. + * Parameters: sdp_ptr The SDP handle returned by sdp_init_description. + * level The media level to set the param. Will be 1-n. + * Returns: sctp_port or -1 on failure + */ +int32 sdp_get_media_sctp_port(void *sdp_ptr, u16 level) +{ + sdp_t *sdp_p = (sdp_t *)sdp_ptr; + sdp_mca_t *mca_p; + + if (!sdp_verify_sdp_ptr(sdp_p)) { + return -1; + } + + mca_p = sdp_find_media_level(sdp_p, level); + if (!mca_p) { + sdp_p->conf_p->num_invalid_param++; + return -1; + } + + return mca_p->sctpport; +} + /* Function: sdp_set_media_portcount * Description: Sets the value of the port count parameter for the m= * media token line. If the port count is not valid with the diff --git a/media/webrtc/signaling/src/sipcc/core/sdp/sdp_attr_access.c b/media/webrtc/signaling/src/sipcc/core/sdp/sdp_attr_access.c index 13994455a0ea..38363cef2635 100644 --- a/media/webrtc/signaling/src/sipcc/core/sdp/sdp_attr_access.c +++ b/media/webrtc/signaling/src/sipcc/core/sdp/sdp_attr_access.c @@ -5,6 +5,7 @@ #include "sdp_os_defs.h" #include "sdp.h" #include "sdp_private.h" +#include "fsm.h" #include "CSFLog.h" static const char* logTag = "sdp_attr_access"; @@ -6659,6 +6660,7 @@ sdp_result_e sdp_attr_get_fmtp_streams (void *sdp_ptr, u16 level, "not found.", sdp_p->debug_str, level, inst_num); } sdp_p->conf_p->num_invalid_param++; + *val = WEBRTC_DATACHANNEL_STREAMS_DEFAULT; return (SDP_INVALID_PARAMETER); } else { *val = attr_p->attr.fmtp.streams; diff --git a/media/webrtc/signaling/src/sipcc/include/vcm.h b/media/webrtc/signaling/src/sipcc/include/vcm.h index 6db623087b92..711ff0a79a30 100755 --- a/media/webrtc/signaling/src/sipcc/include/vcm.h +++ b/media/webrtc/signaling/src/sipcc/include/vcm.h @@ -672,9 +672,10 @@ int vcmTxStart(cc_mcapid_t mcap_id, size_t max_digest_len); - short vcmSetDataChannelParameters(const char *peerconnection, + short vcmInitializeDataChannel(const char *peerconnection, cc_uint16_t streams, - int sctp_port, + int local_datachannel_port, + int remote_datachannel_port, const char* protocol); /*! diff --git a/netwerk/sctp/datachannel/Makefile.in b/netwerk/sctp/datachannel/Makefile.in index 7472d0b02546..6801d917e8ff 100644 --- a/netwerk/sctp/datachannel/Makefile.in +++ b/netwerk/sctp/datachannel/Makefile.in @@ -13,6 +13,7 @@ include $(DEPTH)/config/autoconf.mk LIBRARY_NAME = nkdatachan_s LIBXUL_LIBRARY = 1 FORCE_STATIC_LIB = 1 +NO_PROFILE_GUIDED_OPTIMIZE = 1 # Don't PGO EXPORTS_NAMESPACES = mozilla/net diff --git a/netwerk/sctp/src/Makefile.in b/netwerk/sctp/src/Makefile.in index 173ded319138..09e8c728fa29 100644 --- a/netwerk/sctp/src/Makefile.in +++ b/netwerk/sctp/src/Makefile.in @@ -17,6 +17,7 @@ include $(DEPTH)/config/autoconf.mk LIBRARY_NAME = nksctp_s LIBXUL_LIBRARY = 1 FORCE_STATIC_LIB = 1 +NO_PROFILE_GUIDED_OPTIMIZE = 1 # Don't PGO EXPORTS_NAMESPACES = mozilla/net