[libcamera-devel] [PATCH 3/9] libcamera: pipeline: raspberrypi: Add some debug logging

[Naushir Patuck]

Hi Niklas,


On Mon, 13 Jul 2020 at 12:39, Niklas Söderlund
<niklas.soderlund at ragnatech.se> wrote:
>
> Hi Naushir,
>
> Thanks for your work.
>
> On 2020-07-13 09:47:22 +0100, Naushir Patuck wrote:
> > No functional changes, only added some more trace points.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index 8b6f578f..65c8557d 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -1175,6 +1175,11 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)
> >       if (state_ == State::Stopped)
> >               return;
> >
> > +     LOG(RPI, Debug) << "Stream ISP Input buffer complete"
> > +                     << ", buffer id " << buffer->cookie()
>
> This worries me a bit. For buffers coming from the application the
> cookie is controlled by the application. This means that pipeline
> handlers can't set the cookie for any buffer it receives in a Request.
> Maybe we need to create something in libcamera core to allow
> pipelines/IPA to have an internal cookie value for buffers?
>

Actually I did not realise that the application was meant to use the
cookie.  I thought it was a pipeline handler thing to identify
buffers.

> Or maybe I'm misunderstanding the code and cookies are only used for
> buffers that are not seen by the applications at all (such as ISP
> statistics and parameters)?

This is correct, it was only used for internal buffers, but I relied
on it for this change as well.  This is not a problem, I use it
currently to identify buffers passed to/from the IPA, and I can use a
simple linear index to do the same.

>
> > +                     << ", timestamp: " << buffer->metadata().timestamp;
> > +
> > +     /* The ISP input buffer gets re-queued into Unicam. */
> >       handleStreamBuffer(buffer, &unicam_[Unicam::Image]);
> >       handleState();
> >  }
> > @@ -1339,6 +1344,8 @@ void RPiCameraData::checkRequestCompleted()
> >               pipe_->completeRequest(camera_, request);
> >               requestQueue_.pop_front();
> >               requestCompleted = true;
> > +
> > +             LOG(RPI, Debug) << "Request is complete";
> >       }
> >
> >       /*
> > @@ -1477,7 +1484,7 @@ FrameBuffer *RPiCameraData::updateQueue(std::queue<FrameBuffer *> &q, uint64_t t
> >               if (b->metadata().timestamp < timestamp) {
> >                       q.pop();
> >                       dev->queueBuffer(b);
> > -                     LOG(RPI, Error) << "Dropping input frame!";
> > +                     LOG(RPI, Warning) << "Dropping input frame!";
> >               } else if (b->metadata().timestamp == timestamp) {
> >                       /* The calling function will pop the item from the queue. */
> >                       return b;
> > --
> > 2.25.1
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund

Regards,
Naush