[1/2] libcamera: v4l2: Implement the equality and inequality operators for V4L2DeviceFormat
diff mbox series

Message ID 20241009200142.3213065-2-chenghaoyang@chromium.org
State New
Headers show
Series
  • Add V4L2DeviceFormat support
Related show

Commit Message

Cheng-Hao Yang Oct. 9, 2024, 7:58 p.m. UTC 
From: Yunke Cao <yunkec@chromium.org>

The comparators allow us to compare format directly instead of
comparing each field.

Signed-off-by: Yunke Cao <yunkec@chromium.org>
Co-developed-by: Han-Lin Chen <hanlinchen@chromium.org>
Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
---
 include/libcamera/internal/v4l2_videodevice.h |  6 +++++
 src/libcamera/v4l2_videodevice.cpp            | 27 +++++++++++++++++++
 2 files changed, 33 insertions(+)

Comments

Kieran Bingham Oct. 9, 2024, 11:17 p.m. UTC | #1 
Hi Harvey,

Quoting Harvey Yang (2024-10-09 20:58:50)
> From: Yunke Cao <yunkec@chromium.org>
> 
> The comparators allow us to compare format directly instead of
> comparing each field.
> 

I haven't seen how this will get used yet, but it seems reasonable on
it's own.

Perhaps add some unit tests to test/v4l2_videodevice/formats.cpp ?

