[libcamera-devel,v2,1/2] android: CameraBuffer: Add a static function to check a buffer validness
diff mbox series

Message ID 20210407043620.1756347-1-hiroh@chromium.org
State Superseded
Headers show
Series
  • [libcamera-devel,v2,1/2] android: CameraBuffer: Add a static function to check a buffer validness
Related show

Commit Message

Hirokazu Honda April 7, 2021, 4:36 a.m. UTC 
This adds a static function to CameraBuffer class that checks if
a buffer_handle is valid with a platform dependent framework.
For example, the function validates a buffer using
cros::CameraBufferManager on ChromeOS.

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
---
 src/android/camera_buffer.h              |  6 ++++++
 src/android/mm/cros_camera_buffer.cpp    | 19 +++++++++++++++++++
 src/android/mm/generic_camera_buffer.cpp |  9 +++++++++
 3 files changed, 34 insertions(+)

Comments

Jacopo Mondi April 7, 2021, 8:06 a.m. UTC | #1 
Hi Hiro,

On Wed, Apr 07, 2021 at 01:36:19PM +0900, Hirokazu Honda wrote:
> This adds a static function to CameraBuffer class that checks if
> a buffer_handle is valid with a platform dependent framework.
> For example, the function validates a buffer using
> cros::CameraBufferManager on ChromeOS.
>
> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> ---
>  src/android/camera_buffer.h              |  6 ++++++
>  src/android/mm/cros_camera_buffer.cpp    | 19 +++++++++++++++++++
>  src/android/mm/generic_camera_buffer.cpp |  9 +++++++++
>  3 files changed, 34 insertions(+)
>
> diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h
> index 7e8970b4..3bb9a435 100644
> --- a/src/android/camera_buffer.h
> +++ b/src/android/camera_buffer.h
> @@ -20,6 +20,8 @@ public:
>  	CameraBuffer(buffer_handle_t camera3Buffer, int flags);
>  	~CameraBuffer();
>
> +	static bool isValidBuffer(buffer_handle_t camera3Buffer);
> +
>  	bool isValid() const;
>
>  	unsigned int numPlanes() const;
> @@ -38,6 +40,10 @@ CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags)	\
>  CameraBuffer::~CameraBuffer()						\
>  {									\
>  }									\
> +bool CameraBuffer::isValidBuffer(buffer_handle_t camera3Buffer)		\


The closing '\' renders un-aligned in my mail client

> +{									\
> +	return Private::isValidBuffer(camera3Buffer);			\
> +}									\
>  bool CameraBuffer::isValid() const					\
>  {									\
>  	const Private *const d = LIBCAMERA_D_PTR();			\
> diff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp
> index 1a4fd5d1..371a2511 100644
> --- a/src/android/mm/cros_camera_buffer.cpp
> +++ b/src/android/mm/cros_camera_buffer.cpp
> @@ -24,6 +24,8 @@ public:
>  		buffer_handle_t camera3Buffer, int flags);
>  	~Private();
>
> +	static bool isValidBuffer(buffer_handle_t camera3Buffer);
> +
>  	bool isValid() const { return valid_; }
>
>  	unsigned int numPlanes() const;
> @@ -133,4 +135,21 @@ size_t CameraBuffer::Private::jpegBufferSize(size_t maxJpegBufferSize) const
>  	return bufferManager_->GetPlaneSize(handle_, 0);
>  }
>
> +// static

No C++ comments please

> +bool CameraBuffer::Private::isValidBuffer(buffer_handle_t camera3Buffer)
> +{
> +	cros::CameraBufferManager * bufferManager =

*bufferManager

> +		cros::CameraBufferManager::GetInstance();
> +
> +	int ret = bufferManager->Register(camera3Buffer);
> +	if (ret) {
> +		return false;
> +	}

No enclosing braces for 1-liners

> +
> +	bufferManager->Deregister(camera3Buffer);
> +
> +	return true;
> +}
> +

Double empty line

> +
>  PUBLIC_CAMERA_BUFFER_IMPLEMENTATION
> diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp
> index 929e078a..0c52e196 100644
> --- a/src/android/mm/generic_camera_buffer.cpp
> +++ b/src/android/mm/generic_camera_buffer.cpp
> @@ -24,6 +24,8 @@ public:
>  		buffer_handle_t camera3Buffer, int flags);
>  	~Private();
>
> +	static bool isValidBuffer(buffer_handle_t camera3Buffer);
> +
>  	unsigned int numPlanes() const;
>
>  	Span<uint8_t> plane(unsigned int plane);
> @@ -85,4 +87,11 @@ size_t CameraBuffer::Private::jpegBufferSize(size_t maxJpegBufferSize) const
>  				      maxJpegBufferSize);
>  }
>
> +// static

