[libcamera-devel] [PATCH v2] cam: sdl_sink: Add SDL sink
Kieran Bingham
kieran.bingham at ideasonboard.com
Wed Mar 23 00:34:28 CET 2022
Hi Eric,
Quoting Eric Curtin via libcamera-devel (2022-03-22 19:09:36)
> This adds more portability to existing cam sinks. You can pass a
> YUYV camera buffer for example and SDL will handle the pixel buffer
> conversion, although SDL does not support decompression for pixelformats
> like MJPEG. This allows cam reference implementation to display images
> on VMs, Mac M1, Raspberry Pi, etc. This also enables cam reference
> implementation, to run as a desktop application in wayland or x11.
> SDL also has support for Android and ChromeOS which I have not tested.
> Also tested on simpledrm raspberry pi 4 framebuffer successfully where
> existing kms sink did not work.
This sounds very helpful and useful to me...
>
> Signed-off-by: Eric Curtin <ecurtin at redhat.com>
> ---
> Changes in v2:
> - Remove hardcoded pixel format from SDL_CreateTexture call
> ---
> src/cam/camera_session.cpp | 8 +++
> src/cam/main.cpp | 6 ++
> src/cam/main.h | 1 +
> src/cam/meson.build | 10 +++
> src/cam/sdl_sink.cpp | 125 +++++++++++++++++++++++++++++++++++++
> src/cam/sdl_sink.h | 40 ++++++++++++
> 6 files changed, 190 insertions(+)
> create mode 100644 src/cam/sdl_sink.cpp
> create mode 100644 src/cam/sdl_sink.h
>
> diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
> index 0428b538..30162dbd 100644
> --- a/src/cam/camera_session.cpp
> +++ b/src/cam/camera_session.cpp
> @@ -19,6 +19,9 @@
> #ifdef HAVE_KMS
> #include "kms_sink.h"
> #endif
> +#ifdef HAVE_SDL
> +#include "sdl_sink.h"
> +#endif
> #include "main.h"
> #include "stream_options.h"
>
> @@ -187,6 +190,11 @@ int CameraSession::start()
> sink_ = std::make_unique<KMSSink>(options_[OptDisplay].toString());
> #endif
>
> +#ifdef HAVE_SDL
> + if (options_.isSet(OptSDL))
> + sink_ = std::make_unique<SDLSink>();
> +#endif
> +
> if (options_.isSet(OptFile)) {
> if (!options_[OptFile].toString().empty())
> sink_ = std::make_unique<FileSink>(streamNames_,
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index c7f664b9..406d61bc 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -138,6 +138,12 @@ int CamApp::parseOptions(int argc, char *argv[])
> "display", ArgumentOptional, "connector", false,
> OptCamera);
> #endif
> +
> +#ifdef HAVE_SDL
> + parser.addOption(OptSDL, OptionNone,
> + "Display viewfinder through SDL",
> + "sdl", ArgumentNone, "", false, OptCamera);
> +#endif
> parser.addOption(OptFile, OptionString,
> "Write captured frames to disk\n"
> "If the file name ends with a '/', it sets the directory in which\n"
> diff --git a/src/cam/main.h b/src/cam/main.h
> index 62f7bbc9..a64f95a0 100644
> --- a/src/cam/main.h
> +++ b/src/cam/main.h
> @@ -11,6 +11,7 @@ enum {
> OptCamera = 'c',
> OptCapture = 'C',
> OptDisplay = 'D',
> + OptSDL = 'S',
> OptFile = 'F',
> OptHelp = 'h',
> OptInfo = 'I',
> diff --git a/src/cam/meson.build b/src/cam/meson.build
> index e8e2ae57..44202ef0 100644
> --- a/src/cam/meson.build
> +++ b/src/cam/meson.build
> @@ -32,11 +32,21 @@ cam_sources += files([
> ])
> endif
>
> +libsdl2 = dependency('SDL2', required : false)
> +
> +if libsdl2.found()
> +cam_cpp_args += [ '-DHAVE_SDL' ]
> +cam_sources += files([
> + 'sdl_sink.cpp'
> +])
> +endif
> +
> cam = executable('cam', cam_sources,
> dependencies : [
> libatomic,
> libcamera_public,
> libdrm,
> + libsdl2,
> libevent,
> ],
> cpp_args : cam_cpp_args,
> diff --git a/src/cam/sdl_sink.cpp b/src/cam/sdl_sink.cpp
> new file mode 100644
> index 00000000..480df1b4
> --- /dev/null
> +++ b/src/cam/sdl_sink.cpp
> @@ -0,0 +1,125 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * sdl_sink.cpp - SDL Sink
> + */
> +
> +#include "sdl_sink.h"
> +
> +#include <assert.h>
> +#include <fcntl.h>
> +#include <iomanip>
> +#include <iostream>
> +#include <signal.h>
> +#include <sstream>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include <libcamera/camera.h>
> +#include <libcamera/formats.h>
> +
> +#include "image.h"
> +
> +using namespace libcamera;
> +
> +SDLSink::SDLSink()
> +{
> + memset(&sdlRect_, 0, sizeof(sdlRect_));
> +}
> +
> +SDLSink::~SDLSink()
> +{
> + SDL_Quit();
> +}
> +
> +int SDLSink::configure(const libcamera::CameraConfiguration &cfg)
> +{
> + int ret = FrameSink::configure(cfg);
> + if (ret < 0)
> + return ret;
> +
> + if ((ret = SDL_Init(SDL_INIT_VIDEO))) {
> + std::cout << "Could not initialize SDL - " << SDL_GetError() << "\n";
> + return ret;
> + }
> +
> + const libcamera::StreamConfiguration &sCfg = cfg.at(0);
> + sdlScreen_ = SDL_CreateWindow("", SDL_WINDOWPOS_UNDEFINED,
> + SDL_WINDOWPOS_UNDEFINED, sCfg.size.width,
> + sCfg.size.height, SDL_WINDOW_SHOWN | SDL_WINDOW_RESIZABLE);
> + if (!sdlScreen_) {
> + std::cerr << "SDL: could not create window - exiting: " << SDL_GetError() << "\n";
> + return -1;
is an errno more appropriate?
Same for the others..
> + }
> +
> + sdlRenderer_ = SDL_CreateRenderer(
> + sdlScreen_, -1, 0);
> + if (!sdlRenderer_) {
> + std::cerr << "SDL_CreateRenderer Error\n";
> + return -2;
> + }
> +
> + SDL_RenderSetLogicalSize(sdlRenderer_, sCfg.size.width,
> + sCfg.size.height);
> +
> + SDL_PixelFormatEnum pf = (SDL_PixelFormatEnum)sCfg.pixelFormat.fourcc();
> + if (pf == SDL_DEFINE_PIXELFOURCC('Y', 'U', 'Y', 'V')) {
> + pf = SDL_PIXELFORMAT_YUY2;
> + }
Is this some sort of conversion that most fourccs are compatible
'except' YUY2? I'd be wary if there is even a single difference, and
it might be better to put in an explicit conversion table or map?
Particularly as libcamera fourcc's are based on DRM fourccs... are SDL
fourcc's defined as directly compatible there? I.e. there are frequently
differences in byte ordering on the RGB/BGR formats that cause issues.
> +
> + if (!(ret = strcmp(SDL_GetPixelFormatName(pf), "SDL_PIXELFORMAT_UNKNOWN"))) {
> + std::cerr << "error: SDL_PIXELFORMAT_UNKNOWN, no " << sCfg.pixelFormat.toString() << " support\n";
> + return -3;
> + }
> +
> + sdlTexture_ =
> + SDL_CreateTexture(sdlRenderer_, pf,
> + SDL_TEXTUREACCESS_STREAMING, sCfg.size.width,
> + sCfg.size.height);
> + sdlRect_.w = sCfg.size.width;
> + sdlRect_.h = sCfg.size.height;
> +
> + return 0;
> +}
> +
> +void SDLSink::mapBuffer(FrameBuffer *buffer)
> +{
> + std::unique_ptr<Image> image =
> + Image::fromFrameBuffer(buffer, Image::MapMode::ReadOnly);
> + assert(image != nullptr);
> +
> + mappedBuffers_[buffer] = std::move(image);
> +}
> +
> +bool SDLSink::processRequest(Request *request)
> +{
> + for (auto [stream, buffer] : request->buffers())
> + writeBuffer(buffer);
> +
> + return true;
> +}
> +
> +void SDLSink::writeBuffer(FrameBuffer *buffer)
> +{
> + Image *image = mappedBuffers_[buffer].get();
> +
> + for (unsigned int i = 0; i < buffer->planes().size(); ++i) {
> + const FrameMetadata::Plane &meta = buffer->metadata().planes()[i];
> +
> + Span<uint8_t> data = image->data(i);
> + if (meta.bytesused > data.size())
> + std::cerr << "payload size " << meta.bytesused
> + << " larger than plane size " << data.size()
> + << std::endl;
> +
> + SDL_UpdateTexture(sdlTexture_, &sdlRect_, data.data(), sdlRect_.w * 2);
> + SDL_RenderClear(sdlRenderer_);
> + SDL_RenderCopy(sdlRenderer_, sdlTexture_, NULL, NULL);
> + SDL_RenderPresent(sdlRenderer_);
> + SDL_Event e;
> + while (SDL_PollEvent(&e)) {
> + if (e.type == SDL_QUIT) { // click close icon then quit
> + kill(getpid(), SIGINT);
> + }
> + };
This seems like an odd place to be doing the event loop polling. I guess
it's intentional to be able to 'hook' it in somewhere?
I'd be tempted to pull it out to a separate function at least.
Having never looked at SDL code before, I must say, - that looks like
it's really quite easy to pull together a window and display!
> + }
> +}
> diff --git a/src/cam/sdl_sink.h b/src/cam/sdl_sink.h
> new file mode 100644
> index 00000000..f4f843fa
> --- /dev/null
> +++ b/src/cam/sdl_sink.h
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * sdl_sink.h - SDL Sink
> + */
> +
> +#pragma once
> +
> +#include <map>
> +#include <memory>
> +#include <string>
> +
> +#include <libcamera/stream.h>
> +
> +#include <SDL2/SDL.h>
> +
> +#include "frame_sink.h"
> +
> +class Image;
> +
> +class SDLSink : public FrameSink
> +{
> +public:
> + SDLSink();
> + ~SDLSink();
> +
> + int configure(const libcamera::CameraConfiguration &cfg) override;
> +
> + void mapBuffer(libcamera::FrameBuffer *buffer) override;
> +
> + bool processRequest(libcamera::Request *request) override;
> +
> +private:
> + void writeBuffer(libcamera::FrameBuffer *buffer);
> +
> + std::map<libcamera::FrameBuffer *, std::unique_ptr<Image>> mappedBuffers_;
> + SDL_Window *sdlScreen_;
> + SDL_Renderer *sdlRenderer_;
> + SDL_Texture *sdlTexture_;
> + SDL_Rect sdlRect_;
> +};
> --
> 2.35.1
>
More information about the libcamera-devel
mailing list