SDL: camera: Massive code reworking.

From d3e6ef3cc6e04139204302f07b59a08b3d41cbd1 Mon Sep 17 00:00:00 2001
From: "Ryan C. Gordon" <[EMAIL REDACTED]>
Date: Fri, 15 Dec 2023 11:45:11 -0500
Subject: [PATCH] camera: Massive code reworking.

- Simplified public API, simplified backend interface.
- Camera device hotplug events.
- Thread code is split up so it backends that provide own threads can use it.
- Added "dummy" backend.

Note that CoreMedia (Apple) and Android backends need to be updated, as does
the testcamera app (testcameraminimal works).
---
 include/SDL3/SDL_camera.h                   |  315 ++---
 include/SDL3/SDL_events.h                   |   17 +
 src/camera/SDL_camera.c                     | 1218 ++++++++++++-------
 src/camera/SDL_camera_c.h                   |    3 +
 src/camera/SDL_syscamera.h                  |  132 +-
 src/camera/coremedia/SDL_camera_coremedia.m |   15 +-
 src/camera/dummy/SDL_camera_dummy.c         |   80 ++
 src/camera/v4l2/SDL_camera_v4l2.c           | 1170 +++++++-----------
 src/dynapi/SDL_dynapi.sym                   |   17 +-
 src/dynapi/SDL_dynapi_overrides.h           |   17 +-
 src/dynapi/SDL_dynapi_procs.h               |   27 +-
 src/events/SDL_events.c                     |   14 +
 test/testcamera.c                           |   10 +-
 test/testcameraminimal.c                    |   94 +-
 14 files changed, 1657 insertions(+), 1472 deletions(-)
 create mode 100644 src/camera/dummy/SDL_camera_dummy.c

diff --git a/include/SDL3/SDL_camera.h b/include/SDL3/SDL_camera.h
index e2b13cfe82dd..eb6a9f2d8295 100644
--- a/include/SDL3/SDL_camera.h
+++ b/include/SDL3/SDL_camera.h
@@ -49,23 +49,16 @@ typedef Uint32 SDL_CameraDeviceID;
 
 
 /**
- * The structure used to identify an SDL camera device
+ * The structure used to identify an opened SDL camera
  */
-struct SDL_CameraDevice;
-typedef struct SDL_CameraDevice SDL_CameraDevice;
-
-#define SDL_CAMERA_ALLOW_ANY_CHANGE          1
+struct SDL_Camera;
+typedef struct SDL_Camera SDL_Camera;
 
 /**
  *  SDL_CameraSpec structure
  *
- *  Only those field can be 'desired' when configuring the device:
- *  - format
- *  - width
- *  - height
- *
- *  \sa SDL_GetCameraFormat
- *  \sa SDL_GetCameraFrameSize
+ * \sa SDL_GetCameraDeviceSupportedSpecs
+ * \sa SDL_GetCameraSpec
  *
  */
 typedef struct SDL_CameraSpec
@@ -75,39 +68,6 @@ typedef struct SDL_CameraSpec
     int height;             /**< Frame height */
 } SDL_CameraSpec;
 
