[libcamera-devel,8/8] android: camera_device: Add stream mapping log

Message ID 20200909155457.153907-9-jacopo@jmondi.org
State Accepted
Headers show
Series
  • android: camera_device: Allocate internal buffers
Related show

Commit Message

Jacopo Mondi Sept. 9, 2020, 3:54 p.m. UTC
To ease following how Android stream gets mapped onto libcamera ones
add a (quite verbose) printout before queueing a request to libcamera.

The output looks like:
 0 -  (320x240)[0x00000022] -> (320x240)[NV12] (external)
 1 -  (640x480)[0x00000021] -> (640x480)[NV12] (internal)

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/android/camera_device.cpp | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

Comments

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

Thanks for your patch.

On 2020-09-09 17:54:57 +0200, Jacopo Mondi wrote:
> To ease following how Android stream gets mapped onto libcamera ones
> add a (quite verbose) printout before queueing a request to libcamera.
> 
> The output looks like:
>  0 -  (320x240)[0x00000022] -> (320x240)[NV12] (external)
>  1 -  (640x480)[0x00000021] -> (640x480)[NV12] (internal)
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

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

> ---
>  src/android/camera_device.cpp | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index f94b313581e7..811f0a66cf2f 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1419,9 +1419,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  	Request *request =
>  		camera_->createRequest(reinterpret_cast<uint64_t>(descriptor));
>  
> +	LOG(HAL, Debug) << "Queueing Request to libcamera with "
> +			<< descriptor->numBuffers << " streams";
>  	for (unsigned int i = 0; i < descriptor->numBuffers; ++i) {
> +		camera3_stream *camera3Stream = camera3Buffers[i].stream;
>  		CameraStream *cameraStream =
> -			static_cast<CameraStream *>(camera3Buffers[i].stream->priv);
> +			static_cast<CameraStream *>(camera3Stream->priv);
>  		const StreamConfiguration &config = config_->at(cameraStream->index());
>  		Stream *stream = config.stream();
>  
> @@ -1432,9 +1435,18 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  		descriptor->buffers[i].stream = camera3Buffers[i].stream;
>  		descriptor->buffers[i].buffer = camera3Buffers[i].buffer;
>  
> -		/* Mapped streams don't need to be added to the Request. */
> -		if (cameraStream->type() == CameraStream::Type::Mapped)
> +		std::stringstream ss;
> +		ss << i << " - (";
> +		ss << camera3Stream->width << "x" << camera3Stream->height << ")"
> +		   << "[" << utils::hex(camera3Stream->format) << "] -> "
> +		   << "(" << config.size.toString() << ")["
> +		   << config.pixelFormat.toString() << "]";
> +
> +		/* Mapped streams don't need buffers added to the Request. */
> +		if (cameraStream->type() == CameraStream::Type::Mapped) {
> +			LOG(HAL, Debug) << ss.str() << " (mapped)";
>  			continue;
> +		}
>  
>  		FrameBuffer *buffer;
>  		if (cameraStream->type() == CameraStream::Type::Direct) {
> @@ -1446,6 +1458,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  			 */
>  			buffer = createFrameBuffer(*camera3Buffers[i].buffer);
>  			descriptor->frameBuffers.emplace_back(buffer);
> +			ss << " (external)";
>  
>  		} else {
>  			/*
> @@ -1458,7 +1471,11 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  			 * allocator once it has been processed.
>  			 */
>  			buffer = allocator_.getBuffer(stream);
> +			ss << " (internal)";
>  		}
> +
> +		LOG(HAL, Debug) << ss.str();
> +
>  		if (!buffer) {
>  			LOG(HAL, Error) << "Failed to create buffer";
>  			delete request;
> -- 
> 2.28.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Hirokazu Honda Sept. 11, 2020, 8:55 a.m. UTC | #2
On Thu, Sep 10, 2020 at 8:36 PM Niklas Söderlund
<niklas.soderlund@ragnatech.se> wrote:
>
> Hi Jacopo,
>
> Thanks for your patch.
>
> On 2020-09-09 17:54:57 +0200, Jacopo Mondi wrote:
> > To ease following how Android stream gets mapped onto libcamera ones
> > add a (quite verbose) printout before queueing a request to libcamera.
> >
> > The output looks like:
> >  0 -  (320x240)[0x00000022] -> (320x240)[NV12] (external)
> >  1 -  (640x480)[0x00000021] -> (640x480)[NV12] (internal)
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
>
> > ---
> >  src/android/camera_device.cpp | 23 ++++++++++++++++++++---
> >  1 file changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index f94b313581e7..811f0a66cf2f 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -1419,9 +1419,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> >       Request *request =
> >               camera_->createRequest(reinterpret_cast<uint64_t>(descriptor));
> >
> > +     LOG(HAL, Debug) << "Queueing Request to libcamera with "
> > +                     << descriptor->numBuffers << " streams";
> >       for (unsigned int i = 0; i < descriptor->numBuffers; ++i) {
> > +             camera3_stream *camera3Stream = camera3Buffers[i].stream;
> >               CameraStream *cameraStream =
> > -                     static_cast<CameraStream *>(camera3Buffers[i].stream->priv);
> > +                     static_cast<CameraStream *>(camera3Stream->priv);
> >               const StreamConfiguration &config = config_->at(cameraStream->index());
> >               Stream *stream = config.stream();
> >
> > @@ -1432,9 +1435,18 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> >               descriptor->buffers[i].stream = camera3Buffers[i].stream;
> >               descriptor->buffers[i].buffer = camera3Buffers[i].buffer;
> >
> > -             /* Mapped streams don't need to be added to the Request. */
> > -             if (cameraStream->type() == CameraStream::Type::Mapped)
> > +             std::stringstream ss;
> > +             ss << i << " - (";
> > +             ss << camera3Stream->width << "x" << camera3Stream->height << ")"
> > +                << "[" << utils::hex(camera3Stream->format) << "] -> "
> > +                << "(" << config.size.toString() << ")["
> > +                << config.pixelFormat.toString() << "]";
> > +

nit: ss is used only in debug mode. Can we execute this process in
debug mode only as well?

Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
> > +             /* Mapped streams don't need buffers added to the Request. */
> > +             if (cameraStream->type() == CameraStream::Type::Mapped) {
> > +                     LOG(HAL, Debug) << ss.str() << " (mapped)";
> >                       continue;
> > +             }
> >
> >               FrameBuffer *buffer;
> >               if (cameraStream->type() == CameraStream::Type::Direct) {
> > @@ -1446,6 +1458,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> >                        */
> >                       buffer = createFrameBuffer(*camera3Buffers[i].buffer);
> >                       descriptor->frameBuffers.emplace_back(buffer);
> > +                     ss << " (external)";
> >
> >               } else {
> >                       /*
> > @@ -1458,7 +1471,11 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> >                        * allocator once it has been processed.
> >                        */
> >                       buffer = allocator_.getBuffer(stream);
> > +                     ss << " (internal)";
> >               }
> > +
> > +             LOG(HAL, Debug) << ss.str();
> > +
> >               if (!buffer) {
> >                       LOG(HAL, Error) << "Failed to create buffer";
> >                       delete request;
> > --
> > 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
Jacopo Mondi Sept. 11, 2020, 9:08 a.m. UTC | #3
Hi Hiro,

On Fri, Sep 11, 2020 at 05:55:55PM +0900, Hirokazu Honda wrote:
> On Thu, Sep 10, 2020 at 8:36 PM Niklas Söderlund
> <niklas.soderlund@ragnatech.se> wrote:
> >
> > Hi Jacopo,
> >
> > Thanks for your patch.
> >
> > On 2020-09-09 17:54:57 +0200, Jacopo Mondi wrote:
> > > To ease following how Android stream gets mapped onto libcamera ones
> > > add a (quite verbose) printout before queueing a request to libcamera.
> > >
> > > The output looks like:
> > >  0 -  (320x240)[0x00000022] -> (320x240)[NV12] (external)
> > >  1 -  (640x480)[0x00000021] -> (640x480)[NV12] (internal)
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> >
> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> >
> > > ---
> > >  src/android/camera_device.cpp | 23 ++++++++++++++++++++---
> > >  1 file changed, 20 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > > index f94b313581e7..811f0a66cf2f 100644
> > > --- a/src/android/camera_device.cpp
> > > +++ b/src/android/camera_device.cpp
> > > @@ -1419,9 +1419,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> > >       Request *request =
> > >               camera_->createRequest(reinterpret_cast<uint64_t>(descriptor));
> > >
> > > +     LOG(HAL, Debug) << "Queueing Request to libcamera with "
> > > +                     << descriptor->numBuffers << " streams";
> > >       for (unsigned int i = 0; i < descriptor->numBuffers; ++i) {
> > > +             camera3_stream *camera3Stream = camera3Buffers[i].stream;
> > >               CameraStream *cameraStream =
> > > -                     static_cast<CameraStream *>(camera3Buffers[i].stream->priv);
> > > +                     static_cast<CameraStream *>(camera3Stream->priv);
> > >               const StreamConfiguration &config = config_->at(cameraStream->index());
> > >               Stream *stream = config.stream();
> > >
> > > @@ -1432,9 +1435,18 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> > >               descriptor->buffers[i].stream = camera3Buffers[i].stream;
> > >               descriptor->buffers[i].buffer = camera3Buffers[i].buffer;
> > >
> > > -             /* Mapped streams don't need to be added to the Request. */
> > > -             if (cameraStream->type() == CameraStream::Type::Mapped)
> > > +             std::stringstream ss;
> > > +             ss << i << " - (";
> > > +             ss << camera3Stream->width << "x" << camera3Stream->height << ")"
> > > +                << "[" << utils::hex(camera3Stream->format) << "] -> "
> > > +                << "(" << config.size.toString() << ")["
> > > +                << config.pixelFormat.toString() << "]";
> > > +
>
> nit: ss is used only in debug mode. Can we execute this process in
> debug mode only as well?

Do you mean assembling the debug string only if a certain debug level
is enabled ? I'm afraid that's not possible, libcamera debug and
logging system implementes a run-time selectable log filtering system,
we don't have anything that allows us to enable/disable debug at compile
time afaict.

>
> Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
> > > +             /* Mapped streams don't need buffers added to the Request. */
> > > +             if (cameraStream->type() == CameraStream::Type::Mapped) {
> > > +                     LOG(HAL, Debug) << ss.str() << " (mapped)";
> > >                       continue;
> > > +             }
> > >
> > >               FrameBuffer *buffer;
> > >               if (cameraStream->type() == CameraStream::Type::Direct) {
> > > @@ -1446,6 +1458,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> > >                        */
> > >                       buffer = createFrameBuffer(*camera3Buffers[i].buffer);
> > >                       descriptor->frameBuffers.emplace_back(buffer);
> > > +                     ss << " (external)";
> > >
> > >               } else {
> > >                       /*
> > > @@ -1458,7 +1471,11 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> > >                        * allocator once it has been processed.
> > >                        */
> > >                       buffer = allocator_.getBuffer(stream);
> > > +                     ss << " (internal)";
> > >               }
> > > +
> > > +             LOG(HAL, Debug) << ss.str();
> > > +
> > >               if (!buffer) {
> > >                       LOG(HAL, Error) << "Failed to create buffer";
> > >                       delete request;
> > > --
> > > 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
Hirokazu Honda Sept. 11, 2020, 10:17 a.m. UTC | #4
Hi Jacopo.

On Fri, Sep 11, 2020 at 6:04 PM Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> Hi Hiro,
>
> On Fri, Sep 11, 2020 at 05:55:55PM +0900, Hirokazu Honda wrote:
> > On Thu, Sep 10, 2020 at 8:36 PM Niklas Söderlund
> > <niklas.soderlund@ragnatech.se> wrote:
> > >
> > > Hi Jacopo,
> > >
> > > Thanks for your patch.
> > >
> > > On 2020-09-09 17:54:57 +0200, Jacopo Mondi wrote:
> > > > To ease following how Android stream gets mapped onto libcamera ones
> > > > add a (quite verbose) printout before queueing a request to libcamera.
> > > >
> > > > The output looks like:
> > > >  0 -  (320x240)[0x00000022] -> (320x240)[NV12] (external)
> > > >  1 -  (640x480)[0x00000021] -> (640x480)[NV12] (internal)
> > > >
> > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > >
> > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > >
> > > > ---
> > > >  src/android/camera_device.cpp | 23 ++++++++++++++++++++---
> > > >  1 file changed, 20 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > > > index f94b313581e7..811f0a66cf2f 100644
> > > > --- a/src/android/camera_device.cpp
> > > > +++ b/src/android/camera_device.cpp
> > > > @@ -1419,9 +1419,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> > > >       Request *request =
> > > >               camera_->createRequest(reinterpret_cast<uint64_t>(descriptor));
> > > >
> > > > +     LOG(HAL, Debug) << "Queueing Request to libcamera with "
> > > > +                     << descriptor->numBuffers << " streams";
> > > >       for (unsigned int i = 0; i < descriptor->numBuffers; ++i) {
> > > > +             camera3_stream *camera3Stream = camera3Buffers[i].stream;
> > > >               CameraStream *cameraStream =
> > > > -                     static_cast<CameraStream *>(camera3Buffers[i].stream->priv);
> > > > +                     static_cast<CameraStream *>(camera3Stream->priv);
> > > >               const StreamConfiguration &config = config_->at(cameraStream->index());
> > > >               Stream *stream = config.stream();
> > > >
> > > > @@ -1432,9 +1435,18 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> > > >               descriptor->buffers[i].stream = camera3Buffers[i].stream;
> > > >               descriptor->buffers[i].buffer = camera3Buffers[i].buffer;
> > > >
> > > > -             /* Mapped streams don't need to be added to the Request. */
> > > > -             if (cameraStream->type() == CameraStream::Type::Mapped)
> > > > +             std::stringstream ss;
> > > > +             ss << i << " - (";
> > > > +             ss << camera3Stream->width << "x" << camera3Stream->height << ")"
> > > > +                << "[" << utils::hex(camera3Stream->format) << "] -> "
> > > > +                << "(" << config.size.toString() << ")["
> > > > +                << config.pixelFormat.toString() << "]";
> > > > +
> >
> > nit: ss is used only in debug mode. Can we execute this process in
> > debug mode only as well?
>
> Do you mean assembling the debug string only if a certain debug level
> is enabled ? I'm afraid that's not possible, libcamera debug and
> logging system implementes a run-time selectable log filtering system,
> we don't have anything that allows us to enable/disable debug at compile
> time afaict.
>

Thanks for info. Got it.
> >
> > Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
> > > > +             /* Mapped streams don't need buffers added to the Request. */
> > > > +             if (cameraStream->type() == CameraStream::Type::Mapped) {
> > > > +                     LOG(HAL, Debug) << ss.str() << " (mapped)";
> > > >                       continue;
> > > > +             }
> > > >
> > > >               FrameBuffer *buffer;
> > > >               if (cameraStream->type() == CameraStream::Type::Direct) {
> > > > @@ -1446,6 +1458,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> > > >                        */
> > > >                       buffer = createFrameBuffer(*camera3Buffers[i].buffer);
> > > >                       descriptor->frameBuffers.emplace_back(buffer);
> > > > +                     ss << " (external)";
> > > >
> > > >               } else {
> > > >                       /*
> > > > @@ -1458,7 +1471,11 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> > > >                        * allocator once it has been processed.
> > > >                        */
> > > >                       buffer = allocator_.getBuffer(stream);
> > > > +                     ss << " (internal)";
> > > >               }
> > > > +
> > > > +             LOG(HAL, Debug) << ss.str();
> > > > +
> > > >               if (!buffer) {
> > > >                       LOG(HAL, Error) << "Failed to create buffer";
> > > >                       delete request;
> > > > --
> > > > 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 --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index f94b313581e7..811f0a66cf2f 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -1419,9 +1419,12 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 	Request *request =
 		camera_->createRequest(reinterpret_cast<uint64_t>(descriptor));
 
+	LOG(HAL, Debug) << "Queueing Request to libcamera with "
+			<< descriptor->numBuffers << " streams";
 	for (unsigned int i = 0; i < descriptor->numBuffers; ++i) {
+		camera3_stream *camera3Stream = camera3Buffers[i].stream;
 		CameraStream *cameraStream =
-			static_cast<CameraStream *>(camera3Buffers[i].stream->priv);
+			static_cast<CameraStream *>(camera3Stream->priv);
 		const StreamConfiguration &config = config_->at(cameraStream->index());
 		Stream *stream = config.stream();
 
@@ -1432,9 +1435,18 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 		descriptor->buffers[i].stream = camera3Buffers[i].stream;
 		descriptor->buffers[i].buffer = camera3Buffers[i].buffer;
 
-		/* Mapped streams don't need to be added to the Request. */
-		if (cameraStream->type() == CameraStream::Type::Mapped)
+		std::stringstream ss;
+		ss << i << " - (";
+		ss << camera3Stream->width << "x" << camera3Stream->height << ")"
+		   << "[" << utils::hex(camera3Stream->format) << "] -> "
+		   << "(" << config.size.toString() << ")["
+		   << config.pixelFormat.toString() << "]";
+
+		/* Mapped streams don't need buffers added to the Request. */
+		if (cameraStream->type() == CameraStream::Type::Mapped) {
+			LOG(HAL, Debug) << ss.str() << " (mapped)";
 			continue;
+		}
 
 		FrameBuffer *buffer;
 		if (cameraStream->type() == CameraStream::Type::Direct) {
@@ -1446,6 +1458,7 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 			 */
 			buffer = createFrameBuffer(*camera3Buffers[i].buffer);
 			descriptor->frameBuffers.emplace_back(buffer);
+			ss << " (external)";
 
 		} else {
 			/*
@@ -1458,7 +1471,11 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 			 * allocator once it has been processed.
 			 */
 			buffer = allocator_.getBuffer(stream);
+			ss << " (internal)";
 		}
+
+		LOG(HAL, Debug) << ss.str();
+
 		if (!buffer) {
 			LOG(HAL, Error) << "Failed to create buffer";
 			delete request;