[libcamera-devel] [PATCH v2 1/3] libcamera: v4l2_videodevice: Zero-initialize planes in V4L2DeviceFormat
Niklas Söderlund
niklas.soderlund at ragnatech.se
Fri Nov 6 01:14:54 CET 2020
Hi Laurent,
Thanks for your work.
On 2020-11-04 09:48:39 +0200, Laurent Pinchart wrote:
> The V4L2DeviceFormat class doesn't have a default constructor, neither
> does it specifies default member initializers for the plane-related
> members. This results in the planes array and planesCount members being
> uninitialized by default, leading to undefined behaviour if the user of
> the class doesn't initialize it explicitly.
>
> Most users initialize V4L2DeviceFormat instances, but some don't. We
> could fix them, but that would likely turn into a game of whack-a-mole.
> As there's no use case for instantiating a large number of
> V4L2DeviceFormat instances in a performance-critical code path, let's
> instead add default initializers to avoid future issues.
>
> While at it, define a type of the structures containing plane
> information, and use an std::array.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> include/libcamera/internal/v4l2_videodevice.h | 13 ++++++++-----
> src/libcamera/v4l2_videodevice.cpp | 9 +++++++++
> 2 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> index 53f6a2d5515b..dcb9654a34b8 100644
> --- a/include/libcamera/internal/v4l2_videodevice.h
> +++ b/include/libcamera/internal/v4l2_videodevice.h
> @@ -7,6 +7,7 @@
> #ifndef __LIBCAMERA_INTERNAL_V4L2_VIDEODEVICE_H__
> #define __LIBCAMERA_INTERNAL_V4L2_VIDEODEVICE_H__
>
> +#include <array>
> #include <atomic>
> #include <memory>
> #include <stdint.h>
> @@ -153,14 +154,16 @@ private:
> class V4L2DeviceFormat
> {
> public:
> + struct Plane {
> + uint32_t size = 0;
> + uint32_t bpl = 0;
> + };
> +
> V4L2PixelFormat fourcc;
> Size size;
>
> - struct {
> - uint32_t size;
> - uint32_t bpl;
> - } planes[3];
> - unsigned int planesCount;
> + std::array<Plane, 3> planes;
> + unsigned int planesCount = 0;
>
> const std::string toString() const;
> };
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 36d7d9a0f27a..d07c3530eea5 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -351,6 +351,15 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const
> * V4L2DeviceFormat::planesCount value.
> */
>
> +/**
> + * \struct V4L2DeviceFormat::Plane
> + * \brief Per-plane memory size information
> + * \var V4L2DeviceFormat::Plane::size
> + * \brief The plane total memory size (in bytes)
> + * \var V4L2DeviceFormat::Plane::bpl
> + * \brief The plane line stride (in bytes)
> + */
> +
> /**
> * \var V4L2DeviceFormat::size
> * \brief The image size in pixels
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list