-/**
- *  SDL Camera Status
- *
- *  Change states but calling the function in this order:
- *
- *  SDL_OpenCamera()
- *  SDL_SetCameraSpec()  -> Init
- *  SDL_StartCamera()    -> Playing
- *  SDL_StopCamera()     -> Stopped
- *  SDL_CloseCamera()
- *
- */
-typedef enum
-{
-    SDL_CAMERA_FAIL = -1,    /**< Failed */
-    SDL_CAMERA_INIT = 0,     /**< Init, spec hasn't been set */
-    SDL_CAMERA_STOPPED,      /**< Stopped */
-    SDL_CAMERA_PLAYING       /**< Playing */
-} SDL_CameraStatus;
-
-/**
- *  SDL Video Capture Status
- */
-typedef struct SDL_CameraFrame
-{
-    Uint64 timestampNS;         /**< Frame timestamp in nanoseconds when read from the driver */
-    int num_planes;             /**< Number of planes */
-    Uint8 *data[3];             /**< Pointer to data of i-th plane */
-    int pitch[3];               /**< Pitch of i-th plane */
-    void *internal;             /**< Private field */
-} SDL_CameraFrame;
-
-
 /**
  * Use this function to get the number of built-in camera drivers.
  *
@@ -176,11 +136,13 @@ extern DECLSPEC const char *SDLCALL SDL_GetCurrentCameraDriver(void);
 /**
  * Get a list of currently connected camera devices.
  *
- * \param count a pointer filled in with the number of camera devices
+ * \param count a pointer filled in with the number of camera devices. Can be NULL.
  * \returns a 0 terminated array of camera instance IDs which should be
  *          freed with SDL_free(), or NULL on error; call SDL_GetError() for
  *          more details.
  *
+ * \threadsafety It is safe to call this function from any thread.
+ *
  * \since This function is available since SDL 3.0.0.
  *
  * \sa SDL_OpenCamera
@@ -188,237 +150,202 @@ extern DECLSPEC const char *SDLCALL SDL_GetCurrentCameraDriver(void);
 extern DECLSPEC SDL_CameraDeviceID *SDLCALL SDL_GetCameraDevices(int *count);
 
 /**
- * Open a Video Capture device
+ * Get the list of native formats/sizes a camera supports.
  *
- * \param instance_id the camera device instance ID
- * \returns device, or NULL on failure; call SDL_GetError() for more
- *          information.
+ * This returns a list of all formats and frame sizes that a specific
+ * camera can offer. This is useful if your app can accept a variety
+ * of image formats and sizes and so want to find the optimal spec
+ * that doesn't require conversion.
  *
- * \since This function is available since SDL 3.0.0.
+ * This function isn't strictly required; if you call SDL_OpenCameraDevice
+ * with a NULL spec, SDL will choose a native format for you, and if you
+ * instead specify a desired format, it will transparently convert to the
+ * requested format on your behalf.
  *
- * \sa SDL_GetCameraDeviceName
- * \sa SDL_GetCameraDevices
- * \sa SDL_OpenCameraWithSpec
- */
-extern DECLSPEC SDL_CameraDevice *SDLCALL SDL_OpenCamera(SDL_CameraDeviceID instance_id);
-
-/**
- * Set specification
+ * If `count` is not NULL, it will be filled with the number of elements
+ * in the returned array. Additionally, the last element of the array
+ * has all fields set to zero (this element is not included in `count`).
  *
- * \param device opened camera device
- * \param desired desired camera spec
- * \param obtained obtained camera spec
- * \param allowed_changes allow changes or not
- * \returns 0 on success or a negative error code on failure; call
- *          SDL_GetError() for more information.
+ * The returned list is owned by the caller, and should be released with
+ * SDL_free() when no longer needed.
  *
- * \since This function is available since SDL 3.0.0.
- *
- * \sa SDL_OpenCamera
- * \sa SDL_OpenCameraWithSpec
- * \sa SDL_GetCameraSpec
- */
-extern DECLSPEC int SDLCALL SDL_SetCameraSpec(SDL_CameraDevice *device,
-                                                    const SDL_CameraSpec *desired,
-                                                    SDL_CameraSpec *obtained,
-                                                    int allowed_changes);
-
-/**
- * Open a Video Capture device and set specification
+ * \param devid the camera device instance ID to query.
+ * \param count a pointer filled in with the number of elements in the list. Can be NULL.
+ * \returns a 0 terminated array of SDL_CameraSpecs, which should be
+ *          freed with SDL_free(), or NULL on error; call SDL_GetError() for
+ *          more details.
  *
- * \param instance_id the camera device instance ID
- * \param desired desired camera spec
- * \param obtained obtained camera spec
- * \param allowed_changes allow changes or not
- * \returns device, or NULL on failure; call SDL_GetError() for more
- *          information.
+ * \threadsafety It is safe to call this function from any thread.
  *
  * \since This function is available since SDL 3.0.0.
  *
- * \sa SDL_OpenCamera
- * \sa SDL_SetCameraSpec
- * \sa SDL_GetCameraSpec
+ * \sa SDL_GetCameraDevices
+ * \sa SDL_OpenCameraDevice
  */
