[libcamera-devel] [PATCH v9 3/4] cam: sdl_sink: Add SDL sink with initial YUYV support
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun May 22 19:31:51 CEST 2022
Another small comment.
On Sun, May 22, 2022 at 01:36:28PM +0300, Laurent Pinchart via libcamera-devel wrote:
> On Fri, May 20, 2022 at 08:01:05PM +0100, Eric Curtin wrote:
> > This adds more portability to existing cam sinks. You can pass a
> > YUYV camera buffer and SDL will handle the pixel buffer conversion
> > to the display. 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 has not been
> > tested. Also tested on simpledrm Raspberry Pi 4 framebuffer
> > successfully where existing kms sink did not work. Can also be used as
> > kmsdrm sink. Only supports one camera stream at present.
> >
> > Signed-off-by: Eric Curtin <ecurtin at redhat.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > Tested-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> > src/cam/camera_session.cpp | 8 ++
> > src/cam/main.cpp | 4 +
> > src/cam/main.h | 1 +
> > src/cam/meson.build | 12 +++
> > src/cam/sdl_sink.cpp | 194 +++++++++++++++++++++++++++++++++++
> > src/cam/sdl_sink.h | 49 +++++++++
> > src/cam/sdl_texture.cpp | 37 +++++++
> > src/cam/sdl_texture.h | 29 ++++++
> > src/cam/sdl_texture_yuyv.cpp | 20 ++++
> > src/cam/sdl_texture_yuyv.h | 17 +++
> > 10 files changed, 371 insertions(+)
> > create mode 100644 src/cam/sdl_sink.cpp
> > create mode 100644 src/cam/sdl_sink.h
> > create mode 100644 src/cam/sdl_texture.cpp
> > create mode 100644 src/cam/sdl_texture.h
> > create mode 100644 src/cam/sdl_texture_yuyv.cpp
> > create mode 100644 src/cam/sdl_texture_yuyv.h
> >
> > diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
> > index efffafbf..336ae471 100644
> > --- a/src/cam/camera_session.cpp
> > +++ b/src/cam/camera_session.cpp
> > @@ -20,6 +20,9 @@
> > #include "kms_sink.h"
> > #endif
> > #include "main.h"
> > +#ifdef HAVE_SDL
> > +#include "sdl_sink.h"
> > +#endif
> > #include "stream_options.h"
> >
> > using namespace libcamera;
> > @@ -186,6 +189,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..962262a8 100644
> > --- a/src/cam/main.cpp
> > +++ b/src/cam/main.cpp
> > @@ -147,6 +147,10 @@ int CamApp::parseOptions(int argc, char *argv[])
> > "The default file name is 'frame-#.bin'.",
> > "file", ArgumentOptional, "filename", false,
> > OptCamera);
> > +#ifdef HAVE_SDL
> > + parser.addOption(OptSDL, OptionNone, "Display viewfinder through SDL",
> > + "sdl", ArgumentNone, "", false, OptCamera);
> > +#endif
> > parser.addOption(OptStream, &streamKeyValue,
> > "Set configuration of a camera stream", "stream", true,
> > OptCamera);
> > diff --git a/src/cam/main.h b/src/cam/main.h
> > index 62f7bbc9..507a184f 100644
> > --- a/src/cam/main.h
> > +++ b/src/cam/main.h
> > @@ -17,6 +17,7 @@ enum {
> > OptList = 'l',
> > OptListProperties = 'p',
> > OptMonitor = 'm',
> > + OptSDL = 'S',
> > OptStream = 's',
> > OptListControls = 256,
> > OptStrictFormats = 257,
> > diff --git a/src/cam/meson.build b/src/cam/meson.build
> > index 5bab8c9e..7d714152 100644
> > --- a/src/cam/meson.build
> > +++ b/src/cam/meson.build
> > @@ -32,12 +32,24 @@ if libdrm.found()
> > ])
> > endif
> >
> > +libsdl2 = dependency('SDL2', required : false)
> > +
> > +if libsdl2.found()
> > + cam_cpp_args += ['-DHAVE_SDL']
> > + cam_sources += files([
> > + 'sdl_sink.cpp',
> > + 'sdl_texture.cpp',
> > + 'sdl_texture_yuyv.cpp'
> > + ])
> > +endif
> > +
> > cam = executable('cam', cam_sources,
> > dependencies : [
> > libatomic,
> > libcamera_public,
> > libdrm,
> > libevent,
> > + libsdl2,
> > ],
> > cpp_args : cam_cpp_args,
> > install : true)
> > diff --git a/src/cam/sdl_sink.cpp b/src/cam/sdl_sink.cpp
> > new file mode 100644
> > index 00000000..65430efb
> > --- /dev/null
> > +++ b/src/cam/sdl_sink.cpp
> > @@ -0,0 +1,194 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2022, Ideas on Board Oy
> > + *
> > + * sdl_sink.h - 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 "event_loop.h"
> > +#include "image.h"
> > +#include "sdl_texture_yuyv.h"
> > +
> > +using namespace libcamera;
> > +
> > +using namespace std::chrono_literals;
> > +
> > +SDLSink::SDLSink()
> > + : window_(nullptr), renderer_(nullptr), rect_({}),
> > + init_(false)
> > +{
> > +}
> > +
> > +SDLSink::~SDLSink()
> > +{
> > + stop();
> > +}
> > +
> > +int SDLSink::configure(const libcamera::CameraConfiguration &config)
> > +{
> > + const int ret = FrameSink::configure(config);
>
> No need to make ret const.
>
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (config.size() > 1) {
> > + std::cerr
> > + << "SDL sink only supports one camera stream at present, streaming first camera stream"
> > + << std::endl;
> > + } else if (config.empty()) {
> > + std::cerr << "Require at least one camera stream to process"
> > + << std::endl;
> > + return -EINVAL;
> > + }
> > +
> > + const libcamera::StreamConfiguration &cfg = config.at(0);
> > + rect_.w = cfg.size.width;
> > + rect_.h = cfg.size.height;
> > +
> > + switch (cfg.pixelFormat) {
> > + case libcamera::formats::YUYV:
> > + texture_ = std::make_unique<SDLTextureYUYV>(rect_);
> > + break;
> > + default:
> > + std::cerr << "Unsupported pixel format "
> > + << cfg.pixelFormat.toString() << std::endl;
> > + return -EINVAL;
> > + };
> > +
> > + return 0;
> > +}
> > +
> > +int SDLSink::start()
> > +{
> > + int ret = SDL_Init(SDL_INIT_VIDEO);
> > + if (ret) {
> > + std::cerr << "Failed to initialize SDL: " << SDL_GetError()
> > + << std::endl;
> > + return ret;
> > + }
> > +
> > + init_ = true;
> > + window_ = SDL_CreateWindow("", SDL_WINDOWPOS_UNDEFINED,
> > + SDL_WINDOWPOS_UNDEFINED, rect_.w,
> > + rect_.h,
> > + SDL_WINDOW_SHOWN | SDL_WINDOW_RESIZABLE);
> > + if (!window_) {
> > + std::cerr << "Failed to create SDL window: " << SDL_GetError()
> > + << std::endl;
> > + return -EINVAL;
> > + }
> > +
> > + renderer_ = SDL_CreateRenderer(window_, -1, 0);
> > + if (!renderer_) {
> > + std::cerr << "Failed to create SDL renderer: " << SDL_GetError()
> > + << std::endl;
> > + return -EINVAL;
> > + }
> > +
> > + ret = SDL_RenderSetLogicalSize(renderer_, rect_.w, rect_.h);
> > + if (ret) { /* Not critical to set, don't return in this case, set for scaling purposes */
> > + std::cerr << "Failed to set SDL render logical size: "
> > + << SDL_GetError() << std::endl;
> > + }
>
> /*
> * Not critical to set, don't return in this case, set for scaling
> * purposes.
> */
> if (ret)
> std::cerr << "Failed to set SDL render logical size: "
> << SDL_GetError() << std::endl;
>
> > +
> > + ret = texture_->create(renderer_);
> > + if (ret) {
> > + return ret;
> > + }
> > +
> > + /* \todo Make the event cancellable to support stop/start cycles. */
> > + EventLoop::instance()->addTimerEvent(
> > + 10ms, std::bind(&SDLSink::processSDLEvents, this));
> > +
> > + return 0;
> > +}
> > +
> > +int SDLSink::stop()
> > +{
> > + if (texture_) {
> > + texture_.reset();
> > + }
>
> This can be written as
>
> texture_.reset();
>
> or
>
> texture_ = nullptr;
>
> > +
> > + if (renderer_) {
> > + SDL_DestroyRenderer(renderer_);
> > + renderer_ = nullptr;
> > + }
> > +
> > + if (window_) {
> > + SDL_DestroyWindow(window_);
> > + window_ = nullptr;
> > + }
> > +
> > + if (init_) {
> > + SDL_Quit();
> > + init_ = false;
> > + }
> > +
> > + return FrameSink::stop();
> > +}
> > +
> > +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()) {
> > + renderBuffer(buffer);
> > + break; /* to be expanded to launch SDL window per buffer */
> > + }
> > +
> > + return true;
> > +}
> > +
> > +/*
> > + * Process SDL events, required for things like window resize and quit button
> > + */
> > +void SDLSink::processSDLEvents()
> > +{
> > + for (SDL_Event e; SDL_PollEvent(&e);) {
> > + if (e.type == SDL_QUIT) {
> > + /* Click close icon then quit */
> > + EventLoop::instance()->exit(0);
> > + }
> > + }
> > +}
> > +
> > +void SDLSink::renderBuffer(FrameBuffer *buffer)
> > +{
> > + Image *image = mappedBuffers_[buffer].get();
> > +
> > + /* \todo Implement support for multi-planar formats. */
> > + const FrameMetadata::Plane &meta =
> > + buffer->metadata().planes()[0];
>
> This holds on a single line.
>
> > +
> > + Span<uint8_t> data = image->data(0);
> > + if (meta.bytesused > data.size())
> > + std::cerr << "payload size " << meta.bytesused
> > + << " larger than plane size " << data.size()
> > + << std::endl;
> > +
> > + texture_->update(data);
> > +
> > + SDL_RenderClear(renderer_);
> > + SDL_RenderCopy(renderer_, texture_->get(), nullptr, nullptr);
> > + SDL_RenderPresent(renderer_);
> > +}
> > diff --git a/src/cam/sdl_sink.h b/src/cam/sdl_sink.h
> > new file mode 100644
> > index 00000000..83171cca
> > --- /dev/null
> > +++ b/src/cam/sdl_sink.h
> > @@ -0,0 +1,49 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2022, Ideas on Board Oy
> > + *
> > + * sdl_sink.h - SDL Sink
> > + */
> > +
> > +#pragma once
> > +
> > +#include <map>
> > +#include <memory>
> > +
> > +#include <libcamera/stream.h>
> > +
> > +#include <SDL2/SDL.h>
> > +
> > +#include "frame_sink.h"
> > +
> > +class Image;
> > +class SDLTexture;
> > +
> > +class SDLSink : public FrameSink
> > +{
> > +public:
> > + SDLSink();
> > + ~SDLSink();
> > +
> > + int configure(const libcamera::CameraConfiguration &config) override;
> > + int start() override;
> > + int stop() override;
> > + void mapBuffer(libcamera::FrameBuffer *buffer) override;
> > +
> > + bool processRequest(libcamera::Request *request) override;
> > +
> > +private:
> > + void renderBuffer(libcamera::FrameBuffer *buffer);
> > + void processSDLEvents();
> > +
> > + std::map<libcamera::FrameBuffer *, std::unique_ptr<Image>>
> > + mappedBuffers_;
> > +
> > + std::unique_ptr<SDLTexture> texture_;
> > +
> > + SDL_Window *window_;
> > + SDL_Renderer *renderer_;
> > + SDL_Rect rect_;
> > + SDL_PixelFormatEnum pixelFormat_;
This isn't used and can be dropped.
> > + bool init_;
> > +};
> > diff --git a/src/cam/sdl_texture.cpp b/src/cam/sdl_texture.cpp
> > new file mode 100644
> > index 00000000..ac355a97
> > --- /dev/null
> > +++ b/src/cam/sdl_texture.cpp
> > @@ -0,0 +1,37 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2022, Ideas on Board Oy
> > + *
> > + * sdl_texture.cpp - SDL Texture
> > + */
> > +
> > +#include "sdl_texture.h"
> > +
> > +#include <iostream>
> > +
> > +SDLTexture::SDLTexture(const SDL_Rect &rect, SDL_PixelFormatEnum pixelFormat,
> > + const int pitch)
> > + : ptr_(nullptr), rect_(rect), pixelFormat_(pixelFormat), pitch_(pitch)
> > +{
> > +}
> > +
> > +SDLTexture::~SDLTexture()
> > +{
> > + if (ptr_) {
> > + SDL_DestroyTexture(ptr_);
> > + }
>
> No need for curly braces.
>
> > +}
> > +
> > +int SDLTexture::create(SDL_Renderer *renderer_)
> > +{
> > + ptr_ = SDL_CreateTexture(renderer_, pixelFormat_,
> > + SDL_TEXTUREACCESS_STREAMING, rect_.w,
> > + rect_.h);
> > + if (!ptr_) {
> > + std::cerr << "Failed to create SDL texture: " << SDL_GetError()
> > + << std::endl;
> > + return -ENOMEM;
> > + }
> > +
> > + return 0;
> > +}
> > diff --git a/src/cam/sdl_texture.h b/src/cam/sdl_texture.h
> > new file mode 100644
> > index 00000000..b04eece0
> > --- /dev/null
> > +++ b/src/cam/sdl_texture.h
> > @@ -0,0 +1,29 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2022, Ideas on Board Oy
> > + *
> > + * sdl_texture.h - SDL Texture
> > + */
> > +
> > +#pragma once
> > +
> > +#include <SDL2/SDL.h>
> > +
> > +#include "image.h"
> > +
> > +class SDLTexture
> > +{
> > +public:
> > + SDLTexture(const SDL_Rect &rect, SDL_PixelFormatEnum pixelFormat,
> > + const int pitch);
> > + virtual ~SDLTexture();
> > + int create(SDL_Renderer *renderer_);
>
> s/renderer_/renderer/
>
> > + virtual void update(const libcamera::Span<uint8_t> &data) = 0;
> > + SDL_Texture *get() const { return ptr_; }
> > +
> > +protected:
> > + SDL_Texture *ptr_;
> > + const SDL_Rect &rect_;
>
> This should be a copy, not a reference, as the caller shouldn't need to
> guarantee that the rectangle passed to the constructor will not be
> destroyed as long as the texture exists.
>
> These are small issues, I can fix them when applying.
>
> > + SDL_PixelFormatEnum pixelFormat_;
> > + const int pitch_;
> > +};
> > diff --git a/src/cam/sdl_texture_yuyv.cpp b/src/cam/sdl_texture_yuyv.cpp
> > new file mode 100644
> > index 00000000..a219097b
> > --- /dev/null
> > +++ b/src/cam/sdl_texture_yuyv.cpp
> > @@ -0,0 +1,20 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2022, Ideas on Board Oy
> > + *
> > + * sdl_texture_yuyv.cpp - SDL Texture YUYV
> > + */
> > +
> > +#include "sdl_texture_yuyv.h"
> > +
> > +using namespace libcamera;
> > +
> > +SDLTextureYUYV::SDLTextureYUYV(const SDL_Rect &sdlRect)
> > + : SDLTexture(sdlRect, SDL_PIXELFORMAT_YUY2, 4 * ((sdlRect.w + 1) / 2))
> > +{
> > +}
> > +
> > +void SDLTextureYUYV::update(const Span<uint8_t> &data)
> > +{
> > + SDL_UpdateTexture(ptr_, &rect_, data.data(), pitch_);
> > +}
> > diff --git a/src/cam/sdl_texture_yuyv.h b/src/cam/sdl_texture_yuyv.h
> > new file mode 100644
> > index 00000000..38c51c74
> > --- /dev/null
> > +++ b/src/cam/sdl_texture_yuyv.h
> > @@ -0,0 +1,17 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2022, Ideas on Board Oy
> > + *
> > + * sdl_texture_yuyv.h - SDL Texture YUYV
> > + */
> > +
> > +#pragma once
> > +
> > +#include "sdl_texture.h"
> > +
> > +class SDLTextureYUYV : public SDLTexture
> > +{
> > +public:
> > + SDLTextureYUYV(const SDL_Rect &sdlRect);
> > + void update(const libcamera::Span<uint8_t> &data) override;
> > +};
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list