[libcamera-devel] [PATCH] cam: sdl: Use uint32_t in place of SDL_PixelFormatEnum

[Laurent Pinchart]

On Fri, Jul 15, 2022 at 09:22:15AM +0100, Kieran Bingham via libcamera-devel wrote:
> Quoting Jacopo Mondi via libcamera-devel (2022-07-14 19:24:23)
> > The SDL_PixelFormatEnum type has been introduced in libsdl by
> > 1a4c0d4e17e6 ("Fixed bug 4377 - SDL_PIXELFORMAT enum is anonymous, which
> > prevents its use in a templated function") which is only available after
> > release 2.0.10 of the library.
> > 
> > Debian 10 ships libsdl at version 2.0.9 and building cam with sdl
> > support there fails with error:
> > ./src/cam/sdl_texture.h:27:8: error: ‘SDL_PixelFormatEnum’ does not name
> > a type; did you mean ‘SDL_PixelFormat’?
> > 
> > Fix that by using the base type uint32_t in place of
> > SDL_PixelFormatEnum.
> 
> Is there scope to fix this by checking the version of the header being
> compiled and defining SDL_PixelFormatEnum instead?
> 
> I would expect something in src/cam/sdl_texture.h such as:
> 
> #if !SDL_VERSION_ATLEAST(2.0.10)
> typedef uint32_t SDL_PixelFormatEnum;
> #endif

It could be done. It won't make any difference at compilation time as
SDL_PixelFormatEnum is a plain C enum, the compiler will thus not catch
SDLTexture::SDLTexture() being called with a wrong value, but it may
make it clearer to the person reading the code what type of pixel format
is expected. I don't have any strong opinion either way.

> > Reported-by: https://buildbot.libcamera.org/#/builders/6/builds/355
> > Fixes: 11554a259f4e ("cam: sdl_sink: Add SDL sink with initial YUYV support")
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/cam/sdl_texture.cpp | 2 +-
> >  src/cam/sdl_texture.h   | 4 ++--
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/cam/sdl_texture.cpp b/src/cam/sdl_texture.cpp
> > index 2ca2add2f00c..02a8ff28b669 100644
> > --- a/src/cam/sdl_texture.cpp
> > +++ b/src/cam/sdl_texture.cpp
> > @@ -9,7 +9,7 @@
> > 
> >  #include <iostream>
> > 
> > -SDLTexture::SDLTexture(const SDL_Rect &rect, SDL_PixelFormatEnum pixelFormat,
> > +SDLTexture::SDLTexture(const SDL_Rect &rect, uint32_t pixelFormat,
> >                        const int pitch)
> >         : ptr_(nullptr), rect_(rect), pixelFormat_(pixelFormat), pitch_(pitch)
> >  {
> > diff --git a/src/cam/sdl_texture.h b/src/cam/sdl_texture.h
> > index 9097479846f7..2275b4e605d9 100644
> > --- a/src/cam/sdl_texture.h
> > +++ b/src/cam/sdl_texture.h
> > @@ -14,7 +14,7 @@
> >  class SDLTexture
> >  {
> >  public:
> > -       SDLTexture(const SDL_Rect &rect, SDL_PixelFormatEnum pixelFormat,
> > +       SDLTexture(const SDL_Rect &rect, uint32_t pixelFormat,
> >                    const int pitch);
> >         virtual ~SDLTexture();
> >         int create(SDL_Renderer *renderer);
> > @@ -24,6 +24,6 @@ public:
> >  protected:
> >         SDL_Texture *ptr_;
> >         const SDL_Rect rect_;
> > -       const SDL_PixelFormatEnum pixelFormat_;
> > +       const uint32_t pixelFormat_;
> >         const int pitch_;
> >  };
> > --
> > 2.36.1
> >

-- 
Regards,

Laurent Pinchart