-extern DECLSPEC SDL_CameraDevice *SDLCALL SDL_OpenCameraWithSpec(SDL_CameraDeviceID instance_id,
-                                                                              const SDL_CameraSpec *desired,
-                                                                              SDL_CameraSpec *obtained,
-                                                                              int allowed_changes);
+extern DECLSPEC SDL_CameraSpec *SDLCALL SDL_GetCameraDeviceSupportedSpecs(SDL_CameraDeviceID devid, int *count);
 
 /**
- * Get device name
+ * Get human-readable device name for a camera.
  *
- * \param instance_id the camera device instance ID
- * \returns device name, shouldn't be freed
+ * The returned string is owned by the caller; please release it with
+ * SDL_free() when done with it.
  *
- * \since This function is available since SDL 3.0.0.
- *
- * \sa SDL_GetCameraDevices
- */
-extern DECLSPEC const char * SDLCALL SDL_GetCameraDeviceName(SDL_CameraDeviceID instance_id);
-
-/**
- * Get the obtained camera spec
+ * \param instance_id the camera device instance ID
+ * \returns Human-readable device name, or NULL on error; call SDL_GetError() for more information.
  *
- * \param device opened camera device
- * \param spec The SDL_CameraSpec to be initialized by this function.
- * \returns 0 on success or a negative error code on failure; call
- *          SDL_GetError() for more information.
+ * \threadsafety It is safe to call this function from any thread.
  *
  * \since This function is available since SDL 3.0.0.
  *
- * \sa SDL_SetCameraSpec
- * \sa SDL_OpenCameraWithSpec
+ * \sa SDL_GetCameraDevices
  */
-extern DECLSPEC int SDLCALL SDL_GetCameraSpec(SDL_CameraDevice *device, SDL_CameraSpec *spec);
-
+extern DECLSPEC char * SDLCALL SDL_GetCameraDeviceName(SDL_CameraDeviceID instance_id);
 
 /**
- * Get frame format of camera device.
+ * Open a video capture device (a "camera").
  *
- * The value can be used to fill SDL_CameraSpec structure.
+ * You can open the device with any reasonable spec, and if the hardware can't
+ * directly support it, it will convert data seamlessly to the requested
+ * format. This might incur overhead, including scaling of image data.
  *
- * \param device opened camera device
- * \param index format between 0 and num -1
- * \param format pointer output format (SDL_PixelFormatEnum)
- * \returns 0 on success or a negative error code on failure; call
- *          SDL_GetError() for more information.
+ * If you would rather accept whatever format the device offers, you can
+ * pass a NULL spec here and it will choose one for you (and you can use
+ * SDL_Surface's conversion/scaling functions directly if necessary).
  *
- * \since This function is available since SDL 3.0.0.
+ * You can call SDL_GetCameraSpec() to get the actual data format if
+ * passing a NULL spec here. You can see the exact specs a device can
+ * support without conversion with SDL_GetCameraSupportedSpecs().
  *
- * \sa SDL_GetNumCameraFormats
- */
-extern DECLSPEC int SDLCALL SDL_GetCameraFormat(SDL_CameraDevice *device,
-                                                      int index,
-                                                      Uint32 *format);
-
-/**
- * Number of available formats for the device
+ * \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
+ *          information.
  *
- * \param device opened camera device
- * \returns number of formats or a negative error code on failure; call
- *          SDL_GetError() for more information.
+ * \threadsafety It is safe to call this function from any thread.
  *
  * \since This function is available since SDL 3.0.0.
  *
- * \sa SDL_GetCameraFormat
- * \sa SDL_SetCameraSpec
+ * \sa SDL_GetCameraDeviceName
+ * \sa SDL_GetCameraDevices
+ * \sa SDL_OpenCameraWithSpec
  */
-extern DECLSPEC int SDLCALL SDL_GetNumCameraFormats(SDL_CameraDevice *device);
+extern DECLSPEC SDL_Camera *SDLCALL SDL_OpenCameraDevice(SDL_CameraDeviceID instance_id, const SDL_CameraSpec *spec);
 
 /**
- * Get frame sizes of the device and the specified input format.
+ * Get the instance ID of an opened camera.
  *
- * The value can be used to fill SDL_CameraSpec structure.
+ * \param device an SDL_Camera to query
+ * \returns the instance ID of the specified camera on success or 0 on
+ *          failure; call SDL_GetError() for more information.
  *
- * \param device opened camera device
- * \param format a format that can be used by the device (SDL_PixelFormatEnum)
- * \param index framesize between 0 and num -1
- * \param width output width
- * \param height output height
- * \returns 0 on success or a negative error code on failure; call
- *          SDL_GetError() for more information.
+ * \threadsafety It is safe to call this function from any thread.
  *
  * \since This function is available since SDL 3.0.0.
  *
- * \sa SDL_GetNumCameraFrameSizes
+ * \sa SDL_OpenCameraDevice
  */
