SDL icon indicating copy to clipboard operation
SDL copied to clipboard

SDL3 video capture changes

Open icculus opened this issue 2 years ago • 31 comments

This is a draft PR to work towards my camera API wish list in #8626. It will be getting more commits, and probably a force-push or two.

TODO list/wishlist:

  • [x] Should this be named "Camera" instead of "VideoCapture"? I kind of regret using the word "capture" in the audio subsystem but feel like we're probably just stuck with it now. (I know not everything is a camera that generates video frames, but still.)
  • [x] Should it be in its own subdirectory instead of in src/video?
  • [x] This should use a driver interface, like we do for other subsystems.
  • [x] There should be an SDL_INIT_CAMERA flag.
  • [x] Remove open_devices array, so there isn't a limit.
  • [x] We need a Windows implementation. The win32 level is apparently called "Media Foundation API" that's available for Win7(?) and later (DirectShow is the older option, and I'm not bothering with it).There's some C++ish stuff at the WinRT level that I don't intend to implement right now, either.
  • [x] Android 5.0 and later can use android.hardware.camera2.
  • [x] Emscripten can probably use Media Streams
  • [x] This is a problem we also haven't solved for the audio subsystem, but we should probably have a mechanism for dealing with platform permissions: you open a camera and iOS (etc) pops up a diaog asking the user for permission before you actually get access to it. Should we feed empty frames to the app before we get approval? No frames? Should we have a callback or event that lets you know when the user has responded to the prompt? Should we have a way to query if we have permission, or force the permission prompt before we actually open the device? I don't have answers to this, and maybe "it's fine as-is" is the correct solution.
  • [x] In the audio subsystem, we have an SDL_AudioSpec, but all the things that operate on it refer to it as a "Format" in their function names (int SDL_GetAudioDeviceFormat(SDL_AudioSpec *spec); etc). Maybe we should rename the functions here to match. SDL_AudioFormat refers to the sample format (Sint16, float, etc), like this currently has a pixel format as part of the Spec.
  • [x] I would rather things like SDL_GetVideoCaptureFormat() return an array with X elements that the caller will SDL_free later instead of calling X times, to match other APIs that have moved to this.
  • [x] SDL_OpenVideoCapture* works a lot like SDL2's audio devices. I'd prefer to see this work like the SDL3 audio equivalent: a single open function that takes a SDL_VideoCaptureSpec* ... if NULL, you get whatever format the device chooses to offer, if not NULL, you get exactly that data format, and if the hardware couldn't directly support it, SDL will quietly convert it as if it did. I'd remove the allowed_changes stuff, like we did for audio. I don't know how much pain is involved in conversion, though; do we already have code in SDL_surface that can handle it?
  • [x] Is hotplug possible? Can we reasonably detect when a device is added/removed and send an event?
  • [x] ~~It would be nice to have a guarantee that multiple opens of the same device ID will work and provide a unique SDL_VideoCaptureDevice * for each time; they can share a thread that reads the device and manages frame copies to each opened object. While it wouldn't have to be as complex as the audio subsystem equivalent turned out to be, it would still be a bit of an undertaking and can be added later without changing the API.~~
  • [x] Opened cameras should have optional SDL_Properties, like other SDL objects.
  • [x] Framerate should be queryable/settable.
  • [x] ~~Allow devices to be "paused" (which just throws away frames in the device thread).~~
  • [x] Need better timestamps for individual frames.
  • [ ] Make the first camera in the device list the default, if we can reasonably provide a default.
  • [x] Revive the CoreMedia backend.
  • [x] Revive the Android backend.
  • [ ] Add "zombie" functions that make a lost device continue to output blank frames at regular intervals.

icculus avatar Nov 28 '23 04:11 icculus

An enormous amount of work has been happening in my working copy, but it's not quite ready to commit.

