[libcamera-devel] [PATCH 02/30] libcamera: Remove buffer index from logging

[Jacopo Mondi]

Hi Niklas,

On Wed, Nov 27, 2019 at 12:35:52AM +0100, Niklas Söderlund wrote:
> The buffer index is a V4L2 concept that will be hidden from users when
> the switch to FrameBuffer is complete, in preparation remove it from

What's a FrameBuffer ? I guess I'll find it out, but it's mentioned in
a few commit messages before it's actually introdduced...

> logging.
>
> Keep and move one debug log message where the index is available as the
> V4L2 buffer is being dequeued for the video device and it's useful when
> debugging.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/cam/capture.cpp                      | 3 +--
>  src/libcamera/v4l2_videodevice.cpp       | 3 +--
>  src/qcam/main_window.cpp                 | 3 +--
>  test/v4l2_videodevice/buffer_sharing.cpp | 8 ++++----
>  test/v4l2_videodevice/capture_async.cpp  | 2 +-
>  test/v4l2_videodevice/v4l2_m2mdevice.cpp | 4 ++--
>  6 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> index e665d819fb777a90..4b65b1d0a2dbed35 100644
> --- a/src/cam/capture.cpp
> +++ b/src/cam/capture.cpp
> @@ -155,7 +155,6 @@ void Capture::requestComplete(Request *request)
>  		const std::string &name = streamName_[stream];
>
>  		info << " " << name
> -		     << " (" << buffer->index() << ")"
>  		     << " seq: " << std::setw(6) << std::setfill('0') << buffer->sequence()
>  		     << " bytesused: " << buffer->bytesused();
>
> @@ -182,7 +181,7 @@ void Capture::requestComplete(Request *request)
>
>  		std::unique_ptr<Buffer> newBuffer = stream->createBuffer(index);
>  		if (!newBuffer) {
> -			std::cerr << "Can't create buffer " << index << std::endl;
> +			std::cerr << "Can't create buffer" << std::endl;
>  			return;
>  		}
>
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 992130751286994c..dbb5c3982334e243 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -1103,6 +1103,7 @@ Buffer *V4L2VideoDevice::dequeueBuffer()
>  	}
>
>  	ASSERT(buf.index < bufferPool_->count());
> +	LOG(V4L2, Debug) << "Buffer " << buf.index << " is available";
>
>  	auto it = queuedBuffers_.find(buf.index);
>  	Buffer *buffer = it->second;
> @@ -1138,8 +1139,6 @@ void V4L2VideoDevice::bufferAvailable(EventNotifier *notifier)
>  	if (!buffer)
>  		return;
>
> -	LOG(V4L2, Debug) << "Buffer " << buffer->index() << " is available";
> -
>  	/* Notify anyone listening to the device. */
>  	bufferReady.emit(buffer);
>  }
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index cca7365ae75687f9..0c7ca61ac12ec41c 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -264,7 +264,6 @@ void MainWindow::requestComplete(Request *request)
>  	lastBufferTime_ = buffer->timestamp();
>
>  	std::cout << "seq: " << std::setw(6) << std::setfill('0') << buffer->sequence()
> -		  << " buf: " << buffer->index()
>  		  << " bytesused: " << buffer->bytesused()
>  		  << " timestamp: " << buffer->timestamp()
>  		  << " fps: " << std::fixed << std::setprecision(2) << fps
> @@ -285,7 +284,7 @@ void MainWindow::requestComplete(Request *request)
>
>  		std::unique_ptr<Buffer> newBuffer = stream->createBuffer(index);
>  		if (!newBuffer) {
> -			std::cerr << "Can't create buffer " << index << std::endl;
> +			std::cerr << "Can't create buffer" << std::endl;
>  			return;
>  		}
>
> diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp
> index 1629f34cfa6c79fe..d02c391b95933922 100644
> --- a/test/v4l2_videodevice/buffer_sharing.cpp
> +++ b/test/v4l2_videodevice/buffer_sharing.cpp
> @@ -92,8 +92,8 @@ protected:
>
>  	void captureBufferReady(Buffer *buffer)
>  	{
> -		std::cout << "Received capture buffer: " << buffer->index()
> -			  << " sequence " << buffer->sequence() << std::endl;
> +		std::cout << "Received capture buffer  sequence "
                                                      ^- Double space
I would drop 'sequence' completely

> +			  << buffer->sequence() << std::endl;
>
>  		if (buffer->status() != Buffer::BufferSuccess)
>  			return;
> @@ -104,8 +104,8 @@ protected:
>
>  	void outputBufferReady(Buffer *buffer)
>  	{
> -		std::cout << "Received output buffer: " << buffer->index()
> -			  << " sequence " << buffer->sequence() << std::endl;
> +		std::cout << "Received output buffer sequence "
> +			  << buffer->sequence() << std::endl;
>
>  		if (buffer->status() != Buffer::BufferSuccess)
>  			return;
> diff --git a/test/v4l2_videodevice/capture_async.cpp b/test/v4l2_videodevice/capture_async.cpp
> index 442a4fe56eace57e..0bc0067c50909c9d 100644
> --- a/test/v4l2_videodevice/capture_async.cpp
> +++ b/test/v4l2_videodevice/capture_async.cpp
> @@ -22,7 +22,7 @@ public:
>
>  	void receiveBuffer(Buffer *buffer)
>  	{
> -		std::cout << "Received buffer " << buffer->index() << std::endl;
> +		std::cout << "Received buffer" << std::endl;

Without the index this should be "Buffer received"
Here and below

>  		frames++;
>
>  		/* Requeue the buffer for further use. */
> diff --git a/test/v4l2_videodevice/v4l2_m2mdevice.cpp b/test/v4l2_videodevice/v4l2_m2mdevice.cpp
> index 4d3644c2d28792f1..442bcac5dc49cc59 100644
> --- a/test/v4l2_videodevice/v4l2_m2mdevice.cpp
> +++ b/test/v4l2_videodevice/v4l2_m2mdevice.cpp
> @@ -31,7 +31,7 @@ public:
>
>  	void outputBufferComplete(Buffer *buffer)
>  	{
> -		cout << "Received output buffer " << buffer->index() << endl;
> +		cout << "Received output buffer" << endl;
>
>  		outputFrames_++;
>
> @@ -41,7 +41,7 @@ public:
>
>  	void receiveCaptureBuffer(Buffer *buffer)
>  	{
> -		cout << "Received capture buffer " << buffer->index() << endl;
> +		cout << "Received capture buffer" << endl;
>

All minors, please add
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

>  		captureFrames_++;
>
> --
> 2.24.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20191127/3248d59c/attachment.sig>