Switched the camera format to use framerate ratio instead of interval ratio

This is more intuitive for game developers and users.

Fixes https://github.com/libsdl-org/SDL/issues/9896

include/SDL3/SDL_camera.h

@@ -82,8 +82,8 @@ typedef struct SDL_CameraSpec
     SDL_Colorspace colorspace;  /**< Frame colorspace */
     int width;                  /**< Frame width */
     int height;                 /**< Frame height */
-    int interval_numerator;     /**< Frame rate numerator ((dom / num) == fps, (num / dom) == duration) */
-    int interval_denominator;   /**< Frame rate demoninator ((dom / num) == fps, (num / dom) == duration) */
+    int framerate_numerator;     /**< Frame rate numerator ((num / denom) == FPS, (denom / num) == duration in seconds) */
+    int framerate_denominator;   /**< Frame rate demoninator ((num / denom) == FPS, (denom / num) == duration in seconds) */
 } SDL_CameraSpec;
 
 /**

src/camera/SDL_camera.c

@@ -84,7 +84,7 @@ char *SDL_GetCameraThreadName(SDL_CameraDevice *device, char *buf, size_t buflen
     return buf;
 }
 
-int SDL_AddCameraFormat(CameraFormatAddData *data, SDL_PixelFormatEnum format, SDL_Colorspace colorspace, int w, int h, int interval_numerator, int interval_denominator)
+int SDL_AddCameraFormat(CameraFormatAddData *data, SDL_PixelFormatEnum format, SDL_Colorspace colorspace, int w, int h, int framerate_numerator, int framerate_denominator)
 {
     SDL_assert(data != NULL);
     if (data->allocated_specs <= data->num_specs) {
@@ -102,8 +102,8 @@ int SDL_AddCameraFormat(CameraFormatAddData *data, SDL_PixelFormatEnum format, S
     spec->colorspace = colorspace;
     spec->width = w;
     spec->height = h;
-    spec->interval_numerator = interval_numerator;
-    spec->interval_denominator = interval_denominator;
+    spec->framerate_numerator = framerate_numerator;
+    spec->framerate_denominator = framerate_denominator;
 
     data->num_specs++;
 
@@ -119,7 +119,7 @@ static int ZombieWaitDevice(SDL_CameraDevice *device)
 {
     if (!SDL_AtomicGet(&device->shutdown)) {
         // !!! FIXME: this is bad for several reasons (uses double, could be precalculated, doesn't track elasped time).
-        const double duration = ((double) device->actual_spec.interval_numerator / ((double) device->actual_spec.interval_denominator));
+        const double duration = ((double) device->actual_spec.framerate_denominator / ((double) device->actual_spec.framerate_numerator));
         SDL_Delay((Uint32) (duration * 1000.0));
     }
     return 0;
@@ -388,14 +388,14 @@ static int SDLCALL CameraSpecCmp(const void *vpa, const void *vpb)
     }
 
     // still here? We care about framerate less than format or size, but faster is better than slow.
-    if (a->interval_numerator && !b->interval_numerator) {
+    if (a->framerate_numerator && !b->framerate_numerator) {
         return -1;
-    } else if (!a->interval_numerator && b->interval_numerator) {
+    } else if (!a->framerate_numerator && b->framerate_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);
+    const float fpsa = ((float)a->framerate_numerator / a->framerate_denominator);
+    const float fpsb = ((float)b->framerate_numerator / b->framerate_denominator);
     if (fpsa > fpsb) {
         return -1;
     } else if (fpsb > fpsa) {
@@ -483,7 +483,7 @@ SDL_CameraDevice *SDL_AddCameraDevice(const char *name, SDL_CameraPosition posit
     SDL_Log("CAMERA: Adding device '%s' (%s) with %d spec%s%s", name, posstr, num_specs, (num_specs == 1) ? "" : "s", (num_specs == 0) ? "" : ":");
     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);
+        SDL_Log("CAMERA:   - fmt=%s, w=%d, h=%d, numerator=%d, denominator=%d", SDL_GetPixelFormatName(spec->format), spec->width, spec->height, spec->framerate_numerator, spec->framerate_denominator);
     }
     #endif
 
@@ -1024,23 +1024,23 @@ static void ChooseBestCameraSpec(SDL_CameraDevice *device, const SDL_CameraSpec
         closest->format = bestfmt;
 
         // We have a resolution and a format, find the closest framerate...
-        const float wantfps = spec->interval_denominator ? (spec->interval_numerator / spec->interval_denominator) : 0.0f;
+        const float wantfps = spec->framerate_denominator ? ((float)spec->framerate_numerator / spec->framerate_denominator) : 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;
+                if ((thisspec->framerate_numerator == spec->framerate_numerator) && (thisspec->framerate_denominator == spec->framerate_denominator)) {
+                    closest->framerate_numerator = thisspec->framerate_numerator;
+                    closest->framerate_denominator = thisspec->framerate_denominator;
                     break;  // exact match, stop looking.
                 }
 
-                const float thisfps = thisspec->interval_denominator ? (thisspec->interval_numerator / thisspec->interval_denominator) : 0.0f;
+                const float thisfps = thisspec->framerate_denominator ? ((float)thisspec->framerate_numerator / thisspec->framerate_denominator) : 0.0f;
                 const float fpsdiff = SDL_fabsf(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;
+                    closest->framerate_numerator = thisspec->framerate_numerator;
+                    closest->framerate_denominator = thisspec->framerate_denominator;
                 }
             }
         }
@@ -1075,9 +1075,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 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);
+    SDL_Log("CAMERA: App wanted [(%dx%d) fmt=%s framerate=%d/%d], chose [(%dx%d) fmt=%s framerate=%d/%d]",
+            spec ? spec->width : -1, spec ? spec->height : -1, spec ? SDL_GetPixelFormatName(spec->format) : "(null)", spec ? spec->framerate_numerator : -1, spec ? spec->framerate_denominator : -1,
+            closest.width, closest.height, SDL_GetPixelFormatName(closest.format), closest.framerate_numerator, closest.framerate_denominator);
     #endif
 
     if (camera_driver.impl.OpenDevice(device, &closest) < 0) {
@@ -1095,9 +1095,9 @@ SDL_Camera *SDL_OpenCameraDevice(SDL_CameraDeviceID instance_id, const SDL_Camer
         if (spec->format == SDL_PIXELFORMAT_UNKNOWN) {
             device->spec.format = closest.format;
         }
-        if (spec->interval_denominator == 0) {
-            device->spec.interval_numerator = closest.interval_numerator;
-            device->spec.interval_denominator = closest.interval_denominator;
+        if (spec->framerate_denominator == 0) {
+            device->spec.framerate_numerator = closest.framerate_numerator;
+            device->spec.framerate_denominator = closest.framerate_denominator;
         }
     } else {
         SDL_copyp(&device->spec, &closest);

src/camera/SDL_syscamera.h

@@ -64,7 +64,7 @@ typedef struct CameraFormatAddData
     int allocated_specs;
 } CameraFormatAddData;
 
-int SDL_AddCameraFormat(CameraFormatAddData *data, SDL_PixelFormatEnum format, SDL_Colorspace colorspace, int w, int h, int interval_numerator, int interval_denominator);
+int SDL_AddCameraFormat(CameraFormatAddData *data, SDL_PixelFormatEnum format, SDL_Colorspace colorspace, int w, int h, int framerate_numerator, int framerate_denominator);
 
 typedef struct SurfaceList
 {

src/camera/android/SDL_camera_android.c

@@ -655,11 +655,11 @@ static void GatherCameraSpecs(const char *devid, CameraFormatAddData *add_data,
             const long long duration = (long long) i64ptr[3];
             SDL_Log("CAMERA: possible fps %s %dx%d duration=%lld", SDL_GetPixelFormatName(sdlfmt), fpsw, fpsh, duration);
             if ((duration > 0) && (fpsfmt == fmt) && (fpsw == w) && (fpsh == h)) {
-                SDL_AddCameraFormat(add_data, sdlfmt, colorspace, w, h, duration, 1000000000);
+                SDL_AddCameraFormat(add_data, sdlfmt, colorspace, w, h, 1000000000, duration);
             }
         }
 #else
-        SDL_AddCameraFormat(add_data, sdlfmt, colorspace, w, h, 1, 30);
+        SDL_AddCameraFormat(add_data, sdlfmt, colorspace, w, h, 30, 1);
 #endif
     }
 

src/camera/coremedia/SDL_camera_coremedia.m

@@ -255,7 +255,7 @@ static int COREMEDIA_OpenDevice(SDL_CameraDevice *device, const SDL_CameraSpec *
     // Pick format that matches the spec
     const int w = spec->width;
     const int h = spec->height;
-    const int rate = spec->interval_denominator;
+    const float rate = (float)spec->framerate_numerator / spec->framerate_denominator;
     AVCaptureDeviceFormat *spec_format = nil;
     NSArray<AVCaptureDeviceFormat *> *formats = [avdevice formats];
     for (AVCaptureDeviceFormat *format in formats) {
@@ -273,7 +273,7 @@ static int COREMEDIA_OpenDevice(SDL_CameraDevice *device, const SDL_CameraSpec *
         }
 
         for (AVFrameRateRange *framerate in format.videoSupportedFrameRateRanges) {
-            if ((rate == (int) SDL_ceil((double) framerate.minFrameRate)) || (rate == (int) SDL_floor((double) framerate.maxFrameRate))) {
+            if (rate >= framerate.minFrameRate && rate <= framerate.maxFrameRate) {
                 spec_format = format;
                 break;
             }
@@ -381,7 +381,7 @@ static void GatherCameraSpecs(AVCaptureDevice *device, CameraFormatAddData *add_
             continue;
         }
 
-NSLog(@"Available camera format: %@\n", fmt);
+//NSLog(@"Available camera format: %@\n", fmt);
         SDL_PixelFormatEnum device_format = SDL_PIXELFORMAT_UNKNOWN;
         SDL_Colorspace device_colorspace = SDL_COLORSPACE_UNKNOWN;
         CoreMediaFormatToSDL(CMFormatDescriptionGetMediaSubType(fmt.formatDescription), &device_format, &device_colorspace);
@@ -393,16 +393,12 @@ NSLog(@"Available camera format: %@\n", fmt);
         const int w = (int) dims.width;
         const int h = (int) dims.height;
         for (AVFrameRateRange *framerate in fmt.videoSupportedFrameRateRanges) {
-            int rate;
+            int numerator = 0, denominator = 1;
 
-            rate = (int) SDL_ceil((double) framerate.minFrameRate);
-            if (rate) {
-                SDL_AddCameraFormat(add_data, device_format, device_colorspace, w, h, 1, rate);
-            }
-            rate = (int) SDL_floor((double) framerate.maxFrameRate);
-            if (rate) {
-                SDL_AddCameraFormat(add_data, device_format, device_colorspace, w, h, 1, rate);
-            }
+            SDL_CalculateFraction(framerate.minFrameRate, &numerator, &denominator);
+            SDL_AddCameraFormat(add_data, device_format, device_colorspace, w, h, numerator, denominator);
+            SDL_CalculateFraction(framerate.maxFrameRate, &numerator, &denominator);
+            SDL_AddCameraFormat(add_data, device_format, device_colorspace, w, h, numerator, denominator);
         }
     }
 }

src/camera/emscripten/SDL_camera_emscripten.c

@@ -102,8 +102,8 @@ static void SDLEmscriptenCameraDevicePermissionOutcome(SDL_CameraDevice *device,
 {
     device->spec.width = device->actual_spec.width = w;
     device->spec.height = device->actual_spec.height = h;
-    device->spec.interval_numerator = device->actual_spec.interval_numerator = 1;
-    device->spec.interval_denominator = device->actual_spec.interval_denominator = fps;
+    device->spec.framerate_numerator = device->actual_spec.framerate_numerator = fps;
+    device->spec.framerate_denominator = device->actual_spec.framerate_denominator = 1;
     SDL_CameraDevicePermissionOutcome(device, approved ? SDL_TRUE : SDL_FALSE);
 }
 
@@ -115,8 +115,8 @@ static int EMSCRIPTENCAMERA_OpenDevice(SDL_CameraDevice *device, const SDL_Camer
         const device = $0;
         const w = $1;
         const h = $2;
-        const interval_numerator = $3;
-        const interval_denominator = $4;
+        const framerate_numerator = $3;
+        const framerate_denominator = $4;
         const outcome = $5;
         const iterate = $6;
 
@@ -129,8 +129,8 @@ static int EMSCRIPTENCAMERA_OpenDevice(SDL_CameraDevice *device, const SDL_Camer
             constraints.video.height = h;
         }
 
-        if ((interval_numerator > 0) && (interval_denominator > 0)) {
-            var fps = interval_denominator / interval_numerator;
+        if ((framerate_numerator > 0) && (framerate_denominator > 0)) {
+            var fps = framerate_numerator / framerate_denominator;
             constraints.video.frameRate = { ideal: fps };
         }
 
@@ -199,7 +199,7 @@ static int EMSCRIPTENCAMERA_OpenDevice(SDL_CameraDevice *device, const SDL_Camer
                 console.error("Tried to open camera but it threw an error! " + err.name + ": " +  err.message);
                 dynCall('viiiii', outcome, [device, 0, 0, 0, 0]);   // we call this a permission error, because it probably is.
             });
-    }, device, spec->width, spec->height, spec->interval_numerator, spec->interval_denominator, SDLEmscriptenCameraDevicePermissionOutcome, SDL_CameraThreadIterate);
+    }, device, spec->width, spec->height, spec->framerate_numerator, spec->framerate_denominator, SDLEmscriptenCameraDevicePermissionOutcome, SDL_CameraThreadIterate);
 
     return 0;  // the real work waits until the user approves a camera.
 }

src/camera/mediafoundation/SDL_camera_mediafoundation.c

@@ -765,7 +765,7 @@ static int MEDIAFOUNDATION_OpenDevice(SDL_CameraDevice *device, const SDL_Camera
     ret = IMFMediaType_SetUINT64(mediatype, &SDL_MF_MT_FRAME_SIZE, (((UINT64)spec->width) << 32) | ((UINT64)spec->height));
     CHECK_HRESULT("MFSetAttributeSize(frame_size)", ret);
 
-    ret = IMFMediaType_SetUINT64(mediatype, &SDL_MF_MT_FRAME_RATE, (((UINT64)spec->interval_numerator) << 32) | ((UINT64)spec->interval_denominator));
+    ret = IMFMediaType_SetUINT64(mediatype, &SDL_MF_MT_FRAME_RATE, (((UINT64)spec->framerate_numerator) << 32) | ((UINT64)spec->framerate_denominator));
     CHECK_HRESULT("MFSetAttributeRatio(frame_rate)", ret);
 
     ret = IMFSourceReader_SetCurrentMediaType(srcreader, (DWORD)MF_SOURCE_READER_FIRST_VIDEO_STREAM, NULL, mediatype);
@@ -940,12 +940,12 @@ static void GatherCameraSpecs(IMFMediaSource *source, CameraFormatAddData *add_d
                                 w = (UINT32)(val >> 32);
                                 h = (UINT32)val;
                                 if (SUCCEEDED(ret) && w && h) {
-                                    UINT32 interval_numerator = 0, interval_denominator = 0;
+                                    UINT32 framerate_numerator = 0, framerate_denominator = 0;
                                     ret = IMFMediaType_GetUINT64(mediatype, &SDL_MF_MT_FRAME_RATE, &val);
-                                    interval_numerator = (UINT32)(val >> 32);
-                                    interval_denominator = (UINT32)val;
-                                    if (SUCCEEDED(ret) && interval_numerator && interval_denominator) {
-                                        SDL_AddCameraFormat(add_data, sdlfmt, colorspace, (int) w, (int) h, (int)interval_numerator, (int)interval_denominator);
+                                    framerate_numerator = (UINT32)(val >> 32);
+                                    framerate_denominator = (UINT32)val;
+                                    if (SUCCEEDED(ret) && framerate_numerator && framerate_denominator) {
+                                        SDL_AddCameraFormat(add_data, sdlfmt, colorspace, (int) w, (int) h, (int)framerate_numerator, (int)framerate_denominator);
                                     }
                                 }
                             }

src/camera/pipewire/SDL_camera_pipewire.c

@@ -517,7 +517,7 @@ static int PIPEWIRECAMERA_OpenDevice(SDL_CameraDevice *device, const SDL_CameraS
                     SPA_FORMAT_VIDEO_format, SPA_POD_Id(sdl_format_to_id(spec->format)),
                     SPA_FORMAT_VIDEO_size, SPA_POD_Rectangle(&SPA_RECTANGLE(spec->width, spec->height)),
                     SPA_FORMAT_VIDEO_framerate,
-		        SPA_POD_Fraction(&SPA_FRACTION(spec->interval_numerator, spec->interval_denominator)));
+		        SPA_POD_Fraction(&SPA_FRACTION(spec->framerate_numerator, spec->framerate_denominator)));
 
     if ((res = PIPEWIRE_pw_stream_connect(device->hidden->stream,
                                     PW_DIRECTION_INPUT,
@@ -625,8 +625,7 @@ static void collect_rates(CameraFormatAddData *data, struct param *p, SDL_PixelF
     SPA_FALLTHROUGH;
     case SPA_CHOICE_Enum:
         for (i = 0; i < n_vals; i++) {
-            // denom and num are switched, because SDL expects an interval, while pw provides a rate
-            if (SDL_AddCameraFormat(data, sdlfmt, colorspace, size->width, size->height, rates[i].denom, rates[i].num) < 0) {
+            if (SDL_AddCameraFormat(data, sdlfmt, colorspace, size->width, size->height, rates[i].num, rates[i].denom) < 0) {
                 return;  // Probably out of memory; we'll go with what we have, if anything.
             }
         }

src/camera/v4l2/SDL_camera_v4l2.c

@@ -538,16 +538,16 @@ 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) {
+    if (spec->framerate_numerator && spec->framerate_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) ) {
+            if ( (setfps.parm.capture.timeperframe.denominator != spec->framerate_numerator) ||
+                 (setfps.parm.capture.timeperframe.numerator = spec->framerate_denominator) ) {
                 setfps.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-                setfps.parm.capture.timeperframe.numerator = spec->interval_numerator;
-                setfps.parm.capture.timeperframe.denominator = spec->interval_denominator;
+                setfps.parm.capture.timeperframe.numerator = spec->framerate_numerator;
+                setfps.parm.capture.timeperframe.denominator = spec->framerate_denominator;
                 if (xioctl(fd, VIDIOC_S_PARM, &setfps) == -1) {
                     return SDL_SetError("Error VIDIOC_S_PARM");
                 }
@@ -661,7 +661,7 @@ static int AddCameraFormat(const int fd, CameraFormatAddData *data, SDL_PixelFor
             const float fps = (float) denominator / (float) numerator;
             SDL_Log("CAMERA:       * Has discrete frame interval (%d / %d), fps=%f", numerator, denominator, fps);
             #endif
-            if (SDL_AddCameraFormat(data, sdlfmt, colorspace, w, h, numerator, denominator) == -1) {
+            if (SDL_AddCameraFormat(data, sdlfmt, colorspace, w, h, denominator, numerator) == -1) {
                 return -1;  // Probably out of memory; we'll go with what we have, if anything.
             }
             frmivalenum.index++;  // set up for the next one.
@@ -673,7 +673,8 @@ static int AddCameraFormat(const int fd, CameraFormatAddData *data, SDL_PixelFor
                 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 (SDL_AddCameraFormat(data, sdlfmt, colorspace, w, h, n, d) == -1) {
+                // SDL expects framerate, V4L2 provides interval
+                if (SDL_AddCameraFormat(data, sdlfmt, colorspace, w, h, d, n) == -1) {
                     return -1;  // Probably out of memory; we'll go with what we have, if anything.
                 }
                 d += (int) frmivalenum.stepwise.step.denominator;

test/testcamera.c

@@ -134,8 +134,8 @@ int SDL_AppInit(void **appstate, int argc, char *argv[])
     }
 
     SDL_CameraSpec *pspec = &spec;
-    spec.interval_numerator = 1000;
-    spec.interval_denominator = 1;
+    spec.framerate_numerator = 1000;
+    spec.framerate_denominator = 1;
 
     camera = SDL_OpenCameraDevice(camera_id, pspec);
     if (!camera) {