Some notable executive decisions I've made since the initial wishlist:

  • StartCamera and StopCamera are gone; you can open a device and close it (and I might add a pause/unpause replacement that just makes SDL throw away frames internally), but it seemed kinda redundant, and there was a FIXME for this not working with mmap()'d devices in Video4Linux2.
  • I've decided not to allow multiple opens of a single camera, at least, for now. It adds a ton of complexity, race condition risks, etc, which was a giant pain to go through with the audio subsystem rewrite, and I see less need for multiple opens of a camera than I do for audio hardware. So for now, it'll just see the device is already open during SDL_OpenCameraDevice() and return -1.
  • During hotplug, the device is queried for all the pertinent info (available formats/sizes, real name, etc), and SDL keeps track of it. The backends are much simpler now since they just have to init, get frames from, and deinit hardware, and a lot of the upkeep has moved to the higher level.

I'm hoping to have something to show for this in a day or two, at most, if everything goes okay.

icculus avatar Dec 11 '23 16:12 icculus

great :) Indeed, pause seems not to work with v4l2 this is why I didn't add this state..

1bsyl avatar Dec 11 '23 17:12 1bsyl

Is there a way/need to find the accompanying audio capture device of a video capture device (and vice versa)?

madebr avatar Dec 12 '23 08:12 madebr

Not at the moment, but also: while some webcams have mics, most don't, and even if they do, often times the user is going to want to use an external mic (a headset or some sort of podcasting desk mic thing), so apps should probably let the user choose.

icculus avatar Dec 12 '23 13:12 icculus

Ton of work hit revision control today. Got more to do, but this is more or less how I want this to look. Still need to make some small API additions, update the Apple/Android backends, and implement some new ones, but this is more or less the redesign as planned.

The diffs are hard to look at since they touch basically everything, so the important thing to focus on is the public API changes, since anything internal can change as necessary once 3.2.0 locks down, but the API cannot.

icculus avatar Dec 15 '23 19:12 icculus

Just one note about: SDL_Surface * SDLCALL SDL_AcquireCameraFrame it now returns a SDL_Surface. it's better as a design. If i remember correctly I started with that, but there is an issue with other back-end. For linux/v4l2 memory is contiguous and that's ok, but on ios/android, not always, and you cannot use "surface->pixels" to point the memory. So this would need some alloc/memcpy per frame, and would be less efficient.

1bsyl avatar Dec 16 '23 13:12 1bsyl

Mmmm...I think I'd rather take the memcpy hit here; having these in SDL_Surfaces is super convenient, both for being able to trivially scale/convert internally, and for consistency with the rest of the API.

Maybe we change SDL_Surface to permit multiple planes/pitches?

icculus avatar Dec 16 '23 16:12 icculus

Maybe we change SDL_Surface to permit multiple planes/pitches?

(I think this is a good idea, camera or not, but I have no idea what sort of things would blow up if we did this.)

icculus avatar Dec 16 '23 17:12 icculus

Maybe we change SDL_Surface to permit multiple planes/pitches?

I think we need to do this for SDL3, to clearly represent YUV surfaces.

This does mean we'll need to translate between SDL2 and SDL3 surfaces in sdl2-compat though. This would help https://github.com/libsdl-org/SDL/pull/8691 as well.

slouken avatar Dec 16 '23 17:12 slouken

This does mean we'll need to translate between SDL2 and SDL3 surfaces in sdl2-compat though.

We need to do this anyhow, since we added SDL_PropertiesID to the struct. Right now sdl2-compat works with surfaces as long as you only use them as opaque objects, but would explode if you allocate your own or try to access its fields.

icculus avatar Dec 16 '23 18:12 icculus

so we need, to remove pixels / pitch from SDL_surface and add

     Uint8 *planes[3];
     int pitches[3];

should we use different names ? so that this breaks at compilation time. there ~200 occurrence of pixels, and same for pitch. so that's not a big deal I guess. most of them, should be planes[0] / pitches[0]

