| Message ID | 20250425104703.805170-5-barnabas.pocze@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Quoting Barnabás Pőcze (2025-04-25 11:47:03) > With the newly introduced `SDLTexture1Plane` it is easy to handle > any single-plane format that has an SDL equivalent. So use it for > more YUV and RGB formats. > > The mapping of RGB formats is not entirely straightforward because > `SDL_PIXELFORMAT_ZZZ...888...` defines a format where the order of > the components is endian dependent, while libcamera's `ZZZ...888...` > formats are derived from the matching DRM formats, and the RGB formats > in question are defined to be little-endian there. So the > endian-independent `SDL_PIXELFORMAT_{ZZZ24,ZZZZ32}` are used. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/apps/cam/sdl_sink.cpp | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/src/apps/cam/sdl_sink.cpp b/src/apps/cam/sdl_sink.cpp > index b295675dc..2edbb523d 100644 > --- a/src/apps/cam/sdl_sink.cpp > +++ b/src/apps/cam/sdl_sink.cpp > @@ -77,6 +77,42 @@ int SDLSink::configure(const libcamera::CameraConfiguration &config) > case libcamera::formats::YUYV: > texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_YUY2, cfg.stride); > break; > + case libcamera::formats::UYVY: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_UYVY, cfg.stride); > + break; > + case libcamera::formats::YVYU: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_YVYU, cfg.stride); > + break; > + case libcamera::formats::ARGB8888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_BGRA32, cfg.stride); > + break; > + case libcamera::formats::XRGB8888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_BGRX32, cfg.stride); > + break; > + case libcamera::formats::RGBA8888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_ABGR32, cfg.stride); > + break; > + case libcamera::formats::RGBX8888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_XBGR32, cfg.stride); > + break; > + case libcamera::formats::ABGR8888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_RGBA32, cfg.stride); > + break; > + case libcamera::formats::XBGR8888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_RGBX32, cfg.stride); > + break; > + case libcamera::formats::BGRA8888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_ARGB32, cfg.stride); > + break; > + case libcamera::formats::BGRX8888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_XRGB32, cfg.stride); > + break; > + case libcamera::formats::RGB888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_BGR24, cfg.stride); > + break; > + case libcamera::formats::BGR888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_RGB24, cfg.stride); > + break; all of those are in the ordering I would expect: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > default: > std::cerr << "Unsupported pixel format " > << cfg.pixelFormat.toString() << std::endl; > -- > 2.49.0 >
Hi Barnabás, Thank you for the patch. On Fri, Apr 25, 2025 at 12:47:03PM +0200, Barnabás Pőcze wrote: > With the newly introduced `SDLTexture1Plane` it is easy to handle > any single-plane format that has an SDL equivalent. So use it for > more YUV and RGB formats. > > The mapping of RGB formats is not entirely straightforward because > `SDL_PIXELFORMAT_ZZZ...888...` defines a format where the order of > the components is endian dependent, while libcamera's `ZZZ...888...` > formats are derived from the matching DRM formats, and the RGB formats > in question are defined to be little-endian there. So the > endian-independent `SDL_PIXELFORMAT_{ZZZ24,ZZZZ32}` are used. Format mapping is always painful :-( > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/apps/cam/sdl_sink.cpp | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/src/apps/cam/sdl_sink.cpp b/src/apps/cam/sdl_sink.cpp > index b295675dc..2edbb523d 100644 > --- a/src/apps/cam/sdl_sink.cpp > +++ b/src/apps/cam/sdl_sink.cpp > @@ -77,6 +77,42 @@ int SDLSink::configure(const libcamera::CameraConfiguration &config) > case libcamera::formats::YUYV: > texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_YUY2, cfg.stride); > break; > + case libcamera::formats::UYVY: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_UYVY, cfg.stride); > + break; > + case libcamera::formats::YVYU: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_YVYU, cfg.stride); > + break; > + case libcamera::formats::ARGB8888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_BGRA32, cfg.stride); > + break; > + case libcamera::formats::XRGB8888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_BGRX32, cfg.stride); > + break; > + case libcamera::formats::RGBA8888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_ABGR32, cfg.stride); > + break; > + case libcamera::formats::RGBX8888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_XBGR32, cfg.stride); > + break; > + case libcamera::formats::ABGR8888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_RGBA32, cfg.stride); > + break; > + case libcamera::formats::XBGR8888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_RGBX32, cfg.stride); > + break; > + case libcamera::formats::BGRA8888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_ARGB32, cfg.stride); > + break; > + case libcamera::formats::BGRX8888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_XRGB32, cfg.stride); > + break; > + case libcamera::formats::RGB888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_BGR24, cfg.stride); > + break; > + case libcamera::formats::BGR888: > + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_RGB24, cfg.stride); > + break; I'm tempted to avoid all the constructor calls by instead adding a sdlFormat variable and filling it in the switch, and then creating the texture after the switch. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > default: > std::cerr << "Unsupported pixel format " > << cfg.pixelFormat.toString() << std::endl;
Hi 2025. 04. 25. 14:45 keltezéssel, Laurent Pinchart írta: > Hi Barnabás, > > Thank you for the patch. > > On Fri, Apr 25, 2025 at 12:47:03PM +0200, Barnabás Pőcze wrote: >> With the newly introduced `SDLTexture1Plane` it is easy to handle >> any single-plane format that has an SDL equivalent. So use it for >> more YUV and RGB formats. >> >> The mapping of RGB formats is not entirely straightforward because >> `SDL_PIXELFORMAT_ZZZ...888...` defines a format where the order of >> the components is endian dependent, while libcamera's `ZZZ...888...` >> formats are derived from the matching DRM formats, and the RGB formats >> in question are defined to be little-endian there. So the >> endian-independent `SDL_PIXELFORMAT_{ZZZ24,ZZZZ32}` are used. > > Format mapping is always painful :-( > >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> src/apps/cam/sdl_sink.cpp | 36 ++++++++++++++++++++++++++++++++++++ >> 1 file changed, 36 insertions(+) >> >> diff --git a/src/apps/cam/sdl_sink.cpp b/src/apps/cam/sdl_sink.cpp >> index b295675dc..2edbb523d 100644 >> --- a/src/apps/cam/sdl_sink.cpp >> +++ b/src/apps/cam/sdl_sink.cpp >> @@ -77,6 +77,42 @@ int SDLSink::configure(const libcamera::CameraConfiguration &config) >> case libcamera::formats::YUYV: >> texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_YUY2, cfg.stride); >> break; >> + case libcamera::formats::UYVY: >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_UYVY, cfg.stride); >> + break; >> + case libcamera::formats::YVYU: >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_YVYU, cfg.stride); >> + break; >> + case libcamera::formats::ARGB8888: >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_BGRA32, cfg.stride); >> + break; >> + case libcamera::formats::XRGB8888: >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_BGRX32, cfg.stride); >> + break; >> + case libcamera::formats::RGBA8888: >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_ABGR32, cfg.stride); >> + break; >> + case libcamera::formats::RGBX8888: >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_XBGR32, cfg.stride); >> + break; >> + case libcamera::formats::ABGR8888: >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_RGBA32, cfg.stride); >> + break; >> + case libcamera::formats::XBGR8888: >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_RGBX32, cfg.stride); >> + break; >> + case libcamera::formats::BGRA8888: >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_ARGB32, cfg.stride); >> + break; >> + case libcamera::formats::BGRX8888: >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_XRGB32, cfg.stride); >> + break; >> + case libcamera::formats::RGB888: >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_BGR24, cfg.stride); >> + break; >> + case libcamera::formats::BGR888: >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_RGB24, cfg.stride); >> + break; > > I'm tempted to avoid all the constructor calls by instead adding a > sdlFormat variable and filling it in the switch, and then creating the > texture after the switch. Yes, indeed, and I had thought about that for a bit, but there is also SDLTextureNV12 (conditionally), and I didn't really see a good way to reconcile the two. Regards, Barnabás Pőcze > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> default: >> std::cerr << "Unsupported pixel format " >> << cfg.pixelFormat.toString() << std::endl; >
On Fri, Apr 25, 2025 at 02:58:08PM +0200, Barnabás Pőcze wrote: > Hi > > > 2025. 04. 25. 14:45 keltezéssel, Laurent Pinchart írta: > > Hi Barnabás, > > > > Thank you for the patch. > > > > On Fri, Apr 25, 2025 at 12:47:03PM +0200, Barnabás Pőcze wrote: > >> With the newly introduced `SDLTexture1Plane` it is easy to handle > >> any single-plane format that has an SDL equivalent. So use it for > >> more YUV and RGB formats. > >> > >> The mapping of RGB formats is not entirely straightforward because > >> `SDL_PIXELFORMAT_ZZZ...888...` defines a format where the order of > >> the components is endian dependent, while libcamera's `ZZZ...888...` > >> formats are derived from the matching DRM formats, and the RGB formats > >> in question are defined to be little-endian there. So the > >> endian-independent `SDL_PIXELFORMAT_{ZZZ24,ZZZZ32}` are used. > > > > Format mapping is always painful :-( > > > >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > >> --- > >> src/apps/cam/sdl_sink.cpp | 36 ++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 36 insertions(+) > >> > >> diff --git a/src/apps/cam/sdl_sink.cpp b/src/apps/cam/sdl_sink.cpp > >> index b295675dc..2edbb523d 100644 > >> --- a/src/apps/cam/sdl_sink.cpp > >> +++ b/src/apps/cam/sdl_sink.cpp > >> @@ -77,6 +77,42 @@ int SDLSink::configure(const libcamera::CameraConfiguration &config) > >> case libcamera::formats::YUYV: > >> texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_YUY2, cfg.stride); > >> break; > >> + case libcamera::formats::UYVY: > >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_UYVY, cfg.stride); > >> + break; > >> + case libcamera::formats::YVYU: > >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_YVYU, cfg.stride); > >> + break; > >> + case libcamera::formats::ARGB8888: > >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_BGRA32, cfg.stride); > >> + break; > >> + case libcamera::formats::XRGB8888: > >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_BGRX32, cfg.stride); > >> + break; > >> + case libcamera::formats::RGBA8888: > >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_ABGR32, cfg.stride); > >> + break; > >> + case libcamera::formats::RGBX8888: > >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_XBGR32, cfg.stride); > >> + break; > >> + case libcamera::formats::ABGR8888: > >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_RGBA32, cfg.stride); > >> + break; > >> + case libcamera::formats::XBGR8888: > >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_RGBX32, cfg.stride); > >> + break; > >> + case libcamera::formats::BGRA8888: > >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_ARGB32, cfg.stride); > >> + break; > >> + case libcamera::formats::BGRX8888: > >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_XRGB32, cfg.stride); > >> + break; > >> + case libcamera::formats::RGB888: > >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_BGR24, cfg.stride); > >> + break; > >> + case libcamera::formats::BGR888: > >> + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_RGB24, cfg.stride); > >> + break; > > > > I'm tempted to avoid all the constructor calls by instead adding a > > sdlFormat variable and filling it in the switch, and then creating the > > texture after the switch. > > Yes, indeed, and I had thought about that for a bit, but there is also > SDLTextureNV12 (conditionally), and I didn't really see a good way to > reconcile the two. switch (cfg.pixelFormat) { #ifdef HAVE_LIBJPEG case libcamera::formats::MJPEG: texture_ = std::make_unique<SDLTextureMJPG>(rect_); break; #endif #if SDL_VERSION_ATLEAST(2, 0, 16) case libcamera::formats::NV12: texture_ = std::make_unique<SDLTextureNV12>(rect_, cfg.stride); break; #endif case libcamera::formats::YUYV: sdlFormat = SDL_PIXELFORMAT_YUY2; break; case libcamera::formats::UYVY: sdlFormat = SDL_PIXELFORMAT_UYVY; break; ... default: std::cerr << "Unsupported pixel format " << cfg.pixelFormat.toString() << std::endl; return -EINVAL; }; if (sdlFormat) texture_ = std::make_unique<SDLTexture1Plane>(rect_, sdlFormat, cfg.stride); > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > >> default: > >> std::cerr << "Unsupported pixel format " > >> << cfg.pixelFormat.toString() << std::endl;
diff --git a/src/apps/cam/sdl_sink.cpp b/src/apps/cam/sdl_sink.cpp index b295675dc..2edbb523d 100644 --- a/src/apps/cam/sdl_sink.cpp +++ b/src/apps/cam/sdl_sink.cpp @@ -77,6 +77,42 @@ int SDLSink::configure(const libcamera::CameraConfiguration &config) case libcamera::formats::YUYV: texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_YUY2, cfg.stride); break; + case libcamera::formats::UYVY: + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_UYVY, cfg.stride); + break; + case libcamera::formats::YVYU: + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_YVYU, cfg.stride); + break; + case libcamera::formats::ARGB8888: + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_BGRA32, cfg.stride); + break; + case libcamera::formats::XRGB8888: + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_BGRX32, cfg.stride); + break; + case libcamera::formats::RGBA8888: + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_ABGR32, cfg.stride); + break; + case libcamera::formats::RGBX8888: + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_XBGR32, cfg.stride); + break; + case libcamera::formats::ABGR8888: + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_RGBA32, cfg.stride); + break; + case libcamera::formats::XBGR8888: + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_RGBX32, cfg.stride); + break; + case libcamera::formats::BGRA8888: + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_ARGB32, cfg.stride); + break; + case libcamera::formats::BGRX8888: + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_XRGB32, cfg.stride); + break; + case libcamera::formats::RGB888: + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_BGR24, cfg.stride); + break; + case libcamera::formats::BGR888: + texture_ = std::make_unique<SDLTexture1Plane>(rect_, SDL_PIXELFORMAT_RGB24, cfg.stride); + break; default: std::cerr << "Unsupported pixel format " << cfg.pixelFormat.toString() << std::endl;
With the newly introduced `SDLTexture1Plane` it is easy to handle any single-plane format that has an SDL equivalent. So use it for more YUV and RGB formats. The mapping of RGB formats is not entirely straightforward because `SDL_PIXELFORMAT_ZZZ...888...` defines a format where the order of the components is endian dependent, while libcamera's `ZZZ...888...` formats are derived from the matching DRM formats, and the RGB formats in question are defined to be little-endian there. So the endian-independent `SDL_PIXELFORMAT_{ZZZ24,ZZZZ32}` are used. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/apps/cam/sdl_sink.cpp | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)