[PATCH] pipeline: rkisp1: clear pending requests when stopping camera device

[Umang Jain]

Hello Javier

On 25/10/24 2:08 pm, Javier Carrasco wrote:
> If the camera device is stopped, there may still be requests within
> 'data->queuedRequests_' that are mistakenly never popped. If this is the
> case, libcamera terminates with the assertion
> 'ASSERT(data->queuedRequests_.empty())'.
>
> Walk over the pending queued requests, destroy the info frames and
> complete the requests to ensure that no requests are left behind.
>
> Co-developed-by: Lukas Riezler <lukas.riezler at wolfvision.net>
> Signed-off-by: Lukas Riezler <lukas.riezler at wolfvision.net>
> Signed-off-by: Javier Carrasco <javier.carrasco at wolfvision.net>
>
> ---
> This is my first patch for libcamera, apologies for anything I might
> have overlooked from the "Submitting Patches" section in the docs.
>
> There is already a patch to fix a similar issue for swISP[1], which
> adresses a bug tracked on Bugzilla[2]. The latter contains some mentions
> to the possibility of having the same issue for rkisp1, and that is what
> we are observing on our RK3568-based system. I did not add a 'Bug:' tag
> as [2] addresses a different pipeline, but the problem seems to be the
> same.
>
> The developers involved in this fix are by no means libcamera experts,
> and we have tried to follow a similar approach as in [1] by walking over
> pending requests and completing them before stopDevice() is completed.
> The proposed patch fixes the issue, but we are open to any suggestions
> and improvements.
>
> Link: https://patchwork.libcamera.org/patch/21537/
> <https://patchwork.libcamera.org/patch/21537/> [1]
> Link: https://bugs.libcamera.org/show_bug.cgi?id=234
> <https://bugs.libcamera.org/show_bug.cgi?id=234> [2]
> ---
>    src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 +++++++
>    1 file changed, 7 insertions(+)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index c02c7cf3..efa7dbd5 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -997,6 +997,13 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera)
>                                    << "Failed to stop parameters for " <<
> camera->id();
>            }
>
> +       while (!data->queuedRequests_.empty()) {
> +               Request *req = data->queuedRequests_.front();
> +               RkISP1FrameInfo *info = data->frameInfo_.find(req);
> +               data->frameInfo_.destroy(info->frame);
> +               completeRequest(req);
> +       }
> +

We should never do this - if we need to this, there is clearly a bug 
somewhere. stop() makes sure all requests are completed (even with 
status = cancelled)

Refer to PipelineHandler::stop() documentation

>            ASSERT(data->queuedRequests_.empty());

What I think is, this is a redundant assert and should be dropped from 
RkISP1PipelineHandler::stopDevice()

We are already in stopDevice() which is called by 
PipelineHandler::stop() - which already has this ASSERT
https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline_handler.cpp#n377

So, we should the drop the ASSERT from rkisp1 pipeline handler and let 
the base pipeline handler class handle the ASSERT (which it already does).

It's redundant and called in wrong order currently. That should solve 
the issue!

>            data->frameInfo_.clear();
>
> --
> 2.43.0
>
> Javier Carrasco
> R&D Engineer
> Mail: javier.carrasco at wolfvision.net <mailto:%7BE-mail%7D>
>
> WolfVision GmbH
> Oberes Ried 14 | 6833 Klaus | Österreich
> T: +43 5523 52250 <tel:+43552352250>
>
> Webpage: www.wolfvision.com <https://www.wolfvision.com/> |
> www.wolfvision.com/green <https://www.wolfvision.com/green>
> Firmenbuch / Commercial Register: FN283521v Feldkirch/Austria
>
> RegisterNow-Signatur_ISE25_v1.jpg
> <https://marketing.wolfvision.com/l/908602/2024-10-24/by7n7><https://info.wolfvision.com/ise/?utm_source=email&utm_medium=referral&utm_campaign=ise2024&utm_content=signature>
> See our new solutions LIVE in Barcelona at ISE booth 3N500
> Use our discount code QK7Z38BS to secure your free ticket here
> <https://marketing.wolfvision.com/l/908602/2024-10-24/by7n7>
>