-extern DECLSPEC int SDLCALL SDL_GetCameraFrameSize(SDL_CameraDevice *device, Uint32 format, int index, int *width, int *height);
+extern DECLSPEC SDL_CameraDeviceID SDLCALL SDL_GetCameraInstanceID(SDL_Camera *camera);
 
 /**
- * Number of different framesizes available for the device and pixel format.
+ * Get the properties associated with an opened camera.
  *
- * \param device opened camera device
- * \param format frame pixel format (SDL_PixelFormatEnum)
- * \returns number of framesizes or a negative error code on failure; call
+ * \param device the SDL_Camera obtained from SDL_OpenCameraDevice()
+ * \returns a valid property ID on success or 0 on failure; call
  *          SDL_GetError() for more information.
  *
- * \since This function is available since SDL 3.0.0.
- *
- * \sa SDL_GetCameraFrameSize
- * \sa SDL_SetCameraSpec
- */
-extern DECLSPEC int SDLCALL SDL_GetNumCameraFrameSizes(SDL_CameraDevice *device, Uint32 format);
-
-
-/**
- * Get camera status
- *
- * \param device opened camera device
- * \returns 0 on success or a negative error code on failure; call
- *          SDL_GetError() for more information.
+ * \threadsafety It is safe to call this function from any thread.
  *
  * \since This function is available since SDL 3.0.0.
  *
- * \sa SDL_CameraStatus
+ * \sa SDL_GetProperty
+ * \sa SDL_SetProperty
  */
-extern DECLSPEC SDL_CameraStatus SDLCALL SDL_GetCameraStatus(SDL_CameraDevice *device);
+extern DECLSPEC SDL_PropertiesID SDLCALL SDL_GetCameraProperties(SDL_Camera *camera);
 
 /**
- * Start camera
+ * Get the spec that a camera is using when generating images.
+ *
+ * Note that this might not be the native format of the hardware, as SDL
+ * might be converting to this format behind the scenes.
  *
  * \param device opened camera device
+ * \param spec The SDL_CameraSpec to be initialized by this function.
  * \returns 0 on success or a negative error code on failure; call
  *          SDL_GetError() for more information.
  *
+ * \threadsafety It is safe to call this function from any thread.
+ *
  * \since This function is available since SDL 3.0.0.
  *
- * \sa SDL_StopCamera
+ * \sa SDL_OpenCameraDevice
  */
-extern DECLSPEC int SDLCALL SDL_StartCamera(SDL_CameraDevice *device);
+extern DECLSPEC int SDLCALL SDL_GetCameraSpec(SDL_Camera *camera, SDL_CameraSpec *spec);
 
 /**
  * Acquire a frame.
  *
  * The frame is a memory pointer to the image data, whose size and format are
- * given by the the obtained spec.
+ * given by the spec requested when opening the device.
+ *
+ * This is a non blocking API. If there is a frame available, a non-NULL surface is
+ * returned, and timestampNS will be filled with a non-zero value.
+ *
+ * Note that an error case can also return NULL, but a NULL by itself is normal
+ * and just signifies that a new frame is not yet available. Note that even if a
+ * camera device fails outright (a USB camera is unplugged while in use, etc), SDL
+ * will send an event separately to notify the app, but continue to provide blank
+ * frames at ongoing intervals until SDL_CloseCamera() is called, so real
+ * failure here is almost always an out of memory condition.
  *
- * Non blocking API. If there is a frame available, frame->num_planes is non
- * 0. If frame->num_planes is 0 and returned code is 0, there is no frame at
- * that time.
+ * After use, the frame should be released with SDL_ReleaseCameraFrame(). If you
+ * don't do this, the system may stop providing more video! If the hardware is
+ * using DMA to write directly into memory, frames held too long may be overwritten
+ * with new data.
  *
- * After used, the frame should be released with SDL_ReleaseCameraFrame
+ * Do not call SDL_FreeSurface() on the returned surface! It must be given back
+ * to the camera subsystem with SDL_ReleaseCameraFrame!
  *
  * \param device opened camera device
- * \param frame pointer to get the frame
- * \returns 0 on success or a negative error code on failure; call
- *          SDL_GetError() for more information.
+ * \param timestampNS a pointer filled in with the frame's timestamp, or 0 on error. Can be NULL.
+ * \returns A new frame of video on success, NULL if none is currently available.
+ *
+ * \threadsafety It is safe to call this function from any thread.
  *
  * \since This function is available since SDL 3.0.0.
  *
  * \sa SDL_ReleaseCameraFrame
  */
