[libcamera-devel] [PATCH v3 4/9] libcamera: bayer_format: Add PixelFormat conversion helpers to BayerFormat class

[Kieran Bingham]

Quoting Naushir Patuck (2021-10-27 10:27:58)
> Add BayerFormat::toPixelFormat() and BayerFormat::fromPixelFormat() helper
> functions to convert between BayerFormat and PixelFormat types.
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  include/libcamera/internal/bayer_format.h |  4 ++++
>  src/libcamera/bayer_format.cpp            | 29 +++++++++++++++++++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h
> index 723382d4168d..ee96b9a86707 100644
> --- a/include/libcamera/internal/bayer_format.h
> +++ b/include/libcamera/internal/bayer_format.h
> @@ -10,6 +10,8 @@
>  #include <stdint.h>
>  #include <string>
>  
> +#include <libcamera/pixel_format.h>
> +
>  #include "libcamera/internal/v4l2_pixelformat.h"
>  
>  namespace libcamera {
> @@ -50,6 +52,8 @@ public:
>  
>         V4L2PixelFormat toV4L2PixelFormat() const;
>         static BayerFormat fromV4L2PixelFormat(V4L2PixelFormat v4l2Format);
> +       PixelFormat toPixelFormat() const;
> +       static BayerFormat fromPixelFormat(PixelFormat format);
>         BayerFormat transform(Transform t) const;
>  
>         Order order;
> diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp
> index 94e2294d7f6c..1edd238829ce 100644
> --- a/src/libcamera/bayer_format.cpp
> +++ b/src/libcamera/bayer_format.cpp
> @@ -305,6 +305,35 @@ BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format)
>         return BayerFormat();
>  }
>  
> +/**
> + * \brief Convert a BayerFormat into the corresponding PixelFormat
> + * \return The PixelFormat corresponding to this BayerFormat
> + */
> +PixelFormat BayerFormat::toPixelFormat() const
> +{
> +       const auto it = bayerToFormat.find(*this);
> +       if (it != bayerToFormat.end())
> +               return it->second.pixelFormat;
> +
> +       return PixelFormat();
> +}
> +
> +/**
> + * \brief Convert a PixelFormat into the corresponding BayerFormat
> + * \return The BayerFormat corresponding to this PixelFormat
> + */
> +BayerFormat BayerFormat::fromPixelFormat(PixelFormat format)
> +{
> +       auto it = std::find_if(bayerToFormat.begin(), bayerToFormat.end(),
> +                              [format](const auto &i) {
> +                                      return i.second.pixelFormat == format;
> +                              });
> +       if (it == bayerToFormat.end())
> +               return BayerFormat{};
> +
> +       return it->first;

I'm curious why you have used two styles in the two functions.

BayerFormat::toPixelFormat is 
  if (it != end)
     return it;
  return Empty;

while BayerFormat::fromPixelFormat is
  if (it == end)
     return Empty;

  return it;

If there's no technical reason for this, I think keeping them both using
the same pattern would be more consistent, but ... they are both
functional, so I don't see this as a blocker. (Personally I'd use the
BayerFormat::toPixelFormat style on both)

So either way:

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

> +}
> +
>  /**
>   * \brief Apply a transform to this BayerFormat
>   * \param[in] t The transform to apply
> -- 
> 2.25.1
>