[libcamera-devel] [RFC PATCH v1 03/12] libcamera: formats: Add planeSize() helpers to PixelFormatInfo
Hirokazu Honda
hiroh at chromium.org
Fri Sep 3 12:35:12 CEST 2021
Hi Laurent, thank you for the patch.
On Thu, Sep 2, 2021 at 5:49 PM Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> On 02/09/2021 05:22, Laurent Pinchart wrote:
> > Add two helpers functions to the PixelFormatInfo class to compute the
> > byte size of a given plane, taking the frame size, the stride, the
> > alignement constraints and the vertical subsampling into account.
>
> s/alignement/alignment/
>
>
> > Use the new functions through the code base to replace manual
> > implementations.
>
>
> \o/ that makes me happy.
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > include/libcamera/internal/formats.h | 4 ++
> > src/android/mm/generic_camera_buffer.cpp | 11 +---
> > src/android/yuv/post_processor_yuv.cpp | 10 ++-
> > src/libcamera/formats.cpp | 82 ++++++++++++++++++++----
> > src/libcamera/v4l2_videodevice.cpp | 6 +-
> > 5 files changed, 79 insertions(+), 34 deletions(-)
> >
> > diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h
> > index 51a8a6b8b0ae..8f314e99889b 100644
> > --- a/include/libcamera/internal/formats.h
> > +++ b/include/libcamera/internal/formats.h
> > @@ -42,6 +42,10 @@ public:
> >
> > unsigned int stride(unsigned int width, unsigned int plane,
> > unsigned int align = 1) const;
> > + unsigned int planeSize(const Size &size, unsigned int plane,
> > + unsigned int align = 1) const;
> > + unsigned int planeSize(unsigned int height, unsigned int plane,
> > + unsigned int stride) const;
> > unsigned int frameSize(const Size &size, unsigned int align = 1) const;
> > unsigned int frameSize(const Size &size,
> > const std::array<unsigned int, 3> &strides) const;
> > diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp
> > index 22efc4d4b13a..93aa5821e470 100644
> > --- a/src/android/mm/generic_camera_buffer.cpp
> > +++ b/src/android/mm/generic_camera_buffer.cpp
> > @@ -108,16 +108,9 @@ CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer,
> >
> > unsigned int offset = 0;
> > for (unsigned int i = 0; i < numPlanes; ++i) {
> > - /*
> > - * \todo Remove if this plane size computation function is
> > - * added to PixelFormatInfo.
> > - */
> > - const unsigned int vertSubSample = info.planes[i].verticalSubSampling;
> > - const unsigned int stride = info.stride(size.width, i, 1u);
> > - const unsigned int planeSize =
> > - stride * ((size.height + vertSubSample - 1) / vertSubSample);
> > + const unsigned int planeSize = info.planeSize(size, i);
> >
> > - planeInfo_[i].stride = stride;
> > + planeInfo_[i].stride = info.stride(size.width, i, 1u);
> > planeInfo_[i].offset = offset;
> > planeInfo_[i].size = planeSize;
> >
> > diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
> > index 6952fc38b0ef..7b3b49609cb1 100644
> > --- a/src/android/yuv/post_processor_yuv.cpp
> > +++ b/src/android/yuv/post_processor_yuv.cpp
> > @@ -134,11 +134,9 @@ void PostProcessorYuv::calculateLengths(const StreamConfiguration &inCfg,
> > sourceStride_[i] = inCfg.stride;
> > destinationStride_[i] = nv12Info.stride(destinationSize_.width, i, 1);
> >
> > - const unsigned int vertSubSample =
> > - nv12Info.planes[i].verticalSubSampling;
> > - sourceLength_[i] = sourceStride_[i] *
> > - ((sourceSize_.height + vertSubSample - 1) / vertSubSample);
> > - destinationLength_[i] = destinationStride_[i] *
> > - ((destinationSize_.height + vertSubSample - 1) / vertSubSample);
> > + sourceLength_[i] = nv12Info.planeSize(sourceSize_.height, i,
> > + sourceStride_[i]);
> > + destinationLength_[i] = nv12Info.planeSize(destinationSize_.height, i,
> > + destinationStride_[i]);
> > }
> > }
> > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> > index 603d88619fe0..76be93bc1c5c 100644
> > --- a/src/libcamera/formats.cpp
> > +++ b/src/libcamera/formats.cpp
> > @@ -11,6 +11,7 @@
> > #include <errno.h>
> >
> > #include <libcamera/base/log.h>
> > +#include <libcamera/base/utils.h>
> >
> > #include <libcamera/formats.h>
> >
> > @@ -801,32 +802,85 @@ unsigned int PixelFormatInfo::stride(unsigned int width, unsigned int plane,
> > }
> >
> > /**
> > - * \brief Compute the number of bytes necessary to store a frame
> > + * \brief Compute the number of bytes necessary to store a plane of a frame
> > * \param[in] size The size of the frame, in pixels
> > + * \param[in] plane The plane index
> > * \param[in] align The stride alignment, in bytes (1 for default alignment)
> > *
> > - * The frame is computed by adding the product of the line stride and the frame
> > - * height for all planes, taking subsampling and other format characteristics
> > - * into account. Additional stride alignment constraints may be specified
> > - * through the \a align parameter, and will apply to all planes. For more
> > - * complex stride constraints, use the frameSize() overloaded version that takes
> > - * an array of stride values.
> > + * The plane size is computed by multiplying the line stride and the frame
> > + * height, taking subsampling and other format characteristics into account.
> > + * Stride alignment constraints may be specified through the \a align parameter.
> > *
> > * \sa stride()
> > *
> > + * \return The number of bytes necessary to store the plane, or 0 if the
> > + * PixelFormatInfo instance is not valid or the plane number isn't valid for the
> > + * format
> > + */
> > +unsigned int PixelFormatInfo::planeSize(const Size &size, unsigned int plane,
> > + unsigned int align) const
> > +{
> > + unsigned int stride = PixelFormatInfo::stride(size.width, plane, align);
> > + if (!stride)
> > + return 0;
> > +
The below can be replaced with
return planeSize(size.height, plane, stride);
Nice clean up!
Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
-Hiro
> > + unsigned int vertSubSample = planes[plane].verticalSubSampling;
> > + if (!vertSubSample)
> > + return 0;
> > +
> > + /* stride * ceil(height / verticalSubSampling) */
> > + return stride * ((size.height + vertSubSample - 1) / vertSubSample);
> > +}
> > +
> > +/**
> > + * \brief Compute the number of bytes necessary to store a plane of a frame
> > + * \param[in] height The height of the frame, in pixels
> > + * \param[in] plane The plane index
> > + * \param[in] stride The plane stride, in bytes
> > + *
> > + * The plane size is computed by multiplying the line stride and the frame
> > + * height, taking subsampling and other format characteristics into account.
> > + * Stride alignment constraints may be specified through the \a align parameter.
> > + *
> > + * \return The number of bytes necessary to store the plane, or 0 if the
> > + * PixelFormatInfo instance is not valid or the plane number isn't valid for the
> > + * format
> > + */
> > +unsigned int PixelFormatInfo::planeSize(unsigned int height, unsigned int plane,
> > + unsigned int stride) const
> > +{
> > + unsigned int vertSubSample = planes[plane].verticalSubSampling;
> > + if (!vertSubSample)
> > + return 0;
> > +
> > + /* stride * ceil(height / verticalSubSampling) */
> > + return stride * ((height + vertSubSample - 1) / vertSubSample);
> > +}
> > +
> > +/**
> > + * \brief Compute the number of bytes necessary to store a frame
> > + * \param[in] size The size of the frame, in pixels
> > + * \param[in] align The stride alignment, in bytes (1 for default alignment)
> > + *
> > + * The frame size is computed by adding the size of all planes, as computed by
> > + * planeSize(), using the specified alignment constraints for all planes. For
> > + * more complex stride constraints, use the frameSize() overloaded version that
> > + * takes an array of stride values.
> > + *
> > + * \sa planeSize()
> > + *
> > * \return The number of bytes necessary to store the frame, or 0 if the
> > * PixelFormatInfo instance is not valid
> > */
> > unsigned int PixelFormatInfo::frameSize(const Size &size, unsigned int align) const
> > {
> > - /* stride * ceil(height / verticalSubSampling) */
> > unsigned int sum = 0;
> > - for (unsigned int i = 0; i < 3; i++) {
> > - unsigned int vertSubSample = planes[i].verticalSubSampling;
> > - if (!vertSubSample)
> > - continue;
> > - sum += stride(size.width, i, align)
> > - * ((size.height + vertSubSample - 1) / vertSubSample);
> > +
> > + for (const auto &[i, plane] : utils::enumerate(planes)) {
> > + if (plane.bytesPerGroup == 0)
> > + break;
> > +
> > + sum += planeSize(size, i, align);
> > }
> >
> > return sum;
> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > index 4e1c2b7cef5e..adabd4720668 100644
> > --- a/src/libcamera/v4l2_videodevice.cpp
> > +++ b/src/libcamera/v4l2_videodevice.cpp
> > @@ -1337,11 +1337,7 @@ std::unique_ptr<FrameBuffer> V4L2VideoDevice::createBuffer(unsigned int index)
> > planes[i].offset = offset;
> >
> > /* \todo Take the V4L2 stride into account */
> > - const unsigned int vertSubSample =
> > - info.planes[i].verticalSubSampling;
> > - planes[i].length =
> > - info.stride(format_.size.width, i, 1u) *
> > - ((format_.size.height + vertSubSample - 1) / vertSubSample);
> > + planes[i].length = info.planeSize(format_.size, i);
> > offset += planes[i].length;
> > }
> > }
> >
More information about the libcamera-devel
mailing list