[libcamera-devel,v3,04/11] android: camera_device: Break out size calculation
diff mbox series

Message ID 20200908134142.27470-5-jacopo@jmondi.org
State Accepted
Headers show
Series
  • android: camera_device: Fix JPEG/RAW sizes
Related show

Commit Message

Jacopo Mondi Sept. 8, 2020, 1:41 p.m. UTC 
As the RAW stream sizes needs to be calculated differently from the
processed one, break out the procedure to calculate the processed
(RGB/YUV) resolutions from initializeStreamConfigurations() in order to
prepare for RAW sizes calculation.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/android/camera_device.cpp | 51 +++++++++++++++++++++++------------
 src/android/camera_device.h   |  5 ++++
 2 files changed, 39 insertions(+), 17 deletions(-)

Comments

Niklas Söderlund Sept. 10, 2020, 10:38 a.m. UTC | #1 
Hi Jacopo,

Thanks for your work.

On 2020-09-08 15:41:35 +0200, Jacopo Mondi wrote:
> As the RAW stream sizes needs to be calculated differently from the
> processed one, break out the procedure to calculate the processed
> (RGB/YUV) resolutions from initializeStreamConfigurations() in order to
> prepare for RAW sizes calculation.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/android/camera_device.cpp | 51 +++++++++++++++++++++++------------
>  src/android/camera_device.h   |  5 ++++
>  2 files changed, 39 insertions(+), 17 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 17b5fd5f59eb..9d460906ab08 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -283,6 +283,36 @@ int CameraDevice::initialize()
>  	return ret;
>  }
>  
> +std::vector<Size> CameraDevice::filterYUVResolutions(CameraConfiguration *cameraConfig,
> +						     const PixelFormat &pixelFormat,
> +						     const std::vector<Size> &resolutions)
> +{
> +	std::vector<Size> supportedResolutions;
> +
> +	StreamConfiguration &cfg = cameraConfig->at(0);
> +	for (const Size &res : resolutions) {
> +		cfg.pixelFormat = pixelFormat;
> +		cfg.size = res;
> +
> +		std::stringstream ss;
> +		ss << "Testing " << cfg.toString();
> +
> +		CameraConfiguration::Status status = cameraConfig->validate();
> +		if (status != CameraConfiguration::Valid) {
> +			ss << " not supported";
> +			LOG(HAL, Debug) << ss.str();
> +			continue;
> +		}
> +
> +		ss << " supported";
> +		LOG(HAL, Debug) << ss.str();
> +
> +		supportedResolutions.push_back(res);
> +	}
> +
> +	return supportedResolutions;
> +}
> +
>  /*
>   * Initialize the format conversion map to translate from Android format
>   * identifier to libcamera pixel formats and fill in the list of supported
> @@ -427,23 +457,10 @@ int CameraDevice::initializeStreamConfigurations()
>  				<< camera3Format.name << " to "
>  				<< mappedFormat.toString();
>  
> -		for (const Size &res : cameraResolutions) {
> -			cfg.pixelFormat = mappedFormat;
> -			cfg.size = res;
> -
> -			std::stringstream ss;
> -			ss << "Testing " << cfg.toString();
> -
> -			CameraConfiguration::Status status = cameraConfig->validate();
> -			if (status != CameraConfiguration::Valid) {
> -				ss << " not supported";
> -				LOG(HAL, Debug) << ss.str();
> -				continue;
> -			}
> -
> -			ss << " supported";
> -			LOG(HAL, Debug) << ss.str();
> -
> +		std::vector<Size> resolutions = filterYUVResolutions(cameraConfig.get(),
> +								     mappedFormat,
> +								     cameraResolutions);
> +		for (const Size &res : resolutions) {
>  			streamConfigurations_.push_back({ res, androidFormat });
>  
>  			/*
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 3934f194f1b5..359a163ebab9 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -93,6 +93,11 @@ private:
>  	};
>  
>  	int initializeStreamConfigurations();
> +	std::vector<libcamera::Size>
> +	filterYUVResolutions(libcamera::CameraConfiguration *cameraConfig,
> +			     const libcamera::PixelFormat &pixelFormat,
> +			     const std::vector<libcamera::Size> &resolutions);
> +
>  	std::tuple<uint32_t, uint32_t> calculateStaticMetadataSize();
>  	libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);
>  	void notifyShutter(uint32_t frameNumber, uint64_t timestamp);
> -- 
> 2.28.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Hirokazu Honda Sept. 11, 2020, 2:12 a.m. UTC | #2 
On Thu, Sep 10, 2020 at 7:38 PM Niklas Söderlund
<niklas.soderlund@ragnatech.se> wrote:
>
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2020-09-08 15:41:35 +0200, Jacopo Mondi wrote:
> > As the RAW stream sizes needs to be calculated differently from the
> > processed one, break out the procedure to calculate the processed
> > (RGB/YUV) resolutions from initializeStreamConfigurations() in order to
> > prepare for RAW sizes calculation.
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
>
> > ---
> >  src/android/camera_device.cpp | 51 +++++++++++++++++++++++------------
> >  src/android/camera_device.h   |  5 ++++
> >  2 files changed, 39 insertions(+), 17 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 17b5fd5f59eb..9d460906ab08 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -283,6 +283,36 @@ int CameraDevice::initialize()
> >       return ret;
> >  }
> >
> > +std::vector<Size> CameraDevice::filterYUVResolutions(CameraConfiguration *cameraConfig,
> > +                                                  const PixelFormat &pixelFormat,
> > +                                                  const std::vector<Size> &resolutions)
> > +{
> > +     std::vector<Size> supportedResolutions;
> > +
> > +     StreamConfiguration &cfg = cameraConfig->at(0);
> > +     for (const Size &res : resolutions) {
> > +             cfg.pixelFormat = pixelFormat;
> > +             cfg.size = res;
> > +
> > +             std::stringstream ss;
> > +             ss << "Testing " << cfg.toString();
> > +
> > +             CameraConfiguration::Status status = cameraConfig->validate();
> > +             if (status != CameraConfiguration::Valid) {
> > +                     ss << " not supported";
> > +                     LOG(HAL, Debug) << ss.str();
> > +                     continue;
> > +             }
> > +
> > +             ss << " supported";
> > +             LOG(HAL, Debug) << ss.str();
> > +

nit: You may want to apply the same Nioklas's comment as Patch 03/12 here.

Reviewed-by: Hirokazu Honda <hiroh@chromium.org>

> > +             supportedResolutions.push_back(res);
> > +     }
> > +
> > +     return supportedResolutions;
> > +}
> > +
> >  /*
> >   * Initialize the format conversion map to translate from Android format
> >   * identifier to libcamera pixel formats and fill in the list of supported
> > @@ -427,23 +457,10 @@ int CameraDevice::initializeStreamConfigurations()
> >                               << camera3Format.name << " to "
> >                               << mappedFormat.toString();
> >
> > -             for (const Size &res : cameraResolutions) {
> > -                     cfg.pixelFormat = mappedFormat;
> > -                     cfg.size = res;
> > -
> > -                     std::stringstream ss;
> > -                     ss << "Testing " << cfg.toString();
> > -
> > -                     CameraConfiguration::Status status = cameraConfig->validate();
> > -                     if (status != CameraConfiguration::Valid) {
> > -                             ss << " not supported";
> > -                             LOG(HAL, Debug) << ss.str();
> > -                             continue;
> > -                     }
> > -
> > -                     ss << " supported";
> > -                     LOG(HAL, Debug) << ss.str();
> > -
> > +             std::vector<Size> resolutions = filterYUVResolutions(cameraConfig.get(),
> > +                                                                  mappedFormat,
> > +                                                                  cameraResolutions);
> > +             for (const Size &res : resolutions) {
> >                       streamConfigurations_.push_back({ res, androidFormat });
> >
> >                       /*
> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> > index 3934f194f1b5..359a163ebab9 100644
> > --- a/src/android/camera_device.h
> > +++ b/src/android/camera_device.h
> > @@ -93,6 +93,11 @@ private:
> >       };
> >
> >       int initializeStreamConfigurations();
> > +     std::vector<libcamera::Size>
> > +     filterYUVResolutions(libcamera::CameraConfiguration *cameraConfig,
> > +                          const libcamera::PixelFormat &pixelFormat,
> > +                          const std::vector<libcamera::Size> &resolutions);
> > +
> >       std::tuple<uint32_t, uint32_t> calculateStaticMetadataSize();
> >       libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);
> >       void notifyShutter(uint32_t frameNumber, uint64_t timestamp);
> > --
> > 2.28.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund
> _______________________________________________
> 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_device.cpp b/src/android/camera_device.cpp
index 17b5fd5f59eb..9d460906ab08 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -283,6 +283,36 @@  int CameraDevice::initialize()
 	return ret;
 }
 
+std::vector<Size> CameraDevice::filterYUVResolutions(CameraConfiguration *cameraConfig,
+						     const PixelFormat &pixelFormat,
+						     const std::vector<Size> &resolutions)
+{
+	std::vector<Size> supportedResolutions;
+
+	StreamConfiguration &cfg = cameraConfig->at(0);
+	for (const Size &res : resolutions) {
+		cfg.pixelFormat = pixelFormat;
+		cfg.size = res;
+
+		std::stringstream ss;
+		ss << "Testing " << cfg.toString();
+
+		CameraConfiguration::Status status = cameraConfig->validate();
+		if (status != CameraConfiguration::Valid) {
+			ss << " not supported";
+			LOG(HAL, Debug) << ss.str();
+			continue;
+		}
+
+		ss << " supported";
+		LOG(HAL, Debug) << ss.str();
+
+		supportedResolutions.push_back(res);
+	}
+
+	return supportedResolutions;
+}
+
 /*
  * Initialize the format conversion map to translate from Android format
  * identifier to libcamera pixel formats and fill in the list of supported
@@ -427,23 +457,10 @@  int CameraDevice::initializeStreamConfigurations()
 				<< camera3Format.name << " to "
 				<< mappedFormat.toString();
 
-		for (const Size &res : cameraResolutions) {
-			cfg.pixelFormat = mappedFormat;
-			cfg.size = res;
-
-			std::stringstream ss;
-			ss << "Testing " << cfg.toString();
-
-			CameraConfiguration::Status status = cameraConfig->validate();
-			if (status != CameraConfiguration::Valid) {
-				ss << " not supported";
-				LOG(HAL, Debug) << ss.str();
-				continue;
-			}
-
-			ss << " supported";
-			LOG(HAL, Debug) << ss.str();
-
+		std::vector<Size> resolutions = filterYUVResolutions(cameraConfig.get(),
+								     mappedFormat,
+								     cameraResolutions);
+		for (const Size &res : resolutions) {
 			streamConfigurations_.push_back({ res, androidFormat });
 
 			/*
diff --git a/src/android/camera_device.h b/src/android/camera_device.h
index 3934f194f1b5..359a163ebab9 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -93,6 +93,11 @@  private:
 	};
 
 	int initializeStreamConfigurations();
+	std::vector<libcamera::Size>
+	filterYUVResolutions(libcamera::CameraConfiguration *cameraConfig,
+			     const libcamera::PixelFormat &pixelFormat,
+			     const std::vector<libcamera::Size> &resolutions);
+
 	std::tuple<uint32_t, uint32_t> calculateStaticMetadataSize();
 	libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);
 	void notifyShutter(uint32_t frameNumber, uint64_t timestamp);