fix(recording): let a webcam's MJPEG mode win when it is faster or sharper

electron/native/wgc-capture/src/webcam_format.cpp

@@ -22,6 +22,21 @@ int usefulFps(const WebcamFormat& format, int targetFps) {
     return std::min(format.fps, targetFps);
 }
 
+// The most a compressed mode may cost to decode, in pixels per second: 1080p
+// at 30. Decoding MJPEG to NV12 through the helper's own source-reader setup,
+// on an otherwise idle Ryzen 7 5800X, takes 4.9 ms a frame at 1920x1080,
+// 9.2 ms at 2560x1440 and 19.7 ms at 3840x2160 (p95 22.7). 1080p30 is 15% of
+// the 33 ms frame interval, a margin for slower CPUs and the screen encode
+// running beside it; 4K30 already takes 60% on a fast desktop.
+constexpr long long kMaxDecodedPixelsPerSecond = 1920LL * 1080LL * 30LL;
+
+// A compressed mode is decoded at the rate the camera runs, not the rate we
+// encode at, so a 60 fps mode costs twice its 30 fps twin.
+bool decodable(const WebcamFormat& format, int targetFps) {
+    const int fps = format.fps > 0 ? format.fps : targetFps;
+    return !format.compressed || pixels(format) * fps <= kMaxDecodedPixelsPerSecond;
+}
+
 // True when `candidate` is a better frame rate than `best` for a target of
 // `targetFps`: the lowest rate that still reaches the target, or -- when no
 // mode reaches it -- the highest one on offer.
