From 9474a3d33635bac77b0b9fc49d4d8d206d53d408 Mon Sep 17 00:00:00 2001 From: Samy Ouyahia Date: Tue, 16 Apr 2024 18:55:08 +0200 Subject: [PATCH] Fixed the stream dimensions logic in the Camera component --- packages/camera-web/src/Camera/Camera.tsx | 8 +- .../src/Camera/hooks/useCameraPreview.ts | 2 +- .../src/Camera/hooks/useUserMedia.ts | 62 +++---- .../camera-web/test/Camera/Camera.test.tsx | 40 ++++- .../Camera/hooks/useCameraPreview.test.tsx | 9 +- .../test/Camera/hooks/useUserMedia.test.ts | 158 +++++++++++------- .../test/mocks/getUserMedia.mock.ts | 9 +- 7 files changed, 184 insertions(+), 104 deletions(-) diff --git a/packages/camera-web/src/Camera/Camera.tsx b/packages/camera-web/src/Camera/Camera.tsx index ee6c5a88a..24997379f 100644 --- a/packages/camera-web/src/Camera/Camera.tsx +++ b/packages/camera-web/src/Camera/Camera.tsx @@ -1,5 +1,6 @@ import React, { useMemo } from 'react'; import { AllOrNone, RequiredKeys } from '@monkvision/types'; +import { isMobileDevice } from '@monkvision/common'; import { CameraFacingMode, CameraResolution, @@ -97,6 +98,10 @@ export function Camera({ monitoring, onPictureTaken, }: CameraProps) { + const previewResolution = useMemo( + () => (isMobileDevice() ? CameraResolution.UHD_4K : CameraResolution.FHD_1080P), + [], + ); const { ref: videoRef, dimensions: streamDimensions, @@ -104,7 +109,7 @@ export function Camera({ retry, isLoading: isPreviewLoading, } = useCameraPreview({ - resolution: CameraResolution.UHD_4K, + resolution: previewResolution, facingMode: CameraFacingMode.ENVIRONMENT, }); const { ref: canvasRef, dimensions: canvasDimensions } = useCameraCanvas({ @@ -134,6 +139,7 @@ export function Camera({ autoPlay playsInline={true} controls={false} + muted={true} data-testid='camera-video-preview' /> diff --git a/packages/camera-web/src/Camera/hooks/useCameraPreview.ts b/packages/camera-web/src/Camera/hooks/useCameraPreview.ts index 20b7beb41..969195b8e 100644 --- a/packages/camera-web/src/Camera/hooks/useCameraPreview.ts +++ b/packages/camera-web/src/Camera/hooks/useCameraPreview.ts @@ -20,7 +20,7 @@ export interface CameraPreviewHandle extends UserMediaResult { export function useCameraPreview(config: CameraConfig): CameraPreviewHandle { const ref = useRef(null); const { handleError } = useMonitoring(); - const userMediaResult = useUserMedia(getMediaConstraints(config)); + const userMediaResult = useUserMedia(getMediaConstraints(config), ref); useEffect(() => { if (userMediaResult.stream && ref.current) { diff --git a/packages/camera-web/src/Camera/hooks/useUserMedia.ts b/packages/camera-web/src/Camera/hooks/useUserMedia.ts index 146f40c29..bb64ef3b6 100644 --- a/packages/camera-web/src/Camera/hooks/useUserMedia.ts +++ b/packages/camera-web/src/Camera/hooks/useUserMedia.ts @@ -1,6 +1,6 @@ import { useMonitoring } from '@monkvision/monitoring'; import deepEqual from 'fast-deep-equal'; -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { RefObject, useCallback, useEffect, useMemo, useState } from 'react'; import { PixelDimensions } from '@monkvision/types'; import { isMobileDevice } from '@monkvision/common'; import { getValidCameraDeviceIds } from './utils'; @@ -111,7 +111,14 @@ export interface UserMediaResult { retry: () => void; } -function getStreamDimensions(stream: MediaStream): PixelDimensions { +function swapDimensions(dimensions: PixelDimensions): PixelDimensions { + return { + width: dimensions.height, + height: dimensions.width, + }; +} + +function getStreamDimensions(stream: MediaStream, checkOrientation: boolean): PixelDimensions { const videoTracks = stream.getVideoTracks(); if (videoTracks.length === 0) { throw new InvalidStreamError( @@ -132,14 +139,14 @@ function getStreamDimensions(stream: MediaStream): PixelDimensions { InvalidStreamErrorName.NO_DIMENSIONS, ); } - return { width, height }; -} + const dimensions = { width, height }; + if (!isMobileDevice() || !checkOrientation) { + return dimensions; + } -function swapWidthAndHeight(dimensions: PixelDimensions): PixelDimensions { - return { - width: dimensions.height, - height: dimensions.width, - }; + const isStreamInPortrait = width < height; + const isDeviceInPortrait = window.matchMedia('(orientation: portrait)').matches; + return isStreamInPortrait !== isDeviceInPortrait ? swapDimensions(dimensions) : dimensions; } /** @@ -152,12 +159,16 @@ function swapWidthAndHeight(dimensions: PixelDimensions): PixelDimensions { * * @param constraints The same media constraints you would pass to the `getUserMedia` function. Note that this hook has * been designed for video only, so audio constraints could provoke unexpected behaviour. + * @param videoRef The ref to the video element displaying the camera preview stream. * @return The result of this hook contains the resulting video stream, an error object if there has been an error, a * loading indicator and a retry function that tries to get a camera stream again. See the `UserMediaResult` interface * for more information. * @see UserMediaResult */ -export function useUserMedia(constraints: MediaStreamConstraints): UserMediaResult { +export function useUserMedia( + constraints: MediaStreamConstraints, + videoRef: RefObject, +): UserMediaResult { const [stream, setStream] = useState(null); const [dimensions, setDimensions] = useState(null); const [isLoading, setIsLoading] = useState(false); @@ -215,7 +226,7 @@ export function useUserMedia(constraints: MediaStreamConstraints): UserMediaResu const updatedConstraints = { ...constraints, video: { - ...(constraints ? (constraints.video as MediaStreamConstraints) : null), + ...(constraints ? (constraints.video as MediaTrackConstraints) : {}), deviceId: { exact: cameraDeviceIds }, }, }; @@ -223,13 +234,7 @@ export function useUserMedia(constraints: MediaStreamConstraints): UserMediaResu str?.addEventListener('inactive', onStreamInactive); setStream(str); - const dimensionsStr = getStreamDimensions(str); - const isPortrait = window.matchMedia('(orientation: portrait)').matches; - setDimensions( - dimensionsStr.width > dimensionsStr.height && isMobileDevice() && isPortrait - ? swapWidthAndHeight(dimensionsStr) - : dimensionsStr, - ); + setDimensions(getStreamDimensions(str, true)); setIsLoading(false); } catch (err) { handleGetUserMediaError(err); @@ -240,18 +245,17 @@ export function useUserMedia(constraints: MediaStreamConstraints): UserMediaResu }, [constraints, stream, error, isLoading, lastConstraintsApplied]); useEffect(() => { - const portrait = window.matchMedia('(orientation: portrait)'); - - const handleOrientationChange = () => { - if (stream) { - const dimensionsStr = getStreamDimensions(stream); - setDimensions(isMobileDevice() ? swapWidthAndHeight(dimensionsStr) : dimensionsStr); - } - }; - portrait.addEventListener('change', handleOrientationChange); - + let isActive = true; + if (stream && videoRef.current) { + // eslint-disable-next-line no-param-reassign + videoRef.current.onresize = () => { + if (isActive) { + setDimensions(getStreamDimensions(stream, false)); + } + }; + } return () => { - portrait.removeEventListener('change', handleOrientationChange); + isActive = false; }; }, [stream]); diff --git a/packages/camera-web/test/Camera/Camera.test.tsx b/packages/camera-web/test/Camera/Camera.test.tsx index 44b85c25d..ceae12c88 100644 --- a/packages/camera-web/test/Camera/Camera.test.tsx +++ b/packages/camera-web/test/Camera/Camera.test.tsx @@ -1,5 +1,9 @@ import React, { createRef } from 'react'; +Object.defineProperty(HTMLMediaElement.prototype, 'muted', { + set: () => {}, +}); + jest.mock('../../src/Camera/hooks', () => ({ ...jest.requireActual('../../src/Camera/hooks'), useCameraPreview: jest.fn(() => ({ @@ -35,6 +39,7 @@ import { useCompression, useTakePicture, } from '../../src/Camera/hooks'; +import { isMobileDevice } from '@monkvision/common'; const VIDEO_PREVIEW_TEST_ID = 'camera-video-preview'; const CANVAS_TEST_ID = 'camera-canvas'; @@ -44,13 +49,38 @@ describe('Camera component', () => { jest.clearAllMocks(); }); - it('should pass the proper props to the useCameraPreview hook', () => { + it('should ask for an environment camera for the camera preview', () => { const { unmount } = render(); - expect(useCameraPreview).toHaveBeenCalledWith({ - facingMode: CameraFacingMode.ENVIRONMENT, - resolution: CameraResolution.UHD_4K, - }); + expect(useCameraPreview).toHaveBeenCalledWith( + expect.objectContaining({ + facingMode: CameraFacingMode.ENVIRONMENT, + }), + ); + unmount(); + }); + + it('should ask for a 4K camera for the camera preview on mobile devices', () => { + (isMobileDevice as jest.Mock).mockImplementationOnce(() => true); + const { unmount } = render(); + + expect(useCameraPreview).toHaveBeenCalledWith( + expect.objectContaining({ + resolution: CameraResolution.UHD_4K, + }), + ); + unmount(); + }); + + it('should ask for a FHD camera for the camera preview on desktop devices', () => { + (isMobileDevice as jest.Mock).mockImplementationOnce(() => false); + const { unmount } = render(); + + expect(useCameraPreview).toHaveBeenCalledWith( + expect.objectContaining({ + resolution: CameraResolution.FHD_1080P, + }), + ); unmount(); }); diff --git a/packages/camera-web/test/Camera/hooks/useCameraPreview.test.tsx b/packages/camera-web/test/Camera/hooks/useCameraPreview.test.tsx index 7204682ec..e1a8796ea 100644 --- a/packages/camera-web/test/Camera/hooks/useCameraPreview.test.tsx +++ b/packages/camera-web/test/Camera/hooks/useCameraPreview.test.tsx @@ -40,10 +40,13 @@ describe('useCameraPreview hook', () => { unmount(); }); - it('should make a call to useUserMedia with constraints obtained from useUserMedia', async () => { - const { unmount } = renderHook(useCameraPreview); + it('should make a call to useUserMedia with constraints obtained from useUserMedia and the video ref', async () => { + const { result, unmount } = renderHook(useCameraPreview); await waitFor(() => { - expect(useUserMedia).toHaveBeenCalledWith((getMediaConstraints as jest.Mock)()); + expect(useUserMedia).toHaveBeenCalledWith( + (getMediaConstraints as jest.Mock)(), + result.current.ref, + ); }); unmount(); }); diff --git a/packages/camera-web/test/Camera/hooks/useUserMedia.test.ts b/packages/camera-web/test/Camera/hooks/useUserMedia.test.ts index adc53bd02..55a46cdc4 100644 --- a/packages/camera-web/test/Camera/hooks/useUserMedia.test.ts +++ b/packages/camera-web/test/Camera/hooks/useUserMedia.test.ts @@ -5,6 +5,19 @@ import { useMonitoring } from '@monkvision/monitoring'; import { UserMediaErrorType } from '../../../src'; import { InvalidStreamErrorName, useUserMedia } from '../../../src/Camera/hooks'; import { GetUserMediaMock, mockGetUserMedia } from '../../mocks'; +import { RefObject } from 'react'; +import { createFakePromise } from '@monkvision/test-utils'; + +function renderUseUserMedia(initialProps: { + constraints: MediaStreamConstraints; + videoRef: RefObject; +}) { + return renderHook( + (props: { constraints: MediaStreamConstraints; videoRef: RefObject }) => + useUserMedia(props.constraints, props.videoRef), + { initialProps }, + ); +} describe('useUserMedia hook', () => { let gumMock: GetUserMediaMock | null = null; @@ -18,13 +31,12 @@ describe('useUserMedia hook', () => { }); it('should make a call to the getUserMedia with the given constraints', async () => { + const videoRef = { current: {} } as RefObject; const constraints: MediaStreamConstraints = { audio: false, video: { width: 123, height: 456 }, }; - const { unmount } = renderHook(useUserMedia, { - initialProps: constraints, - }); + const { unmount } = renderUseUserMedia({ constraints, videoRef }); await waitFor(() => { expect(gumMock?.getUserMediaSpy).toHaveBeenCalledTimes(1); expect(gumMock?.getUserMediaSpy).toHaveBeenCalledWith(constraints); @@ -33,13 +45,12 @@ describe('useUserMedia hook', () => { }); it('should return the stream obtained with getUserMedia in case of success', async () => { + const videoRef = { current: {} } as RefObject; const constraints: MediaStreamConstraints = { audio: false, video: { width: 123, height: 456 }, }; - const { result, unmount } = renderHook(useUserMedia, { - initialProps: constraints, - }); + const { result, unmount } = renderUseUserMedia({ constraints, videoRef }); await waitFor(() => { expect(result.current).toEqual({ stream: gumMock?.stream, @@ -53,26 +64,20 @@ describe('useUserMedia hook', () => { }); it('should be loading while the stream is being fetched', async () => { - let done = false; + const videoRef = { current: {} } as RefObject; + const fakePromise = createFakePromise(); + let stream = {} as MediaStream; mockGetUserMedia({ - createMock: (stream) => - jest.fn( - () => - new Promise((resolve) => { - const intervalId = setInterval(() => { - if (done) { - resolve(stream); - clearInterval(intervalId); - } - }, 300); - }), - ), + createMock: (str) => { + stream = str; + return jest.fn(() => fakePromise); + }, }); - const { result, unmount } = renderHook(useUserMedia); + const { result, unmount } = renderUseUserMedia({ constraints: {}, videoRef }); await waitFor(() => { expect(result.current.isLoading).toBe(true); }); - done = true; + fakePromise.resolve(stream); await waitFor(() => { expect(result.current.isLoading).toBe(false); }); @@ -80,10 +85,11 @@ describe('useUserMedia hook', () => { }); it('should return a NotAllowed error in case of camera permission error', async () => { + const videoRef = { current: {} } as RefObject; const nativeError = new Error(); nativeError.name = 'NotAllowedError'; mockGetUserMedia({ createMock: () => jest.fn(() => Promise.reject(nativeError)) }); - const { result, unmount } = renderHook(useUserMedia); + const { result, unmount } = renderUseUserMedia({ constraints: {}, videoRef }); await waitFor(() => { expect(result.current).toEqual({ stream: null, @@ -100,8 +106,9 @@ describe('useUserMedia hook', () => { }); it('should return an InvalidStream error if the stream has no tracks', async () => { + const videoRef = { current: {} } as RefObject; mockGetUserMedia({ tracks: [] }); - const { result, unmount } = renderHook(useUserMedia); + const { result, unmount } = renderUseUserMedia({ constraints: {}, videoRef }); await waitFor(() => { expect(result.current).toEqual({ stream: null, @@ -118,6 +125,7 @@ describe('useUserMedia hook', () => { }); it('should return an InvalidStream error if the stream has more than one track', async () => { + const videoRef = { current: {} } as RefObject; const tracks = [ { kind: 'video', @@ -133,7 +141,7 @@ describe('useUserMedia hook', () => { }, ] as unknown as MediaStreamTrack[]; mockGetUserMedia({ tracks }); - const { result, unmount } = renderHook(useUserMedia); + const { result, unmount } = renderUseUserMedia({ constraints: {}, videoRef }); await waitFor(() => { expect(result.current).toEqual({ stream: null, @@ -152,6 +160,7 @@ describe('useUserMedia hook', () => { }); it("should return an InvalidStream error if the stream's track has no dimensions", async () => { + const videoRef = { current: {} } as RefObject; const invalidSettings = [{ width: 456 }, { height: 123 }, {}]; for (let i = 0; i < invalidSettings.length; i++) { const tracks = [ @@ -163,7 +172,7 @@ describe('useUserMedia hook', () => { }, ] as unknown as MediaStreamTrack[]; mockGetUserMedia({ tracks }); - const { result, unmount } = renderHook(useUserMedia); + const { result, unmount } = renderUseUserMedia({ constraints: {}, videoRef }); // eslint-disable-next-line no-await-in-loop await waitFor(() => { expect(result.current).toEqual({ @@ -184,9 +193,10 @@ describe('useUserMedia hook', () => { }); it('should return an OtherType error in case of unknown error', async () => { + const videoRef = { current: {} } as RefObject; const nativeError = new Error('hello'); mockGetUserMedia({ createMock: () => jest.fn(() => Promise.reject(nativeError)) }); - const { result, unmount } = renderHook(useUserMedia); + const { result, unmount } = renderUseUserMedia({ constraints: {}, videoRef }); await waitFor(() => { expect(result.current).toEqual({ stream: null, @@ -203,9 +213,10 @@ describe('useUserMedia hook', () => { }); it('should call handleError in case of an error', async () => { + const videoRef = { current: {} } as RefObject; const nativeError = new Error('test'); mockGetUserMedia({ createMock: () => jest.fn(() => Promise.reject(nativeError)) }); - const { unmount } = renderHook(useUserMedia); + const { unmount } = renderUseUserMedia({ constraints: {}, videoRef }); const handleErrorMock = (useMonitoring as jest.Mock).mock.results[0].value.handleError; await waitFor(() => { @@ -215,6 +226,7 @@ describe('useUserMedia hook', () => { }); it('should provide a working retry function', async () => { + const videoRef = { current: {} } as RefObject; let gumCallCount = 0; const nativeError = new Error('test'); const mock = mockGetUserMedia({ @@ -224,7 +236,7 @@ describe('useUserMedia hook', () => { return gumCallCount === 1 ? Promise.reject(nativeError) : Promise.resolve(stream); }), }); - const { result, unmount } = renderHook(useUserMedia); + const { result, unmount } = renderUseUserMedia({ constraints: {}, videoRef }); await waitFor(() => { expect(result.current.error?.type).toEqual(UserMediaErrorType.OTHER); }); @@ -240,12 +252,14 @@ describe('useUserMedia hook', () => { }); it('should stop the stream and call getUserMedia again when the constraints change', async () => { + const videoRef = { current: {} } as RefObject; const initialConstraints: MediaStreamConstraints = { audio: false, video: { width: 356, height: 234 }, }; - const { result, unmount, rerender } = renderHook(useUserMedia, { - initialProps: initialConstraints, + const { result, unmount, rerender } = renderUseUserMedia({ + constraints: initialConstraints, + videoRef, }); await waitFor(() => { expect(result.current.stream).toEqual(gumMock?.stream); @@ -259,7 +273,7 @@ describe('useUserMedia hook', () => { }, }; - rerender(newConstraints); + rerender({ constraints: newConstraints, videoRef }); await waitFor(() => { expect(gumMock?.stream.removeEventListener).toHaveBeenCalledWith( 'inactive', @@ -273,30 +287,8 @@ describe('useUserMedia hook', () => { unmount(); }); - it('should switch the dimensions if the device is mobile', async () => { - const userAgentGetter = jest.spyOn(window.navigator, 'userAgent', 'get'); - userAgentGetter.mockReturnValue('iphone'); - const isMobileDeviceMock = isMobileDevice as jest.Mock; - isMobileDeviceMock.mockReturnValue(true); - const constraints: MediaStreamConstraints = { - audio: false, - video: { width: 123, height: 456 }, - }; - const { result, unmount } = renderHook(useUserMedia, { - initialProps: constraints, - }); - await waitFor(() => { - expect(result.current.dimensions).toEqual({ - height: 456, - width: 123, - }); - }); - unmount(); - }); - it('should filter the video constraints by removing: Telephoto and wide camera', async () => { - const userAgentGetter = jest.spyOn(window.navigator, 'userAgent', 'get'); - userAgentGetter.mockReturnValue('iphone'); + const videoRef = { current: {} } as RefObject; const constraints: MediaStreamConstraints = { audio: false, video: { width: 123, height: 456 }, @@ -307,9 +299,7 @@ describe('useUserMedia hook', () => { { kind: 'videoinput', label: 'Wide Angle Camera', deviceId: 'wideDeviceId' }, { kind: 'videoinput', label: 'Telephoto Angle Camera', deviceId: 'wideDeviceId' }, ]); - const { unmount } = renderHook(useUserMedia, { - initialProps: constraints, - }); + const { unmount } = renderUseUserMedia({ constraints, videoRef }); await waitFor(() => { expect(gumMock?.getUserMediaSpy).toHaveBeenCalledWith({ audio: false, @@ -322,4 +312,58 @@ describe('useUserMedia hook', () => { }); unmount(); }); + + it("should switch the dimensions of the stream if it doesn't match the orientation of the mobile device", async () => { + const videoRef = { current: {} } as RefObject; + (isMobileDevice as jest.Mock).mockReturnValue(true); + (global.window.matchMedia as jest.Mock).mockReturnValueOnce({ matches: true }); + const { result, unmount } = renderUseUserMedia({ constraints: {}, videoRef }); + await waitFor(() => { + expect(global.window.matchMedia).toHaveBeenCalledWith('(orientation: portrait)'); + expect(result.current.dimensions).toEqual({ + height: 456, + width: 123, + }); + }); + unmount(); + }); + + it('should not switch the dimensions of the stream on desktop', async () => { + const videoRef = { current: {} } as RefObject; + (isMobileDevice as jest.Mock).mockReturnValue(false); + const { result, unmount } = renderUseUserMedia({ constraints: {}, videoRef }); + await waitFor(() => { + expect(result.current.dimensions).toEqual({ + height: 123, + width: 456, + }); + }); + unmount(); + }); + + it('should update the stream dimensions when the video ref resizes', async () => { + const videoRef = { current: {} } as RefObject; + const { result, rerender, unmount } = renderUseUserMedia({ constraints: {}, videoRef }); + await waitFor(() => { + expect(result.current.dimensions).toEqual({ + width: 456, + height: 123, + }); + }); + (gumMock?.stream.getVideoTracks()[0].getSettings as jest.Mock).mockImplementation(() => ({ + width: 222, + height: 111, + })); + act(() => { + videoRef.current?.onresize?.({} as any); + }); + rerender(); + await waitFor(() => { + expect(result.current.dimensions).toEqual({ + width: 222, + height: 111, + }); + }); + unmount(); + }); }); diff --git a/packages/camera-web/test/mocks/getUserMedia.mock.ts b/packages/camera-web/test/mocks/getUserMedia.mock.ts index c5970466b..a25812c5f 100644 --- a/packages/camera-web/test/mocks/getUserMedia.mock.ts +++ b/packages/camera-web/test/mocks/getUserMedia.mock.ts @@ -47,14 +47,7 @@ export function mockGetUserMedia(params?: MockGetUserMediaParams): GetUserMediaM }); Object.defineProperty(window, 'matchMedia', { writable: true, - value: jest.fn().mockImplementation((query) => ({ - matches: true, // Set the default value as needed - media: query, - onchange: null, - addEventListener: jest.fn(), - removeEventListener: jest.fn(), - dispatchEvent: jest.fn(), - })), + value: jest.fn().mockImplementation(() => ({ matches: true })), }); return { tracks,