Remove fps24 option from getMediaConstraints

This fps24 is always on in pwa now, so this constant `true` arg can be replaced with a default 24 fps in code.
whereby · Feb 26, 2024 · 3ae8323 · 3ae8323
1 parent 92c8301
commit 3ae8323
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 20 deletions.
diff --git a/src/webrtc/mediaConstraints.js b/src/webrtc/mediaConstraints.js
@@ -11,7 +11,6 @@ const parseResolution = (res) => res.split(/[^\d]/g).map((n) => parseInt(n, 10))
 export function getMediaConstraints({
     disableAEC,
     disableAGC,
-    fps24,
     hd,
     lax,
     lowDataMode,
@@ -22,7 +21,6 @@ export function getMediaConstraints({
 }) {
     let HIGH_HEIGHT = 480;
     let LOW_HEIGHT = 240;
-    let NON_STANDARD_FPS = 0;
 
     if (hd) {
         // respect user choice, but default to HD for pro, and SD for free
@@ -37,18 +35,14 @@ export function getMediaConstraints({
         }
     }
 
-    // Set framerate to 24 to increase quality/bandwidth
-    if (fps24) NON_STANDARD_FPS = 24;
-
-    // Set framerate for low data, but only for non-simulcast
-    if (lowDataMode && !simulcast) NON_STANDARD_FPS = 15;
-
     const constraints = {
         audio: { ...(preferredDeviceIds.audioId && { deviceId: preferredDeviceIds.audioId }) },
         video: {
             ...(preferredDeviceIds.videoId ? { deviceId: preferredDeviceIds.videoId } : { facingMode: "user" }),
             height: lowDataMode ? LOW_HEIGHT : HIGH_HEIGHT,
-            ...(NON_STANDARD_FPS && { frameRate: NON_STANDARD_FPS }),
+            // Set a lower frame rate (15fps) for low data, but only for non-simulcast.
+            // Otherwise use 24fps to increase quality/bandwidth.
+            frameRate: lowDataMode && !simulcast ? 15 : 24,
         },
     };
     if (lax) {

diff --git a/tests/webrtc/mediaConstraints.spec.js b/tests/webrtc/mediaConstraints.spec.js
@@ -1,4 +1,4 @@
-import getConstraints from "../../src/webrtc/mediaConstraints";
+import getConstraints, { getMediaConstraints } from "../../src/webrtc/mediaConstraints";
 
 // the selectGetConstraintsOptions is testing most permutations of this
 describe("getConstraints", () => {
@@ -15,17 +15,24 @@ describe("getConstraints", () => {
 
         expect(result).toEqual({ video: expect.any(Object) });
     });
+});
 
-    it("should set fps to 24 if fps24 is true", () => {
-        const result = getConstraints({ devices: [vdev1], options: { fps24: true, hd: true } });
+describe("getMediaConstraints", () => {
+    describe("frameRate", () => {
+        it.each`
+            lowDataMode | simulcast | expected
+            ${false}    | ${false}  | ${24}
+            ${true}     | ${false}  | ${15}
+            ${true}     | ${true}   | ${24}
+        `(
+            "should set frameRate to $expected if lowDataMode is $lowDataMode and simulcast is $simulcast",
+            ({ lowDataMode, simulcast, expected }) => {
+                const preferredDeviceIds = { audioId: "audioId", videoId: "videoId" };
 
-        expect(result).toEqual({
-            video: {
-                aspectRatio: 1.3333333333333333,
-                facingMode: "user",
-                frameRate: 24,
-                height: { ideal: 720, min: 360 },
-            },
-        });
+                const result = getMediaConstraints({ lowDataMode, preferredDeviceIds, simulcast });
+
+                expect(result.video.frameRate).toBe(expected);
+            }
+        );
     });
 });