[libcamera-devel] [PATCH] pipeline: rpi: Add some useful logging messages

[Laurent Pinchart]

On Mon, Nov 06, 2023 at 11:47:28AM +0000, Kieran Bingham via libcamera-devel wrote:
> Quoting Naushir Patuck via libcamera-devel (2023-10-24 14:27:46)
> > Add a bunch of logging messages that have come in handy debugging
> > various issues with the pipeline handler code.
> 
> No specific objection to these, but some of them sound more like
> 'tracepoints'. Maybe it's worth looking at that more in the future ...

Indeed. Naush, have you looked at tracepoints ? I'll merge the patch in
the meantime, but for this kind of debugging features, you may want to
switch to tracing at some point. Or if the tracing infrastructure
doesn't provide what you need, I would be interested in knowing why, in
order to improve it.

> but for now I'd say:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> >  src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 11 ++++++++++-
> >  src/libcamera/pipeline/rpi/vc4/vc4.cpp              |  3 +++
> >  2 files changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > index 9f788c9d1bc2..8e1cbbe67a52 100644
> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > @@ -731,7 +731,8 @@ int PipelineHandlerBase::queueRequestDevice(Camera *camera, Request *request)
> >         if (!data->isRunning())
> >                 return -EINVAL;
> >  
> > -       LOG(RPI, Debug) << "queueRequestDevice: New request.";
> > +       LOG(RPI, Debug) << "queueRequestDevice: New request sequence: "
> > +                       << request->sequence();
> >  
> >         /* Push all buffers supplied in the Request to the respective streams. */
> >         for (auto stream : data->streams_) {
> > @@ -1436,6 +1437,8 @@ void CameraData::handleStreamBuffer(FrameBuffer *buffer, RPi::Stream *stream)
> >                  * Tag the buffer as completed, returning it to the
> >                  * application.
> >                  */
> > +               LOG(RPI, Debug) << "Completing request buffer for stream "
> > +                               << stream->name();
> >                 pipe()->completeBuffer(request, buffer);
> >         } else {
> >                 /*
> > @@ -1444,6 +1447,8 @@ void CameraData::handleStreamBuffer(FrameBuffer *buffer, RPi::Stream *stream)
> >                  * unconditionally for internal streams), or there is no pending
> >                  * request, so we can recycle it.
> >                  */
> > +               LOG(RPI, Debug) << "Returning buffer to stream "
> > +                               << stream->name();
> >                 stream->returnBuffer(buffer);
> >         }
> >  }
> > @@ -1487,6 +1492,9 @@ void CameraData::checkRequestCompleted()
> >                 if (state_ != State::IpaComplete)
> >                         return;
> >  
> > +               LOG(RPI, Debug) << "Completing request sequence: "
> > +                               << request->sequence();
> > +
> >                 pipe()->completeRequest(request);
> >                 requestQueue_.pop();
> >                 requestCompleted = true;
> > @@ -1499,6 +1507,7 @@ void CameraData::checkRequestCompleted()
> >         if (state_ == State::IpaComplete &&
> >             ((ispOutputCount_ == ispOutputTotal_ && dropFrameCount_) ||
> >              requestCompleted)) {
> > +               LOG(RPI, Debug) << "Going into Idle state";
> >                 state_ = State::Idle;
> >                 if (dropFrameCount_) {
> >                         dropFrameCount_--;
> > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> > index 616e0bc95021..816de14e5b22 100644
> > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> > @@ -283,6 +283,9 @@ int PipelineHandlerVc4::prepareBuffers(Camera *camera)
> >                         numBuffers = 1;
> >                 }
> >  
> > +               LOG(RPI, Debug) << "Preparing " << numBuffers
> > +                               << " buffers for stream " << stream->name();
> > +
> >                 ret = stream->prepareBuffers(numBuffers);
> >                 if (ret < 0)
> >                         return ret;

-- 
Regards,

Laurent Pinchart