[libcamera-devel] libcamera: v4l2_pixelformat: Implement std::hash specialization

16922 diff mbox series

Inject a specialization of std::hash<> for the V4L2PixelFormat class in
the std namespace to make it possible to store instances of the class in
the std::unordered_map and std::unordered_set containers.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/internal/v4l2_pixelformat.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Hi LAurent,

On Tue, Aug 02, 2022 at 10:47:05PM +0300, Laurent Pinchart wrote:
> Inject a specialization of std::hash<> for the V4L2PixelFormat class in
> the std namespace to make it possible to store instances of the class in
> the std::unordered_map and std::unordered_set containers.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I started with the same, then I thought polluting the std namespace
was not a good idea ? Do you think it's not an issue ?

> ---
>  include/libcamera/internal/v4l2_pixelformat.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/include/libcamera/internal/v4l2_pixelformat.h b/include/libcamera/internal/v4l2_pixelformat.h
> index d5400f90a67e..fcc1dadd4de0 100644
> --- a/include/libcamera/internal/v4l2_pixelformat.h
> +++ b/include/libcamera/internal/v4l2_pixelformat.h
> @@ -8,6 +8,7 @@
>
>  #pragma once
>
> +#include <functional>
>  #include <ostream>
>  #include <stdint.h>
>  #include <string>
> @@ -55,3 +56,15 @@ private:
>  std::ostream &operator<<(std::ostream &out, const V4L2PixelFormat &f);
>
>  } /* namespace libcamera */
> +
> +namespace std {
> +
> +template<>
> +struct hash<libcamera::V4L2PixelFormat> {
> +	size_t operator()(libcamera::V4L2PixelFormat const &format) const noexcept
> +	{
> +		return hash<uint32_t>{}(format.fourcc());

iirc just "return format.fourcc();" was enough

> +	}
> +};
> +
> +} /* namespace std */
> --
> Regards,
>
> Laurent Pinchart
>

Hi Jacopo,

On Wed, Aug 03, 2022 at 08:52:08AM +0200, Jacopo Mondi wrote:
> On Tue, Aug 02, 2022 at 10:47:05PM +0300, Laurent Pinchart wrote:
> > Inject a specialization of std::hash<> for the V4L2PixelFormat class in
> > the std namespace to make it possible to store instances of the class in
> > the std::unordered_map and std::unordered_set containers.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> I started with the same, then I thought polluting the std namespace
> was not a good idea ? Do you think it's not an issue ?

In general it's a bad idea to add custom code to the std namespace, but
for hash specializations, that's specifically allowed and recommended.

> > ---
> >  include/libcamera/internal/v4l2_pixelformat.h | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/include/libcamera/internal/v4l2_pixelformat.h b/include/libcamera/internal/v4l2_pixelformat.h
> > index d5400f90a67e..fcc1dadd4de0 100644
> > --- a/include/libcamera/internal/v4l2_pixelformat.h
> > +++ b/include/libcamera/internal/v4l2_pixelformat.h
> > @@ -8,6 +8,7 @@
> >
> >  #pragma once
> >
> > +#include <functional>
> >  #include <ostream>
> >  #include <stdint.h>
> >  #include <string>
> > @@ -55,3 +56,15 @@ private:
> >  std::ostream &operator<<(std::ostream &out, const V4L2PixelFormat &f);
> >
> >  } /* namespace libcamera */
> > +
> > +namespace std {
> > +
> > +template<>
> > +struct hash<libcamera::V4L2PixelFormat> {
> > +	size_t operator()(libcamera::V4L2PixelFormat const &format) const noexcept
> > +	{
> > +		return hash<uint32_t>{}(format.fourcc());
> 
> iirc just "return format.fourcc();" was enough

That would match the Hash requirement
(https://en.cppreference.com/w/cpp/named_req/Hash) indeed:

    h(k) -> std::size_t

    The returned value depends only on the value of k for the duration of
    the program.

    All evaluations of h(k) executed within a given execution of a program
    yield the same result for the same value of k.

    The probability of h(a)==h(b) for a!=b should approach
    1.0/std::numeric_limits<std::size_t>::max().

and that's actually how std::hash<unsigned int> is implemented in libc++
and libstdc++. I went for hash<uint32_t>{}(format.fourcc()) to use the
same hash mechanism as in the C++ library without having to check how
it's implemented. All this will be inlined by the compiler anyway.

> > +	}
> > +};
> > +
> > +} /* namespace std */