SDL: camera: framerate support.

From 0b5875825e42eb56623847a7c812fc01d03182c4 Mon Sep 17 00:00:00 2001
From: "Ryan C. Gordon" <[EMAIL REDACTED]>
Date: Sun, 17 Dec 2023 13:38:36 -0500
Subject: [PATCH] camera: framerate support.

---
 include/SDL3/SDL_camera.h         | 11 ++++-
 src/camera/SDL_camera.c           | 51 +++++++++++++++++++++-
 src/camera/v4l2/SDL_camera_v4l2.c | 70 ++++++++++++++++++++++++++++---
 3 files changed, 122 insertions(+), 10 deletions(-)

diff --git a/include/SDL3/SDL_camera.h b/include/SDL3/SDL_camera.h
index d31e04550826..c19e50db9e9a 100644
--- a/include/SDL3/SDL_camera.h
+++ b/include/SDL3/SDL_camera.h
@@ -66,6 +66,8 @@ typedef struct SDL_CameraSpec
     Uint32 format;          /**< Frame SDL_PixelFormatEnum format */
     int width;              /**< Frame width */
     int height;             /**< Frame height */
+    int interval_numerator;  /**< Frame rate numerator ((dom / num) == fps) */
+    int interval_denominator;  /**< Frame rate demoninator ((dom / num) == fps)*/
 } SDL_CameraSpec;
 
 /**
@@ -216,6 +218,12 @@ extern DECLSPEC char * SDLCALL SDL_GetCameraDeviceName(SDL_CameraDeviceID instan
  * passing a NULL spec here. You can see the exact specs a device can
  * support without conversion with SDL_GetCameraSupportedSpecs().
  *
+ * SDL will not attempt to emulate framerate; it will try to set the
+ * hardware to the rate closest to the requested speed, but it won't
+ * attempt to limit or duplicate frames artificially; call
+ * SDL_GetCameraFormat() to see the actual framerate of the opened the device,
+ * and check your timestamps if this is crucial to your app!
+ *
  * \param instance_id the camera device instance ID
  * \param spec The desired format for data the device will provide. Can be NULL.
  * \returns device, or NULL on failure; call SDL_GetError() for more
@@ -225,9 +233,8 @@ extern DECLSPEC char * SDLCALL SDL_GetCameraDeviceName(SDL_CameraDeviceID instan
  *
  * \since This function is available since SDL 3.0.0.
  *
- * \sa SDL_GetCameraDeviceName
  * \sa SDL_GetCameraDevices
- * \sa SDL_OpenCameraWithSpec
+ * \sa SDL_GetCameraFormat
  */
 extern DECLSPEC SDL_Camera *SDLCALL SDL_OpenCameraDevice(SDL_CameraDeviceID instance_id, const SDL_CameraSpec *spec);
 
diff --git a/src/camera/SDL_camera.c b/src/camera/SDL_camera.c
index 18efa3f282e7..c8af9f0e2e28 100644
--- a/src/camera/SDL_camera.c
+++ b/src/camera/SDL_camera.c
@@ -224,6 +224,21 @@ static int SDLCALL CameraSpecCmp(const void *vpa, const void *vpb)
         return 1;
     }
 
+    // still here? We care about framerate less than format or size, but faster is better than slow.
+    if (a->interval_numerator && !b->interval_numerator) {
+        return -1;
+    } else if (!a->interval_numerator && b->interval_numerator) {
+        return 1;
+    }
+
+    const float fpsa = ((float) a->interval_denominator)/ ((float) a->interval_numerator);
+    const float fpsb = ((float) b->interval_denominator)/ ((float) b->interval_numerator);
+    if (fpsa > fpsb) {
+        return -1;
+    } else if (fpsb > fpsa) {
+        return 1;
+    }
+
     return 0;  // apparently, they're equal.
 }
 
