SDL icon indicating copy to clipboard operation
SDL copied to clipboard

Add video capture API proposal

Open 1bsyl opened this issue 3 years ago • 44 comments

I thought it could be interesting to add a SDL Video Capture API (was also suggested in #4)

This only provides a set of Open/Start/Stop, Get/Release Frame + query format/size features. With implementation for

  • linux using 'Video4Linux' (software data + dmabuf hardware)
  • Android using ndk / libcamera2 (hardcoded NV12) (software data + hardware buffer)
  • IOS : AVCapture API (hardcoded NV12) (software data)
  • MACOSX: same code as IOS

there is a test program (testvideocapture)

to build, you need to run first ./src/dynapi/gendynapi.py

1bsyl avatar Mar 31 '22 21:03 1bsyl

Separation of list procedures is a good thing that can be done without this PR.

sezero avatar Mar 31 '22 21:03 sezero

@sezero yes, this is done thanks ! Should be ok !

1bsyl avatar Apr 01 '22 06:04 1bsyl

A few notes (all of which can be rectified easily later when you finalize things):

  • The current code relies on static linkage to SDL2, because SDL_VideoCaptureInit and SDL_VideoCaptureQuit are not public in the public header.

  • Dynamic linkage needs dynapi on linux & co, but dynapi fails because the SDL_VideoCaptureDeviceID type isn't known to it.

  • testvideocapture.c is plagued with c99-isms.

sezero avatar Apr 02 '22 12:04 sezero

@sezero thanks ! I've fixed. The whole thing is mostly a draft now ... I just want to call "gendynapi.pl" at the very last moment, to avoid merge each time this is modified

1bsyl avatar Apr 02 '22 15:04 1bsyl

Is there a specific reason you want a dedicated SDL_VideoCaptureDeviceID type instead of plain Sint32 - that way, you won't have to include video_capture.h from within SDL.h and only its users will have to include it.

Here is a very minor tidy-up: test/Makefile.os2 part is commented out because we don't have dyapi yet and I don't want CI failures.

diff --git a/Makefile.os2 b/Makefile.os2
index 9060e0e..3902d25 100644
--- a/Makefile.os2
+++ b/Makefile.os2
@@ -77,3 +77,4 @@ SRCS+= SDL_blit.c SDL_blit_0.c SDL_blit_1.c SDL_blit_A.c SDL_blit_auto.c &
        SDL_pixels.c SDL_rect.c SDL_RLEaccel.c SDL_shape.c SDL_stretch.c &
-       SDL_surface.c SDL_video.c SDL_video_capture.c SDL_clipboard.c SDL_vulkan_utils.c SDL_egl.c
+       SDL_surface.c SDL_video.c SDL_clipboard.c SDL_video_capture.c SDL_egl.c &
+       SDL_vulkan_utils.c
 
diff --git a/test/testvideocapture.c b/test/testvideocapture.c
index e1a7816..3672092 100644
--- a/test/testvideocapture.c
+++ b/test/testvideocapture.c
@@ -353,4 +353,6 @@ int main(int argc, char **argv)
         {
+            static int x = 0;
             SDL_Rect r;
-            static int x = 0; x += 10; if (x > 1000) x = 0;
+            x += 10;
+            if (x > 1000) x = 0;
             r.x = x;
diff --git a/test/Makefile.os2 b/test/Makefile.os2
index e238af4..2ea51b0 100644
--- a/test/Makefile.os2
+++ b/test/Makefile.os2
@@ -29,3 +29,3 @@ TARGETS = testatomic.exe testdisplayinfo.exe testbounds.exe testdraw2.exe &
           testsurround.exe testyuv.exe testgl2.exe testvulkan.exe testnative.exe &
-          testautomation.exe
+          testautomation.exe #testvideocapture.exe
 

sezero avatar Apr 02 '22 15:04 sezero

@sezero I think this is fixed. Do you receive also the notification the ci build failed for this branch ? I hope this is ok now. SDL_VideoCaptureDeviceID sound better to read the API but that can be changed, there are also type that would fail the dynapi (like SDL_VideoCaptureSpec)

1bsyl avatar Apr 02 '22 15:04 1bsyl

@sezero I think this is fixed. Do you receive also the notification the ci build failed for this branch ? I hope this is ok now.

Yes, CI seems fixed.

SDL_VideoCaptureDeviceID sound better to read the API but that can be changed, there are also type that would fail the dynapi (like SDL_VideoCaptureSpec)

OK

sezero avatar Apr 02 '22 15:04 sezero

I've been chewing on all the ways to enumerate things for a couple days, it just seemed odd to me and I needed to think about why.

It works like this:

enumerate cameras
    enumerate formats
        enumerate dimensions (frame sizes)

The indentation shows the "dependency chain" of the functions. You need a camera, then a format the camera supports, then a frame size.

Looking at how this works, and given the fact that this API returns pixels instead of a Surface, and it makes me think this is meant for use with Textures, with LockTexture or UpdateTexture. Because with that, the user is looking for a valid webcam configuration in a pixelformat supported by the Renderer. Tell me if I'm assuming this all wrong. :smile:

Here's an alternative idea- Given a camera, you can enumerate all workable VideoCaptureSpec structs. Putting the format and dimension information in one "plane" allows easier searching for people who are doing that. But since lots of people will presumably want to restrict to a certain range of pixelformats, you should be able to enumerate workable VideoCaptureSpecs in a set of pixelformats.

enumerate cameras
    enumerate VideoCaptureSpecs
    enumerate VideoCaptureSpecs in set of pixelformats

Maybe these VideoCaptureSpecs should be sorted so the highest resolution modes are first.

Starbuck5 avatar Apr 14 '22 08:04 Starbuck5

@Starbuck5 this is still a work in progress. things are changing while trying on backend. but currently it starts working on ios/android/linux. you can try to implement for the window backend. I've no camera so I can´t do this on window.

This can be done progressively:enumerate device / format / sizes. start capture. There is a small test app And it's not very long: for IOS, it's ~400 lines. for Android, ~650 lines linux was longer (because there are several methods inside: mmap or useptr)

1bsyl avatar Apr 14 '22 10:04 1bsyl

autotools need the equivalent changes made in cmake. Untested patch to configure.ac follows

diff --git a/configure.ac b/configure.ac
index 1d96004..b18d214 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2189,13 +2189,22 @@ CheckCOCOA()
           #import <Cocoa/Cocoa.h>
         ]],[])], [have_cocoa=yes],[])
         AC_MSG_RESULT($have_cocoa)
