| Message ID | 20200720091311.805092-4-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush, On 20/07/2020 10:13, Naushir Patuck wrote: > No functional changes, only added some more trace points. Well, that could be classed as a form of a functional change, At least that's what Dr Heisenbug told me ... :-D > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> Still, this looks fine to me: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.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 d3639ce1..35f14dda 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -1160,6 +1160,11 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer) > if (state_ == State::Stopped) > return; > > + LOG(RPI, Debug) << "Stream ISP Input buffer complete" > + << ", buffer id " << buffer->cookie() > + << ", timestamp: " << buffer->metadata().timestamp; > + > + /* The ISP input buffer gets re-queued into Unicam. */ > handleStreamBuffer(buffer, &unicam_[Unicam::Image]); > handleState(); > } > @@ -1324,6 +1329,8 @@ void RPiCameraData::checkRequestCompleted() > pipe_->completeRequest(camera_, request); > requestQueue_.pop_front(); > requestCompleted = true; > + > + LOG(RPI, Debug) << "Request is complete"; > } > > /* > @@ -1462,7 +1469,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; >
Hi Naush, Thank you for the patch. On Mon, Jul 20, 2020 at 10:13:05AM +0100, Naushir Patuck wrote: > No functional changes, only added some more trace points. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > 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 d3639ce1..35f14dda 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -1160,6 +1160,11 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer) > if (state_ == State::Stopped) > return; > > + LOG(RPI, Debug) << "Stream ISP Input buffer complete" Did you mean s/Stream ISP/ISP/ ? > + << ", buffer id " << buffer->cookie() Maybe s/id/cookie/ ? > + << ", timestamp: " << buffer->metadata().timestamp; > + > + /* The ISP input buffer gets re-queued into Unicam. */ > handleStreamBuffer(buffer, &unicam_[Unicam::Image]); > handleState(); > } > @@ -1324,6 +1329,8 @@ void RPiCameraData::checkRequestCompleted() > pipe_->completeRequest(camera_, request); > requestQueue_.pop_front(); > requestCompleted = true; > + > + LOG(RPI, Debug) << "Request is complete"; Would it make sense to add this to Request::complete() instead ? And maybe print the request cookie ? > } > > /* > @@ -1462,7 +1469,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;
Hi Laurent, Thank you for the review. On Wed, 22 Jul 2020 at 15:58, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Naush, > > Thank you for the patch. > > On Mon, Jul 20, 2020 at 10:13:05AM +0100, Naushir Patuck wrote: > > No functional changes, only added some more trace points. > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > 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 d3639ce1..35f14dda 100644 > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > @@ -1160,6 +1160,11 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer) > > if (state_ == State::Stopped) > > return; > > > > + LOG(RPI, Debug) << "Stream ISP Input buffer complete" > > Did you mean s/Stream ISP/ISP/ ? I did mean to use the stream qualifier, in order to be consistent with the other log points in ispOutputDequeue() and unicamBufferDequeue(). > > > + << ", buffer id " << buffer->cookie() > > Maybe s/id/cookie/ ? Possibly, but see patch 9/9. We stop using buffer->cookie() entirely, so id will be the right term eventually :-) > > > + << ", timestamp: " << buffer->metadata().timestamp; > > + > > + /* The ISP input buffer gets re-queued into Unicam. */ > > handleStreamBuffer(buffer, &unicam_[Unicam::Image]); > > handleState(); > > } > > @@ -1324,6 +1329,8 @@ void RPiCameraData::checkRequestCompleted() > > pipe_->completeRequest(camera_, request); > > requestQueue_.pop_front(); > > requestCompleted = true; > > + > > + LOG(RPI, Debug) << "Request is complete"; > > Would it make sense to add this to Request::complete() instead ? And > maybe print the request cookie ? Sure, I can add a log point in Request::complete() to replace this one. > > > } > > > > /* > > @@ -1462,7 +1469,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; > > -- > Regards, > > Laurent Pinchart Regards, Naush
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index d3639ce1..35f14dda 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1160,6 +1160,11 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer) if (state_ == State::Stopped) return; + LOG(RPI, Debug) << "Stream ISP Input buffer complete" + << ", buffer id " << buffer->cookie() + << ", timestamp: " << buffer->metadata().timestamp; + + /* The ISP input buffer gets re-queued into Unicam. */ handleStreamBuffer(buffer, &unicam_[Unicam::Image]); handleState(); } @@ -1324,6 +1329,8 @@ void RPiCameraData::checkRequestCompleted() pipe_->completeRequest(camera_, request); requestQueue_.pop_front(); requestCompleted = true; + + LOG(RPI, Debug) << "Request is complete"; } /* @@ -1462,7 +1469,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;