| Message ID | 20200729112524.498276-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thanks for your work. On 2020-07-29 12:25:24 +0100, Kieran Bingham wrote: > Determine the number of planes used by a format by counting the number > of PxielFormatPlaneInfo entries with a valid entry. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > include/libcamera/internal/formats.h | 2 ++ > src/libcamera/formats.cpp | 22 ++++++++++++++++++++++ > 2 files changed, 24 insertions(+) > > diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h > index 0bb151044294..beeaab1ae2f5 100644 > --- a/include/libcamera/internal/formats.h > +++ b/include/libcamera/internal/formats.h > @@ -45,6 +45,8 @@ public: > unsigned int frameSize(const Size &size, > const std::array<unsigned int, 3> &strides) const; > > + unsigned int planeCount() 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 cd63c15cb926..70d31fb36c37 100644 > --- a/src/libcamera/formats.cpp > +++ b/src/libcamera/formats.cpp > @@ -805,4 +805,26 @@ PixelFormatInfo::frameSize(const Size &size, > return sum; > } > > +/** > + * \brief Identify the number of planes represented by the format > + * > + * This function counts the number of planes which have a valid configuration > + * and uses that to determine the number of planes used by the format. > + * > + * \return The number of planes used by the format > + */ > +unsigned int PixelFormatInfo::planeCount() const > +{ > + unsigned int count = 0; > + > + for (const PixelFormatPlaneInfo &p : planes) { > + if (p.bytesPerGroup == 0) > + break; > + > + count++; > + } > + > + return count; > +} > + > } /* namespace libcamera */ > -- > 2.25.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Kieran, Thank you for the patch. On Wed, Jul 29, 2020 at 12:25:24PM +0100, Kieran Bingham wrote: > Determine the number of planes used by a format by counting the number > of PxielFormatPlaneInfo entries with a valid entry. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > include/libcamera/internal/formats.h | 2 ++ > src/libcamera/formats.cpp | 22 ++++++++++++++++++++++ > 2 files changed, 24 insertions(+) > > diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h > index 0bb151044294..beeaab1ae2f5 100644 > --- a/include/libcamera/internal/formats.h > +++ b/include/libcamera/internal/formats.h > @@ -45,6 +45,8 @@ public: > unsigned int frameSize(const Size &size, > const std::array<unsigned int, 3> &strides) const; > > + unsigned int planeCount() 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 cd63c15cb926..70d31fb36c37 100644 > --- a/src/libcamera/formats.cpp > +++ b/src/libcamera/formats.cpp > @@ -805,4 +805,26 @@ PixelFormatInfo::frameSize(const Size &size, > return sum; > } > > +/** > + * \brief Identify the number of planes represented by the format > + * > + * This function counts the number of planes which have a valid configuration > + * and uses that to determine the number of planes used by the format. I think this is just an internal implementation detail. * \brief Retrieve the number of planes used by the format * \return The number of planes used by the format should be enough. And if we hide the fact that we count them, maybe s/planeCount/numPlanes/ ? I think that's a bit more consistent with how we name fields (haven't checked though). > + * > + * \return The number of planes used by the format > + */ > +unsigned int PixelFormatInfo::planeCount() const > +{ > + unsigned int count = 0; > + > + for (const PixelFormatPlaneInfo &p : planes) { > + if (p.bytesPerGroup == 0) > + break; > + > + count++; > + } > + > + return count; return std::count_if(planes.begin(), planes.end(), [](const PixelFormatPlaneInfo &p) { return p.bytesPerGroup != 0; }); would also work, entirely up to you. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > +} > + > } /* namespace libcamera */
Hi Laurent, On 29/07/2020 14:36, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Wed, Jul 29, 2020 at 12:25:24PM +0100, Kieran Bingham wrote: >> Determine the number of planes used by a format by counting the number >> of PxielFormatPlaneInfo entries with a valid entry. >> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> --- >> include/libcamera/internal/formats.h | 2 ++ >> src/libcamera/formats.cpp | 22 ++++++++++++++++++++++ >> 2 files changed, 24 insertions(+) >> >> diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h >> index 0bb151044294..beeaab1ae2f5 100644 >> --- a/include/libcamera/internal/formats.h >> +++ b/include/libcamera/internal/formats.h >> @@ -45,6 +45,8 @@ public: >> unsigned int frameSize(const Size &size, >> const std::array<unsigned int, 3> &strides) const; >> >> + unsigned int planeCount() 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 cd63c15cb926..70d31fb36c37 100644 >> --- a/src/libcamera/formats.cpp >> +++ b/src/libcamera/formats.cpp >> @@ -805,4 +805,26 @@ PixelFormatInfo::frameSize(const Size &size, >> return sum; >> } >> >> +/** >> + * \brief Identify the number of planes represented by the format >> + * >> + * This function counts the number of planes which have a valid configuration >> + * and uses that to determine the number of planes used by the format. > > I think this is just an internal implementation detail. > > * \brief Retrieve the number of planes used by the format > * \return The number of planes used by the format > > should be enough. Sure, shorter is sweeter ... > > And if we hide the fact that we count them, maybe > s/planeCount/numPlanes/ ? I think that's a bit more consistent with how > we name fields (haven't checked though). numPlanes is indeed better, and doesn't define how it's implemented. >> + * >> + * \return The number of planes used by the format >> + */ >> +unsigned int PixelFormatInfo::planeCount() const >> +{ >> + unsigned int count = 0; >> + >> + for (const PixelFormatPlaneInfo &p : planes) { >> + if (p.bytesPerGroup == 0) >> + break; >> + >> + count++; >> + } >> + >> + return count; > > return std::count_if(planes.begin(), planes.end(), > [](const PixelFormatPlaneInfo &p) { > return p.bytesPerGroup != 0; > }); > > would also work, entirely up to you. > I find the lambda's are often less readable. Yes, more C++'ified. but ... not very friendly at all on the eye. I assume there would be no performance gain or improvement, so I'd prefer to keep the visible count ... Maybe I'll change my mind in the future ... > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks, >> +} >> + >> } /* namespace libcamera */ >
diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h index 0bb151044294..beeaab1ae2f5 100644 --- a/include/libcamera/internal/formats.h +++ b/include/libcamera/internal/formats.h @@ -45,6 +45,8 @@ public: unsigned int frameSize(const Size &size, const std::array<unsigned int, 3> &strides) const; + unsigned int planeCount() 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 cd63c15cb926..70d31fb36c37 100644 --- a/src/libcamera/formats.cpp +++ b/src/libcamera/formats.cpp @@ -805,4 +805,26 @@ PixelFormatInfo::frameSize(const Size &size, return sum; } +/** + * \brief Identify the number of planes represented by the format + * + * This function counts the number of planes which have a valid configuration + * and uses that to determine the number of planes used by the format. + * + * \return The number of planes used by the format + */ +unsigned int PixelFormatInfo::planeCount() const +{ + unsigned int count = 0; + + for (const PixelFormatPlaneInfo &p : planes) { + if (p.bytesPerGroup == 0) + break; + + count++; + } + + return count; +} + } /* namespace libcamera */
Determine the number of planes used by a format by counting the number of PxielFormatPlaneInfo entries with a valid entry. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- include/libcamera/internal/formats.h | 2 ++ src/libcamera/formats.cpp | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+)