-        CFLAGS="$save_CFLAGS"
         if test x$have_cocoa = xyes; then
             AC_DEFINE(SDL_VIDEO_DRIVER_COCOA, 1, [ ])
             SOURCES="$SOURCES $srcdir/src/video/cocoa/*.m"
             SUMMARY_video="${SUMMARY_video} cocoa"
             have_video=yes
         fi
+        AC_MSG_CHECKING(for CoreMedia framework)
+        have_coremedia=no
+        AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
+          #import <AVFoundation/AVFoundation.h>
+          #import <CoreMedia/CoreMedia.h>
+        ]],[])], [have_coremedia=yes],[])
+        CFLAGS="$save_CFLAGS"
+        if test x$have_coremedia = xyes; then
+            SOURCES="$SOURCES $srcdir/src/video/SDL_video_capture.m"
+        fi
     fi
 }
 
@@ -4218,12 +4227,14 @@     *-ios-*)
         SOURCES="$SOURCES $srcdir/src/video/uikit/*.m"
         SUMMARY_video="${SUMMARY_video} uikit"
         have_video=yes
+        SOURCES="$SOURCES $srcdir/src/video/SDL_video_capture.m"
         EXTRA_LDFLAGS="$EXTRA_LDFLAGS -lm -liconv -lobjc"
         EXTRA_LDFLAGS="$EXTRA_LDFLAGS -Wl,-framework,AVFoundation"
         EXTRA_LDFLAGS="$EXTRA_LDFLAGS -Wl,-framework,AudioToolbox"
         EXTRA_LDFLAGS="$EXTRA_LDFLAGS -Wl,-framework,CoreAudio"
         EXTRA_LDFLAGS="$EXTRA_LDFLAGS -Wl,-framework,CoreGraphics"
         EXTRA_LDFLAGS="$EXTRA_LDFLAGS -Wl,-framework,CoreMotion"