> Signed-off-by: Yunke Cao <yunkec@chromium.org>
> Co-developed-by: Han-Lin Chen <hanlinchen@chromium.org>
> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> ---
>  include/libcamera/internal/v4l2_videodevice.h |  6 +++++
>  src/libcamera/v4l2_videodevice.cpp            | 27 +++++++++++++++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> index f021c2a01..9f53e37cd 100644
> --- a/include/libcamera/internal/v4l2_videodevice.h
> +++ b/include/libcamera/internal/v4l2_videodevice.h
> @@ -182,6 +182,12 @@ public:
>         const std::string toString() const;
>  };
>  
> +bool operator==(const V4L2DeviceFormat &lhs, const V4L2DeviceFormat &rhs);
> +static inline bool operator!=(const V4L2DeviceFormat &lhs, const V4L2DeviceFormat &rhs)
> +{
> +       return !(lhs == rhs);
> +}
> +
>  std::ostream &operator<<(std::ostream &out, const V4L2DeviceFormat &f);
>  
>  class V4L2VideoDevice : public V4L2Device
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 14eba0561..1110fb535 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -437,6 +437,33 @@ const std::string V4L2DeviceFormat::toString() const
>         return ss.str();
>  }
>  
> +/**
> + * \brief Compare V4L2DeviceFormat for equality
> + * \return True if the two formats are identical, false otherwise
> + */
> +bool operator==(const V4L2DeviceFormat &lhs, const V4L2DeviceFormat &rhs)
> +{
> +       if (!(lhs.fourcc == rhs.fourcc &&
> +             lhs.size == rhs.size &&
> +             lhs.colorSpace == rhs.colorSpace &&
> +             lhs.planesCount == rhs.planesCount))
> +               return false;
> +
> +       for (unsigned int i = 0; i < lhs.planesCount; ++i) {
> +               if (lhs.planes[i].size != rhs.planes[i].size ||
> +                   lhs.planes[i].bpl != rhs.planes[i].bpl)
> +                       return false;
> +       }
> +
> +       return true;
> +}
> +
> +/**
> + * \fn bool operator!=(const V4L2DeviceFormat &lhs, const V4L2DeviceFormat &rhs)
> + * \brief Comparetwo formats for inequality

s/Comparetwo/Compare two/

> + * \return True if the two formats are not identical, false otherwise
> + */
> +
>  /**
>   * \brief Insert a text representation of a V4L2DeviceFormat into an output
>   * stream
> -- 
> 2.47.0.rc0.187.ge670bccf7e-goog
>
Cheng-Hao Yang Oct. 11, 2024, 4:37 p.m. UTC | #2 
Hi Kieran,

On Thu, Oct 10, 2024 at 7:17 AM Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Hi Harvey,
>
> Quoting Harvey Yang (2024-10-09 20:58:50)
> > From: Yunke Cao <yunkec@chromium.org>
> >
> > The comparators allow us to compare format directly instead of
> > comparing each field.
> >
>
> I haven't seen how this will get used yet, but it seems reasonable on
> it's own.

It'll be used here:
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/mtkisp7/src/libcamera/pipeline/mtkisp7/imgsys/imgsys.cpp;l=202

>
> Perhaps add some unit tests to test/v4l2_videodevice/formats.cpp ?

Added in the next version, while I think I did that pretty ugly... Please
help instruct me how to do that properly :)))

To prevent the spam, let me provide the version on gitlab for now:
https://gitlab.freedesktop.org/chenghaoyang/libcamera/-/commit/465ce9ce1e14526144a13ef57530310fa9764609

>
> > Signed-off-by: Yunke Cao <yunkec@chromium.org>
> > Co-developed-by: Han-Lin Chen <hanlinchen@chromium.org>
> > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> > ---
> >  include/libcamera/internal/v4l2_videodevice.h |  6 +++++
> >  src/libcamera/v4l2_videodevice.cpp            | 27 +++++++++++++++++++
> >  2 files changed, 33 insertions(+)
> >
> > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> > index f021c2a01..9f53e37cd 100644
> > --- a/include/libcamera/internal/v4l2_videodevice.h
> > +++ b/include/libcamera/internal/v4l2_videodevice.h
> > @@ -182,6 +182,12 @@ public:
> >         const std::string toString() const;
> >  };
> >
> > +bool operator==(const V4L2DeviceFormat &lhs, const V4L2DeviceFormat &rhs);
> > +static inline bool operator!=(const V4L2DeviceFormat &lhs, const V4L2DeviceFormat &rhs)
> > +{
> > +       return !(lhs == rhs);
> > +}
> > +
> >  std::ostream &operator<<(std::ostream &out, const V4L2DeviceFormat &f);
> >
> >  class V4L2VideoDevice : public V4L2Device
> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > index 14eba0561..1110fb535 100644
> > --- a/src/libcamera/v4l2_videodevice.cpp
> > +++ b/src/libcamera/v4l2_videodevice.cpp
> > @@ -437,6 +437,33 @@ const std::string V4L2DeviceFormat::toString() const
> >         return ss.str();
> >  }
> >
> > +/**
> > + * \brief Compare V4L2DeviceFormat for equality
> > + * \return True if the two formats are identical, false otherwise
> > + */
> > +bool operator==(const V4L2DeviceFormat &lhs, const V4L2DeviceFormat &rhs)
> > +{
> > +       if (!(lhs.fourcc == rhs.fourcc &&
> > +             lhs.size == rhs.size &&
> > +             lhs.colorSpace == rhs.colorSpace &&
> > +             lhs.planesCount == rhs.planesCount))
> > +               return false;
> > +
> > +       for (unsigned int i = 0; i < lhs.planesCount; ++i) {
> > +               if (lhs.planes[i].size != rhs.planes[i].size ||
> > +                   lhs.planes[i].bpl != rhs.planes[i].bpl)
> > +                       return false;
> > +       }
> > +
> > +       return true;
> > +}
> > +
> > +/**
> > + * \fn bool operator!=(const V4L2DeviceFormat &lhs, const V4L2DeviceFormat &rhs)
> > + * \brief Comparetwo formats for inequality
>
> s/Comparetwo/Compare two/

Done

BR,
Harvey


>
> > + * \return True if the two formats are not identical, false otherwise
> > + */
> > +
> >  /**
> >   * \brief Insert a text representation of a V4L2DeviceFormat into an output
> >   * stream
> > --
> > 2.47.0.rc0.187.ge670bccf7e-goog
> >

Patch
diff mbox series 

diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
index f021c2a01..9f53e37cd 100644
--- a/include/libcamera/internal/v4l2_videodevice.h
+++ b/include/libcamera/internal/v4l2_videodevice.h
@@ -182,6 +182,12 @@  public:
 	const std::string toString() const;
 };
 
+bool operator==(const V4L2DeviceFormat &lhs, const V4L2DeviceFormat &rhs);
+static inline bool operator!=(const V4L2DeviceFormat &lhs, const V4L2DeviceFormat &rhs)
+{
+	return !(lhs == rhs);
+}
+
 std::ostream &operator<<(std::ostream &out, const V4L2DeviceFormat &f);
 
 class V4L2VideoDevice : public V4L2Device
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index 14eba0561..1110fb535 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -437,6 +437,33 @@  const std::string V4L2DeviceFormat::toString() const
 	return ss.str();
 }
 
+/**
+ * \brief Compare V4L2DeviceFormat for equality
+ * \return True if the two formats are identical, false otherwise
+ */
+bool operator==(const V4L2DeviceFormat &lhs, const V4L2DeviceFormat &rhs)
+{
+	if (!(lhs.fourcc == rhs.fourcc &&
+	      lhs.size == rhs.size &&
+	      lhs.colorSpace == rhs.colorSpace &&
+	      lhs.planesCount == rhs.planesCount))
+		return false;
+
+	for (unsigned int i = 0; i < lhs.planesCount; ++i) {
+		if (lhs.planes[i].size != rhs.planes[i].size ||
+		    lhs.planes[i].bpl != rhs.planes[i].bpl)
+			return false;
+	}
+
+	return true;
+}
+
+/**
+ * \fn bool operator!=(const V4L2DeviceFormat &lhs, const V4L2DeviceFormat &rhs)
+ * \brief Comparetwo formats for inequality
+ * \return True if the two formats are not identical, false otherwise
+ */
+
 /**
  * \brief Insert a text representation of a V4L2DeviceFormat into an output
  * stream