[PATCH v2 3/4] apps: cam: sdl_texture: Add `SDLTexture1Plane`

[Barnabás Pőcze]

Hi


2025. 04. 25. 14:41 keltezéssel, Laurent Pinchart írta:
> Hi Barnabás,
> 
> Thank you for the patch.
> 
> On Fri, Apr 25, 2025 at 12:47:02PM +0200, Barnabás Pőcze wrote:
>> `SDLTextureYUYV` uses `SDL_PIXELFORMAT_YUY2`, which is a single plane
>> format. To support other single plane formats, replace `SDLTextureYUYV`
>> with `SDLTexture1Plane` that can be instantiated with an arbitrary SDL
>> pixel format and that uses `SDL_UpdateTexture()` to update the texture
>> using exactly a single plane.
>>
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
>> ---
>>   src/apps/cam/sdl_sink.cpp         |  3 ++-
>>   src/apps/cam/sdl_texture_1plane.h | 18 ++++++++++++++++++
>>   src/apps/cam/sdl_texture_yuv.cpp  | 10 ----------
>>   src/apps/cam/sdl_texture_yuv.h    |  7 -------
>>   4 files changed, 20 insertions(+), 18 deletions(-)
>>   create mode 100644 src/apps/cam/sdl_texture_1plane.h
>>
>> diff --git a/src/apps/cam/sdl_sink.cpp b/src/apps/cam/sdl_sink.cpp
>> index 8355dd5ed..b295675dc 100644
>> --- a/src/apps/cam/sdl_sink.cpp
>> +++ b/src/apps/cam/sdl_sink.cpp
>> @@ -22,6 +22,7 @@
>>   #include "../common/event_loop.h"
>>   #include "../common/image.h"
>>   
>> +#include "sdl_texture_1plane.h"
>>   #ifdef HAVE_LIBJPEG
>>   #include "sdl_texture_mjpg.h"
>>   #endif
>> @@ -74,7 +75,7 @@ int SDLSink::configure(const libcamera::CameraConfiguration &config)
>>   		break;
>>   #endif
>>   	case libcamera::formats::YUYV:
>> -		texture_ = std::make_unique<SDLTextureYUYV>(rect_, cfg.stride);
>> +		texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_YUY2, cfg.stride);
>>   		break;
>>   	default:
>>   		std::cerr << "Unsupported pixel format "
>> 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_);
>> +	}
> 
> Do we need to inline this function, can't it go to a .cpp file ?

It could certainly be in a cpp file, I can move it if you want me to. I just kept it here
for simplicity and because it is only used in a single cpp file.


Regards,
Barnabás Pőcze

> 
> With that handled,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
>> +};
>> diff --git a/src/apps/cam/sdl_texture_yuv.cpp b/src/apps/cam/sdl_texture_yuv.cpp
>> index 7e2ce3f49..bed297d28 100644
>> --- a/src/apps/cam/sdl_texture_yuv.cpp
>> +++ b/src/apps/cam/sdl_texture_yuv.cpp
>> @@ -21,13 +21,3 @@ void SDLTextureNV12::update(libcamera::Span<const libcamera::Span<const uint8_t>
>>   			    data[1].data(), stride_);
>>   }
>>   #endif
>> -
>> -SDLTextureYUYV::SDLTextureYUYV(const SDL_Rect &rect, unsigned int stride)
>> -	: SDLTexture(rect, SDL_PIXELFORMAT_YUY2, stride)
>> -{
>> -}
>> -
>> -void SDLTextureYUYV::update(libcamera::Span<const libcamera::Span<const uint8_t>> data)
>> -{
>> -	SDL_UpdateTexture(ptr_, nullptr, data[0].data(), stride_);
>> -}
>> diff --git a/src/apps/cam/sdl_texture_yuv.h b/src/apps/cam/sdl_texture_yuv.h
>> index db877f503..c271f901b 100644
>> --- a/src/apps/cam/sdl_texture_yuv.h
>> +++ b/src/apps/cam/sdl_texture_yuv.h
>> @@ -17,10 +17,3 @@ public:
>>   	void update(libcamera::Span<const libcamera::Span<const uint8_t>> data) override;
>>   };
>>   #endif
>> -
>> -class SDLTextureYUYV : public SDLTexture
>> -{
>> -public:
>> -	SDLTextureYUYV(const SDL_Rect &rect, unsigned int stride);
>> -	void update(libcamera::Span<const libcamera::Span<const uint8_t>> data) override;
>> -};
>