+        EXTRA_LDFLAGS="$EXTRA_LDFLAGS -Wl,-framework,CoreMedia"
         EXTRA_LDFLAGS="$EXTRA_LDFLAGS -Wl,-framework,Foundation"
         EXTRA_LDFLAGS="$EXTRA_LDFLAGS -Wl,-framework,GameController"
         EXTRA_LDFLAGS="$EXTRA_LDFLAGS -Wl,-framework,OpenGLES"
@@ -4323,6 +4334,9 @@     *-*-darwin* )
         # The Mac OS X platform requires special setup.
         EXTRA_LDFLAGS="$EXTRA_LDFLAGS -lobjc"
         EXTRA_LDFLAGS="$EXTRA_LDFLAGS -Wl,-framework,CoreVideo"
+        if test x$have_coremedia = xyes; then
+           EXTRA_LDFLAGS="$EXTRA_LDFLAGS -Wl,-framework,CoreMedia"
+        fi
         EXTRA_LDFLAGS="$EXTRA_LDFLAGS -Wl,-framework,Cocoa"
         EXTRA_LDFLAGS="$EXTRA_LDFLAGS -Wl,-framework,Carbon"
         EXTRA_LDFLAGS="$EXTRA_LDFLAGS -Wl,-framework,IOKit"

sezero avatar Apr 14 '22 18:04 sezero

@sezero Updated thanks ! Maybe we also need a true macosx tester with a webcam !

1bsyl avatar Apr 15 '22 08:04 1bsyl

Did some build testing using autotools (cross-compile on Linux). Here goes:

  • SDL_video_capture.m and SDL_video_capture.c names conflict: autofoo doesn't seem to differenciate. My solution: Renamed SDL_video_capture.m to SDL_videocap_apple.m, adjusted configury, Xcode project file, and cmake script.

  • AVCaptureDeviceTypeBuiltInWideAngleCamera requires macOS SDK 10.15 at compile time (and obviously 10.15+ at run time.) Same is true for AVCaptureDeviceDiscoverySession. Solution: Specifically check AVCaptureDeviceTypeBuiltInWideAngleCamera in configury (did not touch cmake), define HAVE_COREMEDIA if all is good. Define HAVE_COREMEDIA in mocosx and iphoneos config files. Undefine HAVE_COREMEDIA in SDL_videocap_apple.m if TVOS or OSX SDK is old.

  • If CoreMedia isn't available, linkage fails. My solution: added stubs for symbols required by SDL_video_capture.c

  • Linking to CoreMedia.framework wasn't enough:

    "_AVCaptureDeviceTypeBuiltInWideAngleCamera", referenced from:
        _get_device_by_name in SDL_videocap_apple.o
        _GetDeviceName in SDL_videocap_apple.o
        _GetNumDevices in SDL_videocap_apple.o
    "_AVCaptureSessionPresetHigh", referenced from:
        _InitDevice in SDL_videocap_apple.o
    "_AVMediaTypeVideo", referenced from:
        _get_device_by_name in SDL_videocap_apple.o
        _GetDeviceName in SDL_videocap_apple.o
        _GetNumDevices in SDL_videocap_apple.o
    "_OBJC_CLASS_$_AVCaptureDeviceDiscoverySession", referenced from:
        objc-class-ref in SDL_videocap_apple.o
    "_OBJC_CLASS_$_AVCaptureDeviceInput", referenced from:
        objc-class-ref in SDL_videocap_apple.o
    "_OBJC_CLASS_$_AVCaptureSession", referenced from:
        objc-class-ref in SDL_videocap_apple.o
    "_OBJC_CLASS_$_AVCaptureVideoDataOutput", referenced from:
        objc-class-ref in SDL_videocap_apple.o
    

    My solution: Weak-link with AVFoundation.framework, as well. (did not touch cmake, and did not touch Xcode: someone else should do them.)

  • You may want to add additional if (@available(macOS 10.15, *)) to other procedures in SDL_videocap_apple.m (or not..)

