[{"id":15865,"web_url":"https://patchwork.libcamera.org/comment/15865/","msgid":"<YFtd9M5KcAwHA1CL@pendragon.ideasonboard.com>","date":"2021-03-24T15:42:44","subject":"Re: [libcamera-devel] [PATCH v3 5/6] libcamera: pipeline: ipu3: Do\n\tnot mark metadata complete early","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Wed, Mar 24, 2021 at 03:01:24PM +0000, Kieran Bingham wrote:\n> When the imguOutputBufferReady() detects a cancelled frame, it is\n> reporting that the metadata has been processed in order to be able to\n> complete the cancelled request.\n> \n> This causes the FrameInfo to be completed and deleted early, but then an\n> active buffer on the IMGU can complete and be unable to find the\n> FrameInfo for it to complete correctly.\n\nThis is because IPU3Frames::tryComplete() will erroneously consider that\nthe frame is complete, right ? I'm a bit puzzled, that function checks\nif there's any pending buffer, wouldn't that condition be true until all\nthe buffers are returned ? What am I missing ?\n\n> Do not mark metadataProcessed early on the event that a single buffer is\n> detected as cancelled. The stopping of the V4L2 devices will ensure\n> that all queued buffers are returned to us and we can follow the normal\n> and expected shutdown sequence.\n> \n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>  src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ---\n>  1 file changed, 3 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 34ee600340b1..a8edf906220b 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -1232,9 +1232,6 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n>  \t\tcropRegion_ = request->controls().get(controls::ScalerCrop);\n>  \trequest->metadata().set(controls::ScalerCrop, cropRegion_);\n>  \n> -\tif (buffer->metadata().status == FrameMetadata::FrameCancelled)\n> -\t\tinfo->metadataProcessed = true;\n> -\n>  \tif (frameInfos_.tryComplete(info))\n>  \t\tpipe_->completeRequest(request);\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 31295C32E7\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 24 Mar 2021 15:43:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8784D68D62;\n\tWed, 24 Mar 2021 16:43:29 +0100 (CET)","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 9FB3A602E3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 24 Mar 2021 16:43:28 +0100 (CET)","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 F2FDC580;\n\tWed, 24 Mar 2021 16:43:27 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"jaFoPN2u\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616600608;\n\tbh=ZhQ4H99417DOXNZg2iQrkoYj2cwPE/oulrkCWhdh7FM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jaFoPN2uWkUE/1FuekieftXipHnr7/mha8KGVA4CUVME5K51uGCMeTFcI5a27AczX\n\tdKp3SykLGEevTtrz2HwWFfqqzh+Indjpzehv0hMDiO0Err/T8vdsCKG3zAvV9WE8oU\n\to1OpEf51hoTiFwwM+OsLjgIEWFNVeTS2If9rPFwY=","Date":"Wed, 24 Mar 2021 17:42:44 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<YFtd9M5KcAwHA1CL@pendragon.ideasonboard.com>","References":"<20210324150125.1318325-1-kieran.bingham@ideasonboard.com>\n\t<20210324150125.1318325-6-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210324150125.1318325-6-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 5/6] libcamera: pipeline: ipu3: Do\n\tnot mark metadata complete early","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>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":15868,"web_url":"https://patchwork.libcamera.org/comment/15868/","msgid":"<e01d2e8a-39d1-d02c-23bb-29949614de3f@ideasonboard.com>","date":"2021-03-24T16:09:35","subject":"Re: [libcamera-devel] [PATCH v3 5/6] libcamera: pipeline: ipu3: Do\n\tnot mark metadata complete early","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\n\nOn 24/03/2021 15:42, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Wed, Mar 24, 2021 at 03:01:24PM +0000, Kieran Bingham wrote:\n>> When the imguOutputBufferReady() detects a cancelled frame, it is\n>> reporting that the metadata has been processed in order to be able to\n>> complete the cancelled request.\n>>\n>> This causes the FrameInfo to be completed and deleted early, but then an\n>> active buffer on the IMGU can complete and be unable to find the\n>> FrameInfo for it to complete correctly.\n> \n> This is because IPU3Frames::tryComplete() will erroneously consider that\n> the frame is complete, right ? I'm a bit puzzled, that function checks\n> if there's any pending buffer, wouldn't that condition be true until all\n> the buffers are returned ? What am I missing ?\n\nIt waits until all application provided buffers are returned. It doesn't\ntrack internal buffers. (assuming you mean the\nrequest->hasPendingBuffers check)\n\nThis is why I previously raised the idea that internal buffers should\nalso be tracked against a request directly, but that should be handled\nthrough a private implementation, as we can't associate them with\nstreams that the application may interrogate.\n\n\ntryComplete requires 3 things.\n\n  1) !request->hasPendingBuffers()\n  2) info->metadataProcessed\n  3) info->paramDequeued\n\n\ninfo->metadataProcessed is set:\n\n A) IPU3CameraData::statBufferReady()\n    If the stat buffer is cancelled, and not sent to the IPA.\n\n B) when ActionMetadataReady is received from the IPA\n    I.e., after successfully parsing the stats in IPA.\n\n C) here in IPU3CameraData::imguOutputBufferReady()\n    This is incorrect, as it's essentially completing the internal stats\n    buffer while it can be potentially still on a V4L2 device, and just\n    about to complete all by itself.\n    imguOutputBufferReady completes the user facing buffer, and that's\n     all it should do.\n\nA, and B are perfectly valid times to complete the stats buffer. C\n(fixed by this patch) is not.\n\n\n>> Do not mark metadataProcessed early on the event that a single buffer is\n>> detected as cancelled. The stopping of the V4L2 devices will ensure\n>> that all queued buffers are returned to us and we can follow the normal\n>> and expected shutdown sequence.\n>>\n>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>> ---\n>>  src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ---\n>>  1 file changed, 3 deletions(-)\n>>\n>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>> index 34ee600340b1..a8edf906220b 100644\n>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>> @@ -1232,9 +1232,6 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n>>  \t\tcropRegion_ = request->controls().get(controls::ScalerCrop);\n>>  \trequest->metadata().set(controls::ScalerCrop, cropRegion_);\n>>  \n>> -\tif (buffer->metadata().status == FrameMetadata::FrameCancelled)\n>> -\t\tinfo->metadataProcessed = true;\n>> -\n>>  \tif (frameInfos_.tryComplete(info))\n>>  \t\tpipe_->completeRequest(request);\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 2F7A5C32E5\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 24 Mar 2021 16:09:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8169F602E3;\n\tWed, 24 Mar 2021 17:09:39 +0100 (CET)","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 5BA97602E3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 24 Mar 2021 17:09:38 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C4C6C580;\n\tWed, 24 Mar 2021 17:09:37 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"PqXhQAJc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616602178;\n\tbh=LkTYeWAKyz1dTT4Hzj+k0VJf5EMD/SNehg3bZPGcqUU=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=PqXhQAJcLrmgYLIkd+V1pePGipI8gRDz40E2QYUtXdiCSnOIl8ABAVZHHjCXP0np/\n\tT5worzXeOOsLRY1LYULrcUerDra1qQm0mIO9FTSVztuZ+zxkwVdLdTp/WRsJZRUaTH\n\t9d2l8x5vWGPeHtMcA3wCtogo1fQ/KDFuIUxnuwfI=","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","References":"<20210324150125.1318325-1-kieran.bingham@ideasonboard.com>\n\t<20210324150125.1318325-6-kieran.bingham@ideasonboard.com>\n\t<YFtd9M5KcAwHA1CL@pendragon.ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<e01d2e8a-39d1-d02c-23bb-29949614de3f@ideasonboard.com>","Date":"Wed, 24 Mar 2021 16:09:35 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<YFtd9M5KcAwHA1CL@pendragon.ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v3 5/6] libcamera: pipeline: ipu3: Do\n\tnot mark metadata complete early","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>","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":15871,"web_url":"https://patchwork.libcamera.org/comment/15871/","msgid":"<YFtpKiEmhI+3aNW1@pendragon.ideasonboard.com>","date":"2021-03-24T16:30:34","subject":"Re: [libcamera-devel] [PATCH v3 5/6] libcamera: pipeline: ipu3: Do\n\tnot mark metadata complete early","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Wed, Mar 24, 2021 at 04:09:35PM +0000, Kieran Bingham wrote:\n> On 24/03/2021 15:42, Laurent Pinchart wrote:\n> > On Wed, Mar 24, 2021 at 03:01:24PM +0000, Kieran Bingham wrote:\n> >> When the imguOutputBufferReady() detects a cancelled frame, it is\n> >> reporting that the metadata has been processed in order to be able to\n> >> complete the cancelled request.\n> >>\n> >> This causes the FrameInfo to be completed and deleted early, but then an\n> >> active buffer on the IMGU can complete and be unable to find the\n> >> FrameInfo for it to complete correctly.\n> > \n> > This is because IPU3Frames::tryComplete() will erroneously consider that\n> > the frame is complete, right ? I'm a bit puzzled, that function checks\n> > if there's any pending buffer, wouldn't that condition be true until all\n> > the buffers are returned ? What am I missing ?\n> \n> It waits until all application provided buffers are returned. It doesn't\n> track internal buffers. (assuming you mean the\n> request->hasPendingBuffers check)\n\nAh of course. That's what I was missing. Thanks.\n\n> This is why I previously raised the idea that internal buffers should\n> also be tracked against a request directly, but that should be handled\n> through a private implementation, as we can't associate them with\n> streams that the application may interrogate.\n> \n> \n> tryComplete requires 3 things.\n> \n>   1) !request->hasPendingBuffers()\n>   2) info->metadataProcessed\n>   3) info->paramDequeued\n> \n> \n> info->metadataProcessed is set:\n> \n>  A) IPU3CameraData::statBufferReady()\n>     If the stat buffer is cancelled, and not sent to the IPA.\n> \n>  B) when ActionMetadataReady is received from the IPA\n>     I.e., after successfully parsing the stats in IPA.\n> \n>  C) here in IPU3CameraData::imguOutputBufferReady()\n>     This is incorrect, as it's essentially completing the internal stats\n>     buffer while it can be potentially still on a V4L2 device, and just\n>     about to complete all by itself.\n>     imguOutputBufferReady completes the user facing buffer, and that's\n>      all it should do.\n> \n> A, and B are perfectly valid times to complete the stats buffer. C\n> (fixed by this patch) is not.\n\nThanks for putting up with me :-)\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> >> Do not mark metadataProcessed early on the event that a single buffer is\n> >> detected as cancelled. The stopping of the V4L2 devices will ensure\n> >> that all queued buffers are returned to us and we can follow the normal\n> >> and expected shutdown sequence.\n> >>\n> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> >> ---\n> >>  src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ---\n> >>  1 file changed, 3 deletions(-)\n> >>\n> >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >> index 34ee600340b1..a8edf906220b 100644\n> >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >> @@ -1232,9 +1232,6 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> >>  \t\tcropRegion_ = request->controls().get(controls::ScalerCrop);\n> >>  \trequest->metadata().set(controls::ScalerCrop, cropRegion_);\n> >>  \n> >> -\tif (buffer->metadata().status == FrameMetadata::FrameCancelled)\n> >> -\t\tinfo->metadataProcessed = true;\n> >> -\n> >>  \tif (frameInfos_.tryComplete(info))\n> >>  \t\tpipe_->completeRequest(request);\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 E69E8C32E7\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 24 Mar 2021 16:31:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3CE0B68D6B;\n\tWed, 24 Mar 2021 17:31:19 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 58B70602E3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 24 Mar 2021 17:31:18 +0100 (CET)","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 C55B3580;\n\tWed, 24 Mar 2021 17:31:17 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kjDavzUh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616603478;\n\tbh=pkHlRP+le/nOYh2nq4tF+dSlg3VlDjWUrg6XnOutMbU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kjDavzUh6rUs1mBjLYX13JES4eHHdKiDUUWJd/ujcuUxAqHK3b8as/12XuY3hLtGk\n\tUdUI3HwltvO4hvAyoEP4dIRivZfBouyoEaWoX8aVxoVtVJkOoBJKdPEa/yFPGYrcKb\n\tn+EatOPmK0OtgJ3Q36coLWisXX99nQl8CEPParJM=","Date":"Wed, 24 Mar 2021 18:30:34 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<YFtpKiEmhI+3aNW1@pendragon.ideasonboard.com>","References":"<20210324150125.1318325-1-kieran.bingham@ideasonboard.com>\n\t<20210324150125.1318325-6-kieran.bingham@ideasonboard.com>\n\t<YFtd9M5KcAwHA1CL@pendragon.ideasonboard.com>\n\t<e01d2e8a-39d1-d02c-23bb-29949614de3f@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<e01d2e8a-39d1-d02c-23bb-29949614de3f@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 5/6] libcamera: pipeline: ipu3: Do\n\tnot mark metadata complete early","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>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]