[libcamera-devel,v8,4/4] cam: sdl_sink: Add MJPG support to SDL sink
diff mbox series

Message ID 20220505151851.2421-4-ecurtin@redhat.com
State Accepted
Headers show
Series
  • [libcamera-devel,v8,1/4] cam: event_loop: Add timer events to event loop
Related show

Commit Message

Eric Curtin May 5, 2022, 3:18 p.m. UTC
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@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

Comments

Kieran Bingham May 17, 2022, 11:42 p.m. UTC | #1
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@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@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
> +
>  #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 @@
> +#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());
> +       SDL_Surface *frame = IMG_Load_RW(buffer_stream, 0);
> +       SDL_UpdateTexture(ptr_, NULL, frame->pixels, frame->pitch);
> +       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;
> +};
> -- 
> 2.35.1
>
Laurent Pinchart May 18, 2022, 7:33 p.m. UTC | #2
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@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@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;
> > +};
Kieran Bingham May 19, 2022, 4:23 p.m. UTC | #3
Quoting Laurent Pinchart (2022-05-18 20:33:22)
> 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@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']

Oh - here it is - but somehow this didn't work or something didn't do
the right thing perhaps ?

--
Kieran


> > > +        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@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
Laurent Pinchart May 19, 2022, 4:31 p.m. UTC | #4
Two more comments.

On Wed, May 18, 2022 at 10:33:22PM +0300, Laurent Pinchart via libcamera-devel wrote:
> 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@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()

This should be libsdl2_image.

> > > +        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@ideasonboard.com>
> > 
> > >  cam  = executable('cam', cam_sources,
> > > @@ -49,6 +57,7 @@ cam  = executable('cam', cam_sources,
> > >                        libcamera_public,
> > >                        libdrm,
> > >                        libsdl2,
> > > +                      libsdl2_image,

If libsdl2 isn't found, then libsdl2_image won't be assigned, and meson
will throw an error here:

src/cam/meson.build:55:0: ERROR: Unknown variable "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;
> > > +};
Laurent Pinchart May 19, 2022, 4:39 p.m. UTC | #5
Hi Kieran,

On Thu, May 19, 2022 at 05:23:41PM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2022-05-18 20:33:22)
> > 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@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']
> 
> Oh - here it is - but somehow this didn't work or something didn't do
> the right thing perhaps ?

I haven't noticed your issue report before sending mine :-)

This could be fixed with

libsdl2 = dependency('SDL2', required : false)
libsdl2_image = dependency('SDL2_image', required : false)

if libsdl2.found()
    cam_cpp_args += ['-DHAVE_SDL']
    cam_sources += files([
        'sdl_sink.cpp',
        'sdl_texture.cpp',
        'sdl_texture_yuyv.cpp'
    ])

    if libsdl2_image.found()
        cam_cpp_args += ['-DHAVE_SDL_IMAGE']
        cam_sources += files([
            'sdl_texture_mjpg.cpp'
        ])
    endif
endif

> > > > +        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@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;
> > > > +};

Patch
diff mbox series

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
 
 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
+
 #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 @@ 
+#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());
+	SDL_Surface *frame = IMG_Load_RW(buffer_stream, 0);
+	SDL_UpdateTexture(ptr_, NULL, frame->pixels, frame->pitch);
+	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;
+};