No C++ comments please

> +bool CameraBuffer::Private::isValidBuffer(
> +       [[maybe_unused]] buffer_handle_t camera3Buffer)
> +{
> +	return true;

Double empty line
> +
> +}

Please make sure to run checkstyle.py before submitting. You can
$ cp utils/hooks/post-commit .git/hooks
to automatize that.

The patch itself looks good
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

>  PUBLIC_CAMERA_BUFFER_IMPLEMENTATION
> --
> 2.31.0.208.g409f899ff0-goog
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Hirokazu Honda April 8, 2021, 3:12 a.m. UTC | #2 
Hi Jacopo, thanks for reviewing.

On Wed, Apr 7, 2021 at 5:06 PM Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> Hi Hiro,
>
> On Wed, Apr 07, 2021 at 01:36:19PM +0900, Hirokazu Honda wrote:
> > This adds a static function to CameraBuffer class that checks if
> > a buffer_handle is valid with a platform dependent framework.
> > For example, the function validates a buffer using
> > cros::CameraBufferManager on ChromeOS.
> >
> > Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> > ---
> >  src/android/camera_buffer.h              |  6 ++++++
> >  src/android/mm/cros_camera_buffer.cpp    | 19 +++++++++++++++++++
> >  src/android/mm/generic_camera_buffer.cpp |  9 +++++++++
> >  3 files changed, 34 insertions(+)
> >
> > diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h
> > index 7e8970b4..3bb9a435 100644
> > --- a/src/android/camera_buffer.h
> > +++ b/src/android/camera_buffer.h
> > @@ -20,6 +20,8 @@ public:
> >       CameraBuffer(buffer_handle_t camera3Buffer, int flags);
> >       ~CameraBuffer();
> >
> > +     static bool isValidBuffer(buffer_handle_t camera3Buffer);
> > +
> >       bool isValid() const;
> >
> >       unsigned int numPlanes() const;
> > @@ -38,6 +40,10 @@ CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags)       \
> >  CameraBuffer::~CameraBuffer()                                                \
> >  {                                                                    \
> >  }                                                                    \
> > +bool CameraBuffer::isValidBuffer(buffer_handle_t camera3Buffer)              \
>
>
> The closing '\' renders un-aligned in my mail client
>
> > +{                                                                    \
> > +     return Private::isValidBuffer(camera3Buffer);                   \
> > +}                                                                    \
> >  bool CameraBuffer::isValid() const                                   \
> >  {                                                                    \
> >       const Private *const d = LIBCAMERA_D_PTR();                     \
> > diff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp
> > index 1a4fd5d1..371a2511 100644
> > --- a/src/android/mm/cros_camera_buffer.cpp
> > +++ b/src/android/mm/cros_camera_buffer.cpp
> > @@ -24,6 +24,8 @@ public:
> >               buffer_handle_t camera3Buffer, int flags);
> >       ~Private();
> >
> > +     static bool isValidBuffer(buffer_handle_t camera3Buffer);
> > +
> >       bool isValid() const { return valid_; }
> >
> >       unsigned int numPlanes() const;
> > @@ -133,4 +135,21 @@ size_t CameraBuffer::Private::jpegBufferSize(size_t maxJpegBufferSize) const
> >       return bufferManager_->GetPlaneSize(handle_, 0);
> >  }
> >
> > +// static
>
> No C++ comments please
>
> > +bool CameraBuffer::Private::isValidBuffer(buffer_handle_t camera3Buffer)
> > +{
> > +     cros::CameraBufferManager * bufferManager =
>
> *bufferManager
>
> > +             cros::CameraBufferManager::GetInstance();
> > +
> > +     int ret = bufferManager->Register(camera3Buffer);
> > +     if (ret) {
> > +             return false;
> > +     }
>
> No enclosing braces for 1-liners
>
> > +
> > +     bufferManager->Deregister(camera3Buffer);
> > +
> > +     return true;
> > +}
> > +
>
> Double empty line
>
> > +
> >  PUBLIC_CAMERA_BUFFER_IMPLEMENTATION
> > diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp
> > index 929e078a..0c52e196 100644
> > --- a/src/android/mm/generic_camera_buffer.cpp
> > +++ b/src/android/mm/generic_camera_buffer.cpp
> > @@ -24,6 +24,8 @@ public:
> >               buffer_handle_t camera3Buffer, int flags);
> >       ~Private();
> >
> > +     static bool isValidBuffer(buffer_handle_t camera3Buffer);
> > +
> >       unsigned int numPlanes() const;
> >
> >       Span<uint8_t> plane(unsigned int plane);
> > @@ -85,4 +87,11 @@ size_t CameraBuffer::Private::jpegBufferSize(size_t maxJpegBufferSize) const
> >                                     maxJpegBufferSize);
> >  }
> >
> > +// static
>
> No C++ comments please
>
> > +bool CameraBuffer::Private::isValidBuffer(
> > +       [[maybe_unused]] buffer_handle_t camera3Buffer)
> > +{
> > +     return true;
>
> Double empty line
> > +
> > +}
>
> Please make sure to run checkstyle.py before submitting. You can
> $ cp utils/hooks/post-commit .git/hooks
> to automatize that.
>

