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

[Naushir Patuck]

On Tue, 21 Nov 2023 at 09:50, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> 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.

To be honest, I have not looked into tracepoints at all so it might
indeed be a replacement for these log messages.  I will have a look
when I get a chance.

Regards,
Naush

>
> > 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