[libcamera-devel,02/12] android: Introduce CameraBuffer interface
diff mbox series

Message ID 20210226132932.165484-3-jacopo@jmondi.org
State Superseded
Headers show
Series
  • android: Support memory backends
Related show

Commit Message

Jacopo Mondi Feb. 26, 2021, 1:29 p.m. UTC 
In order to provide support for different memory backends,
move the MappedCamera3Buffer class definition outside of the
CameraDevice class to its own file.

The interface defined in camera_buffer.h will be implemented by
different backends that will be placed in the src/android/mm
subdirectory.

Provide a first implementation for the 'generic android' backend which
matches the existing one.

The MappedCamera3Buffer interface will be renamed in CameraBuffer
in the next patch to match the name of the file and not in this patch
to ease review.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/android/camera_buffer.h               | 21 ++++++++++
 src/android/camera_device.cpp             | 30 --------------
 src/android/camera_device.h               |  7 +---
 src/android/meson.build                   |  2 +
 src/android/mm/android_generic_buffer.cpp | 48 +++++++++++++++++++++++
 src/android/mm/meson.build                |  6 +++
 6 files changed, 78 insertions(+), 36 deletions(-)
 create mode 100644 src/android/camera_buffer.h
 create mode 100644 src/android/mm/android_generic_buffer.cpp
 create mode 100644 src/android/mm/meson.build

Comments

Laurent Pinchart Feb. 28, 2021, 6:20 p.m. UTC | #1 
Hi Jacopo,

Thank you for the patch.

On Fri, Feb 26, 2021 at 02:29:22PM +0100, Jacopo Mondi wrote:
> In order to provide support for different memory backends,
> move the MappedCamera3Buffer class definition outside of the
> CameraDevice class to its own file.
> 
> The interface defined in camera_buffer.h will be implemented by
> different backends that will be placed in the src/android/mm
> subdirectory.
> 
> Provide a first implementation for the 'generic android' backend which
> matches the existing one.
> 
> The MappedCamera3Buffer interface will be renamed in CameraBuffer
> in the next patch to match the name of the file and not in this patch
> to ease review.

The next patch is fairly small, I'd squash it with this one. Splitting
the two makes 02/12 a bit weird, as you mention introducing CameraBuffer
in the subject while this patch doesn't create such a class.

If you really want to keep the two separate, I'd rename
MappedCamera3Buffer to CameraBuffer first.