Thanks for telling me the track. I manually run checkstyle.py and
forgot sometimes.
It is definitely helpful for me.

> The patch itself looks good
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>
> Thanks
>   j
>
> >  PUBLIC_CAMERA_BUFFER_IMPLEMENTATION
> > --
> > 2.31.0.208.g409f899ff0-goog
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h
index 7e8970b4..3bb9a435 100644
--- a/src/android/camera_buffer.h
+++ b/src/android/camera_buffer.h
@@ -20,6 +20,8 @@  public:
 	CameraBuffer(buffer_handle_t camera3Buffer, int flags);
 	~CameraBuffer();
 
+	static bool isValidBuffer(buffer_handle_t camera3Buffer);
+
 	bool isValid() const;
 
 	unsigned int numPlanes() const;
@@ -38,6 +40,10 @@  CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags)	\
 CameraBuffer::~CameraBuffer()						\
 {									\
 }									\
+bool CameraBuffer::isValidBuffer(buffer_handle_t camera3Buffer)		\
+{									\
+	return Private::isValidBuffer(camera3Buffer);			\
+}									\
 bool CameraBuffer::isValid() const					\
 {									\
 	const Private *const d = LIBCAMERA_D_PTR();			\
diff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp
index 1a4fd5d1..371a2511 100644
--- a/src/android/mm/cros_camera_buffer.cpp
+++ b/src/android/mm/cros_camera_buffer.cpp
@@ -24,6 +24,8 @@  public:
 		buffer_handle_t camera3Buffer, int flags);
 	~Private();
 
+	static bool isValidBuffer(buffer_handle_t camera3Buffer);
+
 	bool isValid() const { return valid_; }
 
 	unsigned int numPlanes() const;
@@ -133,4 +135,21 @@  size_t CameraBuffer::Private::jpegBufferSize(size_t maxJpegBufferSize) const
 	return bufferManager_->GetPlaneSize(handle_, 0);
 }
 
+// static
+bool CameraBuffer::Private::isValidBuffer(buffer_handle_t camera3Buffer)
+{
+	cros::CameraBufferManager * bufferManager =
+		cros::CameraBufferManager::GetInstance();
+
+	int ret = bufferManager->Register(camera3Buffer);
+	if (ret) {
+		return false;
+	}
+
+	bufferManager->Deregister(camera3Buffer);
+
+	return true;
+}
+
+
 PUBLIC_CAMERA_BUFFER_IMPLEMENTATION
diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp
index 929e078a..0c52e196 100644
--- a/src/android/mm/generic_camera_buffer.cpp
+++ b/src/android/mm/generic_camera_buffer.cpp
@@ -24,6 +24,8 @@  public:
 		buffer_handle_t camera3Buffer, int flags);
 	~Private();
 
+	static bool isValidBuffer(buffer_handle_t camera3Buffer);
+
 	unsigned int numPlanes() const;
 
 	Span<uint8_t> plane(unsigned int plane);
@@ -85,4 +87,11 @@  size_t CameraBuffer::Private::jpegBufferSize(size_t maxJpegBufferSize) const
 				      maxJpegBufferSize);
 }
 
+// static
+bool CameraBuffer::Private::isValidBuffer(
+       [[maybe_unused]] buffer_handle_t camera3Buffer)
+{
+	return true;
+
+}
 PUBLIC_CAMERA_BUFFER_IMPLEMENTATION