-extern DECLSPEC int SDLCALL SDL_AcquireCameraFrame(SDL_CameraDevice *device, SDL_CameraFrame *frame);
+extern DECLSPEC SDL_Surface * SDLCALL SDL_AcquireCameraFrame(SDL_Camera *camera, Uint64 *timestampNS);
 
 /**
- * Release a frame.
+ * Release a frame of video acquired from a camera.
  *
  * Let the back-end re-use the internal buffer for camera.
  *
- * All acquired frames should be released before closing the device.
+ * This function _must_ be called only on surface objects returned by
+ * SDL_AcquireCameraFrame(). This function should be called as quickly as
+ * possible after acquisition, as SDL keeps a small FIFO queue of surfaces
+ * for video frames; if surfaces aren't released in a timely manner, SDL
+ * may drop upcoming video frames from the camera.
  *
- * \param device opened camera device
- * \param frame frame pointer.
- * \returns 0 on success or a negative error code on failure; call
- *          SDL_GetError() for more information.
+ * If the app needs to keep the surface for a significant time, they should
+ * make a copy of it and release the original.
  *
- * \since This function is available since SDL 3.0.0.
- *
- * \sa SDL_AcquireCameraFrame
- */
-extern DECLSPEC int SDLCALL SDL_ReleaseCameraFrame(SDL_CameraDevice *device, SDL_CameraFrame *frame);
-
-/**
- * Stop Video Capture
+ * The app should not use the surface again after calling this function;
+ * assume the surface is freed and the pointer is invalid.
  *
  * \param device opened camera device
+ * \param frame The video frame surface to release.
  * \returns 0 on success or a negative error code on failure; call
  *          SDL_GetError() for more information.
  *
+ * \threadsafety It is safe to call this function from any thread.
+ *
  * \since This function is available since SDL 3.0.0.
  *
- * \sa SDL_StartCamera
+ * \sa SDL_AcquireCameraFrame
  */
-extern DECLSPEC int SDLCALL SDL_StopCamera(SDL_CameraDevice *device);
+extern DECLSPEC int SDLCALL SDL_ReleaseCameraFrame(SDL_Camera *camera, SDL_Surface *frame);
 
 /**
  * Use this function to shut down camera processing and close the
@@ -426,12 +353,16 @@ extern DECLSPEC int SDLCALL SDL_StopCamera(SDL_CameraDevice *device);
  *
  * \param device opened camera device
  *
+ * \threadsafety It is safe to call this function from any thread, but
+ *               no thread may reference `device` once this function
+ *               is called.
+ *
  * \since This function is available since SDL 3.0.0.
  *
  * \sa SDL_OpenCameraWithSpec
  * \sa SDL_OpenCamera
  */
-extern DECLSPEC void SDLCALL SDL_CloseCamera(SDL_CameraDevice *device);
+extern DECLSPEC void SDLCALL SDL_CloseCamera(SDL_Camera *camera);
 
 /* Ends C function definitions when using C++ */
 #ifdef __cplusplus
diff --git a/include/SDL3/SDL_events.h b/include/SDL3/SDL_events.h
index d209801ede51..74a39267c188 100644
--- a/include/SDL3/SDL_events.h
+++ b/include/SDL3/SDL_events.h
@@ -205,6 +205,10 @@ typedef enum
     SDL_EVENT_PEN_BUTTON_DOWN,            /**< Pressure-sensitive pen button pressed */
     SDL_EVENT_PEN_BUTTON_UP,              /**< Pressure-sensitive pen button released */
 
+    /* Camera hotplug events */
+    SDL_EVENT_CAMERA_DEVICE_ADDED = 0x1400,  /**< A new camera device is available */
+    SDL_EVENT_CAMERA_DEVICE_REMOVED,         /**< A camera device has been removed. */
+
     /* Render events */
     SDL_EVENT_RENDER_TARGETS_RESET = 0x2000, /**< The render targets have been reset and their contents need to be updated */
     SDL_EVENT_RENDER_DEVICE_RESET, /**< The device has been reset and all textures need to be recreated */
@@ -526,6 +530,18 @@ typedef struct SDL_AudioDeviceEvent
     Uint8 padding3;
 } SDL_AudioDeviceEvent;
 