Here is the patch file I generated: patch_5477a.txt

  • Do read the patch (and the notes I gathered above) before applying.
  • Remember to git mv SDL_video_capture.m SDL_videocap_apple.m before applying.

(EDIT: a bad patch was attached, now corrected.)

sezero avatar Apr 15 '22 11:04 sezero

I've applied the patch 5477a.txt I misread the renaming (SDL_videocap_apple.m vs SDL_video_capture_apple.m) so I've had to change the name inside the patch also

1bsyl avatar Apr 15 '22 20:04 1bsyl

OK, either way the same changes are in.

sezero avatar Apr 15 '22 20:04 sezero

A simple attempt to build on MacOS (./configure, make) failed. Mac OS 10.13 (intel) make-output.txt

Starbuck5 avatar Apr 18 '22 05:04 Starbuck5

A simple attempt to build on MacOS (./configure, make) failed.

Did you run ./autogen.sh before running configure?

sezero avatar Apr 18 '22 05:04 sezero

A simple attempt to build on MacOS (./configure, make) failed.

Did you run ./autogen.sh before running configure?

And here is a patch that just does that for you, for convenience: patch_configure.txt

sezero avatar Apr 18 '22 06:04 sezero

This is probably my inexperience in C that I can't resolve these. I didn't run autogen, but after I did I was able to configure, make, and make install the branch of SDL.

I now get problems trying to compile the test program: compile-output.txt I get the same thing trying to run make testvideocapture.

Starbuck5 avatar Apr 18 '22 06:04 Starbuck5

I now get problems trying to compile the test program: compile-output.txt I get the same thing trying to run make testvideocapture.

That's because @1bsyl hasn't integrated the new capture apis into dynapi yet.

sezero avatar Apr 18 '22 07:04 sezero

That's because @1bsyl hasn't integrated the new capture apis into dynapi yet.

If you want to test things, do:

cd src/dynapi && ./gendynapi.pl
cd ../..

And then build SDL and install. Then testvideocapture should link properly.

sezero avatar Apr 18 '22 08:04 sezero

I can build it now! It doesn't detect my camera. I'm on a macbook pro with an integrated webcam. When I click devices, it says number of devices is 0.

It's probably because of the DiscoverySession thing, that's new in MacOS 10.15, and I'm running 10.13.

Starbuck5 avatar Apr 18 '22 08:04 Starbuck5

I can build it now!

Great!

It doesn't detect my camera. I'm on a macbook pro with an integrated webcam. When I click devices, it says number of devices is 0.

It's probably because of the DiscoverySession thing, that's new in MacOS 10.15, and I'm running 10.13.

That part is for Sylvain

sezero avatar Apr 18 '22 09:04 sezero

@sezero should I apply the previous patch_configure.txt ?

@Starbuck5 indeed, this seems to be a 10.15 function... at least it doesn't crash.. maybe we could do a fallback in the SDL2 code. in src/video/SDL_video_capture_apple.c line 113, discover_devices:

instead of:

return devices;

would do something:

if ([devices count] > 0) {
  return devices;
} else {
  AVCaptureDevice *captureDevice = [AVCaptureDevice defaultDeviceWithMediaType:AVMediaTypeVideo];
  if (captureDevice == nil) {
    return devices;
  } else {
     NSArray<AVCaptureDevice *> default_device = @[ captureDevice ];
     return default_device;
 }
}

(this is un-tested..)

1bsyl avatar Apr 18 '22 18:04 1bsyl

@sezero should I apply the previous patch_configure.txt

I guess it makes it easier for people to test your branch, so yes

sezero avatar Apr 18 '22 18:04 sezero

Okay @1bsyl I've managed to get my webcam light to turn on, which is big progress.

