[libcamera-devel,v3,03/11] android: camera_device: Add debug to stream initialization
diff mbox series

Message ID 20200908134142.27470-4-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 
Add debug printouts to the CameraDevice::initializeStreamConfigurations()
function that help to follow the process of building the stream
configurations map.

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 | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

Comments

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

Thanks for your patch.

On 2020-09-08 15:41:34 +0200, Jacopo Mondi wrote:
> Add debug printouts to the CameraDevice::initializeStreamConfigurations()
> function that help to follow the process of building the stream
> configurations map.
> 
> 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 | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 1b2e12d6d33c..17b5fd5f59eb 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -363,12 +363,19 @@ int CameraDevice::initializeStreamConfigurations()
>  		const std::vector<PixelFormat> &libcameraFormats =
>  			camera3Format.libcameraFormats;
>  
> +		LOG(HAL, Debug) << "Trying to map Android format "
> +				<< camera3Format.name;
> +
>  		/*
>  		 * JPEG is always supported, either produced directly by the
>  		 * camera, or encoded in the HAL.
>  		 */
>  		if (androidFormat == HAL_PIXEL_FORMAT_BLOB) {
>  			formatsMap_[androidFormat] = formats::MJPEG;
> +			LOG(HAL, Debug) << "Mapped Android format "
> +					<< camera3Format.name << " to "
> +					<< formats::MJPEG.toString()
> +					<< " (fixed mapping)";
>  			continue;
>  		}
>  
> @@ -379,6 +386,8 @@ int CameraDevice::initializeStreamConfigurations()
>  		PixelFormat mappedFormat;
>  		for (const PixelFormat &pixelFormat : libcameraFormats) {
>  
> +			LOG(HAL, Debug) << "Testing " << pixelFormat.toString();
> +
>  			/*
>  			 * The stream configuration size can be adjusted,
>  			 * not the pixel format.
> @@ -414,14 +423,26 @@ int CameraDevice::initializeStreamConfigurations()
>  		 * stream configurations map, by testing the image resolutions.
>  		 */
>  		formatsMap_[androidFormat] = mappedFormat;
> +		LOG(HAL, Debug) << "Mapped Android format "
> +				<< camera3Format.name << " to "
> +				<< mappedFormat.toString();
>  
>  		for (const Size &res : cameraResolutions) {
>  			cfg.pixelFormat = mappedFormat;
>  			cfg.size = res;
>  
> +			std::stringstream ss;
> +			ss << "Testing " << cfg.toString();

nit: I would drop the std:;stringstream and either store the common part 
in a std::string or simply duplicated in both LOG() lines as I think it 
makes the code easier to read.

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

> +
>  			CameraConfiguration::Status status = cameraConfig->validate();
> -			if (status != CameraConfiguration::Valid)
> +			if (status != CameraConfiguration::Valid) {
> +				ss << " not supported";
> +				LOG(HAL, Debug) << ss.str();
>  				continue;
> +			}
> +
> +			ss << " supported";
> +			LOG(HAL, Debug) << ss.str();
>  
>  			streamConfigurations_.push_back({ res, androidFormat });
>  
> -- 
> 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:04 a.m. UTC | #2 
On Thu, Sep 10, 2020 at 7:36 PM Niklas Söderlund
<niklas.soderlund@ragnatech.se> wrote:
>
> Hi Jacopo,
>
> Thanks for your patch.
>
> On 2020-09-08 15:41:34 +0200, Jacopo Mondi wrote:
> > Add debug printouts to the CameraDevice::initializeStreamConfigurations()
> > function that help to follow the process of building the stream
> > configurations map.
> >
> > 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 | 23 ++++++++++++++++++++++-
> >  1 file changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 1b2e12d6d33c..17b5fd5f59eb 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -363,12 +363,19 @@ int CameraDevice::initializeStreamConfigurations()
> >               const std::vector<PixelFormat> &libcameraFormats =
> >                       camera3Format.libcameraFormats;
> >
> > +             LOG(HAL, Debug) << "Trying to map Android format "
> > +                             << camera3Format.name;
> > +
> >               /*
> >                * JPEG is always supported, either produced directly by the
> >                * camera, or encoded in the HAL.
> >                */
> >               if (androidFormat == HAL_PIXEL_FORMAT_BLOB) {
> >                       formatsMap_[androidFormat] = formats::MJPEG;
> > +                     LOG(HAL, Debug) << "Mapped Android format "
> > +                                     << camera3Format.name << " to "
> > +                                     << formats::MJPEG.toString()
> > +                                     << " (fixed mapping)";
> >                       continue;
> >               }
> >
> > @@ -379,6 +386,8 @@ int CameraDevice::initializeStreamConfigurations()
> >               PixelFormat mappedFormat;
> >               for (const PixelFormat &pixelFormat : libcameraFormats) {
> >
> > +                     LOG(HAL, Debug) << "Testing " << pixelFormat.toString();
> > +
> >                       /*
> >                        * The stream configuration size can be adjusted,
> >                        * not the pixel format.
> > @@ -414,14 +423,26 @@ int CameraDevice::initializeStreamConfigurations()
> >                * stream configurations map, by testing the image resolutions.
> >                */
> >               formatsMap_[androidFormat] = mappedFormat;
> > +             LOG(HAL, Debug) << "Mapped Android format "
> > +                             << camera3Format.name << " to "
> > +                             << mappedFormat.toString();
> >
> >               for (const Size &res : cameraResolutions) {
> >                       cfg.pixelFormat = mappedFormat;
> >                       cfg.size = res;
> >
> > +                     std::stringstream ss;
> > +                     ss << "Testing " << cfg.toString();
>
> nit: I would drop the std:;stringstream and either store the common part
> in a std::string or simply duplicated in both LOG() lines as I think it
> makes the code easier to read.
>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

+1
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
>
> > +
> >                       CameraConfiguration::Status status = cameraConfig->validate();
> > -                     if (status != CameraConfiguration::Valid)
> > +                     if (status != CameraConfiguration::Valid) {
> > +                             ss << " not supported";
> > +                             LOG(HAL, Debug) << ss.str();
> >                               continue;
> > +                     }
> > +
> > +                     ss << " supported";
> > +                     LOG(HAL, Debug) << ss.str();
> >
> >                       streamConfigurations_.push_back({ res, androidFormat });
> >
> > --
> > 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 1b2e12d6d33c..17b5fd5f59eb 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -363,12 +363,19 @@  int CameraDevice::initializeStreamConfigurations()
 		const std::vector<PixelFormat> &libcameraFormats =
 			camera3Format.libcameraFormats;
 
+		LOG(HAL, Debug) << "Trying to map Android format "
+				<< camera3Format.name;
+
 		/*
 		 * JPEG is always supported, either produced directly by the
 		 * camera, or encoded in the HAL.
 		 */
 		if (androidFormat == HAL_PIXEL_FORMAT_BLOB) {
 			formatsMap_[androidFormat] = formats::MJPEG;
+			LOG(HAL, Debug) << "Mapped Android format "
+					<< camera3Format.name << " to "
+					<< formats::MJPEG.toString()
+					<< " (fixed mapping)";
 			continue;
 		}
 
@@ -379,6 +386,8 @@  int CameraDevice::initializeStreamConfigurations()
 		PixelFormat mappedFormat;
 		for (const PixelFormat &pixelFormat : libcameraFormats) {
 
+			LOG(HAL, Debug) << "Testing " << pixelFormat.toString();
+
 			/*
 			 * The stream configuration size can be adjusted,
 			 * not the pixel format.
@@ -414,14 +423,26 @@  int CameraDevice::initializeStreamConfigurations()
 		 * stream configurations map, by testing the image resolutions.
 		 */
 		formatsMap_[androidFormat] = mappedFormat;
+		LOG(HAL, Debug) << "Mapped Android format "
+				<< 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)
+			if (status != CameraConfiguration::Valid) {
+				ss << " not supported";
+				LOG(HAL, Debug) << ss.str();
 				continue;
+			}
+
+			ss << " supported";
+			LOG(HAL, Debug) << ss.str();
 
 			streamConfigurations_.push_back({ res, androidFormat });