From f5fc524030230b5f120d8a9fdaab5b0792f07757 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C3=BAl=20Ibarra=20Corretg=C3=A9?= Date: Fri, 28 Oct 2022 22:24:44 +0200 Subject: [PATCH] fix(connection-stats) fix rendering codec information Ref: https://github.com/jitsi/lib-jitsi-meet/pull/2143 --- react/features/base/tracks/functions.ts | 15 ---- .../components/AbstractConnectionIndicator.js | 37 +------- .../components/web/ConnectionIndicator.tsx | 5 +- .../web/ConnectionIndicatorContent.js | 19 +--- .../components/ConnectionStatsTable.tsx | 74 ++++++--------- .../native/ConnectionStatusComponent.js | 90 +++++-------------- 6 files changed, 57 insertions(+), 183 deletions(-) diff --git a/react/features/base/tracks/functions.ts b/react/features/base/tracks/functions.ts index 54083ab37..4a08bb12b 100644 --- a/react/features/base/tracks/functions.ts +++ b/react/features/base/tracks/functions.ts @@ -9,7 +9,6 @@ import JitsiMeetJS, { JitsiTrackErrors, browser } from '../lib-jitsi-meet'; import { setAudioMuted } from '../media/actions'; import { MEDIA_TYPE, MediaType, VIDEO_TYPE } from '../media/constants'; import { - getParticipantByIdOrUndefined, getVirtualScreenshareParticipantOwnerId, isScreenShareParticipant } from '../participants/functions'; @@ -445,20 +444,6 @@ export function getVideoTrackByParticipant( return getTrackByMediaTypeAndParticipant(tracks, MEDIA_TYPE.VIDEO, participant.id); } -/** - * Returns source name for specified participant id. - * - * @param {IReduxState} state - The Redux state. - * @param {string} participantId - Participant ID. - * @returns {string | undefined} - */ -export function getSourceNameByParticipantId(state: IReduxState, participantId: string) { - const participant = getParticipantByIdOrUndefined(state, participantId); - const track = getVideoTrackByParticipant(state, participant); - - return track?.jitsiTrack?.getSourceName(); -} - /** * Returns track of specified media type for specified participant id. * diff --git a/react/features/connection-indicator/components/AbstractConnectionIndicator.js b/react/features/connection-indicator/components/AbstractConnectionIndicator.js index 8eda3e959..a0a06192c 100644 --- a/react/features/connection-indicator/components/AbstractConnectionIndicator.js +++ b/react/features/connection-indicator/components/AbstractConnectionIndicator.js @@ -35,17 +35,7 @@ export type Props = { /** * Custom icon style. */ - iconStyle?: Object, - - /** - * The source name of the track. - */ - _sourceName: string, - - /** - * The flag whether source name signaling is enabled. - */ - _sourceNameSignalingEnabled: string + iconStyle?: Object }; /** @@ -100,13 +90,7 @@ class AbstractConnectionIndicator extends Component { * returns {void} */ componentDidMount() { - statsEmitter.subscribeToClientStats( - this.props.participantId, this._onStatsUpdated); - - if (this.props._sourceNameSignalingEnabled) { - statsEmitter.subscribeToClientStats( - this.props._sourceName, this._onStatsUpdated); - } + statsEmitter.subscribeToClientStats(this.props.participantId, this._onStatsUpdated); } /** @@ -122,15 +106,6 @@ class AbstractConnectionIndicator extends Component { statsEmitter.subscribeToClientStats( this.props.participantId, this._onStatsUpdated); } - - if (this.props._sourceNameSignalingEnabled) { - if (prevProps._sourceName !== this.props._sourceName) { - statsEmitter.unsubscribeToClientStats( - prevProps._sourceName, this._onStatsUpdated); - statsEmitter.subscribeToClientStats( - this.props._sourceName, this._onStatsUpdated); - } - } } /** @@ -141,13 +116,7 @@ class AbstractConnectionIndicator extends Component { * @returns {void} */ componentWillUnmount() { - statsEmitter.unsubscribeToClientStats( - this.props.participantId, this._onStatsUpdated); - - if (this.props._sourceNameSignalingEnabled) { - statsEmitter.unsubscribeToClientStats( - this.props._sourceName, this._onStatsUpdated); - } + statsEmitter.unsubscribeToClientStats(this.props.participantId, this._onStatsUpdated); clearTimeout(this.autoHideTimeout); } diff --git a/react/features/connection-indicator/components/web/ConnectionIndicator.tsx b/react/features/connection-indicator/components/web/ConnectionIndicator.tsx index 49edbc533..dff5a5459 100644 --- a/react/features/connection-indicator/components/web/ConnectionIndicator.tsx +++ b/react/features/connection-indicator/components/web/ConnectionIndicator.tsx @@ -18,7 +18,6 @@ import { } from '../../../base/participants/functions'; import Popover from '../../../base/popover/components/Popover.web'; import { - getSourceNameByParticipantId, getTrackByMediaTypeAndParticipant, getVirtualScreenshareParticipantTrack } from '../../../base/tracks/functions'; @@ -420,9 +419,7 @@ export function _mapStateToProps(state: IReduxState, ownProps: Props) { _popoverDisabled: state['features/base/config'].connectionIndicators?.disableDetails, _videoTrack: firstVideoTrack, _isConnectionStatusInactive, - _isConnectionStatusInterrupted, - _sourceName: getSourceNameByParticipantId(state, participantId), - _sourceNameSignalingEnabled: sourceNameSignalingEnabled + _isConnectionStatusInterrupted }; } export default translate(connect(_mapStateToProps)( diff --git a/react/features/connection-indicator/components/web/ConnectionIndicatorContent.js b/react/features/connection-indicator/components/web/ConnectionIndicatorContent.js index 690956e2b..4bd363f17 100644 --- a/react/features/connection-indicator/components/web/ConnectionIndicatorContent.js +++ b/react/features/connection-indicator/components/web/ConnectionIndicatorContent.js @@ -8,7 +8,7 @@ import { translate } from '../../../base/i18n'; import { MEDIA_TYPE } from '../../../base/media'; import { getLocalParticipant, getParticipantById, isScreenShareParticipant } from '../../../base/participants'; import { connect } from '../../../base/redux'; -import { getSourceNameByParticipantId, getTrackByMediaTypeAndParticipant } from '../../../base/tracks'; +import { getTrackByMediaTypeAndParticipant } from '../../../base/tracks'; import { ConnectionStatsTable } from '../../../connection-stats'; import { saveLogs } from '../../actions'; import { @@ -131,17 +131,7 @@ type Props = AbstractProps & { /** * Invoked to obtain translated strings. */ - t: Function, - - /** - * The source name of the track. - */ - _sourceName: string, - - /** - * Whether source name signaling is enabled. - */ - _sourceNameSignalingEnabled: boolean + t: Function }; /** @@ -224,7 +214,6 @@ class ConnectionIndicatorContent extends AbstractConnectionIndicator ); @@ -354,9 +343,7 @@ export function _mapStateToProps(state: Object, ownProps: Props) { _isConnectionStatusInterrupted, _isVirtualScreenshareParticipant: sourceNameSignalingEnabled && isScreenShareParticipant(participant), _isLocalVideo: participant?.local, - _region: participant?.region, - _sourceName: getSourceNameByParticipantId(state, participantId), - _sourceNameSignalingEnabled: sourceNameSignalingEnabled + _region: participant?.region }; if (conference) { diff --git a/react/features/connection-stats/components/ConnectionStatsTable.tsx b/react/features/connection-stats/components/ConnectionStatsTable.tsx index 2c3f1341a..fb03cb0ab 100644 --- a/react/features/connection-stats/components/ConnectionStatsTable.tsx +++ b/react/features/connection-stats/components/ConnectionStatsTable.tsx @@ -58,8 +58,8 @@ interface IProps extends WithTranslation { */ codec: { [key: string]: { - audio: string; - video: string; + audio: string | undefined; + video: string | undefined; }; }; @@ -158,11 +158,6 @@ interface IProps extends WithTranslation { */ shouldShowMore: boolean; - /** - * Whether source name signaling is enabled. - */ - sourceNameSignalingEnabled: boolean; - /** * Statistics related to transports. */ @@ -465,28 +460,22 @@ class ConnectionStatsTable extends Component { * @returns {ReactElement} */ _renderCodecs() { - const { codec, t, sourceNameSignalingEnabled } = this.props; + const { codec, t } = this.props; - if (!codec) { - return; - } + let codecString = 'N/A'; - let codecString; + if (codec) { + const audioCodecs = Object.values(codec) + .map(c => c.audio) + .filter(Boolean); + const videoCodecs = Object.values(codec) + .map(c => c.video) + .filter(Boolean); - if (sourceNameSignalingEnabled) { - codecString = `${codec.audio}, ${codec.video}`; - } else { - // Only report one codec, in case there are multiple for a user. - Object.keys(codec || {}) - .forEach(ssrc => { - const { audio, video } = codec[ssrc]; - - codecString = `${audio}, ${video}`; - }); - } - - if (!codecString) { - codecString = 'N/A'; + if (audioCodecs.length || videoCodecs.length) { + // Use a Set to eliminate duplicates. + codecString = Array.from(new Set([ ...audioCodecs, ...videoCodecs ])).join(', '); + } } return ( @@ -583,17 +572,11 @@ class ConnectionStatsTable extends Component { * @returns {ReactElement} */ _renderFrameRate() { - const { framerate, t, sourceNameSignalingEnabled } = this.props; + const { framerate, t } = this.props; - let frameRateString; - - if (sourceNameSignalingEnabled) { - frameRateString = framerate || 'N/A'; - } else { - frameRateString = Object.keys(framerate || {}) - .map(ssrc => framerate[ssrc as keyof typeof framerate]) - .join(', ') || 'N/A'; - } + const frameRateString = Object.keys(framerate || {}) + .map(ssrc => framerate[ssrc as keyof typeof framerate]) + .join(', ') || 'N/A'; return ( @@ -655,20 +638,15 @@ class ConnectionStatsTable extends Component { * @returns {ReactElement} */ _renderResolution() { - const { resolution, maxEnabledResolution, t, sourceNameSignalingEnabled } = this.props; - let resolutionString; + const { resolution, maxEnabledResolution, t } = this.props; - if (sourceNameSignalingEnabled) { - resolutionString = resolution ? `${resolution.width}x${resolution.height}` : 'N/A'; - } else { - resolutionString = Object.keys(resolution || {}) - .map(ssrc => { - const { width, height } = resolution[ssrc]; + let resolutionString = Object.keys(resolution || {}) + .map(ssrc => { + const { width, height } = resolution[ssrc]; - return `${width}x${height}`; - }) - .join(', ') || 'N/A'; - } + return `${width}x${height}`; + }) + .join(', ') || 'N/A'; if (maxEnabledResolution && maxEnabledResolution < 720) { const maxEnabledResolutionTitle = t('connectionindicator.maxEnabledResolution'); diff --git a/react/features/video-menu/components/native/ConnectionStatusComponent.js b/react/features/video-menu/components/native/ConnectionStatusComponent.js index 8d65980de..d40079176 100644 --- a/react/features/video-menu/components/native/ConnectionStatusComponent.js +++ b/react/features/video-menu/components/native/ConnectionStatusComponent.js @@ -3,7 +3,6 @@ import { Text, View } from 'react-native'; import { withTheme } from 'react-native-paper'; import { Avatar } from '../../../base/avatar'; -import { getSourceNameSignalingFeatureFlag } from '../../../base/config'; import { BottomSheet, hideSheet } from '../../../base/dialog'; import { bottomSheetStyles } from '../../../base/dialog/components/native/styles'; import { translate } from '../../../base/i18n'; @@ -11,7 +10,6 @@ import { IconArrowDownLarge, IconArrowUpLarge } from '../../../base/icons'; import { getParticipantDisplayName } from '../../../base/participants'; import { BaseIndicator } from '../../../base/react'; import { connect } from '../../../base/redux'; -import { getSourceNameByParticipantId } from '../../../base/tracks'; import statsEmitter from '../../../connection-indicator/statsEmitter'; import styles from './styles'; @@ -57,17 +55,7 @@ export type Props = { /** * Theme used for styles. */ - theme: Object, - - /** - * The source name of the track. - */ - _sourceName: string, - - /** - * Whether source name signaling is enabled. - */ - _sourceNameSignalingEnabled: boolean + theme: Object } /** @@ -209,13 +197,7 @@ class ConnectionStatusComponent extends Component { * returns {void} */ componentDidMount() { - statsEmitter.subscribeToClientStats( - this.props.participantID, this._onStatsUpdated); - - if (this.props._sourceNameSignalingEnabled) { - statsEmitter.subscribeToClientStats( - this.props._sourceName, this._onStatsUpdated); - } + statsEmitter.subscribeToClientStats(this.props.participantID, this._onStatsUpdated); } /** @@ -231,15 +213,6 @@ class ConnectionStatusComponent extends Component { statsEmitter.subscribeToClientStats( this.props.participantID, this._onStatsUpdated); } - - if (this.props._sourceNameSignalingEnabled) { - if (prevProps._sourceName !== this.props._sourceName) { - statsEmitter.unsubscribeToClientStats( - prevProps._sourceName, this._onStatsUpdated); - statsEmitter.subscribeToClientStats( - this.props._sourceName, this._onStatsUpdated); - } - } } _onStatsUpdated: Object => void; @@ -294,24 +267,18 @@ class ConnectionStatusComponent extends Component { */ _extractResolutionString(stats) { const { framerate, resolution } = stats; - let frameRateString, resolutionString; - if (this.props._sourceNameSignalingEnabled) { - resolutionString = resolution ? `${resolution.width}x${resolution.height}` : null; - frameRateString = framerate || null; - } else { - resolutionString = Object.keys(resolution || {}) - .map(ssrc => { - const { width, height } = resolution[ssrc]; + const resolutionString = Object.keys(resolution || {}) + .map(ssrc => { + const { width, height } = resolution[ssrc]; - return `${width}x${height}`; - }) - .join(', ') || null; + return `${width}x${height}`; + }) + .join(', ') || null; - frameRateString = Object.keys(framerate || {}) - .map(ssrc => framerate[ssrc]) - .join(', ') || null; - } + const frameRateString = Object.keys(framerate || {}) + .map(ssrc => framerate[ssrc]) + .join(', ') || null; return resolutionString && frameRateString ? `${resolutionString}@${frameRateString}fps` : undefined; } @@ -361,21 +328,20 @@ class ConnectionStatusComponent extends Component { let codecString; - if (this.props._sourceNameSignalingEnabled) { - if (codec) { - codecString = `${codec.audio}, ${codec.video}`; + if (codec) { + const audioCodecs = Object.values(codec) + .map(c => c.audio) + .filter(Boolean); + const videoCodecs = Object.values(codec) + .map(c => c.video) + .filter(Boolean); + + if (audioCodecs.length || videoCodecs.length) { + // Use a Set to eliminate duplicates. + codecString = Array.from(new Set([ ...audioCodecs, ...videoCodecs ])).join(', '); } - } else { - // Only report one codec, in case there are multiple for a user. - Object.keys(codec || {}) - .forEach(ssrc => { - const { audio, video } = codec[ssrc]; - - codecString = `${audio}, ${video}`; - }); } - return codecString; } @@ -403,13 +369,7 @@ class ConnectionStatusComponent extends Component { * @returns {boolean} */ _onCancel() { - statsEmitter.unsubscribeToClientStats( - this.props.participantID, this._onStatsUpdated); - - if (this.props._sourceNameSignalingEnabled) { - statsEmitter.unsubscribeToClientStats( - this.props._sourceName, this._onStatsUpdated); - } + statsEmitter.unsubscribeToClientStats(this.props.participantID, this._onStatsUpdated); this.props.dispatch(hideSheet()); } @@ -450,9 +410,7 @@ function _mapStateToProps(state, ownProps) { const { participantID } = ownProps; return { - _participantDisplayName: getParticipantDisplayName(state, participantID), - _sourceNameSignalingEnabled: getSourceNameSignalingFeatureFlag(state), - _sourceName: getSourceNameByParticipantId(state, ownProps.participantId) + _participantDisplayName: getParticipantDisplayName(state, participantID) }; }