[libcamera-devel,PATCH/RFC,04/11] libcamera: pixel_format: Make PixelFormat usable as a constexpr
diff mbox series

Message ID 20200522145459.16836-6-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • Introduce formats:: namespace for libcamera pixel formats
Related show

Commit Message

Laurent Pinchart May 22, 2020, 2:54 p.m. UTC 
The PixelFormat class is a lightweight wrapper around a 32-bit FourCC
and a 64-bit modifier. Make is usable as a constexpr.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/pixel_format.h | 19 +++++++++++++------
 src/libcamera/pixel_format.cpp   | 10 ++--------
 2 files changed, 15 insertions(+), 14 deletions(-)

Comments

Kieran Bingham May 28, 2020, 9:42 a.m. UTC | #1 
Hi Laurent,

On 22/05/2020 15:54, Laurent Pinchart wrote:
> The PixelFormat class is a lightweight wrapper around a 32-bit FourCC
> and a 64-bit modifier. Make is usable as a constexpr.
> 

Excellent, I tried to use a switch statement with the PixelFormat and
hit this - I used pixelformat.fourcc (or such), and switched on the DRM
fourcc in the end to work around, but this patch would make things much
cleaner I think!

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/libcamera/pixel_format.h | 19 +++++++++++++------
>  src/libcamera/pixel_format.cpp   | 10 ++--------
>  2 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/include/libcamera/pixel_format.h b/include/libcamera/pixel_format.h
> index e3b371ef92bb..8dfeb8922418 100644
> --- a/include/libcamera/pixel_format.h
> +++ b/include/libcamera/pixel_format.h
> @@ -18,18 +18,25 @@ namespace libcamera {
>  class PixelFormat
>  {
>  public:
> -	PixelFormat();
> -	explicit PixelFormat(uint32_t fourcc, uint64_t modifier = 0);
> +	constexpr PixelFormat()
> +		: fourcc_(0), modifier_(0)
> +	{
> +	}
> +
> +	explicit constexpr PixelFormat(uint32_t fourcc, uint64_t modifier = 0)
> +		: fourcc_(fourcc), modifier_(modifier)
> +	{
> +	}
>  
>  	bool operator==(const PixelFormat &other) const;
>  	bool operator!=(const PixelFormat &other) const { return !(*this == other); }
>  	bool operator<(const PixelFormat &other) const;
>  
> -	bool isValid() const { return fourcc_ != 0; }
> +	constexpr bool isValid() const { return fourcc_ != 0; }
>  
> -	operator uint32_t() const { return fourcc_; }
> -	uint32_t fourcc() const { return fourcc_; }
> -	uint64_t modifier() const { return modifier_; }
> +	constexpr operator uint32_t() const { return fourcc_; }
> +	constexpr uint32_t fourcc() const { return fourcc_; }
> +	constexpr uint64_t modifier() const { return modifier_; }
>  
>  	std::string toString() const;
>  
> diff --git a/src/libcamera/pixel_format.cpp b/src/libcamera/pixel_format.cpp
> index d8718739152d..d501c5f09c6b 100644
> --- a/src/libcamera/pixel_format.cpp
> +++ b/src/libcamera/pixel_format.cpp
> @@ -25,25 +25,19 @@ namespace libcamera {
>   */
>  
>  /**
> + * \fn PixelFormat::PixelFormat()
>   * \brief Construct a PixelFormat with an invalid format
>   *
>   * PixelFormat instances constructed with the default constructor are
>   * invalid, calling the isValid() function returns false.
>   */
> -PixelFormat::PixelFormat()
> -	: fourcc_(0)
> -{
> -}
>  
>  /**
> + * \fn PixelFormat::PixelFormat(uint32_t fourcc, uint64_t modifier)
>   * \brief Construct a PixelFormat from a DRM FourCC and a modifier
>   * \param[in] fourcc A DRM FourCC
>   * \param[in] modifier A DRM FourCC modifier
>   */
> -PixelFormat::PixelFormat(uint32_t fourcc, uint64_t modifier)
> -	: fourcc_(fourcc), modifier_(modifier)
> -{
> -}
>  
>  /**
>   * \brief Compare pixel formats for equality
>
Niklas Söderlund June 5, 2020, 8:05 p.m. UTC | #2 
Hi Laurent,

Thanks for your patch.

On 2020-05-22 17:54:52 +0300, Laurent Pinchart wrote:
> The PixelFormat class is a lightweight wrapper around a 32-bit FourCC
> and a 64-bit modifier. Make is usable as a constexpr.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  include/libcamera/pixel_format.h | 19 +++++++++++++------
>  src/libcamera/pixel_format.cpp   | 10 ++--------
>  2 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/include/libcamera/pixel_format.h b/include/libcamera/pixel_format.h
> index e3b371ef92bb..8dfeb8922418 100644
> --- a/include/libcamera/pixel_format.h
> +++ b/include/libcamera/pixel_format.h
> @@ -18,18 +18,25 @@ namespace libcamera {
>  class PixelFormat
>  {
>  public:
> -	PixelFormat();
> -	explicit PixelFormat(uint32_t fourcc, uint64_t modifier = 0);
> +	constexpr PixelFormat()
> +		: fourcc_(0), modifier_(0)
> +	{
> +	}
> +
> +	explicit constexpr PixelFormat(uint32_t fourcc, uint64_t modifier = 0)
> +		: fourcc_(fourcc), modifier_(modifier)
> +	{
> +	}
>  
>  	bool operator==(const PixelFormat &other) const;
>  	bool operator!=(const PixelFormat &other) const { return !(*this == other); }
>  	bool operator<(const PixelFormat &other) const;
>  
> -	bool isValid() const { return fourcc_ != 0; }
> +	constexpr bool isValid() const { return fourcc_ != 0; }
>  
> -	operator uint32_t() const { return fourcc_; }
> -	uint32_t fourcc() const { return fourcc_; }
> -	uint64_t modifier() const { return modifier_; }
> +	constexpr operator uint32_t() const { return fourcc_; }
> +	constexpr uint32_t fourcc() const { return fourcc_; }
> +	constexpr uint64_t modifier() const { return modifier_; }
>  
>  	std::string toString() const;
>  
> diff --git a/src/libcamera/pixel_format.cpp b/src/libcamera/pixel_format.cpp
> index d8718739152d..d501c5f09c6b 100644
> --- a/src/libcamera/pixel_format.cpp
> +++ b/src/libcamera/pixel_format.cpp
> @@ -25,25 +25,19 @@ namespace libcamera {
>   */
>  
>  /**
> + * \fn PixelFormat::PixelFormat()
>   * \brief Construct a PixelFormat with an invalid format
>   *
>   * PixelFormat instances constructed with the default constructor are
>   * invalid, calling the isValid() function returns false.
>   */
> -PixelFormat::PixelFormat()
> -	: fourcc_(0)
> -{
> -}
>  
>  /**
> + * \fn PixelFormat::PixelFormat(uint32_t fourcc, uint64_t modifier)
>   * \brief Construct a PixelFormat from a DRM FourCC and a modifier
>   * \param[in] fourcc A DRM FourCC
>   * \param[in] modifier A DRM FourCC modifier
>   */
> -PixelFormat::PixelFormat(uint32_t fourcc, uint64_t modifier)
> -	: fourcc_(fourcc), modifier_(modifier)
> -{
> -}
>  
>  /**
>   * \brief Compare pixel formats for equality
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/include/libcamera/pixel_format.h b/include/libcamera/pixel_format.h
index e3b371ef92bb..8dfeb8922418 100644
--- a/include/libcamera/pixel_format.h
+++ b/include/libcamera/pixel_format.h
@@ -18,18 +18,25 @@  namespace libcamera {
 class PixelFormat
 {
 public:
-	PixelFormat();
-	explicit PixelFormat(uint32_t fourcc, uint64_t modifier = 0);
+	constexpr PixelFormat()
+		: fourcc_(0), modifier_(0)
+	{
+	}
+
+	explicit constexpr PixelFormat(uint32_t fourcc, uint64_t modifier = 0)
+		: fourcc_(fourcc), modifier_(modifier)
+	{
+	}
 
 	bool operator==(const PixelFormat &other) const;
 	bool operator!=(const PixelFormat &other) const { return !(*this == other); }
 	bool operator<(const PixelFormat &other) const;
 
-	bool isValid() const { return fourcc_ != 0; }
+	constexpr bool isValid() const { return fourcc_ != 0; }
 
-	operator uint32_t() const { return fourcc_; }
-	uint32_t fourcc() const { return fourcc_; }
-	uint64_t modifier() const { return modifier_; }
+	constexpr operator uint32_t() const { return fourcc_; }
+	constexpr uint32_t fourcc() const { return fourcc_; }
+	constexpr uint64_t modifier() const { return modifier_; }
 
 	std::string toString() const;
 
diff --git a/src/libcamera/pixel_format.cpp b/src/libcamera/pixel_format.cpp
index d8718739152d..d501c5f09c6b 100644
--- a/src/libcamera/pixel_format.cpp
+++ b/src/libcamera/pixel_format.cpp
@@ -25,25 +25,19 @@  namespace libcamera {
  */
 
 /**
+ * \fn PixelFormat::PixelFormat()
  * \brief Construct a PixelFormat with an invalid format
  *
  * PixelFormat instances constructed with the default constructor are
  * invalid, calling the isValid() function returns false.
  */
-PixelFormat::PixelFormat()
-	: fourcc_(0)
-{
-}
 
 /**
+ * \fn PixelFormat::PixelFormat(uint32_t fourcc, uint64_t modifier)
  * \brief Construct a PixelFormat from a DRM FourCC and a modifier
  * \param[in] fourcc A DRM FourCC
  * \param[in] modifier A DRM FourCC modifier
  */
-PixelFormat::PixelFormat(uint32_t fourcc, uint64_t modifier)
-	: fourcc_(fourcc), modifier_(modifier)
-{
-}
 
 /**
  * \brief Compare pixel formats for equality