[libcamera-devel] [PATCH] libcamera: PixelFormat: Define a FourCC output helper

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thu, Jan 30, 2020 at 09:38:09PM +0000, Kieran Bingham wrote:
> Provide a helper to print the FourCC string representation, and utilise
> it to improve the readability of the StreamConfiguration.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> This changes the output of the line:
> [159:34:26.262068766] [16497] INFO Camera camera.cpp:784 configuring streams: (0) 1920x1080-0x47504a4d
> 
> to read:
> [159:34:26.262068766] [16497] INFO Camera camera.cpp:784 configuring streams: (0) 1920x1080-0x47504a4d MJPG
> 
>  include/libcamera/pixelformats.h |  3 +++
>  src/libcamera/pixelformats.cpp   | 20 ++++++++++++++++++++
>  src/libcamera/stream.cpp         |  3 ++-
>  3 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/pixelformats.h b/include/libcamera/pixelformats.h
> index 6e25b8d8b76e..32cff083cdf9 100644
> --- a/include/libcamera/pixelformats.h
> +++ b/include/libcamera/pixelformats.h
> @@ -8,11 +8,14 @@
>  #define __LIBCAMERA_PIXEL_FORMATS_H__
>  
>  #include <stdint.h>
> +#include <string>
>  
>  namespace libcamera {
>  
>  using PixelFormat = uint32_t;
>  
> +std::string FourCC(const PixelFormat &p);
> +

I think we should turn PixelFormat into a struct or class, and add a
toString() method. Moving away from the uint32_t alias will also
disallow implicit conversions, which would have helped catch bugs in the
past, and should help avoiding them in the future.

>  } /* namespace libcamera */
>  
>  #endif /* __LIBCAMERA_PIXEL_FORMATS_H__ */
> diff --git a/src/libcamera/pixelformats.cpp b/src/libcamera/pixelformats.cpp
> index c03335400b70..d6b2c6a3dba5 100644
> --- a/src/libcamera/pixelformats.cpp
> +++ b/src/libcamera/pixelformats.cpp
> @@ -7,6 +7,8 @@
>  
>  #include <libcamera/pixelformats.h>
>  
> +#include <sstream>
> +
>  /**
>   * \file pixelformats.h
>   * \brief libcamera pixel formats
> @@ -25,4 +27,22 @@ namespace libcamera {
>   * \todo Add support for format modifiers
>   */
>  
> +/**
> + * \brief Return a PixelFormat as a FourCC string representation
> + */
> +std::string FourCC(const PixelFormat &p)
> +{
> +	std::stringstream ss;
> +
> +	ss << static_cast<char>(p & 0x7f)
> +	   << static_cast<char>((p >> 8) & 0x7f)
> +	   << static_cast<char>((p >> 16) & 0x7f)
> +	   << static_cast<char>((p >> 24) & 0x7f);
> +
> +	if (p & (1 << 31))
> +		ss << "-BE";
> +
> +	return ss.str();
> +}
> +
>  } /* namespace libcamera */
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index 13789e9eb344..4efe4385326f 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -348,7 +348,8 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)
>  std::string StreamConfiguration::toString() const
>  {
>  	std::stringstream ss;
> -	ss << size.toString() << "-" << utils::hex(pixelFormat);
> +	ss << size.toString() << "-" << utils::hex(pixelFormat)
> +	   << " " << FourCC(pixelFormat);
>  	return ss.str();
>  }
>  

-- 
Regards,

Laurent Pinchart