To do that:

  • I modified the test in configure to use the old device strategy, rather than the thing that only works in 10.15. The configure wasn't linking CoreMedia in, and not defining HAVE_COREMEDIA, so SDL_video_capture_apple.m had no shot of building its functions on my Mac. It was using the dummy functions at the top.
  • I got rid of a separate MacOS 10.15 check at the top of SDL_video_capture_apple.m that would undefine HAVE_COREMEDIA.
  • I got rid of a separate MacOS 10.15 check in InitDevice
  • I replaced discover_device's contents with this:
    AVCaptureDevice *captureDevice = [AVCaptureDevice defaultDeviceWithMediaType:AVMediaTypeVideo]; 
    NSArray<AVCaptureDevice *> *devices = @[ captureDevice ];
    return devices;
  • I put in more FOURCC to pixelformat conversions. I'm not sure if the thing that picks the right format tries to look at other ones, it seems like it just tries the first format and fails if that's PIXELFORMAT_UNKNOWN, which it was for me.
  • Pixelformat conversions: It took me a lot of effort, but I found this page: https://developer.apple.com/documentation/coremedia/1564244-video_pixel_format_constants. My webcam (built in Apple webcam) is pushing out NV12 recognized already, along with "2vuy" and "yuvs", which can be found under kCMPixelFormat_422YpCbCr8 and kCMPixelFormat_422YpCbCr8_yuvs on that page. I'm not sure these have SDL equivalents, so I guessed they would correspond to IYUV and YUY2 pixelformats.

More things

  • I understand if you don't want to bother supporting MacOS <10.15, however it seems like its just webcam enumeration code different, and I can test.
  • Currently the webcam opens when I open testvidcapture, but nothing is shown, and the FPS reads out as 0. After I close it, it won't reopen it with the DEV0 or DEV1 buttons.

Okay, on testvideocapture run, it opens a webcam, and using printf debugging I can see that the internal AcquireFrame is being called by the video capture thread. However, in the main thread, the SDL_AcquireFrame always falls down the "queue is empty" branch.

Starbuck5 avatar Apr 27 '22 08:04 Starbuck5

@Starbuck5 It sounds great ! I cannot look at it right now. But just answering parts:

  • first, I think you should fork my branch to submit modifications.
  • use SDL_GetError(). It may give you hint where it fails.
  • I think we want to be compatible with macosx below 10.15 . It s just the discovery which isn't available. See my previous, untested, code to fall back to the default open device.(similar to what you propose).
  • so it probably fine to remove some ifdef in the code
  • check the format that are reported by the webcam. So you make sure we don't identify them as SDL_PIXELFORMAT_UNKNOWN.
  • the error you describe is that. All is fine but, the macosx API send no frame... not sure why. Maybe there are some error message/log?
  • strange that closing + really open doesn't work. What about doing the same without even starting video capture?

1bsyl avatar Apr 27 '22 19:04 1bsyl

@Starbuck5 I grabbed some capture device. And I got it somehow working on macosx.

  • need to unlock the buffer (not on IOS ?)
  • when there is a single plane, need to call the non-plane API So you may want to re-try.

I still see some strangeness. but I am not sure if this is the SDL code, or macosx. (eg NV12 on macosx is bad. YUVS using a very big resolution 2560 x 1440 is bad, whereas FHD is ok).

You maybe need to submit some patch for the COREMEDIA compilation thing I haven't updated I have MACOSX 12.3.1 and discovery session wasn't working neither. I thing we should support a much version a possible / at least recent versions.

1bsyl avatar May 09 '22 15:05 1bsyl

This is something we want for SDL 3, can you clean this up and merge it at your convenience?

slouken avatar Nov 23 '22 18:11 slouken

I need to refresh this. I think the API exposed isn't really correct: 1/ the video format wanted (pixel format) is half hard-coded, per platform. So we could remove the possibility to request it. And just let the user open the device, and once we get frame, we can tell what the format is.

2/ we expose also a special type of texture to do 0-copy. which is kind of platform dependant ...

1bsyl avatar Nov 23 '22 20:11 1bsyl

What's the status of this pull request at this point? Now seems like a fine idea to add it for SDL3, if it's robust and complete.

slouken avatar Mar 27 '23 15:03 slouken