diff --git a/ts/calling/constants.ts b/ts/calling/constants.ts index c12ae22af..93275dea7 100644 --- a/ts/calling/constants.ts +++ b/ts/calling/constants.ts @@ -8,5 +8,6 @@ export const REQUESTED_VIDEO_WIDTH = 640; export const REQUESTED_VIDEO_HEIGHT = 480; export const REQUESTED_VIDEO_FRAMERATE = 30; -export const MAX_FRAME_SIZE = 1920 * 1080; -export const FRAME_BUFFER_SIZE = MAX_FRAME_SIZE * 4; +export const MAX_FRAME_WIDTH = 1920; +export const MAX_FRAME_HEIGHT = 1080; +export const FRAME_BUFFER_SIZE = MAX_FRAME_WIDTH * MAX_FRAME_HEIGHT * 4; diff --git a/ts/components/CallingPipRemoteVideo.tsx b/ts/components/CallingPipRemoteVideo.tsx index 6322c9d36..784f539ba 100644 --- a/ts/components/CallingPipRemoteVideo.tsx +++ b/ts/components/CallingPipRemoteVideo.tsx @@ -2,7 +2,7 @@ // SPDX-License-Identifier: AGPL-3.0-only import React, { useMemo, useEffect } from 'react'; -import { maxBy } from 'lodash'; +import { clamp, maxBy } from 'lodash'; import type { VideoFrameSource } from 'ringrtc'; import { Avatar } from './Avatar'; import { CallBackgroundBlur } from './CallBackgroundBlur'; @@ -18,11 +18,13 @@ import { CallMode } from '../types/Calling'; import { AvatarColors } from '../types/Colors'; import type { SetRendererCanvasType } from '../state/ducks/calling'; import { useGetCallingFrameBuffer } from '../calling/useGetCallingFrameBuffer'; +import { MAX_FRAME_WIDTH } from '../calling/constants'; import { usePageVisibility } from '../hooks/usePageVisibility'; import { missingCaseError } from '../util/missingCaseError'; import { nonRenderedRemoteParticipant } from '../util/ringrtc/nonRenderedRemoteParticipant'; -// This value should be kept in sync with the hard-coded CSS height. +// This value should be kept in sync with the hard-coded CSS height. It should also be +// less than `MAX_FRAME_HEIGHT`. const PIP_VIDEO_HEIGHT_PX = 120; const NoVideo = ({ @@ -110,14 +112,13 @@ export const CallingPipRemoteVideo = ({ if (isPageVisible) { setGroupCallVideoRequest( activeCall.remoteParticipants.map(participant => { - const isVisible = - participant === activeGroupCallSpeaker && - participant.hasRemoteVideo; - if (isVisible) { + if (participant === activeGroupCallSpeaker) { return { demuxId: participant.demuxId, - width: Math.floor( - PIP_VIDEO_HEIGHT_PX * participant.videoAspectRatio + width: clamp( + Math.floor(PIP_VIDEO_HEIGHT_PX * participant.videoAspectRatio), + 1, + MAX_FRAME_WIDTH ), height: PIP_VIDEO_HEIGHT_PX, }; diff --git a/ts/components/GroupCallRemoteParticipant.tsx b/ts/components/GroupCallRemoteParticipant.tsx index 4c2421eba..097e92c37 100644 --- a/ts/components/GroupCallRemoteParticipant.tsx +++ b/ts/components/GroupCallRemoteParticipant.tsx @@ -22,7 +22,7 @@ import { ConfirmationDialog } from './ConfirmationDialog'; import { Intl } from './Intl'; import { ContactName } from './conversation/ContactName'; import { useIntersectionObserver } from '../hooks/useIntersectionObserver'; -import { MAX_FRAME_SIZE } from '../calling/constants'; +import { MAX_FRAME_HEIGHT, MAX_FRAME_WIDTH } from '../calling/constants'; const MAX_TIME_TO_SHOW_STALE_VIDEO_FRAMES = 5000; const MAX_TIME_TO_SHOW_STALE_SCREENSHARE_FRAMES = 60000; @@ -150,7 +150,8 @@ export const GroupCallRemoteParticipant: React.FC = React.memo( if ( frameWidth < 2 || frameHeight < 2 || - frameWidth * frameHeight > MAX_FRAME_SIZE + frameWidth > MAX_FRAME_WIDTH || + frameHeight > MAX_FRAME_HEIGHT ) { return; } diff --git a/ts/components/GroupCallRemoteParticipants.tsx b/ts/components/GroupCallRemoteParticipants.tsx index 0b045d27e..906f83be7 100644 --- a/ts/components/GroupCallRemoteParticipants.tsx +++ b/ts/components/GroupCallRemoteParticipants.tsx @@ -3,7 +3,7 @@ import React, { useCallback, useState, useMemo, useEffect } from 'react'; import Measure from 'react-measure'; -import { takeWhile, chunk, maxBy, flatten, noop } from 'lodash'; +import { takeWhile, clamp, chunk, maxBy, flatten, noop } from 'lodash'; import type { VideoFrameSource } from 'ringrtc'; import { GroupCallRemoteParticipant } from './GroupCallRemoteParticipant'; import { @@ -21,9 +21,10 @@ import { useDevicePixelRatio } from '../hooks/useDevicePixelRatio'; import { nonRenderedRemoteParticipant } from '../util/ringrtc/nonRenderedRemoteParticipant'; import { missingCaseError } from '../util/missingCaseError'; import { SECOND } from '../util/durations'; -import { filter } from '../util/iterables'; +import { filter, join } from '../util/iterables'; import * as setUtil from '../util/setUtil'; import * as log from '../logging/log'; +import { MAX_FRAME_HEIGHT, MAX_FRAME_WIDTH } from '../calling/constants'; const MIN_RENDERED_HEIGHT = 180; const PARTICIPANT_MARGIN = 10; @@ -52,9 +53,9 @@ type PropsType = { }; enum VideoRequestMode { - Normal, - LowResolution, - NoVideo, + Normal = 'Normal', + LowResolution = 'LowResolution', + NoVideo = 'NoVideo', } // This component lays out group call remote participants. It uses a custom layout @@ -298,6 +299,9 @@ export const GroupCallRemoteParticipants: React.FC = ({ ); const videoRequestMode = useVideoRequestMode(); + useEffect(() => { + log.info(`Group call now using ${videoRequestMode} video request mode`); + }, [videoRequestMode]); useEffect(() => { let videoRequest: Array; @@ -310,35 +314,44 @@ export const GroupCallRemoteParticipants: React.FC = ({ if (participant.sharingScreen) { // We want best-resolution video if someone is sharing their screen. scalar = Math.max(devicePixelRatio, 1); - } else if (participant.hasRemoteVideo) { - scalar = VIDEO_REQUEST_SCALAR; } else { - scalar = 0; + scalar = VIDEO_REQUEST_SCALAR; } return { demuxId: participant.demuxId, - width: Math.floor( - gridParticipantHeight * participant.videoAspectRatio * scalar + width: clamp( + Math.floor( + gridParticipantHeight * participant.videoAspectRatio * scalar + ), + 1, + MAX_FRAME_WIDTH + ), + height: clamp( + Math.floor(gridParticipantHeight * scalar), + 1, + MAX_FRAME_HEIGHT ), - height: Math.floor(gridParticipantHeight * scalar), }; }), ...overflowedParticipants.map(participant => { - if ( - !participant.hasRemoteVideo || - invisibleDemuxIds.has(participant.demuxId) - ) { + if (invisibleDemuxIds.has(participant.demuxId)) { return nonRenderedRemoteParticipant(participant); } return { demuxId: participant.demuxId, - width: Math.floor( - OVERFLOW_PARTICIPANT_WIDTH * VIDEO_REQUEST_SCALAR + width: clamp( + Math.floor(OVERFLOW_PARTICIPANT_WIDTH * VIDEO_REQUEST_SCALAR), + 1, + MAX_FRAME_WIDTH ), - height: Math.floor( - (OVERFLOW_PARTICIPANT_WIDTH / participant.videoAspectRatio) * - VIDEO_REQUEST_SCALAR + height: clamp( + Math.floor( + (OVERFLOW_PARTICIPANT_WIDTH / participant.videoAspectRatio) * + VIDEO_REQUEST_SCALAR + ), + 1, + MAX_FRAME_HEIGHT ), }; }), @@ -454,6 +467,12 @@ function useInvisibleParticipants( [] ); + useEffect(() => { + log.info( + `Invisible demux IDs changed to [${join(invisibleDemuxIds, ',')}]` + ); + }, [invisibleDemuxIds]); + useEffect(() => { const remoteParticipantDemuxIds = new Set( remoteParticipants.map(r => r.demuxId) diff --git a/ts/test-both/util/iterables_test.ts b/ts/test-both/util/iterables_test.ts index d054ec6f9..709bbe125 100644 --- a/ts/test-both/util/iterables_test.ts +++ b/ts/test-both/util/iterables_test.ts @@ -12,6 +12,7 @@ import { groupBy, isEmpty, isIterable, + join, map, reduce, repeat, @@ -320,6 +321,31 @@ describe('iterable utilities', () => { }); }); + describe('join', () => { + it('returns the empty string for empty iterables', () => { + assert.isEmpty(join([], 'x')); + assert.isEmpty(join(new Set(), 'x')); + }); + + it("returns the stringified value if it's the only value", () => { + assert.strictEqual(join(new Set(['foo']), 'x'), 'foo'); + assert.strictEqual(join(new Set([123]), 'x'), '123'); + assert.strictEqual(join([{ toString: () => 'foo' }], 'x'), 'foo'); + }); + + it('returns each value stringified, joined by separator', () => { + assert.strictEqual( + join(new Set(['foo', 'bar', 'baz']), ' '), + 'foo bar baz' + ); + assert.strictEqual(join(new Set([1, 2, 3]), '--'), '1--2--3'); + }); + + it('handles undefined and null like Array.prototype.join', () => { + assert.strictEqual(join(new Set([undefined, null]), ','), ','); + }); + }); + describe('map', () => { it('returns an empty iterable when passed an empty iterable', () => { const fn = sinon.fake(); diff --git a/ts/util/iterables.ts b/ts/util/iterables.ts index e32940c26..f02240f81 100644 --- a/ts/util/iterables.ts +++ b/ts/util/iterables.ts @@ -133,6 +133,21 @@ export function groupBy( export const isEmpty = (iterable: Iterable): boolean => Boolean(iterable[Symbol.iterator]().next().done); +export function join(iterable: Iterable, separator: string): string { + let hasProcessedFirst = false; + let result = ''; + for (const value of iterable) { + const stringifiedValue = value == null ? '' : String(value); + if (hasProcessedFirst) { + result += separator + stringifiedValue; + } else { + result = stringifiedValue; + } + hasProcessedFirst = true; + } + return result; +} + export function map( iterable: Iterable, fn: (value: T) => ResultT