[libcamera-devel] [PATCH 3/4] cam: sdl_sink: Support multi-planar formats

[Jacopo Mondi]

Hi Laurent

On Sun, Aug 07, 2022 at 05:17:17AM +0300, Laurent Pinchart via libcamera-devel wrote:
> In order to prepare for NV12 support, implement support for multi-planar
> formats in the SDL sink. This mainly consists in passing a vector of
> plane data to the SDLTexture::update() function instead of a single
> value.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/cam/sdl_sink.cpp         | 21 +++++++++++++--------
>  src/cam/sdl_texture.h        |  4 +++-
>  src/cam/sdl_texture_mjpg.cpp |  4 ++--
>  src/cam/sdl_texture_mjpg.h   |  2 +-
>  src/cam/sdl_texture_yuv.cpp  |  4 ++--
>  src/cam/sdl_texture_yuv.h    |  2 +-
>  6 files changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/src/cam/sdl_sink.cpp b/src/cam/sdl_sink.cpp
> index 04350bd5365b..9675cf275186 100644
> --- a/src/cam/sdl_sink.cpp
> +++ b/src/cam/sdl_sink.cpp
> @@ -185,16 +185,21 @@ 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];
> +	std::vector<Span<const uint8_t>> planes;

You could .reserve() but it's not a big deal

> +	unsigned int i = 0;
>
> -	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;
> +	for (const FrameMetadata::Plane &meta : buffer->metadata().planes()) {

I was about to suggest

        for (const auto &[i, meta] : utils::enumerate(buffer->metadata().planes())) {

and to use utils::enumerate() I have to:

#include <libcamera/base/utils.h>

Which gives me

In file included from ../include/libcamera/base/utils.h:23,
                 from ../src/cam/image.h:17,
                 from ../src/cam/image.cpp:8:
../include/libcamera/base/private.h:21:2: error: #error "Private headers must not be included in the libcamera API"
   21 | #error "Private headers must not be included in the libcamera API"

Which is a bit strange as the file includes several headers from the
<libcamera/base/> inclusion path already :/

That apart
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j


> +		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;
>
> -	texture_->update(data);
> +		planes.push_back(data);
> +		i++;
> +	}
> +
> +	texture_->update(planes);
>
>  	SDL_RenderClear(renderer_);
>  	SDL_RenderCopy(renderer_, texture_->get(), nullptr, nullptr);
> diff --git a/src/cam/sdl_texture.h b/src/cam/sdl_texture.h
> index f523fa5ebf51..e4d3fb2bcf39 100644
> --- a/src/cam/sdl_texture.h
> +++ b/src/cam/sdl_texture.h
> @@ -7,6 +7,8 @@
>
>  #pragma once
>
> +#include <vector>
> +
>  #include <SDL2/SDL.h>
>
>  #include "image.h"
> @@ -17,7 +19,7 @@ public:
>  	SDLTexture(const SDL_Rect &rect, uint32_t pixelFormat, const int pitch);
>  	virtual ~SDLTexture();
>  	int create(SDL_Renderer *renderer);
> -	virtual void update(libcamera::Span<const uint8_t> data) = 0;
> +	virtual void update(const std::vector<libcamera::Span<const uint8_t>> &data) = 0;
>  	SDL_Texture *get() const { return ptr_; }
>
>  protected:
> diff --git a/src/cam/sdl_texture_mjpg.cpp b/src/cam/sdl_texture_mjpg.cpp
> index 7542efd75d96..8dd5ee3eb6bc 100644
> --- a/src/cam/sdl_texture_mjpg.cpp
> +++ b/src/cam/sdl_texture_mjpg.cpp
> @@ -76,8 +76,8 @@ int SDLTextureMJPG::decompress(Span<const uint8_t> data)
>  	return 0;
>  }
>
> -void SDLTextureMJPG::update(Span<const uint8_t> data)
> +void SDLTextureMJPG::update(const std::vector<libcamera::Span<const uint8_t>> &data)
>  {
> -	decompress(data);
> +	decompress(data[0]);
>  	SDL_UpdateTexture(ptr_, nullptr, rgb_.get(), pitch_);
>  }
> diff --git a/src/cam/sdl_texture_mjpg.h b/src/cam/sdl_texture_mjpg.h
> index 5141ed73bf70..814ca79ac193 100644
> --- a/src/cam/sdl_texture_mjpg.h
> +++ b/src/cam/sdl_texture_mjpg.h
> @@ -14,7 +14,7 @@ class SDLTextureMJPG : public SDLTexture
>  public:
>  	SDLTextureMJPG(const SDL_Rect &rect);
>
> -	void update(libcamera::Span<const uint8_t> data) override;
> +	void update(const std::vector<libcamera::Span<const uint8_t>> &data) override;
>
>  private:
>  	int decompress(libcamera::Span<const uint8_t> data);
> diff --git a/src/cam/sdl_texture_yuv.cpp b/src/cam/sdl_texture_yuv.cpp
> index 07df4961a1ab..a5721182a68b 100644
> --- a/src/cam/sdl_texture_yuv.cpp
> +++ b/src/cam/sdl_texture_yuv.cpp
> @@ -14,7 +14,7 @@ SDLTextureYUYV::SDLTextureYUYV(const SDL_Rect &rect, unsigned int stride)
>  {
>  }
>
> -void SDLTextureYUYV::update(Span<const uint8_t> data)
> +void SDLTextureYUYV::update(const std::vector<libcamera::Span<const uint8_t>> &data)
>  {
> -	SDL_UpdateTexture(ptr_, &rect_, data.data(), pitch_);
> +	SDL_UpdateTexture(ptr_, &rect_, data[0].data(), pitch_);
>  }
> diff --git a/src/cam/sdl_texture_yuv.h b/src/cam/sdl_texture_yuv.h
> index 81e51381ec62..c9130298b91d 100644
> --- a/src/cam/sdl_texture_yuv.h
> +++ b/src/cam/sdl_texture_yuv.h
> @@ -13,5 +13,5 @@ class SDLTextureYUYV : public SDLTexture
>  {
>  public:
>  	SDLTextureYUYV(const SDL_Rect &rect, unsigned int stride);
> -	void update(libcamera::Span<const uint8_t> data) override;
> +	void update(const std::vector<libcamera::Span<const uint8_t>> &data) override;
>  };
> --
> Regards,
>
> Laurent Pinchart
>