Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(JingleSession) Convert Jingle->SDP directly w/o interop layer. #2590

Merged
merged 5 commits into from
Oct 29, 2024
Merged

feat(JingleSession) Convert Jingle->SDP directly w/o interop layer. #2590

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b0704e9
feat(JingleSession) Convert Jingle->SDP directly w/o interop layer.
jallamsetty1 Oct 24, 2024
e886dd0
Update modules/RTC/TraceablePeerConnection.js
jallamsetty1 Oct 25, 2024
eae7c4d
ref(SDP) Introduce an enum for ssrc-group semantics.
jallamsetty1 Oct 28, 2024
f397968
Update modules/RTC/TraceablePeerConnection.js
jallamsetty1 Oct 29, 2024
d829dd8
squash: Address review comments
jallamsetty1 Oct 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions modules/RTC/MockClasses.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,13 @@ export class MockPeerConnection {
return Array.from(codecs);
}

/**
* {@link TraceablePeerConnection.getDesiredMediaDirection}.
*/
getDesiredMediaDirection() {
return 'sendrecv';
}

/**
* {@link TraceablePeerConnection.isSpatialScalabilityOn}.
*
Expand Down Expand Up @@ -231,6 +238,12 @@ export class MockPeerConnection {
return false;
}

/**
* {@link TraceablePeerConnection.updateRemoteSources}.
*/
updateRemoteSources() {
}

/**
* {@link TraceablePeerConnection.usesUnifiedPlan}.
*/
Expand Down
116 changes: 64 additions & 52 deletions modules/RTC/TraceablePeerConnection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { getLogger } from '@jitsi/logger';
import { Interop } from '@jitsi/sdp-interop';
import { cloneDeep } from 'lodash-es';
import transform from 'sdp-transform';

