From 01549b11d1612b271c9db15c3475d1346a181358 Mon Sep 17 00:00:00 2001 From: Evan Hahn <69474926+EvanHahn-Signal@users.noreply.github.com> Date: Mon, 6 Dec 2021 17:06:13 -0600 Subject: [PATCH] Don't request video for invisible group call participants --- ts/components/CallScreen.stories.tsx | 4 +- .../GroupCallOverflowArea.stories.tsx | 2 + ts/components/GroupCallOverflowArea.tsx | 6 ++ ts/components/GroupCallRemoteParticipant.tsx | 12 ++- ts/components/GroupCallRemoteParticipants.tsx | 85 ++++++++++++++++--- ts/test-both/util/setUtil_test.ts | 56 ++++++++++++ ts/util/setUtil.ts | 22 +++++ 7 files changed, 171 insertions(+), 16 deletions(-) create mode 100644 ts/test-both/util/setUtil_test.ts create mode 100644 ts/util/setUtil.ts diff --git a/ts/components/CallScreen.stories.tsx b/ts/components/CallScreen.stories.tsx index 45c61b25c29c..245b2d09f14f 100644 --- a/ts/components/CallScreen.stories.tsx +++ b/ts/components/CallScreen.stories.tsx @@ -27,7 +27,7 @@ import { import { fakeGetGroupCallVideoFrameSource } from '../test-both/helpers/fakeGetGroupCallVideoFrameSource'; import enMessages from '../../_locales/en/messages.json'; -const MAX_PARTICIPANTS = 32; +const MAX_PARTICIPANTS = 64; const i18n = setupI18n('en', enMessages); @@ -301,7 +301,7 @@ story.add('Group call - Many', () => { callMode: CallMode.Group, remoteParticipants: allRemoteParticipants.slice( 0, - number('Participant count', 3, { + number('Participant count', 40, { range: true, min: 0, max: MAX_PARTICIPANTS, diff --git a/ts/components/GroupCallOverflowArea.stories.tsx b/ts/components/GroupCallOverflowArea.stories.tsx index 1ae13046b7d3..7152a2477d3b 100644 --- a/ts/components/GroupCallOverflowArea.stories.tsx +++ b/ts/components/GroupCallOverflowArea.stories.tsx @@ -6,6 +6,7 @@ import React from 'react'; import { memoize, times } from 'lodash'; import { storiesOf } from '@storybook/react'; import { number } from '@storybook/addon-knobs'; +import { action } from '@storybook/addon-actions'; import { GroupCallOverflowArea } from './GroupCallOverflowArea'; import { setupI18n } from '../util/setupI18n'; @@ -37,6 +38,7 @@ const defaultProps = { getFrameBuffer: memoize(() => new ArrayBuffer(FRAME_BUFFER_SIZE)), getGroupCallVideoFrameSource: fakeGetGroupCallVideoFrameSource, i18n, + onParticipantVisibilityChanged: action('onParticipantVisibilityChanged'), }; // This component is usually rendered on a call screen. diff --git a/ts/components/GroupCallOverflowArea.tsx b/ts/components/GroupCallOverflowArea.tsx index 0042682a74b3..5d5f1ca2d819 100644 --- a/ts/components/GroupCallOverflowArea.tsx +++ b/ts/components/GroupCallOverflowArea.tsx @@ -19,6 +19,10 @@ type PropsType = { getFrameBuffer: () => ArrayBuffer; getGroupCallVideoFrameSource: (demuxId: number) => VideoFrameSource; i18n: LocalizerType; + onParticipantVisibilityChanged: ( + demuxId: number, + isVisible: boolean + ) => unknown; overflowedParticipants: ReadonlyArray; }; @@ -26,6 +30,7 @@ export const GroupCallOverflowArea: FC = ({ getFrameBuffer, getGroupCallVideoFrameSource, i18n, + onParticipantVisibilityChanged, overflowedParticipants, }) => { const overflowRef = useRef(null); @@ -109,6 +114,7 @@ export const GroupCallOverflowArea: FC = ({ getFrameBuffer={getFrameBuffer} getGroupCallVideoFrameSource={getGroupCallVideoFrameSource} i18n={i18n} + onVisibilityChanged={onParticipantVisibilityChanged} width={OVERFLOW_PARTICIPANT_WIDTH} height={Math.floor( OVERFLOW_PARTICIPANT_WIDTH / remoteParticipant.videoAspectRatio diff --git a/ts/components/GroupCallRemoteParticipant.tsx b/ts/components/GroupCallRemoteParticipant.tsx index ee15522cc13d..1819f2f48715 100644 --- a/ts/components/GroupCallRemoteParticipant.tsx +++ b/ts/components/GroupCallRemoteParticipant.tsx @@ -29,6 +29,7 @@ type BasePropsType = { getFrameBuffer: () => ArrayBuffer; getGroupCallVideoFrameSource: (demuxId: number) => VideoFrameSource; i18n: LocalizerType; + onVisibilityChanged?: (demuxId: number, isVisible: boolean) => unknown; remoteParticipant: GroupCallRemoteParticipantType; }; @@ -52,7 +53,12 @@ export type PropsType = BasePropsType & export const GroupCallRemoteParticipant: React.FC = React.memo( props => { - const { getFrameBuffer, getGroupCallVideoFrameSource, i18n } = props; + const { + getFrameBuffer, + getGroupCallVideoFrameSource, + i18n, + onVisibilityChanged, + } = props; const { acceptedMessageRequest, @@ -95,6 +101,10 @@ export const GroupCallRemoteParticipant: React.FC = React.memo( ? intersectionObserverEntry.isIntersecting : true; + useEffect(() => { + onVisibilityChanged?.(demuxId, isVisible); + }, [demuxId, isVisible, onVisibilityChanged]); + const wantsToShowVideo = hasRemoteVideo && !isBlocked && isVisible; const hasVideoToShow = wantsToShowVideo && hasReceivedVideoRecently; diff --git a/ts/components/GroupCallRemoteParticipants.tsx b/ts/components/GroupCallRemoteParticipants.tsx index 184dfb56f346..8b690a66d0d9 100644 --- a/ts/components/GroupCallRemoteParticipants.tsx +++ b/ts/components/GroupCallRemoteParticipants.tsx @@ -1,7 +1,7 @@ // Copyright 2020-2021 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only -import React, { useState, useMemo, useEffect } from 'react'; +import React, { useCallback, useState, useMemo, useEffect } from 'react'; import Measure from 'react-measure'; import { takeWhile, chunk, maxBy, flatten, noop } from 'lodash'; import type { VideoFrameSource } from 'ringrtc'; @@ -20,6 +20,8 @@ import { usePageVisibility } from '../hooks/usePageVisibility'; import { nonRenderedRemoteParticipant } from '../util/ringrtc/nonRenderedRemoteParticipant'; import { missingCaseError } from '../util/missingCaseError'; import { SECOND } from '../util/durations'; +import { filter } from '../util/iterables'; +import * as setUtil from '../util/setUtil'; import * as log from '../logging/log'; const MIN_RENDERED_HEIGHT = 180; @@ -94,6 +96,9 @@ export const GroupCallRemoteParticipants: React.FC = ({ const getFrameBuffer = useGetCallingFrameBuffer(); + const { invisibleDemuxIds, onParticipantVisibilityChanged } = + useInvisibleParticipants(remoteParticipants); + // 1. Figure out the maximum number of possible rows that could fit on the screen. // // We choose the smaller of these two options: @@ -310,19 +315,23 @@ export const GroupCallRemoteParticipants: React.FC = ({ }; }), ...overflowedParticipants.map(participant => { - if (participant.hasRemoteVideo) { - return { - demuxId: participant.demuxId, - width: Math.floor( - OVERFLOW_PARTICIPANT_WIDTH * VIDEO_REQUEST_SCALAR - ), - height: Math.floor( - (OVERFLOW_PARTICIPANT_WIDTH / participant.videoAspectRatio) * - VIDEO_REQUEST_SCALAR - ), - }; + if ( + !participant.hasRemoteVideo || + invisibleDemuxIds.has(participant.demuxId) + ) { + return nonRenderedRemoteParticipant(participant); } - return nonRenderedRemoteParticipant(participant); + + return { + demuxId: participant.demuxId, + width: Math.floor( + OVERFLOW_PARTICIPANT_WIDTH * VIDEO_REQUEST_SCALAR + ), + height: Math.floor( + (OVERFLOW_PARTICIPANT_WIDTH / participant.videoAspectRatio) * + VIDEO_REQUEST_SCALAR + ), + }; }), ]; break; @@ -350,6 +359,7 @@ export const GroupCallRemoteParticipants: React.FC = ({ }, [ gridParticipantHeight, videoRequestMode, + invisibleDemuxIds, overflowedParticipants, remoteParticipants, setGroupCallVideoRequest, @@ -396,6 +406,7 @@ export const GroupCallRemoteParticipants: React.FC = ({ getFrameBuffer={getFrameBuffer} getGroupCallVideoFrameSource={getGroupCallVideoFrameSource} i18n={i18n} + onParticipantVisibilityChanged={onParticipantVisibilityChanged} overflowedParticipants={overflowedParticipants} /> @@ -404,6 +415,54 @@ export const GroupCallRemoteParticipants: React.FC = ({ ); }; +// This function is only meant for use with `useInvisibleParticipants`. It helps avoid +// returning new set instances when the underlying values are equal. +function pickDifferentSet(a: Readonly>, b: Readonly>): Set { + return a.size === b.size ? a : b; +} + +function useInvisibleParticipants( + remoteParticipants: ReadonlyArray +): Readonly<{ + invisibleDemuxIds: Set; + onParticipantVisibilityChanged: (demuxId: number, isVisible: boolean) => void; +}> { + const [invisibleDemuxIds, setInvisibleDemuxIds] = useState(new Set()); + + const onParticipantVisibilityChanged = useCallback( + (demuxId: number, isVisible: boolean) => { + setInvisibleDemuxIds(oldInvisibleDemuxIds => { + const toggled = setUtil.toggle( + oldInvisibleDemuxIds, + demuxId, + !isVisible + ); + return pickDifferentSet(oldInvisibleDemuxIds, toggled); + }); + }, + [] + ); + + useEffect(() => { + const remoteParticipantDemuxIds = new Set( + remoteParticipants.map(r => r.demuxId) + ); + setInvisibleDemuxIds(oldInvisibleDemuxIds => { + const staleIds = filter( + oldInvisibleDemuxIds, + id => !remoteParticipantDemuxIds.has(id) + ); + const withoutStaleIds = setUtil.remove(oldInvisibleDemuxIds, ...staleIds); + return pickDifferentSet(oldInvisibleDemuxIds, withoutStaleIds); + }); + }, [remoteParticipants]); + + return { + invisibleDemuxIds, + onParticipantVisibilityChanged, + }; +} + function useVideoRequestMode(): VideoRequestMode { const isPageVisible = usePageVisibility(); diff --git a/ts/test-both/util/setUtil_test.ts b/ts/test-both/util/setUtil_test.ts new file mode 100644 index 000000000000..798ccb2eccad --- /dev/null +++ b/ts/test-both/util/setUtil_test.ts @@ -0,0 +1,56 @@ +// Copyright 2021 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { assert } from 'chai'; + +import { remove, toggle } from '../../util/setUtil'; + +describe('set utilities', () => { + const original = new Set([1, 2, 3]); + + describe('remove', () => { + it('accepts zero arguments, returning a new set', () => { + const result = remove(original); + assert.deepStrictEqual(result, original); + assert.notStrictEqual(result, original); + }); + + it('accepts 1 argument, returning a new set', () => { + const result = remove(original, 2); + assert.deepStrictEqual(result, new Set([1, 3])); + assert.deepStrictEqual(original, new Set([1, 2, 3])); + }); + + it('accepts multiple arguments, returning a new set', () => { + const result = remove(original, 1, 2, 99); + assert.deepStrictEqual(result, new Set([3])); + assert.deepStrictEqual(original, new Set([1, 2, 3])); + }); + }); + + describe('toggle', () => { + it('returns a clone if trying to remove an item that was never there', () => { + const result = toggle(original, 99, false); + assert.deepStrictEqual(result, new Set([1, 2, 3])); + assert.notStrictEqual(result, original); + }); + + it('returns a clone if trying to add an item that was already there', () => { + const result = toggle(original, 3, true); + assert.deepStrictEqual(result, new Set([1, 2, 3])); + assert.notStrictEqual(result, original); + }); + + it('can add an item to a set', () => { + const result = toggle(original, 4, true); + assert.deepStrictEqual(result, new Set([1, 2, 3, 4])); + assert.deepStrictEqual(original, new Set([1, 2, 3])); + }); + + it('can remove an item from a set', () => { + const result = toggle(original, 2, false); + assert.deepStrictEqual(result, new Set([1, 3])); + assert.deepStrictEqual(original, new Set([1, 2, 3])); + }); + }); +}); diff --git a/ts/util/setUtil.ts b/ts/util/setUtil.ts new file mode 100644 index 000000000000..425fee46a6dc --- /dev/null +++ b/ts/util/setUtil.ts @@ -0,0 +1,22 @@ +// Copyright 2021 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +const add = (set: Readonly>, item: T): Set => + new Set(set).add(item); + +export const remove = ( + set: Readonly>, + ...items: ReadonlyArray +): Set => { + const clone = new Set(set); + for (const item of items) { + clone.delete(item); + } + return clone; +}; + +export const toggle = ( + set: Readonly>, + item: Readonly, + shouldInclude: boolean +): Set => (shouldInclude ? add : remove)(set, item);