+/**
+ *  Camera device event structure (event.cdevice.*)
+ */
+typedef struct SDL_CameraDeviceEvent
+{
+    Uint32 type;        /**< ::SDL_EVENT_CAMERA_DEVICE_ADDED, or ::SDL_EVENT_CAMERA_DEVICE_REMOVED */
+    Uint64 timestamp;   /**< In nanoseconds, populated using SDL_GetTicksNS() */
+    SDL_CameraDeviceID which;       /**< SDL_CameraDeviceID for the device being added or removed or changing */
+    Uint8 padding1;
+    Uint8 padding2;
+    Uint8 padding3;
+} SDL_CameraDeviceEvent;
 
 /**
  *  Touch finger event structure (event.tfinger.*)
@@ -699,6 +715,7 @@ typedef union SDL_Event
     SDL_GamepadTouchpadEvent gtouchpad;     /**< Gamepad touchpad event data */
     SDL_GamepadSensorEvent gsensor;         /**< Gamepad sensor event data */
     SDL_AudioDeviceEvent adevice;           /**< Audio device event data */
+    SDL_CameraDeviceEvent cdevice;          /**< Camera device event data */
     SDL_SensorEvent sensor;                 /**< Sensor event data */
     SDL_QuitEvent quit;                     /**< Quit request event data */
     SDL_UserEvent user;                     /**< Custom event data */
diff --git a/src/camera/SDL_camera.c b/src/camera/SDL_camera.c
index 5a8c08f8c31d..d9c581b04a16 100644
--- a/src/camera/SDL_camera.c
+++ b/src/camera/SDL_camera.c
@@ -25,6 +25,10 @@
 #include "../video/SDL_pixels_c.h"
 #include "../thread/SDL_systhread.h"
 
