| Message ID | 20200708134417.67747-7-paul.elder@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, Thank you for the patch. On Wed, Jul 08, 2020 at 10:44:02PM +0900, Paul Elder wrote: > Add member functions to PixelFormatInfo for calculating stride and frame > size. This will simplify existing code that calculates these things. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > Changes in v4: > - add overloaded frameSize() that takes array of strides > - add optional parameter to stride() for byte alignment > > Changes in v3: > - rename functions to stride and frameSize, from bytesPerLine and > imageSize, respectively > > Changes in v2: > - make these functions const > - add documentation > - inline DIV_ROUND_UP > --- > include/libcamera/internal/formats.h | 6 +++ > src/libcamera/formats.cpp | 73 ++++++++++++++++++++++++++++ > 2 files changed, 79 insertions(+) > > diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h > index 054be37..e347a46 100644 > --- a/include/libcamera/internal/formats.h > +++ b/include/libcamera/internal/formats.h > @@ -52,6 +52,12 @@ public: > static const PixelFormatInfo &info(const PixelFormat &format); > static const PixelFormatInfo &info(const V4L2PixelFormat &format); > > + unsigned int stride(unsigned int width, unsigned int plane, > + unsigned int align = 0) const; > + unsigned int frameSize(const Size &size) const; > + unsigned int frameSize(const Size &size, > + const std::array<unsigned int, 3> &strides) const; > + > /* \todo Add support for non-contiguous memory planes */ > const char *name; > PixelFormat format; > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp > index 6d96055..c355e57 100644 > --- a/src/libcamera/formats.cpp > +++ b/src/libcamera/formats.cpp > @@ -732,4 +732,77 @@ const PixelFormatInfo &PixelFormatInfo::info(const V4L2PixelFormat &format) > return info->second; > } > > +/** > + * \brief Compute the stride > + * \param[in] width The width of the line, in pixels > + * \param[in] plane The index of the plane whose stride is to be computed > + * \param[in] align The number of bytes to align to (0 for default alignment) > + * \return The number of bytes necessary to store a line, or 0 if the > + * PixelFormatInfo instance or the \a plane is not valid > + */ > +unsigned int PixelFormatInfo::stride(unsigned int width, unsigned int plane, > + unsigned int align) const > +{ > + if (!isValid()) > + return 0; > + > + if (plane > planes.size() || !planes[plane].bytesPerGroup) > + return 0; > + > + /* ceil(width / pixelsPerGroup) * bytesPerGroup */ > + unsigned int stride = (width + pixelsPerGroup - 1) / pixelsPerGroup > + * planes[plane].bytesPerGroup; * should be under = > + > + if (!align) > + return stride; I wonder if we should default align to 1 instead of 0 and drop this. > + > + /* ceil(stride / align) * align */ > + return (stride + align - 1) / align * align; > +} > + > +/** > + * \brief Compute the bytes necessary to store the frame s/the bytes/the number of bytes/ s/the frame/a frame/ Same for the next function > + * \param[in] size The size of the frame, in pixels > + * \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) const Would it be useful to add an align parameter to this function, that would default to 0 (or 1 depending on the outcome of the discussion above) ? That should cover most pipeline handlers, with the next function being for really odd cases. > +{ > + /* 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) > + * ((size.height + vertSubSample - 1) / vertSubSample); > + } > + > + return sum; > +} > + > +/** > + * \brief Compute the bytes necessary to store the frame > + * \param[in] size The size of the frame, in pixels > + * \param[in] strides The strides to use for each plane * * This function is an overloaded version that takes custom strides for each * plane, to be used when the device has custom alignment constraints that * can't be described by just an alignment value. * ? Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + * \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, > + const std::array<unsigned int, 3> &strides) 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 += strides[i] > + * ((size.height + vertSubSample - 1) / vertSubSample); > + } > + > + return sum; > +} > + > } /* namespace libcamera */
Hi Paul, On Wed, Jul 08, 2020 at 10:44:02PM +0900, Paul Elder wrote: > Add member functions to PixelFormatInfo for calculating stride and frame > size. This will simplify existing code that calculates these things. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > Changes in v4: > - add overloaded frameSize() that takes array of strides > - add optional parameter to stride() for byte alignment > > Changes in v3: > - rename functions to stride and frameSize, from bytesPerLine and > imageSize, respectively > > Changes in v2: > - make these functions const > - add documentation > - inline DIV_ROUND_UP > --- > include/libcamera/internal/formats.h | 6 +++ > src/libcamera/formats.cpp | 73 ++++++++++++++++++++++++++++ > 2 files changed, 79 insertions(+) > > diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h > index 054be37..e347a46 100644 > --- a/include/libcamera/internal/formats.h > +++ b/include/libcamera/internal/formats.h > @@ -52,6 +52,12 @@ public: > static const PixelFormatInfo &info(const PixelFormat &format); > static const PixelFormatInfo &info(const V4L2PixelFormat &format); > > + unsigned int stride(unsigned int width, unsigned int plane, > + unsigned int align = 0) const; > + unsigned int frameSize(const Size &size) const; > + unsigned int frameSize(const Size &size, > + const std::array<unsigned int, 3> &strides) const; > + > /* \todo Add support for non-contiguous memory planes */ > const char *name; > PixelFormat format; > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp > index 6d96055..c355e57 100644 > --- a/src/libcamera/formats.cpp > +++ b/src/libcamera/formats.cpp > @@ -732,4 +732,77 @@ const PixelFormatInfo &PixelFormatInfo::info(const V4L2PixelFormat &format) > return info->second; > } > > +/** > + * \brief Compute the stride > + * \param[in] width The width of the line, in pixels > + * \param[in] plane The index of the plane whose stride is to be computed s/is/has ? (if this wasn't intentional, your English is slightly better than mine so it might be correct :) > + * \param[in] align The number of bytes to align to (0 for default alignment) I would then make this a default parameter > + * \return The number of bytes necessary to store a line, or 0 if the How is likely be used stride by applications ? Could it be used as divisor ? silent divisios by 0 are nasty to debug, is it worth an Info message ? Alternatively you could return an int with an error code, but I wonder how many would actually check that, so an error message might be better. > + * PixelFormatInfo instance or the \a plane is not valid > + */ > +unsigned int PixelFormatInfo::stride(unsigned int width, unsigned int plane, > + unsigned int align) const ^ allignment > +{ > + if (!isValid()) > + return 0; > + > + if (plane > planes.size() || !planes[plane].bytesPerGroup) > + return 0; > + > + /* ceil(width / pixelsPerGroup) * bytesPerGroup */ > + unsigned int stride = (width + pixelsPerGroup - 1) / pixelsPerGroup > + * planes[plane].bytesPerGroup; > + > + if (!align) > + return stride; yes, maybe a default parameter is better, and maybe > + /* ceil(stride / align) * align */ return !align ? stride : (stride + align - 1) / align * align; > + return (stride + align - 1) / align * align; > +} > + > +/** > + * \brief Compute the bytes necessary to store the frame > + * \param[in] size The size of the frame, in pixels > + * \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) 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) > + * ((size.height + vertSubSample - 1) / vertSubSample); > + } > + > + return sum; > +} > + > +/** > + * \brief Compute the bytes necessary to store the frame > + * \param[in] size The size of the frame, in pixels > + * \param[in] strides The strides to use for each plane > + * \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, > + const std::array<unsigned int, 3> &strides) const ^ alignment (doesn't checkstyle complains?) > +{ > + /* 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 += strides[i] > + * ((size.height + vertSubSample - 1) / vertSubSample); If you make of the stride stride a parameter defaulted to and empty vector you could merge the two functions here. Just unsigned int s = ((size.height + vertSubSample - 1)/ vertSubSample); if (!strides.empty()) s *= stride(size.width, i); else s *= strides[i]; sum += s; or something similar The rest of the function seems identical to me. > + } > + > + return sum; > +} > + > } /* namespace libcamera */ > -- > 2.27.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Laurent, Thank you for the review. On Wed, Jul 08, 2020 at 05:52:10PM +0300, Laurent Pinchart wrote: > Hi Paul, > > Thank you for the patch. > > On Wed, Jul 08, 2020 at 10:44:02PM +0900, Paul Elder wrote: > > Add member functions to PixelFormatInfo for calculating stride and frame > > size. This will simplify existing code that calculates these things. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > > --- > > Changes in v4: > > - add overloaded frameSize() that takes array of strides > > - add optional parameter to stride() for byte alignment > > > > Changes in v3: > > - rename functions to stride and frameSize, from bytesPerLine and > > imageSize, respectively > > > > Changes in v2: > > - make these functions const > > - add documentation > > - inline DIV_ROUND_UP > > --- > > include/libcamera/internal/formats.h | 6 +++ > > src/libcamera/formats.cpp | 73 ++++++++++++++++++++++++++++ > > 2 files changed, 79 insertions(+) > > > > diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h > > index 054be37..e347a46 100644 > > --- a/include/libcamera/internal/formats.h > > +++ b/include/libcamera/internal/formats.h > > @@ -52,6 +52,12 @@ public: > > static const PixelFormatInfo &info(const PixelFormat &format); > > static const PixelFormatInfo &info(const V4L2PixelFormat &format); > > > > + unsigned int stride(unsigned int width, unsigned int plane, > > + unsigned int align = 0) const; > > + unsigned int frameSize(const Size &size) const; > > + unsigned int frameSize(const Size &size, > > + const std::array<unsigned int, 3> &strides) const; > > + > > /* \todo Add support for non-contiguous memory planes */ > > const char *name; > > PixelFormat format; > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp > > index 6d96055..c355e57 100644 > > --- a/src/libcamera/formats.cpp > > +++ b/src/libcamera/formats.cpp > > @@ -732,4 +732,77 @@ const PixelFormatInfo &PixelFormatInfo::info(const V4L2PixelFormat &format) > > return info->second; > > } > > > > +/** > > + * \brief Compute the stride > > + * \param[in] width The width of the line, in pixels > > + * \param[in] plane The index of the plane whose stride is to be computed > > + * \param[in] align The number of bytes to align to (0 for default alignment) > > + * \return The number of bytes necessary to store a line, or 0 if the > > + * PixelFormatInfo instance or the \a plane is not valid > > + */ > > +unsigned int PixelFormatInfo::stride(unsigned int width, unsigned int plane, > > + unsigned int align) const > > +{ > > + if (!isValid()) > > + return 0; > > + > > + if (plane > planes.size() || !planes[plane].bytesPerGroup) > > + return 0; > > + > > + /* ceil(width / pixelsPerGroup) * bytesPerGroup */ > > + unsigned int stride = (width + pixelsPerGroup - 1) / pixelsPerGroup > > + * planes[plane].bytesPerGroup; > > * should be under = > > > + > > + if (!align) > > + return stride; > > I wonder if we should default align to 1 instead of 0 and drop this. Well at first I thought that align to 1 bytes doesn't make sense, until I realized that stride must be a multiple of align, so 1 does actually make sense, both semantically and mathematically. So yeah, I'll change it to 1. > > + > > + /* ceil(stride / align) * align */ > > + return (stride + align - 1) / align * align; > > +} > > + > > +/** > > + * \brief Compute the bytes necessary to store the frame > > s/the bytes/the number of bytes/ > s/the frame/a frame/ > > Same for the next function > > > + * \param[in] size The size of the frame, in pixels > > + * \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) const > > Would it be useful to add an align parameter to this function, that > would default to 0 (or 1 depending on the outcome of the discussion > above) ? That should cover most pipeline handlers, with the next > function being for really odd cases. Yes, I think it would be useful. > > +{ > > + /* 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) > > + * ((size.height + vertSubSample - 1) / vertSubSample); > > + } > > + > > + return sum; > > +} > > + > > +/** > > + * \brief Compute the bytes necessary to store the frame > > + * \param[in] size The size of the frame, in pixels > > + * \param[in] strides The strides to use for each plane > > * > * This function is an overloaded version that takes custom strides for each > * plane, to be used when the device has custom alignment constraints that > * can't be described by just an alignment value. > * > > ? > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > + * \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, > > + const std::array<unsigned int, 3> &strides) 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 += strides[i] > > + * ((size.height + vertSubSample - 1) / vertSubSample); > > + } > > + > > + return sum; > > +} > > + > > } /* namespace libcamera */ Thanks, Paul
Hi Jacopo, Thank you for the review. On Wed, Jul 08, 2020 at 11:10:45PM +0200, Jacopo Mondi wrote: > Hi Paul, > > On Wed, Jul 08, 2020 at 10:44:02PM +0900, Paul Elder wrote: > > Add member functions to PixelFormatInfo for calculating stride and frame > > size. This will simplify existing code that calculates these things. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > > --- > > Changes in v4: > > - add overloaded frameSize() that takes array of strides > > - add optional parameter to stride() for byte alignment > > > > Changes in v3: > > - rename functions to stride and frameSize, from bytesPerLine and > > imageSize, respectively > > > > Changes in v2: > > - make these functions const > > - add documentation > > - inline DIV_ROUND_UP > > --- > > include/libcamera/internal/formats.h | 6 +++ > > src/libcamera/formats.cpp | 73 ++++++++++++++++++++++++++++ > > 2 files changed, 79 insertions(+) > > > > diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h > > index 054be37..e347a46 100644 > > --- a/include/libcamera/internal/formats.h > > +++ b/include/libcamera/internal/formats.h > > @@ -52,6 +52,12 @@ public: > > static const PixelFormatInfo &info(const PixelFormat &format); > > static const PixelFormatInfo &info(const V4L2PixelFormat &format); > > > > + unsigned int stride(unsigned int width, unsigned int plane, > > + unsigned int align = 0) const; > > + unsigned int frameSize(const Size &size) const; > > + unsigned int frameSize(const Size &size, > > + const std::array<unsigned int, 3> &strides) const; > > + > > /* \todo Add support for non-contiguous memory planes */ > > const char *name; > > PixelFormat format; > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp > > index 6d96055..c355e57 100644 > > --- a/src/libcamera/formats.cpp > > +++ b/src/libcamera/formats.cpp > > @@ -732,4 +732,77 @@ const PixelFormatInfo &PixelFormatInfo::info(const V4L2PixelFormat &format) > > return info->second; > > } > > > > +/** > > + * \brief Compute the stride > > + * \param[in] width The width of the line, in pixels > > + * \param[in] plane The index of the plane whose stride is to be computed > > s/is/has ? (if this wasn't intentional, your English is slightly > better than mine so it might be correct :) This is fine as-is :) We /at the moment/ want to compute the stride of a plane, so we can refer to the stride in present tense. > > + * \param[in] align The number of bytes to align to (0 for default alignment) > > I would then make this a default parameter It is, in the header. > > + * \return The number of bytes necessary to store a line, or 0 if the > > How is likely be used stride by applications ? Could it be used as > divisor ? silent divisios by 0 are nasty to debug, is it worth an Info > message ? Alternatively you could return an int with an error code, > but I wonder how many would actually check that, so an error message > might be better. Yeah, that's true. I'll add an Info message at least, then. We return unsigned int for stride (as does struct v4l2_format.fmt.pix.bytesperline), so we can't really return negative error code. > > + * PixelFormatInfo instance or the \a plane is not valid > > + */ > > +unsigned int PixelFormatInfo::stride(unsigned int width, unsigned int plane, > > + unsigned int align) const > ^ allignment This, and the other alignments that you point out, are fine. Maybe something happened in the mail client :/ > > +{ > > + if (!isValid()) > > + return 0; > > + > > + if (plane > planes.size() || !planes[plane].bytesPerGroup) > > + return 0; > > + > > + /* ceil(width / pixelsPerGroup) * bytesPerGroup */ > > + unsigned int stride = (width + pixelsPerGroup - 1) / pixelsPerGroup > > + * planes[plane].bytesPerGroup; > > + > > + if (!align) > > + return stride; > > yes, maybe a default parameter is better, and maybe > > + > /* ceil(stride / align) * align */ > return !align ? stride > : (stride + align - 1) / align * align; After making the default alignment 1 instead of 0, the branch won't be necessary anymore. > > + return (stride + align - 1) / align * align; > > +} > > + > > +/** > > + * \brief Compute the bytes necessary to store the frame > > + * \param[in] size The size of the frame, in pixels > > + * \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) 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) > > + * ((size.height + vertSubSample - 1) / vertSubSample); > > + } > > + > > + return sum; > > +} > > + > > +/** > > + * \brief Compute the bytes necessary to store the frame > > + * \param[in] size The size of the frame, in pixels > > + * \param[in] strides The strides to use for each plane > > + * \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, > > + const std::array<unsigned int, 3> &strides) const > ^ alignment (doesn't checkstyle complains?) > > > +{ > > + /* 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 += strides[i] > > + * ((size.height + vertSubSample - 1) / vertSubSample); > > If you make of the stride stride a parameter defaulted to and empty > vector you could merge the two functions here. Just > > unsigned int s = ((size.height + vertSubSample - 1)/ vertSubSample); > if (!strides.empty()) > s *= stride(size.width, i); > else > s *= strides[i]; > > sum += s; > > or something similar Oh yeah, that's better. > The rest of the function seems identical to me. It is :) > > + } > > + > > + return sum; > > +} > > + > > } /* namespace libcamera */ > > -- > > 2.27.0 Thanks, Paul
diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h index 054be37..e347a46 100644 --- a/include/libcamera/internal/formats.h +++ b/include/libcamera/internal/formats.h @@ -52,6 +52,12 @@ public: static const PixelFormatInfo &info(const PixelFormat &format); static const PixelFormatInfo &info(const V4L2PixelFormat &format); + unsigned int stride(unsigned int width, unsigned int plane, + unsigned int align = 0) const; + unsigned int frameSize(const Size &size) const; + unsigned int frameSize(const Size &size, + const std::array<unsigned int, 3> &strides) const; + /* \todo Add support for non-contiguous memory planes */ const char *name; PixelFormat format; diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp index 6d96055..c355e57 100644 --- a/src/libcamera/formats.cpp +++ b/src/libcamera/formats.cpp @@ -732,4 +732,77 @@ const PixelFormatInfo &PixelFormatInfo::info(const V4L2PixelFormat &format) return info->second; } +/** + * \brief Compute the stride + * \param[in] width The width of the line, in pixels + * \param[in] plane The index of the plane whose stride is to be computed + * \param[in] align The number of bytes to align to (0 for default alignment) + * \return The number of bytes necessary to store a line, or 0 if the + * PixelFormatInfo instance or the \a plane is not valid + */ +unsigned int PixelFormatInfo::stride(unsigned int width, unsigned int plane, + unsigned int align) const +{ + if (!isValid()) + return 0; + + if (plane > planes.size() || !planes[plane].bytesPerGroup) + return 0; + + /* ceil(width / pixelsPerGroup) * bytesPerGroup */ + unsigned int stride = (width + pixelsPerGroup - 1) / pixelsPerGroup + * planes[plane].bytesPerGroup; + + if (!align) + return stride; + + /* ceil(stride / align) * align */ + return (stride + align - 1) / align * align; +} + +/** + * \brief Compute the bytes necessary to store the frame + * \param[in] size The size of the frame, in pixels + * \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) 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) + * ((size.height + vertSubSample - 1) / vertSubSample); + } + + return sum; +} + +/** + * \brief Compute the bytes necessary to store the frame + * \param[in] size The size of the frame, in pixels + * \param[in] strides The strides to use for each plane + * \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, + const std::array<unsigned int, 3> &strides) 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 += strides[i] + * ((size.height + vertSubSample - 1) / vertSubSample); + } + + return sum; +} + } /* namespace libcamera */
Add member functions to PixelFormatInfo for calculating stride and frame size. This will simplify existing code that calculates these things. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- Changes in v4: - add overloaded frameSize() that takes array of strides - add optional parameter to stride() for byte alignment Changes in v3: - rename functions to stride and frameSize, from bytesPerLine and imageSize, respectively Changes in v2: - make these functions const - add documentation - inline DIV_ROUND_UP --- include/libcamera/internal/formats.h | 6 +++ src/libcamera/formats.cpp | 73 ++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+)