| Message ID | 20210823131221.1034059-11-hiroh@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Hiro, Thank you for the patch. On Mon, Aug 23, 2021 at 10:12:21PM +0900, Hirokazu Honda wrote: > The offset variable is introduced to FrameBuffer::Plane. In order to > detect that the plane is used while the offset is not set, this adds > the assertion to FrameBuffer::planes(). It should be removed in the > future. > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > --- > include/libcamera/framebuffer.h | 13 +++++++++++-- > src/libcamera/framebuffer.cpp | 4 ++++ > 2 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h > index 5de3c744..d5aeff00 100644 > --- a/include/libcamera/framebuffer.h > +++ b/include/libcamera/framebuffer.h > @@ -7,6 +7,8 @@ > #ifndef __LIBCAMERA_FRAMEBUFFER_H__ > #define __LIBCAMERA_FRAMEBUFFER_H__ > > +#include <assert.h> > +#include <limits> > #include <stdint.h> > #include <vector> > > @@ -41,14 +43,21 @@ class FrameBuffer final : public Extensible > > public: > struct Plane { > + static constexpr unsigned int kInvalidOffset = std::numeric_limits<unsigned int>::max(); > FileDescriptor fd; > - unsigned int offset; > + unsigned int offset = kInvalidOffset; > unsigned int length; > }; > > FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie = 0); > > - const std::vector<Plane> &planes() const { return planes_; } > + const std::vector<Plane> &planes() const > + { > + /* \todo Remove the assertions after sufficient testing */ > + for (const auto &plane : planes_) > + assert(plane.offset != Plane::kInvalidOffset); > + return planes_; > + } > > Request *request() const; > const FrameMetadata &metadata() const { return metadata_; } > diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp > index e9467aa0..5f612ecd 100644 > --- a/src/libcamera/framebuffer.cpp > +++ b/src/libcamera/framebuffer.cpp > @@ -165,6 +165,10 @@ FrameBuffer::Private::Private() > * multiple dmabufs, based on the camera requirements. > */ > > +/** > + * \var FrameBuffer::Plane::kInvalidOffset > + * \brief The invalid offset value "Invalid offset value, to identify uninitialized planes" > + */ Blank line. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > /** > * \var FrameBuffer::Plane::fd > * \brief The dmabuf file descriptor
diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h index 5de3c744..d5aeff00 100644 --- a/include/libcamera/framebuffer.h +++ b/include/libcamera/framebuffer.h @@ -7,6 +7,8 @@ #ifndef __LIBCAMERA_FRAMEBUFFER_H__ #define __LIBCAMERA_FRAMEBUFFER_H__ +#include <assert.h> +#include <limits> #include <stdint.h> #include <vector> @@ -41,14 +43,21 @@ class FrameBuffer final : public Extensible public: struct Plane { + static constexpr unsigned int kInvalidOffset = std::numeric_limits<unsigned int>::max(); FileDescriptor fd; - unsigned int offset; + unsigned int offset = kInvalidOffset; unsigned int length; }; FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie = 0); - const std::vector<Plane> &planes() const { return planes_; } + const std::vector<Plane> &planes() const + { + /* \todo Remove the assertions after sufficient testing */ + for (const auto &plane : planes_) + assert(plane.offset != Plane::kInvalidOffset); + return planes_; + } Request *request() const; const FrameMetadata &metadata() const { return metadata_; } diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp index e9467aa0..5f612ecd 100644 --- a/src/libcamera/framebuffer.cpp +++ b/src/libcamera/framebuffer.cpp @@ -165,6 +165,10 @@ FrameBuffer::Private::Private() * multiple dmabufs, based on the camera requirements. */ +/** + * \var FrameBuffer::Plane::kInvalidOffset + * \brief The invalid offset value + */ /** * \var FrameBuffer::Plane::fd * \brief The dmabuf file descriptor
The offset variable is introduced to FrameBuffer::Plane. In order to detect that the plane is used while the offset is not set, this adds the assertion to FrameBuffer::planes(). It should be removed in the future. Signed-off-by: Hirokazu Honda <hiroh@chromium.org> --- include/libcamera/framebuffer.h | 13 +++++++++++-- src/libcamera/framebuffer.cpp | 4 ++++ 2 files changed, 15 insertions(+), 2 deletions(-)