@@ -43,14 +58,14 @@ WebcamFormat chooseWebcamFormat(
     int targetFps) {
     const WebcamFormat target{targetWidth, targetHeight, targetFps, false};
 
-    // Compressed modes are a last resort, so they are only considered when the
-    // camera offers nothing else at all.
-    const bool anyUncompressed = std::any_of(available.begin(), available.end(), [](const WebcamFormat& format) {
-        return isUsable(format) && !format.compressed;
+    // Compressed modes too costly to decode are only considered when the camera
+    // offers nothing else at all.
+    const bool anyDecodable = std::any_of(available.begin(), available.end(), [&](const WebcamFormat& format) {
+        return isUsable(format) && decodable(format, targetFps);
     });
 
     const auto eligible = [&](const WebcamFormat& format) {
-        return isUsable(format) && !(anyUncompressed && format.compressed);
+        return isUsable(format) && (!anyDecodable || decodable(format, targetFps));
     };
 
     const bool anyFits = std::any_of(available.begin(), available.end(), [&](const WebcamFormat& format) {
@@ -87,6 +102,13 @@ WebcamFormat chooseWebcamFormat(
             }
             continue;
         }
+        // Same picture either way: skip the decoder.
+        if (format.compressed != best->compressed) {
+            if (!format.compressed) {
+                best = &format;
+            }
+            continue;
+        }
         if (betterFps(format.fps, best->fps, targetFps)) {
             best = &format;
         }

electron/native/wgc-capture/src/webcam_format.h

@@ -9,7 +9,7 @@ struct WebcamFormat {
     int fps = 0;
     /**
      * True for MJPEG/H264 modes, which reach higher resolutions than a camera's
-     * uncompressed ones but need a decoder MFT in front of the RGB32 conversion.
+     * uncompressed ones but cost a decode on the CPU for every frame.
      */
     bool compressed = false;
 };
@@ -24,18 +24,20 @@ struct WebcamFormat {
 // then upscaled into the overlay, which is the pixelation users reported.
 //
 // Rules, in order:
-//   * uncompressed modes beat compressed ones outright, whatever their size.
-//     Pinning a camera's MJPEG mode and asking the source reader for RGB32
-//     leaves it delivering no samples at all on some hosts -- a recording with
-//     no camera in it, which is far worse than a smaller one that works;
+//   * a compressed mode is out when decoding it costs more than 1080p at 30
+//     fps (an MJPEG 4K mode, say), unless the camera offers nothing cheaper.
+//     Under that ceiling it competes like any other mode: a C920 only reaches
+//     1080p at 30 in MJPEG, its uncompressed 1080p runs at 5 fps;
 //   * only formats that fit inside the target in BOTH dimensions are eligible,
 //     so an ultrawide mode cannot sneak in on pixel count alone;
 //   * among those, the frame rate comes first: the highest rate up to the
-//     target wins, so a C920's 2304x1536 at 2 fps loses to its 800x448 at 30.
+//     target wins, so a C920's 2304x1536 at 2 fps loses to its 1080p at 30.
 //     Ranking pixels first drove that camera at 2 to 5 fps on every preset;
 //   * then the most pixels;
-//   * ties go to the lowest frame rate that still reaches the target, else the
-//     highest available -- a 1080p60 mode is no help when we encode at 30;
+//   * then an uncompressed mode over a compressed one, which only adds a decode;
+//   * remaining ties go to the lowest frame rate that still reaches the
+//     target, else the highest available -- a 1080p60 mode is no help when we
+//     encode at 30;
 //   * if nothing fits (a camera that only does 4K), the smallest mode at the
 //     best rate wins, so we scale down rather than push 1 GB/s of RGB32
 //     through the encoder;

electron/native/wgc-capture/src/webcam_format_test.cpp

@@ -46,33 +46,75 @@ const std::vector<WebcamFormat> kC920 = {
     {1600, 896, 30, true},   {1920, 1080, 30, true},
 };
 
+// A Logitech StreamCam as Media Foundation lists it on Windows 11: every mode
+// it has at 30 fps, plus its largest YUY2 ones. The NV12 entries mirror the
+// MJPEG ones size for size, so the camera reaches 1080p at 30 uncompressed.
+const std::vector<WebcamFormat> kStreamCam = {
+    {640, 480, 30, false},   {848, 480, 30, false},   {960, 540, 15, false},
+    {1280, 720, 10, false},  {1600, 896, 8, false},   {1920, 1080, 5, false},
+    {640, 480, 30, false},   {640, 480, 30, true},    {848, 480, 30, false},
+    {848, 480, 30, true},    {960, 540, 30, false},   {960, 540, 30, true},
+    {1280, 720, 30, false},  {1280, 720, 30, true},   {1600, 896, 30, false},
+    {1600, 896, 30, true},   {1920, 1080, 30, false}, {1920, 1080, 30, true},
+};
+
+void expectCompressed(const std::string& label, WebcamFormat actual, bool compressed) {
+    if (actual.compressed == compressed) {
+        return;
+    }
+    std::printf("FAIL %s: expected a%s mode\n", label.c_str(), compressed ? " compressed" : "n uncompressed");
+    ++failures;
+}
+
 }  // namespace
 
 int main() {
     // The regression this file exists for: a BRIO asked for 1080p must not come
     // back with the 640x480 default it enumerates first.
     expectFormat("brio targets 1080p", chooseWebcamFormat(kBrio, 1920, 1080, 30), 1920, 1080, 30);
 
-    // Asked for more than the camera can do uncompressed, it must stop at the
-    // best uncompressed mode rather than pin an MJPEG one. Pinning MJPEG and
-    // asking the source reader for RGB32 produced a reader that delivered no
-    // samples at all, so the take had no camera in it.
-    expectFormat(
-        "4K request settles for the best uncompressed mode",
-        chooseWebcamFormat(kBrio, 3840, 2160, 30),
-        1920,
-        1080,
-        30);
-    if (chooseWebcamFormat(kBrio, 3840, 2160, 30).compressed) {
-        std::printf("FAIL 4K request must not pin a compressed mode\n");
-        ++failures;
+    // Asked for more than 1080p, a BRIO stays on its uncompressed 1080p30: its
+    // larger modes are MJPEG, and decoding 1440p or 4K at 30 costs more than
+    // the 1080p30 ceiling (4K takes 20 ms of every 33 ms frame on a desktop).
+    for (const int height : {1440, 2160}) {
+        const std::string label = "brio on " + std::to_string(height) + "p";
+        const WebcamFormat chosen = chooseWebcamFormat(kBrio, height * 16 / 9, height, 30);
+        expectFormat(label + " stays at 1080p30", chosen, 1920, 1080, 30);
+        expectCompressed(label, chosen, false);
     }
 
-    // getopenscreen/openscreen#1112: ranking pixels first drove a C920 at
-    // 2304x1536 and 2 fps on the default 2160p preset, and at 1920x1080 and
-    // 5 fps on 1080p. A usable rate comes before size.
-    expectFormat("c920 on 2160p keeps 30 fps", chooseWebcamFormat(kC920, 3840, 2160, 30), 800, 448, 30);
-    expectFormat("c920 on 1080p keeps 30 fps", chooseWebcamFormat(kC920, 1920, 1080, 30), 800, 448, 30);
+    // getopenscreen/openscreen#1112: a C920 only reaches 1080p at 30 in MJPEG;
+    // its uncompressed 1080p runs at 5 fps. Excluding MJPEG outright left it on
+    // YUY2 800x448 at 30 on every preset.
+    for (const int height : {1080, 1440, 2160}) {
+        const std::string label = "c920 on " + std::to_string(height) + "p";
+        const WebcamFormat chosen = chooseWebcamFormat(kC920, height * 16 / 9, height, 30);
+        expectFormat(label + " takes MJPEG 1080p30", chosen, 1920, 1080, 30);
+        expectCompressed(label, chosen, true);
+    }
+
+    // The StreamCam already has NV12 1080p30, so MJPEG would only add a decode.
+    for (const int height : {1080, 1440, 2160}) {
+        const std::string label = "streamcam on " + std::to_string(height) + "p";
+        const WebcamFormat chosen = chooseWebcamFormat(kStreamCam, height * 16 / 9, height, 30);
+        expectFormat(label + " keeps NV12 1080p30", chosen, 1920, 1080, 30);
+        expectCompressed(label, chosen, false);
+    }
+
+    // Same size and rate: the uncompressed mode wins wherever it is listed.
+    expectCompressed(
+        "uncompressed breaks a tie",
+        chooseWebcamFormat({{1920, 1080, 30, true}, {1920, 1080, 30, false}}, 1920, 1080, 30),
+        false);
+
+    // The decoder runs at the camera's rate, not ours: MJPEG 1080p60 costs
+    // twice the ceiling even though only 30 of its frames are kept.
+    expectFormat(
+        "a 1080p60 MJPEG mode is too costly to decode",
+        chooseWebcamFormat({{1920, 1080, 60, true}, {1280, 720, 30, false}}, 1920, 1080, 30),
+        1280,
+        720,
+        30);
 
     // A camera with nothing but compressed modes still has to be usable.
     expectFormat(
@@ -82,9 +124,17 @@ int main() {
         1080,
         30);
 
-    // Uncompressed wins even when it is much smaller than a compressed option.
+    // Even one too costly to decode: better a working 4K MJPEG than no camera.
+    expectFormat(
+        "a camera with only costly compressed modes is still driven",
+        chooseWebcamFormat({{3840, 2160, 30, true}}, 1920, 1080, 30),
+        3840,
+        2160,
+        30);
+
+    // A mode too costly to decode loses even to a much smaller one.
     expectFormat(
-        "uncompressed beats a larger compressed mode",
+        "4K MJPEG loses to a cheap mode",
         chooseWebcamFormat({{3840, 2160, 30, true}, {640, 480, 30, false}}, 3840, 2160, 30),
         640,
         480,