sw renderer YUV already has planes/pitches distinction: https://github.com/libsdl-org/SDL/blob/main/src/render/SDL_yuv_sw_c.h#L37 https://github.com/libsdl-org/SDL/blob/main/src/render/SDL_yuv_sw.c#L63

SDL_ConvertPixels currently handles yuv, and it assumes contiguous memory ... maybe we don't need to update this. there is ConvertSurface that can be used

I'd say we should give a try to see where it blows up.

1bsyl avatar Dec 16 '23 21:12 1bsyl

I was thinking adding pixels2 and pixels3 fields (plus pitch fields) so existing source code doesn't break for non-planar formats...but maybe breaking them is good?

icculus avatar Dec 16 '23 23:12 icculus

I was thinking we could have arrays of pixels and pitches and the existing members could point to plane[0].pixels and plane[0].pitch, so we have good source compatibility and new flexibility for YUV surfaces.

Also, assuming contiguous memory breaks in a bunch of cases, so those functions should be updated to use the new multi-planar surface. Maybe if the other planes are NULL, we can assume contiguous memory just to preserve backwards compatibility?

slouken avatar Dec 17 '23 04:12 slouken

Hell yeah!

image

icculus avatar Dec 22 '23 06:12 icculus

great ! 268 lines of code for the back-end!

1bsyl avatar Dec 22 '23 11:12 1bsyl

It's tragic that I'll never get the Nobel Prize for all the effort this took.

image

Also it's totally a coincidence that I'm wearing the same shirt in this picture as I was in the Emscripten one, hahah.

icculus avatar Jan 31 '24 20:01 icculus

Y'all sick of my face yet? :)

Screenshot 2024-02-06 at 1 10 46 AM

macOS (and presumably, iOS) is back online.

icculus avatar Feb 06 '24 06:02 icculus

Keep on rocking @icculus! 🤘

madebr avatar Feb 06 '24 11:02 madebr

When you go after the Android backend, can you check https://github.com/libsdl-org/SDL/issues/894? Do we need to link to camera2ndk and/or mediandk? Do we need to load things dynamically?

madebr avatar Feb 07 '24 16:02 madebr

I think camera2ndk was added in android-24. What is our oldest supported Android API level for SDL3? If it's less than that, then yeah, we'll have to dlopen it.

icculus avatar Feb 08 '24 04:02 icculus

yes, if i remember this was api 24. see https://developer.android.com/ndk/guides/stable_apis#camera

SDL supports min 19. Probably will be bumped soon to min-sdk 21 soon (summer).

But, as long as we linked with a not very very old NDK, it can find libcamera2ndk and compile fine. It just needs a run-time check so that we don't try use libcamera2ndk on older device:

we have this kind of call in SDL: if (SDL_GetAndroidSDKVersion() < 24) return SDL_Unsupported();

I'd say we don´t really need to DLL open the library. and also, we will bump to api 24 one day.The lib will be there anyway. ( btw, not even sure that we could dll open this library. maybe this is only a lib part of ndk, and not a runtime library on device...)

1bsyl avatar Feb 08 '24 08:02 1bsyl

since that works on macos, please, give a try on ios. I remember, there was something strange with the camera image format. the same "420v" apple format was YVYU on macos (with an external camera), and NV12 on ipad.

    /* FIXME
     * on IOS this mode gives 2 planes, and it's NV12
     * on macos, 1 plane/ YVYU
     */
    #ifdef SDL_PLATFORM_MACOS
    if (SDL_strcmp("420v", str) == 0)  return SDL_PIXELFORMAT_YVYU;
    #else
    if (SDL_strcmp("420v", str) == 0)  return SDL_PIXELFORMAT_NV12;
    #endif

1bsyl avatar Feb 08 '24 08:02 1bsyl

the same "420v" apple format was YVYU on macos (with an external camera), and NV12 on ipad.

Ugh!

icculus avatar Feb 08 '24 13:02 icculus

