[libcamera-devel] [PATCH v8 4/4] cam: sdl_sink: Add MJPG support to SDL sink

[Laurent Pinchart]

Hi Eric,

Thank you for the patch.

On Wed, May 18, 2022 at 12:42:03AM +0100, Kieran Bingham via libcamera-devel wrote:
> Quoting Eric Curtin via libcamera-devel (2022-05-05 16:18:51)
> > So we have at least two supported capturing pixel formats (although
> > many possible output pixel formats thanks to SDL conversion). MJPG
> > support only built in if SDL2_image is available, provides
> > decompression.
> > 
> > Signed-off-by: Eric Curtin <ecurtin at redhat.com>
> > ---
> >  src/cam/meson.build          |  9 +++++++++
> >  src/cam/sdl_sink.cpp         |  9 +++++++++
> >  src/cam/sdl_texture_mjpg.cpp | 18 ++++++++++++++++++
> >  src/cam/sdl_texture_mjpg.h   | 10 ++++++++++
> >  4 files changed, 46 insertions(+)
> >  create mode 100644 src/cam/sdl_texture_mjpg.cpp
> >  create mode 100644 src/cam/sdl_texture_mjpg.h
> > 
> > diff --git a/src/cam/meson.build b/src/cam/meson.build
> > index 3032730b..afc0ea9f 100644
> > --- a/src/cam/meson.build
> > +++ b/src/cam/meson.build
> > @@ -41,6 +41,14 @@ if libsdl2.found()
> >          'sdl_texture.cpp',
> >          'sdl_texture_yuyv.cpp'
> >      ])
> > +
> > +    libsdl2_image = dependency('SDL2_image', required : false)
> > +    if libsdl2.found()
> > +        cam_cpp_args += ['-DHAVE_SDL_IMAGE']
> > +        cam_sources += files([
> > +            'sdl_texture_mjpg.cpp'
> > +        ])
> > +    endif
> >  endif
> 
> All this extending complexity that is sdl specific makes me wish more
> that this was under src/cam/sdl/ with a dedicated meson.build there, but
> that could also be done on top.
> 
> This looks quite straightforward so far so:
> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> >  cam  = executable('cam', cam_sources,
> > @@ -49,6 +57,7 @@ cam  = executable('cam', cam_sources,
> >                        libcamera_public,
> >                        libdrm,
> >                        libsdl2,
> > +                      libsdl2_image,
> >                        libevent,
> >                    ],
> >                    cpp_args : cam_cpp_args,
> > diff --git a/src/cam/sdl_sink.cpp b/src/cam/sdl_sink.cpp
> > index 6fbeaf56..f15c01cb 100644
> > --- a/src/cam/sdl_sink.cpp
> > +++ b/src/cam/sdl_sink.cpp
> > @@ -19,6 +19,10 @@
> >  
> >  #include "sdl_texture_yuyv.h"
> >  
> > +#ifdef HAVE_SDL_IMAGE
> > +#include "sdl_texture_mjpg.h"
> > +#endif

I'd move this above the previous header to order them alphabetically.

> > +
> >  #include "event_loop.h"
> >  #include "image.h"
> >  
> > @@ -59,6 +63,11 @@ int SDLSink::configure(const libcamera::CameraConfiguration &config)
> >         case libcamera::formats::YUYV:
> >                 sdlTexture_ = std::make_unique<SDLTextureYUYV>(sdlRect_);
> >                 break;
> > +#ifdef HAVE_SDL_IMAGE
> > +       case libcamera::formats::MJPEG:
> > +               sdlTexture_ = std::make_unique<SDLTextureMJPG>(sdlRect_);
> > +               break;
> > +#endif
> >         default:
> >                 std::cerr << "Unsupported pixel format "
> >                           << cfg.pixelFormat.toString() << std::endl;
> > diff --git a/src/cam/sdl_texture_mjpg.cpp b/src/cam/sdl_texture_mjpg.cpp
> > new file mode 100644
> > index 00000000..636fdbea
> > --- /dev/null
> > +++ b/src/cam/sdl_texture_mjpg.cpp
> > @@ -0,0 +1,18 @@

Missing SPDX tag and copyright header. Same for sdl_texture_mjpg.cpp.

> > +#include "sdl_texture_mjpg.h"
> > +
> > +#include <SDL2/SDL_image.h>
> > +
> > +using namespace libcamera;
> > +
> > +SDLTextureMJPG::SDLTextureMJPG(const SDL_Rect &sdlRect)
> > +       : SDLTexture(sdlRect, SDL_PIXELFORMAT_RGB24, 0)
> > +{
> > +}
> > +
> > +void SDLTextureMJPG::update(const Span<uint8_t> &data)
> > +{
> > +       SDL_RWops *buffer_stream = SDL_RWFromMem(data.data(), data.size());

s/buffer_stream/bufferStream/

> > +       SDL_Surface *frame = IMG_Load_RW(buffer_stream, 0);
> > +       SDL_UpdateTexture(ptr_, NULL, frame->pixels, frame->pitch);

s/NULL/nullptr/

> > +       SDL_FreeSurface(frame);
> > +}
> > diff --git a/src/cam/sdl_texture_mjpg.h b/src/cam/sdl_texture_mjpg.h
> > new file mode 100644
> > index 00000000..fefaaeeb
> > --- /dev/null
> > +++ b/src/cam/sdl_texture_mjpg.h
> > @@ -0,0 +1,10 @@
> > +#pragma once
> > +
> > +#include "sdl_texture.h"
> > +
> > +class SDLTextureMJPG : public SDLTexture
> > +{
> > +public:
> > +       SDLTextureMJPG(const SDL_Rect &sdlRect);
> > +       void update(const libcamera::Span<uint8_t> &data) override;
> > +};

-- 
Regards,

Laurent Pinchart