[libcamera-devel] [RFC PATCH] libcamera: Add FrameFormat class
Hirokazu Honda
hiroh at chromium.org
Tue Aug 10 05:10:17 CEST 2021
Hi Laurent, thank you for the patch.
On Fri, Aug 6, 2021 at 6:48 PM Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Hi Laurent,
>
> On 05/08/2021 19:22, Laurent Pinchart wrote:
> > The FrameFormat class describes how a frame is stored in memory. It
> > groups a pixel format, size in pixels, and per-plane stride and size.
> >
> > This new class will be used in the configuration API rework, but could
> > already be put in use if desired to replace the various data structures
> > used to carry the same information.
>
> I think that second paragraph can go under the ---
>
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >
> > I've developed this as part of the configuration API rework, and
> > realized it could already be useful, in particular if we want to start
> > cleaning up support for multi-planar formats. I'm thus sending it as an
> > RFC.
>
> This looks like a good idea. I was considering what information such as
> pixel-format we might have to add to FrameBuffer to convey the
> information required, but we could equally keep it separate.
>
> >
> > ---
> > include/libcamera/format.h | 43 ++++++++++
> > include/libcamera/meson.build | 1 +
> > src/libcamera/format.cpp | 142 ++++++++++++++++++++++++++++++++++
> > src/libcamera/meson.build | 1 +
> > 4 files changed, 187 insertions(+)
> > create mode 100644 include/libcamera/format.h
> > create mode 100644 src/libcamera/format.cpp
> >
> > diff --git a/include/libcamera/format.h b/include/libcamera/format.h
> > new file mode 100644
> > index 000000000000..4982ab8230dc
> > --- /dev/null
> > +++ b/include/libcamera/format.h
> > @@ -0,0 +1,43 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2021, Google Inc.
> > + *
> > + * format.h - Frame Format
>
> Should this be named frame_format.h?
>
> Otherwise it might be confused the Stream format or others?
>
> > + */
> > +#ifndef __LIBCAMERA_FORMAT_H__
> > +#define __LIBCAMERA_FORMAT_H__
> > +
> > +#include <array>
> > +#include <cstddef>
> > +#include <string>
> > +
> > +#include <libcamera/geometry.h>
> > +#include <libcamera/pixel_format.h>
> > +
> > +namespace libcamera {
> > +
> > +class FrameFormat
> > +{
> > +public:
> > + static constexpr unsigned int kMaxPlanes = 4;
> > +
> > + struct Plane {
> > + std::size_t stride;
> > + std::size_t size;
> > + };
> > +
> > + FrameFormat();
> > + FrameFormat(PixelFormat format, const Size &size);
> > +
> > + std::size_t stride() const;
> > + std::size_t frameSize() const;
> > + std::string toString() const;
> > +
> > + PixelFormat format_;
> > + Size size_;
> > + std::array<Plane, kMaxPlanes> planes_;
> > +};
> > +
> > +} /* namespace libcamera */
> > +
> > +#endif /* __LIBCAMERA_FORMAT_H__ */
> > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> > index 5b25ef847ed4..dcfadb381874 100644
> > --- a/include/libcamera/meson.build
> > +++ b/include/libcamera/meson.build
> > @@ -6,6 +6,7 @@ libcamera_public_headers = files([
> > 'compiler.h',
> > 'controls.h',
> > 'file_descriptor.h',
> > + 'format.h',
> > 'framebuffer.h',
> > 'framebuffer_allocator.h',
> > 'geometry.h',
> > diff --git a/src/libcamera/format.cpp b/src/libcamera/format.cpp
> > new file mode 100644
> > index 000000000000..f77413781de1
> > --- /dev/null
> > +++ b/src/libcamera/format.cpp
> > @@ -0,0 +1,142 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2021, Google Inc.
> > + *
> > + * format.cpp - Frame Format
> > + */
> > +
> > +#include <libcamera/format.h>
> > +
> > +#include <numeric>
> > +
> > +/**
> > + * \file libcamera/format.h
> > + * \brief Frame Format
> > + */
> > +
> > +namespace libcamera {
> > +
> > +/**
> > + * \class FrameFormat
> > + * \brief Format of a frame as stored in memory
> > + *
> > + * The FrameFormat class fully describes how a frame is stored in memory. It
> > + * combines a pixel format, a size in pixels, and memory layout information for
> > + * planes.
> > + *
> > + * Frames are stored in memory in one or multiple planes, as specified by the
> > + * pixel format. A plane is a two-dimensional memory area organized as lines of
> > + * samples. A sample typically stores one or multiple components of a pixel (for
> > + * instance, the NV12 format uses two planes, with the first plane storing only
> > + * the luminance Y component of the pixels, and the second plane storing both
> > + * the chroma Cb and Cr components).
> > + *
> > + * Planes cover the whole frame. Their number of lines and sample per lines
> > + * correspond to the frame width and height in pixels, possibly divided by
> > + * horizontal and vertical subsampling factors as specified by the pixel
> > + * format.
> > + *
> > + * A plane may contain padding at the end of line, to support requirements of
> > + * applications and cameras, such as alignment constraints. This is specified
> > + * as a line stride value, equal to the distance in bytes between the first
> > + * sample of successive lines in memory, including pixel data and padding. All
> > + * lines in a plane have the same stride value, including the last line.
> > + * Additionally, padding may also occur at the end of the plane, specified as
> > + * the total plane size in bytes.
> > + *
> > + * \var FrameFormat::kMaxPlanes
> > + * \brief Maximum number of data planes for a frame
> > + *
> > + * \var FrameFormat::format_
> > + * \brief Frame pixel format, defines how pixel data is packed and stored in
> > + * memory
> > + *
> > + * \var FrameFormat::size_
> > + * \brief Frame size, defines the horizontal and vertical dimensions of the
> > + * frame in pixels (excluding any padding)
> > + *
> > + * \var FrameFormat::planes_
> > + * \brief Frame data planes, defines the memory layout for each data plane of
> > + * the frame
> > + *
> > + * Unused entries in the planes array shall be zero-initialized.
> > + */
> > +
> > +/**
> > + * \class FrameFormat::Plane
> > + * \brief Memory layout configuration for one data plane of a frame
> > + *
> > + * \var FrameFormat::Plane::stride
> > + * \brief Distance in bytes between the beginning of two successive lines
> > + *
> > + * If the strides of different planes can't be set separately, the stride of the
> > + * first plane takes precedence and is used by the camera.
> > + *
> > + * \var FrameFormat::Plane::size
> > + * \brief Maximum number of bytes required to store the plane
> > + *
Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
If there is some extra data between planes. Does the size cover the
extra data size?
That is, plene[0].size is possibly not the offset of plane[1]?
-Hiro
> > + * The \a size reports the number of bytes required to store data for plane.
> > + * For uncompressed formats, it takes into account the plane's stride, the
> > + * frame height, the plane's vertical subsampling and the overall alignment
> > + * constraints of the camera (for instance many cameras require buffer sizes
> > + * to be a multiple of the page size). For compressed formats, it indicates the
> > + * maximum size of the compressed data.
> > + */
> > +
> > +/**
> > + * \brief Construct a zero-initialized FrameFormat
> > + */
> > +FrameFormat::FrameFormat() = default;
> > +
> > +/**
> > + * \brief Construct a FrameFormat
> > + * \param[in] format The pixel format
> > + * \param[in] size The frame size in pixels
> > + */
> > +FrameFormat::FrameFormat(PixelFormat format, const Size &size)
> > + : format_(format), size_(size)
> > +{
> > +}
> > +
> > +/**
> > + * \brief Retrieve the frame stride in bytes
> > + *
> > + * The stride() function is a convenience helper to retrieve the stride of the
> > + * first plane. For multi-planar formats that have different stride values for
> > + * each plane, applications shall access the stride from the \ref planes_
> > + * instead.
> > + *
> > + * \return The frame stride in bytes
> > + */
> > +std::size_t FrameFormat::stride() const
> > +{
> > + return planes_[0].stride;
> > +}
> > +
>
> What about having the plane as a parameter, defaulting to 0.
>
> Then the access to reading the strides can be done through a const
> accessor rather than digging into the planes_ directly.
>
> With a similar accessor for size - could the Plane could be made private
> and populated during the constructor only?
>
> With those considered:
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
>
> > +/**
> > + * \brief Retrieve the full frame size
> > + *
> > + * The frameSize() function is a convenience helper to retrieve the full frame
> > + * size, defined as the sum of the sizes of all planes.
> > + *
> > + * \return The full frame size, in bytes
> > + */
> > +std::size_t FrameFormat::frameSize() const
> > +{
> > + return std::accumulate(planes_.begin(), planes_.end(), 0,
> > + [](std::size_t a, const Plane &b) {
> > + return a + b.size;
> > + });
> > +}
> > +
> > +/**
> > + * \brief Assemble and return a string describing the frame format
> > + *
> > + * \return A string describing the FrameFormat
> > + */
> > +std::string FrameFormat::toString() const
> > +{
> > + return size_.toString() + "-" + format_.toString();
> > +}
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index 4f08580157f9..7cf17ec2c628 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -15,6 +15,7 @@ libcamera_sources = files([
> > 'device_enumerator.cpp',
> > 'device_enumerator_sysfs.cpp',
> > 'file_descriptor.cpp',
> > + 'format.cpp',
> > 'formats.cpp',
> > 'framebuffer.cpp',
> > 'framebuffer_allocator.cpp',
> >
More information about the libcamera-devel
mailing list