Skip to content

Commit

Permalink
fix: Adapt AVS SDP for Firefox (#5207)
Browse files Browse the repository at this point in the history
  • Loading branch information
@atomrc
atomrc committed Nov 26, 2018
1 parent d40419a commit 95ab655
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 26 deletions.
18 changes: 17 additions & 1 deletion app/script/calling/SDPMapper.js
View file
Expand Up @@ -74,13 +74,27 @@ z.calling.SDPMapper = {
const iceCandidates = [];
let sessionDescription;

const isFirefox = z.util.Environment.browser.firefox;

const isIceRestart = flowEntity.negotiationMode() === z.calling.enum.SDP_NEGOTIATION_MODE.ICE_RESTART;
const isLocalSdp = sdpSource === z.calling.enum.SDP_SOURCE.LOCAL;
const isLocalSdpInGroup = isLocalSdp && flowEntity.isGroup;
const isOffer = rtcSdp.type === z.calling.rtc.SDP_TYPE.OFFER;

sessionDescription = isLocalSdp ? sdp.replace('UDP/TLS/', '') : sdp;

if (isFirefox) {
if (isLocalSdp) {
sessionDescription = isOffer
? sessionDescription.replace(/\bUDP\/DTLS\/SCTP\b/, 'DTLS/SCTP')
: sessionDescription;
} else {
sessionDescription = !isOffer
? sessionDescription.replace(/\bDTLS\/SCTP\b/, 'UDP/DTLS/SCTP')
: sessionDescription;
}
}

sessionDescription.split('\r\n').forEach(sdpLine => {
let outline = sdpLine;

Expand All @@ -105,7 +119,7 @@ z.calling.SDPMapper = {
sdpLines.push(sdpLine);
outline = `b=AS:${z.calling.SDPMapper.CONFIG.AUDIO_BITRATE}`;
}
} else if (z.util.Environment.browser.firefox && isLocalSdp && isOffer) {
} else if (isFirefox && isLocalSdp && isOffer) {
// Set ports to activate media in outgoing Firefox SDP to ensure enabled media
outline = sdpLine.replace(/^m=(application|video) 0/, 'm=$1 9');
}
Expand All @@ -115,6 +129,8 @@ z.calling.SDPMapper = {
sdpLines.push(sdpLine);
outline = `a=ptime:${z.calling.SDPMapper.CONFIG.AUDIO_PTIME}`;
}
} else if (isFirefox && !isLocalSdp && sdpLine.startsWith('a=sctpmap:')) {
outline = 'a=sctp-port:5000';
}

if (outline !== undefined) {
Expand Down
48 changes: 23 additions & 25 deletions app/script/calling/entities/FlowEntity.js
View file
Expand Up @@ -229,7 +229,7 @@ z.calling.entities.FlowEntity = class FlowEntity {

this.canSetLocalSdp.subscribe(canSet => {
if (canSet) {
this._setLocalSdp();
this._setLocalSdp(this.localSdp());
}
});

Expand Down Expand Up @@ -858,14 +858,13 @@ z.calling.entities.FlowEntity = class FlowEntity {
return;
}

const mappedSdp = z.calling.SDPMapper.rewriteSdp(
this.peerConnection.localDescription,
z.calling.enum.SDP_SOURCE.LOCAL,
this
);
const rawSdp = this.peerConnection.localDescription;

const mappedSdp = z.calling.SDPMapper.rewriteSdp(rawSdp, z.calling.enum.SDP_SOURCE.LOCAL, this);

Promise.resolve(mappedSdp)
.then(({iceCandidates, sdp: localSdp}) => {
this.localSdp(localSdp);
.then(({iceCandidates, sdp: transformedSdp}) => {
this.localSdp(rawSdp);

const isModeDefault = this.negotiationMode() === z.calling.enum.SDP_NEGOTIATION_MODE.DEFAULT;
if (isModeDefault && sendingOnTimeout) {
Expand Down Expand Up @@ -895,17 +894,17 @@ z.calling.entities.FlowEntity = class FlowEntity {
const logMessage = {
data: {
default: [
localSdp.type,
transformedSdp.type,
iceCandidates.length,
iceCandidateTypesLog,
this.remoteUser.name(),
this.localSdp().sdp,
transformedSdp.sdp,
],
obfuscated: [
localSdp.type,
transformedSdp.type,
iceCandidates.length,
this.callLogger.obfuscate(this.remoteUser.id),
this.callLogger.obfuscateSdp(this.localSdp().sdp),
this.callLogger.obfuscateSdp(transformedSdp.sdp),
],
},
message: `Sending local '{0}' SDP containing '{1}' ICE candidates ({2}) for flow with '{3}'\n{4}`,
Expand All @@ -914,10 +913,10 @@ z.calling.entities.FlowEntity = class FlowEntity {

this.shouldSendLocalSdp(false);

const response = localSdp.type === z.calling.rtc.SDP_TYPE.ANSWER;
const response = transformedSdp.type === z.calling.rtc.SDP_TYPE.ANSWER;
let callMessageEntity;

const additionalPayload = this._createAdditionalPayload();
const additionalPayload = this._createAdditionalPayload(transformedSdp);
const sessionId = this.callEntity.sessionId;
const inDefaultMode = this.negotiationMode() === z.calling.enum.SDP_NEGOTIATION_MODE.DEFAULT;
if (inDefaultMode) {
Expand All @@ -930,7 +929,7 @@ z.calling.entities.FlowEntity = class FlowEntity {

return this.callEntity.sendCallMessage(callMessageEntity).then(() => {
this.telemetry.time_step(z.telemetry.calling.CallSetupSteps.LOCAL_SDP_SEND);
this.callLogger.debug(`Sending local '${localSdp.type}' SDP successful`, this.localSdp());
this.callLogger.debug(`Sending local '${transformedSdp.type}' SDP successful`, transformedSdp.sdp);
});
})
.catch(error => {
Expand Down Expand Up @@ -1023,14 +1022,12 @@ z.calling.entities.FlowEntity = class FlowEntity {
* Creating local SDP succeeded.
*
* @private
* @param {RTCSessionDescription} rctSdp - New local SDP
* @param {RTCSessionDescription} rtcSdp - New local SDP
* @returns {undefined} No return value
*/
_createSdpSuccess(rctSdp) {
this.callLogger.info(`Creating '${rctSdp.type}' successful`, rctSdp);

const mappedSdp = z.calling.SDPMapper.rewriteSdp(rctSdp, z.calling.enum.SDP_SOURCE.LOCAL, this);
this.localSdp(mappedSdp.sdp);
_createSdpSuccess(rtcSdp) {
this.callLogger.info(`Creating '${rtcSdp.type}' successful`, rtcSdp);
this.localSdp(rtcSdp);
}

/**
Expand Down Expand Up @@ -1063,16 +1060,17 @@ z.calling.entities.FlowEntity = class FlowEntity {
/**
* Create the additional payload.
* @private
* @param {RTCSessionDescription} localSdp - local sdp to send
* @returns {Object} Additional payload
*/
_createAdditionalPayload() {
_createAdditionalPayload(localSdp) {
const payload = z.calling.CallMessageBuilder.createPayload(
this.conversationId,
this.selfUserId,
this.remoteUserId,
this.remoteClientId
);
const additionalPayload = Object.assign({remoteUser: this.remoteUser, sdp: this.localSdp().sdp}, payload);
const additionalPayload = Object.assign({remoteUser: this.remoteUser, sdp: localSdp.sdp}, payload);

const selfState = this.callEntity.selfState;
return z.calling.CallMessageBuilder.createPropSync(selfState, additionalPayload);
Expand All @@ -1081,11 +1079,11 @@ z.calling.entities.FlowEntity = class FlowEntity {
/**
* Sets the local Session Description Protocol on the PeerConnection.
* @private
* @param {RTCSessionDescription} localSdp - local sdp to set
* @returns {undefined} No return value
*/
_setLocalSdp() {
_setLocalSdp(localSdp) {
this.sdpStateChanging(true);
const localSdp = this.localSdp();
this.callLogger.debug(`Setting local '${localSdp.type}' SDP`, localSdp);

this.peerConnection
Expand Down
48 changes: 48 additions & 0 deletions test/unit_tests/calling/SDPMapperSpec.js
View file
Expand Up @@ -68,6 +68,52 @@ a=tcap:5 UDP/TLS/RTP/SAVP`.replace(/\n/g, '\r\n');
checkUntouchedLines(rtcSdp.sdp, remoteSdp.sdp);
});

it('adapts protocol for an offer created by Firefox > 63', () => {
const firefoxSdp = `
${sdpStr}
m=application 0 UDP/DTLS/SCTP webrtc-datachannel`.replace(/\n/g, '\r\n');

const rtcSdp = {
sdp: firefoxSdp,
type: z.calling.rtc.SDP_TYPE.OFFER,
};

z.util.Environment.browser.firefox = true;

const flowEntity = {
negotiationMode: () => '',
};

const {sdp: localSdp} = sdpMapper.rewriteSdp(rtcSdp, z.calling.enum.SDP_SOURCE.LOCAL, flowEntity);

expect(localSdp.sdp).not.toContain('UDP/DTLS/');
expect(localSdp.sdp).toContain('DTLS/SCTP');
checkUntouchedLines(rtcSdp.sdp, localSdp.sdp);
});

it('Use the most recent syntax to define the sctp port', () => {
const remoteSdp = `${sdpStr}
a=sctpmap:5000 webrtc-datachannel 1024
`.replace(/\n/g, '\r\n');

const rtcSdp = {
sdp: remoteSdp,
type: z.calling.rtc.SDP_TYPE.OFFER,
};

z.util.Environment.browser.firefox = true;

const flowEntity = {
negotiationMode: () => '',
};

const {sdp: localSdp} = sdpMapper.rewriteSdp(rtcSdp, z.calling.enum.SDP_SOURCE.REMOTE, flowEntity);

expect(localSdp.sdp).not.toContain('a=sctpmap:5000 webrtc-datachannel 1024');
expect(localSdp.sdp).toContain('a=sctp-port:5000');
checkUntouchedLines(rtcSdp.sdp, localSdp.sdp);
});

it('adds the browser name and app version for local SDP', () => {
const rtcSdp = {
sdp: sdpStr,
Expand Down Expand Up @@ -192,6 +238,8 @@ a=candidate:750991856 1 udp 25108223 237.30.30.30 58779 typ relay raddr 47.61.61
.replace(/b=AS:.*?(\r\n|$)/g, '')
.replace(/a=ptime:.*?(\r\n|$)/g, '')
.replace(/m=(application|video).*?(\r\n|$)/g, '')
.replace(/a=sctpmap:.*?(\r\n|$)/g, '')
.replace(/a=sctp-port:.*?(\r\n|$)/g, '')
.replace(/a=rtpmap.*?(\r\n|$)/g, '');
};

Expand Down

0 comments on commit 95ab655

Please sign in to comment.