import { CodecMimeType } from '../../service/RTC/CodecMimeType';
Expand All @@ -8,7 +8,7 @@ import { MediaType } from '../../service/RTC/MediaType';
import RTCEvents from '../../service/RTC/RTCEvents';
import * as SignalingEvents from '../../service/RTC/SignalingEvents';
import { getSourceIndexFromSourceName } from '../../service/RTC/SignalingLayer';
import { VIDEO_QUALITY_LEVELS } from '../../service/RTC/StandardVideoQualitySettings';
import { SSRC_GROUP_SEMANTICS, VIDEO_QUALITY_LEVELS } from '../../service/RTC/StandardVideoQualitySettings';
import { VideoType } from '../../service/RTC/VideoType';
import { VIDEO_CODEC_CHANGED } from '../../service/statistics/AnalyticsEvents';
import { SS_DEFAULT_FRAME_RATE } from '../RTC/ScreenObtainer';
Expand Down Expand Up @@ -289,8 +289,6 @@ export default function TraceablePeerConnection(
*/
this.maxstats = options.maxstats;

this.interop = new Interop();

this.simulcast = new SdpSimulcast();

/**
Expand Down Expand Up @@ -328,12 +326,26 @@ export default function TraceablePeerConnection(
this._localTrackTransceiverMids = new Map();

/**
* Holds the SSRC map for the local tracks.
* Holds the SSRC map for the local tracks mapped by their source names.
*
* @type {Map<string, TPCSSRCInfo>}
* @type {Map<string, TPCSourceInfo>}
* @property {string} msid - The track's MSID.
* @property {Array<string>} ssrcs - The SSRCs associated with the track.
* @property {Array<TPCGroupInfo>} groups - The SSRC groups associated with the track.
*/
this._localSsrcMap = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to initialize this to a new map too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some more work planned around this, i.e., getting rid of localSSRCs and remoteSSRCs in favor of these. Lets restrict this PR to only the SDP conversion, WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just looks odd that one is a map and the other one is null. If it's not a huge change it would be nice to at least have them aligned. No worries if it would break things, we can do it later.


/**
* Holds the SSRC map for the remote tracks mapped by their source names.
*
* @type {Map<string, TPCSourceInfo>}
* @property {string} mediaType - The media type of the track.
* @property {string} msid - The track's MSID.
* @property {Array<TPCGroupInfo>} groups - The SSRC groups associated with the track.
* @property {Array<string>} ssrcs - The SSRCs associated with the track.
*/
this._remoteSsrcMap = new Map();

// override as desired
this.trace = (what, info) => {
logger.trace(what, info);
Expand Down Expand Up @@ -680,7 +692,7 @@ TraceablePeerConnection.prototype.getLocalVideoSSRCs = function(localTrack) {
return ssrcs;
}

const ssrcGroup = this.isSpatialScalabilityOn() ? 'SIM' : 'FID';
const ssrcGroup = this.isSpatialScalabilityOn() ? SSRC_GROUP_SEMANTICS.SIM : SSRC_GROUP_SEMANTICS.FID;
saghul marked this conversation as resolved.
Show resolved Hide resolved

return this.localSSRCs.get(localTrack.rtcId)?.groups?.find(group => group.semantics === ssrcGroup)?.ssrcs || ssrcs;
};
Expand Down Expand Up @@ -769,45 +781,35 @@ TraceablePeerConnection.prototype.getRemoteTracks = function(endpointId, mediaTy
};

/**
* Parses the remote description and returns the sdp lines of the sources associated with a remote participant.
* Returns the remote sourceInfo for a given source name.
*
* @param {string} sourceName - The source name.
* @returns {TPCSourceInfo}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can also return null, but I think undefined is also good, see below.

*/
TraceablePeerConnection.prototype.getRemoteSourceInfoBySourceName = function(sourceName) {
return cloneDeep(this._remoteSsrcMap?.get(sourceName) ?? null);
jallamsetty1 marked this conversation as resolved.
Show resolved Hide resolved
};

/**
* Returns a map of source names and their associated SSRCs for the remote participant.
*
* @param {string} id Endpoint id of the remote participant.
* @returns {Array<string>} The sdp lines that have the ssrc information.
* @returns {Map<string, TPCSourceInfo>} The map of source names and their associated SSRCs.
*/
TraceablePeerConnection.prototype.getRemoteSourceInfoByParticipant = function(id) {
const removeSsrcInfo = [];
const removeSsrcInfo = new Map();
const remoteTracks = this.getRemoteTracks(id);

if (!remoteTracks?.length) {
return removeSsrcInfo;
}
const primarySsrcs = remoteTracks.map(track => track.getSSRC());
const sdp = new SDP(this.remoteDescription.sdp);

primarySsrcs.forEach((ssrc, idx) => {
for (const media of sdp.media) {
let lines = '';
let ssrcLines = SDPUtil.findLines(media, `a=ssrc:${ssrc}`);

if (ssrcLines.length) {
if (!removeSsrcInfo[idx]) {
removeSsrcInfo[idx] = '';
}

// Check if there are any FID groups present for the primary ssrc.
const fidLines = SDPUtil.findLines(media, `a=ssrc-group:FID ${ssrc}`);

if (fidLines.length) {
const secondarySsrc = fidLines[0].split(' ')[2];

lines += `${fidLines[0]}\r\n`;
ssrcLines = ssrcLines.concat(SDPUtil.findLines(media, `a=ssrc:${secondarySsrc}`));
}
removeSsrcInfo[idx] += `${ssrcLines.join('\r\n')}\r\n`;
removeSsrcInfo[idx] += lines;
}
for (const [ sourceName, sourceInfo ] of this._remoteSsrcMap) {
if (sourceInfo.ssrcList?.some(ssrc => primarySsrcs.includes(Number(ssrc)))) {
removeSsrcInfo.set(sourceName, sourceInfo);
}
});
}

return removeSsrcInfo;
};
Expand Down Expand Up @@ -907,7 +909,7 @@ TraceablePeerConnection.prototype._remoteTrackAdded = function(stream, track, tr
return;
}

const remoteSDP = new SDP(this.peerconnection.remoteDescription.sdp);
const remoteSDP = new SDP(this.remoteDescription.sdp);
let mediaLine;

// Find the matching mline using 'mid' or the 'msid' attr of the stream.
Expand Down Expand Up @@ -1136,6 +1138,9 @@ TraceablePeerConnection.prototype._removeRemoteTrack = function(toBeRemoved) {
* @returns {void}
*/
TraceablePeerConnection.prototype._processAndExtractSourceInfo = function(localSDP) {
/**
* @type {Map<string, TPCSourceInfo>} The map of source names and their associated SSRCs.
*/
const ssrcMap = new Map();

if (!localSDP || typeof localSDP !== 'string') {
Expand All @@ -1145,7 +1150,7 @@ TraceablePeerConnection.prototype._processAndExtractSourceInfo = function(localS
const media = session.media.filter(mline => mline.direction === MediaDirection.SENDONLY
|| mline.direction === MediaDirection.SENDRECV);

if (!Array.isArray(media)) {
if (!media.length) {
this._localSsrcMap = ssrcMap;

return;
Expand Down Expand Up @@ -1178,14 +1183,14 @@ TraceablePeerConnection.prototype._processAndExtractSourceInfo = function(localS
ssrcInfo.groups.push(group);
}

const simGroup = ssrcGroups.find(group => group.semantics === 'SIM');
const simGroup = ssrcGroups.find(group => group.semantics === SSRC_GROUP_SEMANTICS.SIM);

// Add a SIM group if its missing in the description (happens on Firefox).
if (this.isSpatialScalabilityOn() && !simGroup) {
const groupSsrcs = ssrcGroups.map(group => group.ssrcs[0]);

ssrcInfo.groups.push({
semantics: 'SIM',
semantics: SSRC_GROUP_SEMANTICS.SIM,
ssrcs: groupSsrcs
});
}
Expand Down Expand Up @@ -1231,7 +1236,7 @@ TraceablePeerConnection.prototype._injectSsrcGroupForUnifiedSimulcast = function

// Check if the browser supports RTX, add only the primary ssrcs to the SIM group if that is the case.
video.ssrcGroups = video.ssrcGroups || [];
const fidGroups = video.ssrcGroups.filter(group => group.semantics === 'FID');
const fidGroups = video.ssrcGroups.filter(group => group.semantics === SSRC_GROUP_SEMANTICS.FID);

if (video.simulcast || video.simulcast_03) {
const ssrcs = [];
Expand All @@ -1247,7 +1252,7 @@ TraceablePeerConnection.prototype._injectSsrcGroupForUnifiedSimulcast = function
}
});
}
if (video.ssrcGroups.find(group => group.semantics === 'SIM')) {
if (video.ssrcGroups.find(group => group.semantics === SSRC_GROUP_SEMANTICS.SIM)) {
// Group already exists, no need to do anything
return desc;
}
Expand All @@ -1257,7 +1262,7 @@ TraceablePeerConnection.prototype._injectSsrcGroupForUnifiedSimulcast = function
const simSsrcs = ssrcs.slice(i, i + 3);

video.ssrcGroups.push({
semantics: 'SIM',
semantics: SSRC_GROUP_SEMANTICS.SIM,
ssrcs: simSsrcs.join(' ')
});
}
Expand Down Expand Up @@ -1315,10 +1320,6 @@ const getters = {
if (this.isP2P) {
// Adjust the media direction for p2p based on whether a local source has been added.
desc = this._adjustRemoteMediaDirection(desc);
} else {
// If this is a jvb connection, transform the SDP to Plan B first.
desc = this.interop.toPlanB(desc);
this.trace('getRemoteDescription::postTransform (Plan B)', dumpSDP(desc));
}

return desc;
Expand Down Expand Up @@ -1591,7 +1592,7 @@ TraceablePeerConnection.prototype.getConfiguredVideoCodec = function(localTrack)
return codecs[0].mimeType.split('/')[1].toLowerCase();
}

const sdp = this.peerconnection.remoteDescription?.sdp;
const sdp = this.remoteDescription?.sdp;
const defaultCodec = CodecMimeType.VP8;

if (!sdp) {
Expand Down Expand Up @@ -1837,6 +1838,23 @@ TraceablePeerConnection.prototype.removeTrackFromPc = function(localTrack) {
return this.tpcUtils.replaceTrack(localTrack, null).then(() => false);
};

/**
* Updates the remote source map with the given source map for adding or removing sources.
*
* @param {Map<string, TPCSourceInfo>} sourceMap - The map of source names to their corresponding SSRCs.
* @param {boolean} isAdd - Whether the sources are being added or removed.
* @returns {void}
*/
TraceablePeerConnection.prototype.updateRemoteSources = function(sourceMap, isAdd) {
for (const [ sourceName, ssrcInfo ] of sourceMap) {
if (isAdd) {
this._remoteSsrcMap.set(sourceName, ssrcInfo);
} else {
this._remoteSsrcMap.delete(sourceName);
}
}
};

/**
* Returns true if the codec selection API is used for switching between codecs for the video sources.
*
Expand Down Expand Up @@ -2217,12 +2235,6 @@ TraceablePeerConnection.prototype.setRemoteDescription = function(description) {
// Munge stereo flag and opusMaxAverageBitrate based on config.js
remoteDescription = this._mungeOpus(remoteDescription);

if (!this.isP2P) {
const currentDescription = this.peerconnection.remoteDescription;

remoteDescription = this.interop.toUnifiedPlan(remoteDescription, currentDescription);
this.trace('setRemoteDescription::postTransform (Unified)', dumpSDP(remoteDescription));
}
if (this.isSpatialScalabilityOn()) {
remoteDescription = this.tpcUtils.insertUnifiedPlanSimulcastReceive(remoteDescription);
this.trace('setRemoteDescription::postTransform (sim receive)', dumpSDP(remoteDescription));
Expand Down
7 changes: 4 additions & 3 deletions modules/sdp/RtxModifier.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { getLogger } from '@jitsi/logger';

import { MediaDirection } from '../../service/RTC/MediaDirection';
import { MediaType } from '../../service/RTC/MediaType';
import { SSRC_GROUP_SEMANTICS } from '../../service/RTC/StandardVideoQualitySettings';

import SDPUtil from './SDPUtil';
import { SdpTransformWrap, parseSecondarySSRC } from './SdpTransformUtil';
Expand Down Expand Up @@ -48,7 +49,7 @@ function updateAssociatedRtxStream(mLine, primarySsrcInfo, rtxSsrc) {
value: primarySsrcMsid
});
mLine.addSSRCGroup({
semantics: 'FID',
semantics: SSRC_GROUP_SEMANTICS.FID,
ssrcs: `${primarySsrc} ${rtxSsrc}`
});
}
Expand Down Expand Up @@ -188,10 +189,10 @@ export default class RtxModifier {
if (videoMLine.direction !== MediaDirection.RECVONLY
&& videoMLine.getSSRCCount()
&& videoMLine.containsAnySSRCGroups()) {
const fidGroups = videoMLine.findGroups('FID');
const fidGroups = videoMLine.findGroups(SSRC_GROUP_SEMANTICS.FID);

// Remove the fid groups from the mline
videoMLine.removeGroupsBySemantics('FID');
videoMLine.removeGroupsBySemantics(SSRC_GROUP_SEMANTICS.FID);

// Get the rtx ssrcs and remove them from the mline
for (const fidGroup of fidGroups) {
Expand Down
Loading