@@ -276,13 +291,21 @@ SDL_CameraDevice *SDL_AddCameraDevice(const char *name, int num_specs, const SDL
     for (int i = 0; i < num_specs; i++) {
         SDL_CameraSpec *a = &device->all_specs[i];
         SDL_CameraSpec *b = &device->all_specs[i + 1];
-        if ((a->format == b->format) && (a->width == b->width) && (a->height == b->height)) {
+        if (SDL_memcmp(a, b, sizeof (*a)) == 0) {
             SDL_memmove(a, b, sizeof (*specs) * (num_specs - i));
             i--;
             num_specs--;
         }
     }
 
+    #if DEBUG_CAMERA
+    SDL_Log("CAMERA: Adding device ('%s') with %d spec%s:", name, num_specs, (num_specs == 1) ? "" : "s");
+    for (int i = 0; i < num_specs; i++) {
+        const SDL_CameraSpec *spec = &device->all_specs[i];
+        SDL_Log("CAMERA:   - fmt=%s, w=%d, h=%d, numerator=%d, denominator=%d", SDL_GetPixelFormatName(spec->format), spec->width, spec->height, spec->interval_numerator, spec->interval_denominator);
+    }
+    #endif
+
     device->num_specs = num_specs;
     device->handle = handle;
     device->instance_id = SDL_GetNextObjectID();
@@ -734,6 +757,28 @@ static void ChooseBestCameraSpec(SDL_CameraDevice *device, const SDL_CameraSpec
 
         SDL_assert(bestfmt != SDL_PIXELFORMAT_UNKNOWN);
         closest->format = bestfmt;
+
+        // We have a resolution and a format, find the closest framerate...
+        const float wantfps = spec->interval_numerator ? (spec->interval_denominator / spec->interval_numerator) : 0.0f;
+        float closestfps = 9999999.0f;
+        for (int i = 0; i < num_specs; i++) {
+            const SDL_CameraSpec *thisspec = &device->all_specs[i];
+            if ((thisspec->format == closest->format) && (thisspec->width == closest->width) && (thisspec->height == closest->height)) {
+                if ((thisspec->interval_numerator == spec->interval_numerator) && (thisspec->interval_denominator == spec->interval_denominator)) {
+                    closest->interval_numerator = thisspec->interval_numerator;
+                    closest->interval_denominator = thisspec->interval_denominator;
+                    break;  // exact match, stop looking.
+                }
+
+                const float thisfps = thisspec->interval_numerator ? (thisspec->interval_denominator / thisspec->interval_numerator) : 0.0f;
+                const float fpsdiff = SDL_fabs(wantfps - thisfps);
+                if (fpsdiff < closestfps) {  // this is a closest FPS? Take it until something closer arrives.
+                    closestfps = fpsdiff;
+                    closest->interval_numerator = thisspec->interval_numerator;
+                    closest->interval_denominator = thisspec->interval_denominator;
+                }
+            }
+        }
     }
 
     SDL_assert(closest->width > 0);
@@ -770,7 +815,9 @@ SDL_Camera *SDL_OpenCameraDevice(SDL_CameraDeviceID instance_id, const SDL_Camer
     ChooseBestCameraSpec(device, spec, &closest);
 
     #if DEBUG_CAMERA
-    SDL_Log("CAMERA: App wanted [(%dx%d) fmt=%s], chose [(%dx%d) fmt=%s]", spec ? spec->width : -1, spec ? spec->height : -1, spec ? SDL_GetPixelFormatName(spec->format) : "(null)", closest.width, closest.height, SDL_GetPixelFormatName(closest.format));
+    SDL_Log("CAMERA: App wanted [(%dx%d) fmt=%s interval=%d/%d], chose [(%dx%d) fmt=%s interval=%d/%d]",
+            spec ? spec->width : -1, spec ? spec->height : -1, spec ? SDL_GetPixelFormatName(spec->format) : "(null)", spec ? spec->interval_numerator : -1, spec ? spec->interval_denominator : -1,
+            closest.width, closest.height, SDL_GetPixelFormatName(closest.format), closest.interval_numerator, closest.interval_denominator);
     #endif
 
     if (camera_driver.impl.OpenDevice(device, &closest) < 0) {
diff --git a/src/camera/v4l2/SDL_camera_v4l2.c b/src/camera/v4l2/SDL_camera_v4l2.c
index 7b68378fd442..6e5cadd7ce91 100644
--- a/src/camera/v4l2/SDL_camera_v4l2.c
+++ b/src/camera/v4l2/SDL_camera_v4l2.c
@@ -524,6 +524,23 @@ static int V4L2_OpenDevice(SDL_CameraDevice *device, const SDL_CameraSpec *spec)
         return SDL_SetError("Error VIDIOC_S_FMT");
     }
 
+    if (spec->interval_numerator && spec->interval_denominator) {
+        struct v4l2_streamparm setfps;
+        SDL_zero(setfps);
+        setfps.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+        if (xioctl(fd, VIDIOC_G_PARM, &setfps) == 0) {
+            if ( (setfps.parm.capture.timeperframe.numerator != spec->interval_numerator) ||
+                 (setfps.parm.capture.timeperframe.denominator = spec->interval_denominator) ) {
+                setfps.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+                setfps.parm.capture.timeperframe.numerator = spec->interval_numerator;
+                setfps.parm.capture.timeperframe.denominator = spec->interval_denominator;
+                if (xioctl(fd, VIDIOC_S_PARM, &setfps) == -1) {
+                    return SDL_SetError("Error VIDIOC_S_PARM");
+                }
+            }
+        }
+    }
+
     SDL_zero(fmt);
     fmt.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
     if (xioctl(fd, VIDIOC_G_FMT, &fmt) == -1) {
@@ -618,7 +635,7 @@ typedef struct FormatAddData
     int allocated_specs;
 } FormatAddData;
 
-static int AddCameraFormat(FormatAddData *data, Uint32 fmt, int w, int h)
+static int AddCameraCompleteFormat(FormatAddData *data, Uint32 fmt, int w, int h, int interval_numerator, int interval_denominator)
 {
     SDL_assert(data != NULL);
     if (data->allocated_specs <= data->num_specs) {
@@ -635,12 +652,55 @@ static int AddCameraFormat(FormatAddData *data, Uint32 fmt, int w, int h)
     spec->format = fmt;
     spec->width = w;
     spec->height = h;
+    spec->interval_numerator = interval_numerator;
+    spec->interval_denominator = interval_denominator;
 
     data->num_specs++;
 
     return 0;
 }
 
+static int AddCameraFormat(const int fd, FormatAddData *data, Uint32 sdlfmt, Uint32 v4l2fmt, int w, int h)
+{
+    struct v4l2_frmivalenum frmivalenum;
+    SDL_zero(frmivalenum);
+    frmivalenum.pixel_format = v4l2fmt;
+    frmivalenum.width = (Uint32) w;
+    frmivalenum.height = (Uint32) h;
+
+    while (ioctl(fd, VIDIOC_ENUM_FRAMEINTERVALS, &frmivalenum) == 0) {
+        if (frmivalenum.type == V4L2_FRMIVAL_TYPE_DISCRETE) {
+            const int numerator = (int) frmivalenum.discrete.numerator;
+            const int denominator = (int) frmivalenum.discrete.denominator;
+            #if DEBUG_CAMERA
+            const float fps = (float) denominator / (float) numerator;
+            SDL_Log("CAMERA:       * Has discrete frame interval (%d / %d), fps=%f", numerator, denominator, fps);
+            #endif
+            if (AddCameraCompleteFormat(data, sdlfmt, w, h, numerator, denominator) == -1) {
+                return -1;  // Probably out of memory; we'll go with what we have, if anything.
+            }
+            frmivalenum.index++;  // set up for the next one.
+        } else if ((frmivalenum.type == V4L2_FRMIVAL_TYPE_STEPWISE) || (frmivalenum.type == V4L2_FRMIVAL_TYPE_CONTINUOUS)) {
+            int d = frmivalenum.stepwise.min.denominator;
+            // !!! FIXME: should we step by the numerator...?
+            for (int n = (int) frmivalenum.stepwise.min.numerator; n <= (int) frmivalenum.stepwise.max.numerator; n += (int) frmivalenum.stepwise.step.numerator) {
+                #if DEBUG_CAMERA
+                const float fps = (float) d / (float) n;
+                SDL_Log("CAMERA:       * Has %s frame interval (%d / %d), fps=%f", (frmivalenum.type == V4L2_FRMIVAL_TYPE_STEPWISE) ? "stepwise" : "continuous", n, d, fps);
+                #endif
+                if (AddCameraCompleteFormat(data, sdlfmt, w, h, n, d) == -1) {
+                    return -1;  // Probably out of memory; we'll go with what we have, if anything.
+                }
+                d += (int) frmivalenum.stepwise.step.denominator;
+            }
+            break;
+        }
+    }
+
+    return 0;
+}
+
+
 static void MaybeAddDevice(const char *path)
 {
     if (!path) {
@@ -699,17 +759,16 @@ static void MaybeAddDevice(const char *path)
 
         struct v4l2_frmsizeenum frmsizeenum;
         SDL_zero(frmsizeenum);
-        frmsizeenum.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
         frmsizeenum.pixel_format = fmtdesc.pixelformat;
 
-        while (ioctl(fd,VIDIOC_ENUM_FRAMESIZES, &frmsizeenum) == 0) {
+        while (ioctl(fd, VIDIOC_ENUM_FRAMESIZES, &frmsizeenum) == 0) {
             if (frmsizeenum.type == V4L2_FRMSIZE_TYPE_DISCRETE) {
                 const int w = (int) frmsizeenum.discrete.width;
                 const int h = (int) frmsizeenum.discrete.height;
                 #if DEBUG_CAMERA
                 SDL_Log("CAMERA:     * Has discrete size %dx%d", w, h);
                 #endif
-                if (AddCameraFormat(&add_data, sdlfmt, w, h) == -1) {
+                if (AddCameraFormat(fd, &add_data, sdlfmt, fmtdesc.pixelformat, w, h) == -1) {
                     break;  // Probably out of memory; we'll go with what we have, if anything.
                 }
                 frmsizeenum.index++;  // set up for the next one.
@@ -725,10 +784,9 @@ static void MaybeAddDevice(const char *path)
                         #if DEBUG_CAMERA
                         SDL_Log("CAMERA:     * Has %s size %dx%d", (frmsizeenum.type == V4L2_FRMSIZE_TYPE_STEPWISE) ? "stepwise" : "continuous", w, h);
                         #endif
-                        if (AddCameraFormat(&add_data, sdlfmt, w, h) == -1) {
+                        if (AddCameraFormat(fd, &add_data, sdlfmt, fmtdesc.pixelformat, w, h) == -1) {
                             break;  // Probably out of memory; we'll go with what we have, if anything.
                         }
-
                     }
                 }
                 break;