[RFC PATCH v1 2/5] apps: cam: sdl_texture: Add single plane generic texture

[Laurent Pinchart]

Hi Barnabás,

On Thu, Apr 24, 2025 at 01:29:54PM +0200, Barnabás Pőcze wrote:
> 2025. 04. 23. 1:40 keltezéssel, Laurent Pinchart írta:
> > On Mon, Apr 21, 2025 at 05:51:06PM +0200, Barnabás Pőcze wrote:
> >> Add the `SDLTexture1Plane` type that can be instantiated with
> >> an arbitrary SDL pixel format and that uses `SDL_UpdateTexture()`
> >> to update the texture using exactly a single plane.
> > 
> > You can squash patches 2/5, 3/5 and 4/5 together. The commit message can
> > be reworded to explain how SDLTextureYUYV is generalized for later use
> > with other 1-plane formats.
> > 
> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
> >> ---
> >>   src/apps/cam/sdl_texture_1plane.h | 18 ++++++++++++++++++
> >>   1 file changed, 18 insertions(+)
> >>   create mode 100644 src/apps/cam/sdl_texture_1plane.h
> >>
> >> diff --git a/src/apps/cam/sdl_texture_1plane.h b/src/apps/cam/sdl_texture_1plane.h
> >> new file mode 100644
> >> index 000000000..ded35c589
> >> --- /dev/null
> >> +++ b/src/apps/cam/sdl_texture_1plane.h
> >> @@ -0,0 +1,18 @@
> >> +#pragma once
> >> +
> >> +#include <assert.h>
> >> +
> >> +#include "sdl_texture.h"
> >> +
> >> +class SDLTexture1Plane final : public SDLTexture
> >> +{
> >> +public:
> >> +	using SDLTexture::SDLTexture;
> >> +
> >> +	void update(libcamera::Span<const libcamera::Span<const uint8_t>> data) override
> >> +	{
> >> +		assert(data.size() == 1);
> >> +		assert(data[0].size_bytes() == std::size_t(rect_.h * stride_));
> >> +		SDL_UpdateTexture(ptr_, nullptr, data[0].data(), stride_);
> > 
> > No need to pass &rect_ as the second argument, like done by
> > SDLTextureYUYV ?
> 
> As far as I can see it is only required if only a part of the texture is to be updated,
> and if it is `nullptr, then SDL updates the full size of the texture, which is what
> is wanted here.
> 
>    https://github.com/libsdl-org/SDL/blob/3343cb21473ea454261bc4187393aaec9cc3f28f/src/render/SDL_render.c#L2153

Indeed. It seems it was a bug to pass &rect_ to the SDL_UpdateTexture()
call. It causes no issue in practice as rect_ always covers the full
size of the image in the current code.

Could you drop the rect argument in a separate patch ? It should also be
dropped from SDL_UpdateNVTexture(), which this series doesn't touch.

> >> +	}
> >> +};

-- 
Regards,

Laurent Pinchart