+
+// A lot of this is a simplified version of SDL_audio.c; if fixing stuff here,
+//  maybe check that file, too.
+
 // Available camera drivers
 static const CameraBootStrap *const bootstrap[] = {
 #ifdef SDL_CAMERA_DRIVER_V4L2
@@ -45,15 +49,6 @@ static const CameraBootStrap *const bootstrap[] = {
 static SDL_CameraDriver camera_driver;
 
 
-// list node entries to share frames between SDL and user app
-// !!! FIXME: do we need this struct?
-typedef struct entry_t
-{
-    SDL_CameraFrame frame;
-} entry_t;
-
-static SDL_CameraDevice *open_devices[16];  // !!! FIXME: remove limit
-
 int SDL_GetNumCameraDrivers(void)
 {
     return SDL_arraysize(bootstrap) - 1;
@@ -72,242 +67,435 @@ const char *SDL_GetCurrentCameraDriver(void)
     return camera_driver.name;
 }
 
-
-static void CloseCameraDevice(SDL_CameraDevice *device)
+static void ClosePhysicalCameraDevice(SDL_CameraDevice *device)
 {
     if (!device) {
         return;
     }
 
     SDL_AtomicSet(&device->shutdown, 1);
-    SDL_AtomicSet(&device->enabled, 1);
+
+// !!! FIXME: the close_cond stuff from audio might help the race condition here.
 
     if (device->thread != NULL) {
         SDL_WaitThread(device->thread, NULL);
+        device->thread = NULL;
     }
-    if (device->device_lock != NULL) {
-        SDL_DestroyMutex(device->device_lock);
+
+    // release frames that are queued up somewhere...
+    if (!device->needs_conversion && !device->needs_scaling) {
+        for (SurfaceList *i = device->filled_output_surfaces.next; i != NULL; i = i->next) {
+            camera_driver.impl.ReleaseFrame(device, i->surface);
+        }
+        for (SurfaceList *i = device->app_held_output_surfaces.next; i != NULL; i = i->next) {
+            camera_driver.impl.ReleaseFrame(device, i->surface);
+        }
     }
-    if (device->acquiring_lock != NULL) {
-        SDL_DestroyMutex(device->acquiring_lock);
+
+    camera_driver.impl.CloseDevice(device);
+
+    SDL_DestroyProperties(device->props);
+
+    SDL_DestroySurface(device->acquire_surface);
+    device->acquire_surface = NULL;
+    SDL_DestroySurface(device->conversion_surface);
+    device->conversion_surface = NULL;
+
+    for (int i = 0; i < SDL_arraysize(device->output_surfaces); i++) {
+        SDL_DestroySurface(device->output_surfaces[i].surface);
     }
+    SDL_zeroa(device->output_surfaces);
 
-    const int n = SDL_arraysize(open_devices);
-    for (int i = 0; i < n; i++) {
-        if (open_devices[i] == device) {
-            open_devices[i] = NULL;
-        }
+    device->filled_output_surfaces.next = NULL;
+    device->empty_output_surfaces.next = NULL;
+    device->app_held_output_surfaces.next = NULL;
+}
+
+// this must not be called while `device` is still in a device list, or while a device's camera thread is still running.
+static void DestroyPhysicalCameraDevice(SDL_CameraDevice *device)
+{
+    if (device) {
+        // Destroy any logical devices that still exist...
+        ClosePhysicalCameraDevice(device);
+        camera_driver.impl.FreeDeviceHandle(device);
+        SDL_DestroyMutex(device->lock);
+        SDL_free(device->all_specs);
+        SDL_free(device->name);
+        SDL_free(device);
     }
+}
 
-    entry_t *entry = NULL;
-    while (device->buffer_queue != NULL) {
-        SDL_ListPop(&device->buffer_queue, (void**)&entry);
-        if (entry) {
-            SDL_CameraFrame f = entry->frame;
-            // Release frames not acquired, if any
-            if (f.timestampNS) {
-                camera_driver.impl.ReleaseFrame(device, &f);
-            }
-            SDL_free(entry);
+
+// Don't hold the device lock when calling this, as we may destroy the device!
+void UnrefPhysicalCameraDevice(SDL_CameraDevice *device)
+{
+    if (SDL_AtomicDecRef(&device->refcount)) {
+        // take it out of the device list.
+        SDL_LockRWLockForWriting(camera_driver.device_hash_lock);
+        if (SDL_RemoveFromHashTable(camera_driver.device_hash, (const void *) (uintptr_t) device->instance_id)) {
+            SDL_AtomicAdd(&camera_driver.device_count, -1);
         }
+        SDL_UnlockRWLock(camera_driver.device_hash_lock);
+        DestroyPhysicalCameraDevice(device);  // ...and nuke it.
     }
+}
 
-    camera_driver.impl.CloseDevice(device);
+void RefPhysicalCameraDevice(SDL_CameraDevice *device)
+{
+    SDL_AtomicIncRef(&device->refcount);
+}
 
-    SDL_free(device->dev_name);
-    SDL_free(device);
+static void ObtainPhysicalCameraDeviceObj(SDL_CameraDevice *device) SDL_NO_THREAD_SAFETY_ANALYSIS  // !!! FIXMEL SDL_ACQUIRE
+{
+    if (device) {
+        RefPhysicalCameraDevice(device);
+        SDL_LockMutex(device->lock);
+    }
 }
 
-void SDL_CloseCamera(SDL_CameraDevice *device)
+static SDL_CameraDevice *ObtainPhysicalCameraDevice(SDL_CameraDeviceID devid)  // !!! FIXME: SDL_ACQUIRE
 {
+    if (!SDL_GetCurrentCameraDriver()) {
+        SDL_SetError("Camera subsystem is not initialized");
+        return NULL;
+    }
+
+    SDL_CameraDevice *device = NULL;
+    SDL_LockRWLockForReading(camera_driver.device_hash_lock);
+    SDL_FindInHashTable(camera_driver.device_hash, (const void *) (uintptr_t) devid, (const void **) &device);
+    SDL_UnlockRWLock(camera_driver.device_hash_lock);
     if (!device) {
-        SDL_InvalidParamError("device");
+        SDL_SetError("Invalid camera device instance ID");
     } else {
-        CloseCameraDevice(device);
+        ObtainPhysicalCameraDeviceObj(device);
     }
+    return device;
 }
 
-int SDL_StartCamera(SDL_CameraDevice *device)
+static void ReleaseCameraDevice(SDL_CameraDevice *device) SDL_NO_THREAD_SAFETY_ANALYSIS  // !!! FIXME: SDL_RELEASE
 {
-    if (!device) {
-        return SDL_InvalidParamError("device");
-    } else if (device->is_spec_set == SDL_FALSE) {
-        return SDL_SetError("no spec set");
-    } else if (SDL_GetCameraStatus(device) != SDL_CAMERA_INIT) {
-        return SDL_SetError("invalid state");
+    if (device) {
+        SDL_UnlockMutex(device->lock);
+        UnrefPhysicalCameraDevice(device);
     }
+}
 
-    const int result = camera_driver.impl.StartCamera(device);
-    if (result < 0) {
-        return result;
-    }
+// we want these sorted by format first, so you can find a block of all
+// resolutions that are supported for a format. The formats are sorted in
+// "best" order, but that's subjective: right now, we prefer planar
+// formats, since they're likely what the cameras prefer to produce
+// anyhow, and they basically send the same information in less space
+// than an RGB-style format. After that, sort by bits-per-pixel.
 
-    SDL_AtomicSet(&device->enabled, 1);
+// we want specs sorted largest to smallest dimensions, larger width taking precedence over larger height.
+static int SDLCALL CameraSpecCmp(const void *vpa, const void *vpb)
+{
+    const SDL_CameraSpec *a = (const SDL_CameraSpec *) vpa;
+    const SDL_CameraSpec *b = (const SDL_CameraSpec *) vpb;
+
+    // driver shouldn't send specs like this, check here since we're eventually going to sniff the whole array anyhow.
+    SDL_assert(a->format != SDL_PIXELFORMAT_UNKNOWN);
+    SDL_assert(a->width > 0);
+    SDL_assert(a->height > 0);
+    SDL_assert(b->format != SDL_PIXELFORMAT_UNKNOWN);
+    SDL_assert(b->width > 0);
+    SDL_assert(b->height > 0);
+
+    const Uint32 afmt = a->format;
+    const Uint32 bfmt = b->format;
+    if (SDL_ISPIXELFORMAT_FOURCC(afmt) && !SDL_ISPIXELFORMAT_FOURCC(bfmt)) {
+        return -1;
+    } else if (!SDL_ISPIXELFORMAT_FOURCC(afmt) && SDL_ISPIXELFORMAT_FOURCC(bfmt)) {
+        return 1;
+    } else if (SDL_BITSPERPIXEL(afmt) > SDL_BITSPERPIXEL(bfmt)) {
+        return -1;
+    } else if (SDL_BITSPERPIXEL(bfmt) > SDL_BITSPERPIXEL(afmt)) {
+        return 1;
+    } else if (a->width > b->width) {
+        return -1;
+    } else if (b->width > a->width) {
+        return 1;
+    } else if (a->height > b->height) {
+        return -1;
+    } else if (b->height > a->height) {
+        return 1;
+    }
 
-    return 0;
+    return 0;  // apparently, they're equal.
 }
 
-int SDL_GetCameraSpec(SDL_CameraDevice *device, SDL_CameraSpec *spec)
+
+// The camera backends call this when a new device is plugged in.
+SDL_CameraDevice *SDL_AddCameraDevice(const char *name, int num_specs, const SDL_CameraSpec *specs, void *handle)
 {
+    SDL_assert(name != NULL);
+    SDL_assert(num_specs > 0);
+    SDL_assert(specs != NULL);
+    SDL_assert(handle != NULL);
+
+    SDL_LockRWLockForReading(camera_driver.device_hash_lock);
+    const int shutting_down = SDL_AtomicGet(&camera_driver.shutting_down);
+    SDL_UnlockRWLock(camera_driver.device_hash_lock);
+    if (shutting_down) {
+        return NULL;  // we're shutting down, don't add any devices that are hotplugged at the last possible moment.
+    }
+
+    SDL_CameraDevice *device = (SDL_CameraDevice *)SDL_calloc(1, sizeof(SDL_CameraDevice));
     if (!device) {
-        return SDL_InvalidParamError("device");
-    } else if (!spec) {
-        return SDL_InvalidParamError("spec");
+        return NULL;
     }
 
-    SDL_zerop(spec);
-    return camera_driver.impl.GetDeviceSpec(device, spec);
-}
+    device->name = SDL_strdup(name);
+    if (!device->name) {
+        SDL_free(device);
+        return NULL;
+    }
 
-int SDL_StopCamera(SDL_CameraDevice *device)
-{
-    if (!device) {
-        return SDL_InvalidParamError("device");
-    } else if (SDL_GetCameraStatus(device) != SDL_CAMERA_PLAYING) {
-        return SDL_SetError("invalid state");
+    device->lock = SDL_CreateMutex();
+    if (!device->lock) {
+        SDL_free(device->name);
+        SDL_free(device);
+        return NULL;
     }
 
-    SDL_AtomicSet(&device->enabled, 0);
-    SDL_AtomicSet(&device->shutdown, 1);
+    device->all_specs = SDL_calloc(num_specs + 1, sizeof (*specs));
+    if (!device->all_specs) {
+        SDL_DestroyMutex(device->lock);
+        SDL_free(de

(Patch may be truncated, please check the link at the top of this post.)