[libcamera-devel] [PATCH v3 4/7] libcamera: pipeline: rkisp1: Do not try to process cancelled frames
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Apr 11 13:41:21 CEST 2020
Hi Niklas,
Thank you for the patch.
On Sat, Apr 11, 2020 at 02:19:08AM +0200, Niklas Söderlund wrote:
> There is no need to try and process cancelled frames, try to finish as
> quickly as possible.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> * Changes since v2
> - If the buffer intended for the application is canceled complete it and
> the request as to not lockup applications.
> ---
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index de90615edf217cca..daf84b5ac129efc8 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -1015,6 +1015,12 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
> RkISP1CameraData *data = cameraData(activeCamera_);
> Request *request = buffer->request();
>
> + if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
> + completeBuffer(activeCamera_, request, buffer);
> + completeRequest(activeCamera_, request);
> + return;
> + }
> +
> data->timeline_.bufferReady(buffer);
>
> if (data->frame_ <= buffer->metadata().sequence)
> @@ -1026,6 +1032,9 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>
> void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
> {
> + if (buffer->metadata().status == FrameMetadata::FrameCancelled)
> + return;
> +
> ASSERT(activeCamera_);
> RkISP1CameraData *data = cameraData(activeCamera_);
>
> @@ -1037,6 +1046,9 @@ void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
>
> void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
> {
> + if (buffer->metadata().status == FrameMetadata::FrameCancelled)
> + return;
> +
This looks fine, but will keep data in frameInfo_ after stop(). I think
you need to add a clear() operation to RkISP1Frames() and call it in
stop(). It can be done in a separate patch, so
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ASSERT(activeCamera_);
> RkISP1CameraData *data = cameraData(activeCamera_);
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list