With this handled,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/android/camera_buffer.h               | 21 ++++++++++
>  src/android/camera_device.cpp             | 30 --------------
>  src/android/camera_device.h               |  7 +---
>  src/android/meson.build                   |  2 +
>  src/android/mm/android_generic_buffer.cpp | 48 +++++++++++++++++++++++
>  src/android/mm/meson.build                |  6 +++
>  6 files changed, 78 insertions(+), 36 deletions(-)
>  create mode 100644 src/android/camera_buffer.h
>  create mode 100644 src/android/mm/android_generic_buffer.cpp
>  create mode 100644 src/android/mm/meson.build
> 
> diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h
> new file mode 100644
> index 000000000000..a1fb97a3c6db
> --- /dev/null
> +++ b/src/android/camera_buffer.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * camera_buffer.h - Frame buffer handling interface definition
> + */
> +#ifndef __ANDROID_CAMERA_BUFFER_H__
> +#define __ANDROID_CAMERA_BUFFER_H__
> +
> +#include <hardware/camera3.h>
> +
> +#include <libcamera/internal/buffer.h>
> +
> +class MappedCamera3Buffer : public libcamera::MappedBuffer
> +{
> +public:
> +	MappedCamera3Buffer(const buffer_handle_t camera3buffer, int flags);
> +	~MappedCamera3Buffer();
> +};
> +
> +#endif /* __ANDROID_CAMERA_BUFFER_H__ */
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 16cb8c6d2b84..a7a5b7986aa4 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -257,36 +257,6 @@ void sortCamera3StreamConfigs(std::vector<Camera3StreamConfig> &unsortedConfigs,
>  
>  } /* namespace */
>  
> -MappedCamera3Buffer::MappedCamera3Buffer(const buffer_handle_t camera3buffer,
> -					 int flags)
> -{
> -	maps_.reserve(camera3buffer->numFds);
> -	error_ = 0;
> -
> -	for (int i = 0; i < camera3buffer->numFds; i++) {
> -		if (camera3buffer->data[i] == -1)
> -			continue;
> -
> -		off_t length = lseek(camera3buffer->data[i], 0, SEEK_END);
> -		if (length < 0) {
> -			error_ = -errno;
> -			LOG(HAL, Error) << "Failed to query plane length";
> -			break;
> -		}
> -
> -		void *address = mmap(nullptr, length, flags, MAP_SHARED,
> -				     camera3buffer->data[i], 0);
> -		if (address == MAP_FAILED) {
> -			error_ = -errno;
> -			LOG(HAL, Error) << "Failed to mmap plane";
> -			break;
> -		}
> -
> -		maps_.emplace_back(static_cast<uint8_t *>(address),
> -				   static_cast<size_t>(length));
> -	}
> -}
> -
>  /*
>   * \struct Camera3RequestDescriptor
>   *
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 9cbfcad38433..e6c192c2100b 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -24,17 +24,12 @@
>  #include "libcamera/internal/log.h"
>  #include "libcamera/internal/message.h"
>  
> +#include "camera_buffer.h"
>  #include "camera_metadata.h"
>  #include "camera_stream.h"
>  #include "camera_worker.h"
>  #include "jpeg/encoder.h"
>  
> -class MappedCamera3Buffer : public libcamera::MappedBuffer
> -{
> -public:
> -	MappedCamera3Buffer(const buffer_handle_t camera3buffer, int flags);
> -};
> -
>  class CameraDevice : protected libcamera::Loggable
>  {
>  public:
> diff --git a/src/android/meson.build b/src/android/meson.build
> index a13ce63b1d58..fd41c74f78ef 100644
> --- a/src/android/meson.build
> +++ b/src/android/meson.build
> @@ -57,6 +57,8 @@ android_hal_sources = files([
>      'yuv/post_processor_yuv.cpp'
>  ])
>  
> +subdir('mm')
> +
>  android_camera_metadata_sources = files([
>      'metadata/camera_metadata.c',
>  ])
> diff --git a/src/android/mm/android_generic_buffer.cpp b/src/android/mm/android_generic_buffer.cpp
> new file mode 100644
> index 000000000000..2504d9276e9e
> --- /dev/null
> +++ b/src/android/mm/android_generic_buffer.cpp
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * android_generic_buffer.cpp - Generic Android frame buffer backend
> + */
> +
> +#include "../camera_buffer.h"
> +
> +#include "libcamera/internal/log.h"
> +
> +using namespace libcamera;
> +
> +LOG_DECLARE_CATEGORY(HAL)
> +
> +MappedCamera3Buffer::MappedCamera3Buffer(const buffer_handle_t camera3buffer,
> +					 int flags)
> +{
> +	maps_.reserve(camera3buffer->numFds);
> +	error_ = 0;
> +
> +	for (int i = 0; i < camera3buffer->numFds; i++) {
> +		if (camera3buffer->data[i] == -1)
> +			continue;
> +
> +		off_t length = lseek(camera3buffer->data[i], 0, SEEK_END);
> +		if (length < 0) {
> +			error_ = -errno;
> +			LOG(HAL, Error) << "Failed to query plane length";
> +			break;
> +		}
> +
> +		void *address = mmap(nullptr, length, flags, MAP_SHARED,
> +				     camera3buffer->data[i], 0);
> +		if (address == MAP_FAILED) {
> +			error_ = -errno;
> +			LOG(HAL, Error) << "Failed to mmap plane";
> +			break;
> +		}
> +
> +		maps_.emplace_back(static_cast<uint8_t *>(address),
> +				   static_cast<size_t>(length));
> +	}
> +}
> +
> +MappedCamera3Buffer::~MappedCamera3Buffer()
> +{
> +}
> diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build
> new file mode 100644
> index 000000000000..39be8fec8567
> --- /dev/null
> +++ b/src/android/mm/meson.build
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +memory_backend = get_option('android_memory_backend')
> +if memory_backend == 'android_generic'
> +    android_hal_sources += files(['android_generic_buffer.cpp'])
> +endif

Patch
diff mbox series 

diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h
new file mode 100644
index 000000000000..a1fb97a3c6db
--- /dev/null
+++ b/src/android/camera_buffer.h
@@ -0,0 +1,21 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2021, Google Inc.
+ *
+ * camera_buffer.h - Frame buffer handling interface definition
+ */
+#ifndef __ANDROID_CAMERA_BUFFER_H__
+#define __ANDROID_CAMERA_BUFFER_H__
+
+#include <hardware/camera3.h>
+
+#include <libcamera/internal/buffer.h>
+
+class MappedCamera3Buffer : public libcamera::MappedBuffer
+{
+public:
+	MappedCamera3Buffer(const buffer_handle_t camera3buffer, int flags);
+	~MappedCamera3Buffer();
+};
+
+#endif /* __ANDROID_CAMERA_BUFFER_H__ */
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 16cb8c6d2b84..a7a5b7986aa4 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -257,36 +257,6 @@  void sortCamera3StreamConfigs(std::vector<Camera3StreamConfig> &unsortedConfigs,
 
 } /* namespace */
 