It just needs a run-time check so that we don't try use libcamera2ndk on older device:

But on older devices, if the system doesn't have libcamera2ndk.so installed, won't the app fail to start if we link directly to it, or does Android do some sort of weak linking thing where it only crashes if you try to reference the missing library?

(it's an .so, not a static library in the NDK.)

icculus avatar Feb 08 '24 14:02 icculus

the same "420v" apple format was YVYU on macos (with an external camera), and NV12 on ipad

Ugh!

(that may also be my mistake using the api)

It just needs a run-time check so that we don't try use libcamera2ndk on older device:

But on older devices, if the system doesn't have libcamera2ndk.so installed, won't the app fail to start if we link directly to it, or does Android do some sort of weak linking thing where it only crashes if you try to reference the missing library?

(it's an .so, not a static library in the NDK.)

Yes, you're right, that's a shared object, I thought that was a static link ... and ndk won't even compile with minSdk = 19 or 21.

(btw, I think google play already requires minSdk 21 it.) but maybe we're close to bump to 24 ... I would rather turn the subsystem to be able to be compiled out, instead of being dlopen'able. (people who really want the camera, will bump to minsdk 24, and other can compile things out)

1bsyl avatar Feb 08 '24 15:02 1bsyl

Yeah, we need to either dlopen or compile out for older SDKs. Steam Link runs on API 19.

Is there a compile time define for target SDK?

slouken avatar Feb 08 '24 15:02 slouken

there is #if defined(SDL_PLATFORM_ANDROID) && __ANDROID_API__ >= 24 or ANDROID_MIN_SDK_VERSION

The minSdkVersion of your application is made available to the preprocessor via the ANDROID_MIN_SDK_VERSION macro (the legacy ANDROID_API is identical, but prefer the former because its meaning is clearer). This macro is defined automatically by Clang, so no header needs to be included to use it. For NDK builds, this macro is always defined.

but the libcamera2ndk should be linked or not based on minSdk. or maybe this can be weaked linked ? .. if this can be weaked linked on older NDK .. we can use the runtime check ... if (SDL_GetAndroidSDKVersion() < 24) return SDL_Unsupported();

1bsyl avatar Feb 08 '24 15:02 1bsyl

Android doesn’t support weak linking as far as I know. The equivalent is dynamically loading.

slouken avatar Feb 08 '24 15:02 slouken

Turns out Android does support weak linking, but it's a fairly new feature, so I'd not recommend relying on it (yet), as devices need to catch up, and it sounds like corner cases are still shaking out of it still anyhow.

https://github.com/android/ndk/issues/837

Right now, our camera code will depend on #if __ANDROID_API__ >= 24, which means setting an older minimum target than Android 7.0 ("Nougat", released in 2016) will get you just the "dummy" backend. This is because older NDK installs wrap the entire ACamera API in an __ANDROID_API__ block, so you can't reasonably compile our code that relies on those headers, even if you intended to dlopen the system library at runtime.

build-scripts/androidbuildlibs.sh (and other places) currently set a minimum target of android-16 (Android 4.0, "Jelly Bean" from 2012). It's probably time to bump this up, camera support or not. :)

icculus avatar Feb 13 '24 14:02 icculus

Spent forever trying to coerce Android to work; turns out my 30-dollar pay-as-you-go budget phone I use for testing has what Android refers to as "LEGACY" level camera hardware, which libcamera2ndk refuses to work with (the Java camera2 API can work with it though). The confusion is that libcamera2ndk will enumerate those devices through the hotplug callback and then fail to actually access them, returning "unknown error" through the API and writing "no such file or directory" to the system log.

I spent forever trying to figure out if it was a permission thing, or something weird with how I was linking to the libraries, etc. Turns out it's just a crappy phone.

My other 30-dollar test phone has supported hardware, so it's fine.

Almost done now.

icculus avatar Feb 15 '24 15:02 icculus