Skip to content

Commit

Permalink
fix(TPC) Use videoType from 'source-add' for remote track creation. (#…
Browse files Browse the repository at this point in the history
…2596)

* fix(TPC) Use videoType from 'source-add' for remote track creation.
If 'source-add' for a remote video source is received before presence for that source, videoType will default to 'camera' and the client wouldn't be able to create the virtual participant tile for rendering the desktop track.

* squash: Include the videoType for no SSRC-rewriting case.
  • Loading branch information
jallamsetty1 authored Nov 14, 2024
1 parent 0ef8314 commit bc446e9
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 40 deletions.
62 changes: 27 additions & 35 deletions modules/RTC/TraceablePeerConnection.js
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,8 @@ export default function TraceablePeerConnection(
* @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.
* @property {Array<string>} ssrcList - The SSRCs associated with the track.
* @property {VideoType} videoType - The videoType of the track (undefined for audio tracks).
*/
this._remoteSsrcMap = new Map();

Expand Down Expand Up @@ -964,47 +965,38 @@ TraceablePeerConnection.prototype._remoteTrackAdded = function(stream, track, tr

const sourceName = this.signalingLayer.getTrackSourceName(trackSsrc);
const peerMediaInfo = this.signalingLayer.getPeerMediaInfo(ownerEndpointId, mediaType, sourceName);
const trackDetails = {
mediaType,
muted: peerMediaInfo?.muted ?? true,
stream,
track,
ssrc: trackSsrc,
videoType: peerMediaInfo?.videoType
};

// Assume default presence state for remote source. Presence can be received after source signaling. This shouldn't
// prevent the endpoint from creating a remote track for the source.
let muted = true;
let videoType = mediaType === MediaType.VIDEO ? VideoType.CAMERA : undefined; // 'camera' by default

if (peerMediaInfo) {
muted = peerMediaInfo.muted;
videoType = peerMediaInfo.videoType; // can be undefined
} else {
logger.info(`${this}: no source-info available for ${ownerEndpointId}:${sourceName}, assuming default state`);
if (this._remoteSsrcMap.has(sourceName) && mediaType === MediaType.VIDEO) {
trackDetails.videoType = this._remoteSsrcMap.get(sourceName).videoType;
}

this._createRemoteTrack(ownerEndpointId, stream, track, mediaType, videoType, trackSsrc, muted, sourceName);
this._createRemoteTrack(ownerEndpointId, sourceName, trackDetails);
};

// FIXME cleanup params
/* eslint-disable max-params */

/**
* Initializes a new JitsiRemoteTrack instance with the data provided by
* the signaling layer and SDP.
* Initializes a new JitsiRemoteTrack instance with the data provided by the signaling layer and SDP.
*
* @param {string} ownerEndpointId the owner's endpoint ID (MUC nickname)
* @param {MediaStream} stream the WebRTC stream instance
* @param {MediaStreamTrack} track the WebRTC track instance
* @param {MediaType} mediaType the track's type of the media
* @param {VideoType} [videoType] the track's type of the video (if applicable)
* @param {number} ssrc the track's main SSRC number
* @param {boolean} muted the initial muted status
* @param {String} sourceName the track's source name
*/
TraceablePeerConnection.prototype._createRemoteTrack = function(
ownerEndpointId,
stream,
track,
mediaType,
videoType,
ssrc,
muted,
sourceName) {
* @param {string} ownerEndpointId - The owner's endpoint ID (MUC nickname)
* @param {String} sourceName - The track's source name
* @param {Object} trackDetails - The track's details.
* @param {MediaType} trackDetails.mediaType - media type, 'audio' or 'video'.
* @param {boolean} trackDetails.muted - The initial muted status.
* @param {number} trackDetails.ssrc - The track's main SSRC number.
* @param {MediaStream} trackDetails.stream - The WebRTC stream instance.
* @param {MediaStreamTrack} trackDetails.track - The WebRTC track instance.
* @param {VideoType} trackDetails.videoType - The track's type of the video (if applicable).
*/
TraceablePeerConnection.prototype._createRemoteTrack = function(ownerEndpointId, sourceName, trackDetails) {
const { mediaType, muted, ssrc, stream, track, videoType } = trackDetails;

logger.info(`${this} creating remote track[endpoint=${ownerEndpointId},ssrc=${ssrc},`
+ `type=${mediaType},sourceName=${sourceName}]`);
let remoteTracksMap;
Expand Down
17 changes: 14 additions & 3 deletions modules/xmpp/JingleHelperFunctions.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { $build } from 'strophe.js';

import { MediaType } from '../../service/RTC/MediaType';
import { SSRC_GROUP_SEMANTICS } from '../../service/RTC/StandardVideoQualitySettings';
import { VideoType } from '../../service/RTC/VideoType';
import { XEP } from '../../service/xmpp/XMPPExtensioProtocols';

const logger = getLogger(__filename);
Expand All @@ -13,13 +14,23 @@ const logger = getLogger(__filename);
* Creates a "source" XML element for the source described in compact JSON format in [sourceCompactJson].
* @param {*} owner the endpoint ID of the owner of the source.
* @param {*} sourceCompactJson the compact JSON representation of the source.
* @param {boolean} isVideo whether the source is a video source
* @returns the created "source" XML element.
*/
function _createSourceExtension(owner, sourceCompactJson) {
function _createSourceExtension(owner, sourceCompactJson, isVideo = false) {
let videoType = sourceCompactJson.v ? VideoType.DESKTOP : undefined;

// If the video type is not specified, it is assumed to be a camera for video sources.
// Jicofo adds the video type only for desktop sharing sources.
if (!videoType && isVideo) {
videoType = VideoType.CAMERA;
}

const node = $build('source', {
xmlns: XEP.SOURCE_ATTRIBUTES,
ssrc: sourceCompactJson.s,
name: sourceCompactJson.n
name: sourceCompactJson.n,
videoType
});

if (sourceCompactJson.m) {
Expand Down Expand Up @@ -157,7 +168,7 @@ export function expandSourcesFromJson(iq, jsonMessageXml) {

if (videoSources?.length) {
for (let i = 0; i < videoSources.length; i++) {
videoRtpDescription.appendChild(_createSourceExtension(owner, videoSources[i]));
videoRtpDescription.appendChild(_createSourceExtension(owner, videoSources[i], true));
ssrcs.push(videoSources[i]?.s);
}
}
Expand Down
7 changes: 5 additions & 2 deletions modules/xmpp/JingleSessionPC.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ function _addSourceElement(description, s, ssrc_, msid) {
description.c('source', {
xmlns: XEP.SOURCE_ATTRIBUTES,
ssrc: ssrc_,
name: s.source
name: s.source,
videoType: s.videoType?.toLowerCase()
})
.c('parameter', {
name: 'msid',
Expand Down Expand Up @@ -561,6 +562,7 @@ export default class JingleSessionPC extends JingleSession {
const msid = $(source)
.find('>parameter[name="msid"]')
.attr('value');
const videoType = $(source).attr('videoType');

if (sourceDescription.has(sourceName)) {
sourceDescription.get(sourceName).ssrcList?.push(ssrc);
Expand All @@ -569,7 +571,8 @@ export default class JingleSessionPC extends JingleSession {
groups: [],
mediaType,
msid,
ssrcList: [ ssrc ]
ssrcList: [ ssrc ],
videoType
});
}

Expand Down

0 comments on commit bc446e9

Please sign in to comment.