-MappedCamera3Buffer::MappedCamera3Buffer(const buffer_handle_t camera3buffer,
-					 int flags)
-{
-	maps_.reserve(camera3buffer->numFds);
-	error_ = 0;
-
-	for (int i = 0; i < camera3buffer->numFds; i++) {
-		if (camera3buffer->data[i] == -1)
-			continue;
-
-		off_t length = lseek(camera3buffer->data[i], 0, SEEK_END);
-		if (length < 0) {
-			error_ = -errno;
-			LOG(HAL, Error) << "Failed to query plane length";
-			break;
-		}
-
-		void *address = mmap(nullptr, length, flags, MAP_SHARED,
-				     camera3buffer->data[i], 0);
-		if (address == MAP_FAILED) {
-			error_ = -errno;
-			LOG(HAL, Error) << "Failed to mmap plane";
-			break;
-		}
-
-		maps_.emplace_back(static_cast<uint8_t *>(address),
-				   static_cast<size_t>(length));
-	}
-}
-
 /*
  * \struct Camera3RequestDescriptor
  *
diff --git a/src/android/camera_device.h b/src/android/camera_device.h
index 9cbfcad38433..e6c192c2100b 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -24,17 +24,12 @@ 
 #include "libcamera/internal/log.h"
 #include "libcamera/internal/message.h"
 
+#include "camera_buffer.h"
 #include "camera_metadata.h"
 #include "camera_stream.h"
 #include "camera_worker.h"
 #include "jpeg/encoder.h"
 
-class MappedCamera3Buffer : public libcamera::MappedBuffer
-{
-public:
-	MappedCamera3Buffer(const buffer_handle_t camera3buffer, int flags);
-};
-
 class CameraDevice : protected libcamera::Loggable
 {
 public:
diff --git a/src/android/meson.build b/src/android/meson.build
index a13ce63b1d58..fd41c74f78ef 100644
--- a/src/android/meson.build
+++ b/src/android/meson.build
@@ -57,6 +57,8 @@  android_hal_sources = files([
     'yuv/post_processor_yuv.cpp'
 ])
 
+subdir('mm')
+
 android_camera_metadata_sources = files([
     'metadata/camera_metadata.c',
 ])
diff --git a/src/android/mm/android_generic_buffer.cpp b/src/android/mm/android_generic_buffer.cpp
new file mode 100644
index 000000000000..2504d9276e9e
--- /dev/null
+++ b/src/android/mm/android_generic_buffer.cpp
@@ -0,0 +1,48 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2021, Google Inc.
+ *
+ * android_generic_buffer.cpp - Generic Android frame buffer backend
+ */
+
+#include "../camera_buffer.h"
+
+#include "libcamera/internal/log.h"
+
+using namespace libcamera;
+
+LOG_DECLARE_CATEGORY(HAL)
+
+MappedCamera3Buffer::MappedCamera3Buffer(const buffer_handle_t camera3buffer,
+					 int flags)
+{
+	maps_.reserve(camera3buffer->numFds);
+	error_ = 0;
+
+	for (int i = 0; i < camera3buffer->numFds; i++) {
+		if (camera3buffer->data[i] == -1)
+			continue;
+
+		off_t length = lseek(camera3buffer->data[i], 0, SEEK_END);
+		if (length < 0) {
+			error_ = -errno;
+			LOG(HAL, Error) << "Failed to query plane length";
+			break;
+		}
+
+		void *address = mmap(nullptr, length, flags, MAP_SHARED,
+				     camera3buffer->data[i], 0);
+		if (address == MAP_FAILED) {
+			error_ = -errno;
+			LOG(HAL, Error) << "Failed to mmap plane";
+			break;
+		}
+
+		maps_.emplace_back(static_cast<uint8_t *>(address),
+				   static_cast<size_t>(length));
+	}
+}
+
+MappedCamera3Buffer::~MappedCamera3Buffer()
+{
+}
diff --git a/src/android/mm/meson.build b/src/android/mm/meson.build
new file mode 100644
index 000000000000..39be8fec8567
--- /dev/null
+++ b/src/android/mm/meson.build
@@ -0,0 +1,6 @@ 
+# SPDX-License-Identifier: CC0-1.0
+
+memory_backend = get_option('android_memory_backend')
+if memory_backend == 'android_generic'
+    android_hal_sources += files(['android_generic_buffer.cpp'])
+endif