[libcamera-devel,v5,1/2] cam: sdl_sink: Use libjpeg over SDL2_image

16702 diff mbox series

From: Eric Curtin <ecurtin@redhat.com>

We were using the libjpeg functionality of SDL2_image only, instead just
use libjpeg directly to reduce our dependancy count, it is a more
commonly available library.

Signed-off-by: Eric Curtin <ecurtin@redhat.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v4:

- Move JpegErrorManager to sdl_texture_mjpg.cpp
- Make JpegErrorManager inherit from jpeg_error_mgr
- Define errorExit and outputMessage as member functions
---
 README.rst                   |  2 +-
 src/cam/meson.build          |  8 ++---
 src/cam/sdl_sink.cpp         |  4 +--
 src/cam/sdl_texture_mjpg.cpp | 70 ++++++++++++++++++++++++++++++++----
 src/cam/sdl_texture_mjpg.h   |  6 ++++
 5 files changed, 77 insertions(+), 13 deletions(-)

Not knowing that much about libjpeg, I can only comment that the
plumbing seems right

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

On Wed, Jul 20, 2022 at 04:03:47PM +0300, Laurent Pinchart via libcamera-devel wrote:
> From: Eric Curtin <ecurtin@redhat.com>
>
> We were using the libjpeg functionality of SDL2_image only, instead just
> use libjpeg directly to reduce our dependancy count, it is a more
> commonly available library.
>
> Signed-off-by: Eric Curtin <ecurtin@redhat.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Changes since v4:
>
> - Move JpegErrorManager to sdl_texture_mjpg.cpp
> - Make JpegErrorManager inherit from jpeg_error_mgr
> - Define errorExit and outputMessage as member functions
> ---
>  README.rst                   |  2 +-
>  src/cam/meson.build          |  8 ++---
>  src/cam/sdl_sink.cpp         |  4 +--
>  src/cam/sdl_texture_mjpg.cpp | 70 ++++++++++++++++++++++++++++++++----
>  src/cam/sdl_texture_mjpg.h   |  6 ++++
>  5 files changed, 77 insertions(+), 13 deletions(-)
>
> diff --git a/README.rst b/README.rst
> index b9e72d81b90c..47b914f02260 100644
> --- a/README.rst
> +++ b/README.rst
> @@ -92,8 +92,8 @@ for cam: [optional]
>          tool:
>
>          - libdrm-dev: Enables the KMS sink
> +        - libjpeg-dev: Enables MJPEG on the SDL sink
>          - libsdl2-dev: Enables the SDL sink
> -        - libsdl2-image-dev: Supports MJPEG on the SDL sink
>
>  for qcam: [optional]
>          qtbase5-dev libqt5core5a libqt5gui5 libqt5widgets5 qttools5-dev-tools libtiff-dev
> diff --git a/src/cam/meson.build b/src/cam/meson.build
> index 5957ce140391..4dfa7b22aea9 100644
> --- a/src/cam/meson.build
> +++ b/src/cam/meson.build
> @@ -24,8 +24,8 @@ cam_sources = files([
>  cam_cpp_args = []
>
>  libdrm = dependency('libdrm', required : false)
> +libjpeg = dependency('libjpeg', required : false)
>  libsdl2 = dependency('SDL2', required : false)
> -libsdl2_image = dependency('SDL2_image', required : false)
>
>  if libdrm.found()
>      cam_cpp_args += [ '-DHAVE_KMS' ]
> @@ -43,8 +43,8 @@ if libsdl2.found()
>          'sdl_texture_yuyv.cpp'
>      ])
>
> -    if libsdl2_image.found()
> -        cam_cpp_args += ['-DHAVE_SDL_IMAGE']
> +    if libjpeg.found()
> +        cam_cpp_args += ['-DHAVE_LIBJPEG']
>          cam_sources += files([
>              'sdl_texture_mjpg.cpp'
>          ])
> @@ -57,8 +57,8 @@ cam  = executable('cam', cam_sources,
>                        libcamera_public,
>                        libdrm,
>                        libevent,
> +                      libjpeg,
>                        libsdl2,
> -                      libsdl2_image,
>                        libyaml,
>                    ],
>                    cpp_args : cam_cpp_args,
> diff --git a/src/cam/sdl_sink.cpp b/src/cam/sdl_sink.cpp
> index f8e3e95dd392..19fdfd6dced5 100644
> --- a/src/cam/sdl_sink.cpp
> +++ b/src/cam/sdl_sink.cpp
> @@ -21,7 +21,7 @@
>
>  #include "event_loop.h"
>  #include "image.h"
> -#ifdef HAVE_SDL_IMAGE
> +#ifdef HAVE_LIBJPEG
>  #include "sdl_texture_mjpg.h"
>  #endif
>  #include "sdl_texture_yuyv.h"
> @@ -62,7 +62,7 @@ int SDLSink::configure(const libcamera::CameraConfiguration &config)
>  	rect_.h = cfg.size.height;
>
>  	switch (cfg.pixelFormat) {
> -#ifdef HAVE_SDL_IMAGE
> +#ifdef HAVE_LIBJPEG
>  	case libcamera::formats::MJPEG:
>  		texture_ = std::make_unique<SDLTextureMJPG>(rect_);
>  		break;
> diff --git a/src/cam/sdl_texture_mjpg.cpp b/src/cam/sdl_texture_mjpg.cpp
> index 69e99ad35219..7eddc00cd210 100644
> --- a/src/cam/sdl_texture_mjpg.cpp
> +++ b/src/cam/sdl_texture_mjpg.cpp
> @@ -7,19 +7,77 @@
>
>  #include "sdl_texture_mjpg.h"
>
> -#include <SDL2/SDL_image.h>
> +#include <iostream>
> +#include <setjmp.h>
> +#include <stdio.h>
> +
> +#include <jpeglib.h>
>
>  using namespace libcamera;
>
> +struct JpegErrorManager : public jpeg_error_mgr {
> +	JpegErrorManager()
> +	{
> +		jpeg_std_error(this);
> +		error_exit = errorExit;
> +		output_message = outputMessage;
> +	}
> +
> +	static void errorExit(j_common_ptr cinfo)
> +	{
> +		JpegErrorManager *self =
> +			static_cast<JpegErrorManager *>(cinfo->err);
> +		longjmp(self->escape_, 1);
> +	}
> +
> +	static void outputMessage([[maybe_unused]] j_common_ptr cinfo)
> +	{
> +	}
> +
> +	jmp_buf escape_;
> +};
> +
>  SDLTextureMJPG::SDLTextureMJPG(const SDL_Rect &rect)
> -	: SDLTexture(rect, SDL_PIXELFORMAT_RGB24, 0)
> +	: SDLTexture(rect, SDL_PIXELFORMAT_RGB24, rect.w * 3),
> +	  rgb_(std::make_unique<unsigned char[]>(pitch_ * rect.h))
>  {
>  }
>
> +int SDLTextureMJPG::decompress(const Span<uint8_t> &data)
> +{
> +	struct jpeg_decompress_struct cinfo;
> +
> +	JpegErrorManager errorManager;
> +	if (setjmp(errorManager.escape_)) {
> +		/* libjpeg found an error */
> +		jpeg_destroy_decompress(&cinfo);
> +		std::cerr << "JPEG decompression error" << std::endl;
> +		return -EINVAL;
> +	}
> +
> +	cinfo.err = &errorManager;
> +	jpeg_create_decompress(&cinfo);
> +
> +	jpeg_mem_src(&cinfo, data.data(), data.size());
> +
> +	jpeg_read_header(&cinfo, TRUE);
> +
> +	jpeg_start_decompress(&cinfo);
> +
> +	for (int i = 0; cinfo.output_scanline < cinfo.output_height; ++i) {
> +		JSAMPROW rowptr = rgb_.get() + i * pitch_;
> +		jpeg_read_scanlines(&cinfo, &rowptr, 1);
> +	}
> +
> +	jpeg_finish_decompress(&cinfo);
> +
> +	jpeg_destroy_decompress(&cinfo);
> +
> +	return 0;
> +}
> +
>  void SDLTextureMJPG::update(const Span<uint8_t> &data)
>  {
> -	SDL_RWops *bufferStream = SDL_RWFromMem(data.data(), data.size());
> -	SDL_Surface *frame = IMG_Load_RW(bufferStream, 0);
> -	SDL_UpdateTexture(ptr_, nullptr, frame->pixels, frame->pitch);
> -	SDL_FreeSurface(frame);
> +	decompress(data);
> +	SDL_UpdateTexture(ptr_, nullptr, rgb_.get(), pitch_);
>  }
> diff --git a/src/cam/sdl_texture_mjpg.h b/src/cam/sdl_texture_mjpg.h
> index b103f801176d..328c45a913c5 100644
> --- a/src/cam/sdl_texture_mjpg.h
> +++ b/src/cam/sdl_texture_mjpg.h
> @@ -13,5 +13,11 @@ class SDLTextureMJPG : public SDLTexture
>  {
>  public:
>  	SDLTextureMJPG(const SDL_Rect &rect);
> +
>  	void update(const libcamera::Span<uint8_t> &data) override;
> +
> +private:
> +	int decompress(const libcamera::Span<uint8_t> &data);
> +
> +	std::unique_ptr<unsigned char[]> rgb_;
>  };
> --
> Regards,
>
> Laurent Pinchart
>