[{"id":18245,"web_url":"https://patchwork.libcamera.org/comment/18245/","msgid":"<CAHW6GYJh1iuH=SQdQUK6dqA4UUX21Pi0kDz6X8bq_iQqXLr-JA@mail.gmail.com>","date":"2021-07-21T09:33:21","subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Fix a bug when\n\tclearing out Request buffers on stop","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThank you very much for fixing this!\n\nOn Wed, 21 Jul 2021 at 10:28, Naushir Patuck <naush@raspberrypi.com> wrote:\n>\n> When RPiCameraData::clearIncompleteRequests() clears out the request queue\n> during a stop condition, it unconditionally calls completeBuffer() on all\n> buffers in each request.  This is wrong, as a buffer could have already been\n> completed as part of the current request, but the request itself may not yet\n> have completed.\n>\n> Fix this by checking if the buffers in the request have been completed before\n> cancelling them.\n>\n> Fixes: d372aaa10ddb (\"pipeline: raspberrypi: Simplify RPiCameraData::clearIncompleteRequests()\")\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n\nI've been trying this patch out and it has indeed fixed the crash I\nwas experiencing.\n\nTested-by: David Plowman <david.plowman@raspberrypi.com>\nReviewed-by: David Plowman <david.plowman@raspberrypi.com>\n\nThanks!\nDavid\n\n> ---\n>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 ++++++++--\n>  1 file changed, 8 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index f821d8fe1b6c..0bab3bedd402 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1520,8 +1520,14 @@ void RPiCameraData::clearIncompleteRequests()\n>\n>                 for (auto &b : request->buffers()) {\n>                         FrameBuffer *buffer = b.second;\n> -                       buffer->cancel();\n> -                       pipe_->completeBuffer(request, buffer);\n> +                       /*\n> +                        * Has the buffer already been handed back to the\n> +                        * request? If not, do so now.\n> +                        */\n> +                       if (buffer->request()) {\n> +                               buffer->cancel();\n> +                               pipe_->completeBuffer(request, buffer);\n> +                       }\n>                 }\n>\n>                 pipe_->completeRequest(request);\n> --\n> 2.25.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E72AEC322B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 21 Jul 2021 09:33:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 60C7368540;\n\tWed, 21 Jul 2021 11:33:34 +0200 (CEST)","from mail-wr1-x434.google.com (mail-wr1-x434.google.com\n\t[IPv6:2a00:1450:4864:20::434])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 569336027A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 21 Jul 2021 11:33:32 +0200 (CEST)","by mail-wr1-x434.google.com with SMTP id f9so1416563wrq.11\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 21 Jul 2021 02:33:32 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"E8z28DXk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=yN/J3agCnFKVhB0qNdy0SqvyT8VQ1E6eZMZlhXVNJn4=;\n\tb=E8z28DXkFbacIFJNXV4wtPz5ZvnCuW1r7JOfX64ZkQZCtvTtce+2Vl33Dj5c7W2Pto\n\tvy7KG6yd+gR8wndt7cbkDL2aROaMiYqdomxqPgvnCCfvenk6SHexs/EvGoNzElNh+79S\n\tSdyIm3EmfMgz81umeEKgP/VVQgsoksdpHOjLrxf/maQE1cPkPMQ84G3cJF0+g6Z4KnS9\n\tgjZPK9wcRqEx/BWgPnjXviNK3EjyQgeQdIEsxJkUrY+ULsKffX0KOTKz5k9LABkHfiPR\n\tmTPoLpoBbj/L94RaQmQ490ifX566rRm28RQaGwFQaLMGK7Niu0qV8jCDmXw0OKpgnBYn\n\tHpqA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=yN/J3agCnFKVhB0qNdy0SqvyT8VQ1E6eZMZlhXVNJn4=;\n\tb=rSMmkM+BHAjcev1CW6+QMwRS7JywOTv/RK+ekAjJMh5ZaEvB6u+Z0T36GxbUprnYGf\n\tIY/mMHZkeXChWVtgLiMZ5qq/C3mWKxT9D8ERk+rHXaobcomcPK3UbLpdJiH3Wg05tsl9\n\tQdf3sMkZGkEuUslek8l2Z2hxz4rSF/27aGxkhl8hzCTYhlr2TAjOeOlVAmq52yQfu4jh\n\tDTv+czf/PWSCPltwRnDkNBct/PW7P7rMaszCH+Djaj4/I6oU5n4nnSosyCBX1bsvqhtg\n\tkM0E2xJau2LLQAYMzfR4vCE7qwF7PLC9DOdY8CoOfqVu33iEtH4MzpLyOgdxuZNnE1gD\n\tzLcg==","X-Gm-Message-State":"AOAM5335T5ffN++PW489sLprqBiZUtboGlwMz72bAMhd5zoqG2BDCJ3O\n\tz32LKF2xQHaQeTKgjk4h8Pa51UAMJf1jLjA30RhdSg==","X-Google-Smtp-Source":"ABdhPJxeMq8lDo5rGFk41ciwvbaE6nZhjtvhc2/D5gJEvfwTrHLqkhXPJk/O4G90ko+a1ihcEZPMPeyqszcSoZFFBsI=","X-Received":"by 2002:a5d:4089:: with SMTP id\n\to9mr41452742wrp.274.1626860012050; \n\tWed, 21 Jul 2021 02:33:32 -0700 (PDT)","MIME-Version":"1.0","References":"<20210721092800.1735805-1-naush@raspberrypi.com>","In-Reply-To":"<20210721092800.1735805-1-naush@raspberrypi.com>","From":"David Plowman <david.plowman@raspberrypi.com>","Date":"Wed, 21 Jul 2021 10:33:21 +0100","Message-ID":"<CAHW6GYJh1iuH=SQdQUK6dqA4UUX21Pi0kDz6X8bq_iQqXLr-JA@mail.gmail.com>","To":"Naushir Patuck <naush@raspberrypi.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Fix a bug when\n\tclearing out Request buffers on stop","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":18395,"web_url":"https://patchwork.libcamera.org/comment/18395/","msgid":"<CAEmqJPp8tWGoJVoRVOFE3RNFF1RhoPVHMvbeW56gyd_CHSVEZQ@mail.gmail.com>","date":"2021-07-28T06:37:17","subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Fix a bug when\n\tclearing out Request buffers on stop","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi,\n\nAnother ping for a review please :-)\n\nRegards,\nNaush\n\nOn Wed, 21 Jul 2021 at 10:28, Naushir Patuck <naush@raspberrypi.com> wrote:\n\n> When RPiCameraData::clearIncompleteRequests() clears out the request queue\n> during a stop condition, it unconditionally calls completeBuffer() on all\n> buffers in each request.  This is wrong, as a buffer could have already\n> been\n> completed as part of the current request, but the request itself may not\n> yet\n> have completed.\n>\n> Fix this by checking if the buffers in the request have been completed\n> before\n> cancelling them.\n>\n> Fixes: d372aaa10ddb (\"pipeline: raspberrypi: Simplify\n> RPiCameraData::clearIncompleteRequests()\")\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> ---\n>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 ++++++++--\n>  1 file changed, 8 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index f821d8fe1b6c..0bab3bedd402 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1520,8 +1520,14 @@ void RPiCameraData::clearIncompleteRequests()\n>\n>                 for (auto &b : request->buffers()) {\n>                         FrameBuffer *buffer = b.second;\n> -                       buffer->cancel();\n> -                       pipe_->completeBuffer(request, buffer);\n> +                       /*\n> +                        * Has the buffer already been handed back to the\n> +                        * request? If not, do so now.\n> +                        */\n> +                       if (buffer->request()) {\n> +                               buffer->cancel();\n> +                               pipe_->completeBuffer(request, buffer);\n> +                       }\n>                 }\n>\n>                 pipe_->completeRequest(request);\n> --\n> 2.25.1\n>\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 281EAC3230\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 28 Jul 2021 06:37:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DACB7687C4;\n\tWed, 28 Jul 2021 08:37:33 +0200 (CEST)","from mail-lf1-x12e.google.com (mail-lf1-x12e.google.com\n\t[IPv6:2a00:1450:4864:20::12e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CA8DF6026F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 28 Jul 2021 08:37:32 +0200 (CEST)","by mail-lf1-x12e.google.com with SMTP id bp1so1981827lfb.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 27 Jul 2021 23:37:32 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"G+lGbzNw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to;\n\tbh=ioCymNTFjlXodOqA58VEVkDyBzZNtPu+eVlV+dAVPHY=;\n\tb=G+lGbzNwS7PgKCfXS/O4dL85lavWBlHcoNVpfSELhUX6PT4WcuFvAHVtLHQU5QQ4bs\n\t1l5Q8OILycOCARf0YrnSPajLodDpaL7wmnjgGVrt2CpaHXwmbrT8aRrx1thWXfqmjZit\n\t56HkxDdUfc7IafVU8XbX/2xF102gbdnSSFYLSDxGPxFOk5XOaloU/EZCaGCMrzI+FFTw\n\taybZCFKQVdclUh+O02k1bAgr5OJLVU5UMIa8/MgJWIViqxs1y2PIKlHEoimyQypEkiyH\n\teCtT6qHNHwdgPSoohJZrxj2tV36Eb3SDAHAafb0bJ/hxabSp5QHNH5pm9KZ143g/WW6D\n\tP2TA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to;\n\tbh=ioCymNTFjlXodOqA58VEVkDyBzZNtPu+eVlV+dAVPHY=;\n\tb=m3ezi3OKbX5lIsEYk4R/KLCjSyjCGysb2XQ27jHqbPKDq1lLcvDFJ5o2MYZtQ7risd\n\tJtqv0yU9DfE9Vrw42CkCBBcJWmivRcxqMO5s1Hi9Mc8YZ1rxmYYRbVZEiOQktDOgtb2V\n\tEkST0poJHSlxrXGkR/dMkLXI/mSu/xiybnsJB/a5cj+StMCUU32IRCChQryRuCELMFDq\n\tENgvICxl5e3NqMwQGT10YxAYXjFtDOQLxdAqH31kR1i+O/WYjR2TwqGV0ZAKxx5DJ+nd\n\tR10ezP8imNYBpZ4YrfeVgBLHcbGwyc/Ur9dKNNVEu58K2aLgFstFUi2fbVmVBXR/l/8s\n\t+acA==","X-Gm-Message-State":"AOAM531daWy2nODhQHWcBPueGOuUZkXgOUDsLinNbWGw5NMatgpoXb5h\n\tSNAhDMAjbLay72ZyurAaJSdbtayQvUAozbu4LGLgiQ6ttr4=","X-Google-Smtp-Source":"ABdhPJxp+yF5vXOEd5CiiZq2T1CjDUiBRd+MGpR6Bo28MIahOeCU3hPND8WWnuzhu50Glx68lHvYy5g7REsVemB/VHk=","X-Received":"by 2002:a19:7716:: with SMTP id\n\ts22mr19464013lfc.272.1627454252104; \n\tTue, 27 Jul 2021 23:37:32 -0700 (PDT)","MIME-Version":"1.0","References":"<20210721092800.1735805-1-naush@raspberrypi.com>","In-Reply-To":"<20210721092800.1735805-1-naush@raspberrypi.com>","From":"Naushir Patuck <naush@raspberrypi.com>","Date":"Wed, 28 Jul 2021 07:37:17 +0100","Message-ID":"<CAEmqJPp8tWGoJVoRVOFE3RNFF1RhoPVHMvbeW56gyd_CHSVEZQ@mail.gmail.com>","To":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"multipart/alternative; boundary=\"00000000000095d17a05c82938d1\"","Subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Fix a bug when\n\tclearing out Request buffers on stop","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":18396,"web_url":"https://patchwork.libcamera.org/comment/18396/","msgid":"<YQD/BBvGMwVg15Td@pendragon.ideasonboard.com>","date":"2021-07-28T06:53:56","subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Fix a bug when\n\tclearing out Request buffers on stop","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Wed, Jul 21, 2021 at 10:28:00AM +0100, Naushir Patuck wrote:\n> When RPiCameraData::clearIncompleteRequests() clears out the request queue\n> during a stop condition, it unconditionally calls completeBuffer() on all\n> buffers in each request.  This is wrong, as a buffer could have already been\n> completed as part of the current request, but the request itself may not yet\n> have completed.\n> \n> Fix this by checking if the buffers in the request have been completed before\n> cancelling them.\n\nThis probably calls for improving the PipelineHandler, FrameBuffer and\nRequest classes to simplify completion handling. For the time being,\nthis patch looks good.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> Fixes: d372aaa10ddb (\"pipeline: raspberrypi: Simplify RPiCameraData::clearIncompleteRequests()\")\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> ---\n>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 ++++++++--\n>  1 file changed, 8 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index f821d8fe1b6c..0bab3bedd402 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1520,8 +1520,14 @@ void RPiCameraData::clearIncompleteRequests()\n>  \n>  \t\tfor (auto &b : request->buffers()) {\n>  \t\t\tFrameBuffer *buffer = b.second;\n> -\t\t\tbuffer->cancel();\n> -\t\t\tpipe_->completeBuffer(request, buffer);\n> +\t\t\t/*\n> +\t\t\t * Has the buffer already been handed back to the\n> +\t\t\t * request? If not, do so now.\n> +\t\t\t */\n> +\t\t\tif (buffer->request()) {\n> +\t\t\t\tbuffer->cancel();\n> +\t\t\t\tpipe_->completeBuffer(request, buffer);\n> +\t\t\t}\n>  \t\t}\n>  \n>  \t\tpipe_->completeRequest(request);","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0A316C322E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 28 Jul 2021 06:54:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 77871687C4;\n\tWed, 28 Jul 2021 08:54:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C48746026F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 28 Jul 2021 08:54:02 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3FF82EE;\n\tWed, 28 Jul 2021 08:54:02 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"iUgzinJq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1627455242;\n\tbh=dXzkGcwL5FwZZ7W49mEuxGRyTz2B519nk5NLnPTX2+k=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iUgzinJqqQu6aqvDpWXdVhWEmbo+c64Eh01WEiAGAIqFM3BJuJnMRITTQwrL4LPxL\n\tFJb0zMJKcwfC7Jo+azCPvhpc0G0YcsQ0AhzSGfwUIgAycoH6y+Jm6CTtvTLs0/vUug\n\tY3SDcjzwhST9pPoy3YnlaRfE1vs8yvRiTAEZarHY=","Date":"Wed, 28 Jul 2021 09:53:56 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<YQD/BBvGMwVg15Td@pendragon.ideasonboard.com>","References":"<20210721092800.1735805-1-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210721092800.1735805-1-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Fix a bug when\n\tclearing out Request buffers on stop","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":18398,"web_url":"https://patchwork.libcamera.org/comment/18398/","msgid":"<CAEmqJPpWO5E_y6s=cNjCeTtQFp65NOhzMR-WCjutE14QnZHpYw@mail.gmail.com>","date":"2021-07-28T07:00:00","subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Fix a bug when\n\tclearing out Request buffers on stop","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThanks for the review.\n\nOn Wed, 28 Jul 2021 at 07:54, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Wed, Jul 21, 2021 at 10:28:00AM +0100, Naushir Patuck wrote:\n> > When RPiCameraData::clearIncompleteRequests() clears out the request\n> queue\n> > during a stop condition, it unconditionally calls completeBuffer() on all\n> > buffers in each request.  This is wrong, as a buffer could have already\n> been\n> > completed as part of the current request, but the request itself may not\n> yet\n> > have completed.\n> >\n> > Fix this by checking if the buffers in the request have been completed\n> before\n> > cancelling them.\n>\n> This probably calls for improving the PipelineHandler, FrameBuffer and\n> Request classes to simplify completion handling. For the time being,\n> this patch looks good.\n>\n\nAgree.  I think perhaps a Request::Cancel() that pipeline handlers could\ncall\nmight be a neat solution.  I'd imagine the implementation would look pretty\nsimilar to the code in this patch.\n\nRegards,\nNaush\n\n\n>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>\n> > Fixes: d372aaa10ddb (\"pipeline: raspberrypi: Simplify\n> RPiCameraData::clearIncompleteRequests()\")\n> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > ---\n> >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 ++++++++--\n> >  1 file changed, 8 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index f821d8fe1b6c..0bab3bedd402 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -1520,8 +1520,14 @@ void RPiCameraData::clearIncompleteRequests()\n> >\n> >               for (auto &b : request->buffers()) {\n> >                       FrameBuffer *buffer = b.second;\n> > -                     buffer->cancel();\n> > -                     pipe_->completeBuffer(request, buffer);\n> > +                     /*\n> > +                      * Has the buffer already been handed back to the\n> > +                      * request? If not, do so now.\n> > +                      */\n> > +                     if (buffer->request()) {\n> > +                             buffer->cancel();\n> > +                             pipe_->completeBuffer(request, buffer);\n> > +                     }\n> >               }\n> >\n> >               pipe_->completeRequest(request);\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A9DF4C3230\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 28 Jul 2021 07:00:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1D8A6687BD;\n\tWed, 28 Jul 2021 09:00:18 +0200 (CEST)","from mail-lf1-x136.google.com (mail-lf1-x136.google.com\n\t[IPv6:2a00:1450:4864:20::136])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DE903687B6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 28 Jul 2021 09:00:15 +0200 (CEST)","by mail-lf1-x136.google.com with SMTP id d17so2133920lfv.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 28 Jul 2021 00:00:15 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"GeaPBBPQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=5D2xnrJSwePw+VyUvYUq7FaDn6gdYkNjreEppwv/VzA=;\n\tb=GeaPBBPQEyGqlIU+KwpflEbWriNGOZp6WzD6dc/xI9LD8k9N0WSiBu/tw/XevphiWI\n\tWeTU5mpjqOloqOwGsMrCLE+s5gnHat5hgaRp2yg8tKa1IAPTjhwvZc4TGvS7yZXQj3dB\n\t9F4mjlbHZM33+ERzaHjLzr9pC8gijbvMBY237LpSKZjPr0MsiEupi0K1CCESICcDc1BP\n\tve05P6AR9iSTR6ysnelxj65Gew3+9CIKpJIfQQK0+nfurrhuCzL7mEwbqyZ9/J18bcbW\n\tDCXmDY2k7aaqvhDTKrSCsxOjuan0i8UvCNaTip5+UKBinGL88kycEB70c7kUR2Zr5JDi\n\toweQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=5D2xnrJSwePw+VyUvYUq7FaDn6gdYkNjreEppwv/VzA=;\n\tb=hauYmKTQOUHQlw/34qrR+cQqsazorekBvfT1zi1q7OcXq49fw4daN7hsyC7MsFkh+K\n\tPuI3VFtibs5Dc4e1y4o1ikuaZgXpEo75bTDuTkfg0KqjCb1TrotJaHn54FR3OB36UczW\n\tx4vi4Q66p+UCIm9yM88gCHYVyvCbBTtj17KaK5JA0xEXyYX9p7tr1mk/Pa2ulQLAIyVH\n\t/GPPppx1b26y2hHMfh8SWfo+ptQpjpMzV/wdBztOoUbD4FAaLQYpFACQpCENz/Gz1W3k\n\tN+iv5sBlUb2bYsLDBSMP0NVpjWiiUGPIxjCZrgVLetdfZ65sNicTLnGQ8gI2zN1tJDtj\n\tm/Zg==","X-Gm-Message-State":"AOAM533U3jLcdpVh17xtjyB01Bf5CuGDLq2aiAXV5GHgxUcYUG6Pn/Z1\n\tEscFjlYPFYBVFJs09XW8GkdPOIqRK6brPR+DLeXvkQvWkjk=","X-Google-Smtp-Source":"ABdhPJzs2fwmUmtBzYxDEdzXlHIG7DC5lrJuQ1p5wNs0oEGCH/62FYIc4rb0H0i0NCXtAW9PRsodIuq1kpA3QwVf3Kc=","X-Received":"by 2002:a19:760c:: with SMTP id\n\tc12mr19012398lff.617.1627455615167; \n\tWed, 28 Jul 2021 00:00:15 -0700 (PDT)","MIME-Version":"1.0","References":"<20210721092800.1735805-1-naush@raspberrypi.com>\n\t<YQD/BBvGMwVg15Td@pendragon.ideasonboard.com>","In-Reply-To":"<YQD/BBvGMwVg15Td@pendragon.ideasonboard.com>","From":"Naushir Patuck <naush@raspberrypi.com>","Date":"Wed, 28 Jul 2021 08:00:00 +0100","Message-ID":"<CAEmqJPpWO5E_y6s=cNjCeTtQFp65NOhzMR-WCjutE14QnZHpYw@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Content-Type":"multipart/alternative; boundary=\"000000000000d4822305c82989d8\"","Subject":"Re: [libcamera-devel] [PATCH] pipeline: raspberrypi: Fix a bug when\n\tclearing out Request buffers on stop","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]