[{"id":24060,"web_url":"https://patchwork.libcamera.org/comment/24060/","msgid":"<d3d878ec-e48a-bed6-0799-c945f407a1dc@ideasonboard.com>","date":"2022-07-22T15:15:05","subject":"Re: [libcamera-devel] [RFC PATCH 01/12] libcamera: request: Add\n\tsupport for error flags","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Kieran,\n\nOn 7/21/22 17:42, Kieran Bingham via libcamera-devel wrote:\n> From: Paul Elder <paul.elder@ideasonboard.com>\n>\n> Add error flags to the Request to indicate non-fatal errors.\n>\n> Applications should check the error() state of the Request to determine\n> if any fault occured while processing the request.\n>\n> Initially, a single error flag ControlError is added to allow a pipeline\n> handler to report a failure to set controls on a hardware device while\n> processing the request.\n>\n> ControlErrors occur when a Request was asked to set a control and the\n> pipeline handler attempted to do so, but it was rejected by the kernel.\n>\n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>\n> ---\n> v2:\n>   - Add documentation for Request::Private::setErrorFlags\n>\n> ---\n> v2\n>   - Extend documentation\n>\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>   include/libcamera/internal/request.h |  3 ++\n>   include/libcamera/request.h          |  9 ++++++\n>   src/libcamera/request.cpp            | 46 ++++++++++++++++++++++++++++\n>   3 files changed, 58 insertions(+)\n>\n> diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\n> index 9dadd6c60361..8e592272cfae 100644\n> --- a/include/libcamera/internal/request.h\n> +++ b/include/libcamera/internal/request.h\n> @@ -38,6 +38,7 @@ public:\n>   \tvoid complete();\n>   \tvoid cancel();\n>   \tvoid reuse();\n> +\tvoid setErrorFlags(ErrorFlags flags);\n>   \n>   \tvoid prepare(std::chrono::milliseconds timeout = 0ms);\n>   \tSignal<> prepared;\n> @@ -59,6 +60,8 @@ private:\n>   \tstd::unordered_set<FrameBuffer *> pending_;\n>   \tstd::map<FrameBuffer *, std::unique_ptr<EventNotifier>> notifiers_;\n>   \tstd::unique_ptr<Timer> timer_;\n> +\n> +\tErrorFlags error_;\n>   };\n>   \n>   } /* namespace libcamera */\n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index dffde1536cad..992629e11aa4 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -15,6 +15,7 @@\n>   #include <unordered_set>\n>   \n>   #include <libcamera/base/class.h>\n> +#include <libcamera/base/flags.h>\n>   #include <libcamera/base/signal.h>\n>   \n>   #include <libcamera/controls.h>\n> @@ -43,6 +44,13 @@ public:\n>   \t\tReuseBuffers = (1 << 0),\n>   \t};\n>   \n> +\tenum ErrorFlag {\n> +\t\tNoError = 0,\n> +\t\tControlError = (1 << 0),\n> +\t};\n> +\n> +\tusing ErrorFlags = Flags<ErrorFlag>;\n> +\n>   \tusing BufferMap = std::map<const Stream *, FrameBuffer *>;\n>   \n>   \tRequest(Camera *camera, uint64_t cookie = 0);\n> @@ -60,6 +68,7 @@ public:\n>   \tuint32_t sequence() const;\n>   \tuint64_t cookie() const { return cookie_; }\n>   \tStatus status() const { return status_; }\n> +\tErrorFlags error() const;\n>   \n>   \tbool hasPendingBuffers() const;\n>   \n> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> index d2af1d2212ad..8b82757ea7e3 100644\n> --- a/src/libcamera/request.cpp\n> +++ b/src/libcamera/request.cpp\n> @@ -162,6 +162,7 @@ void Request::Private::cancel()\n>    */\n>   void Request::Private::reuse()\n>   {\n> +\terror_ = Request::NoError;\n>   \tsequence_ = 0;\n>   \tcancelled_ = false;\n>   \tprepared_ = false;\n> @@ -284,6 +285,21 @@ void Request::Private::notifierActivated(FrameBuffer *buffer)\n>   \temitPrepareCompleted();\n>   }\n>   \n> +/**\n> + * \\brief Update the error flags of the Request\n> + * \\param[in] flags Flags to apply on the Request\n> + *\n> + * Apply \\a flags to the Request to report to the application when the Request\n> + * completes.\n\n\ns/to report to the application/which will get reported to the application/\n\n> + *\n> + * Setting an Error flag does not cause a Request to fail, and once set it can\n> + * only be cleared by the application destroying the Request or calling reuse().\n> + */\n> +void Request::Private::setErrorFlags(ErrorFlags flags)\n> +{\n> +\terror_ |= flags;\n> +}\n> +\n>   void Request::Private::timeout()\n>   {\n>   \t/* A timeout can only happen if there are fences not yet signalled. */\n> @@ -318,6 +334,22 @@ void Request::Private::timeout()\n>    * Reuse the buffers that were previously added by addBuffer()\n>    */\n>   \n> +/**\n> + * \\enum Request::ErrorFlag\n> + * Flags to report non-fatal errors\n> + * \\var Request::NoError\n> + * No error\n> + * \\var Request::ControlError\n> + * Control Error. At least on control was not able to be applied to the device.\n\n\ns/on/one/ maybe?\n\n> + * The application should compare the metadata to the requested control values\n> + * to check which controls weren't applied.\n> + */\n> +\n> +/**\n> + * \\typedef Request::ErrorFlags\n> + * The error state of the request defined by \\a Request::ErrorFlag\n> + */\n> +\n>   /**\n>    * \\typedef Request::BufferMap\n>    * \\brief A map of Stream to FrameBuffer pointers\n> @@ -560,6 +592,20 @@ uint32_t Request::sequence() const\n>    * \\return The request completion status\n>    */\n>   \n> +/**\n> + * \\brief Retrieve the error flags\n> + *\n> + * The request could complete with non-fatal error. The completion status will\n> + * indicate success. This function returns the non-fatal errors that the\n> + * request completed with\n\n\ns/that the request completed with/with which the request was completed.\n\n> + *\n> + * \\return Flags of non-fatal errors that the request completed with\n> + */\n> +Request::ErrorFlags Request::error() const\n> +{\n> +\treturn _d()->error_;\n> +}\n> +\n>   /**\n>    * \\brief Check if a request has buffers yet to be completed\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 6B144BD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 22 Jul 2022 15:15:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B40176330F;\n\tFri, 22 Jul 2022 17:15:11 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7CFDC601B8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 22 Jul 2022 17:15:10 +0200 (CEST)","from [IPV6:2401:4900:1f3f:a2d:c6dd:7a7c:3d3c:dbb9] (unknown\n\t[IPv6:2401:4900:1f3f:a2d:c6dd:7a7c:3d3c:dbb9])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 850036D5;\n\tFri, 22 Jul 2022 17:15:09 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658502911;\n\tbh=1uKNGHy80x9owHCyfvrN5UtgeksOXZRN1v93qXHYobA=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=jjBA7akFVnnXukKSkx49vFSG2XVVERRsG0oLNaPsUaop8s8qHSoKc2wFH1KzRF5N+\n\tuzwOavJV8z8qYPw+PFmTP5TYAKoZa1fZ53l2y72vXYAOwLr5wR1NugYVaGzl2Ke/jg\n\t5iYg02DAm1rBTtfQBl0KktKAxlcxzP/rn7y4o9xbnothsbxOyR5NZ/FTOU0AebywdM\n\th8CCth4fRqQx+g4CUyyiMEchcP6yfJeD9rLPokyqyRh3XgCtz+i70xfzlioHvm8ZDj\n\t0jQbb3v4cPChjLzocXJ5wspnPIGIilYUtl+RL/G1f1U7bZWmPLcIauy2ChD7/yz2LS\n\t/hQQncWCaM8cg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658502910;\n\tbh=1uKNGHy80x9owHCyfvrN5UtgeksOXZRN1v93qXHYobA=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=CWLujHe1U81JzfUjlmVRP+xhGZkoHaIO38KT2bp6pmaZfDq+vT2M1THwIQlmX7SVw\n\tmwHL3HrV6jm9/in/8tiebi1CuLRaeTYzUlcSLo1Y97ZRsyOF0M/IWfTcFMWh4Vvmuc\n\tf20rboBQnGhiV7B1irhGxCzd/omz1iKEVevzaIqU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"CWLujHe1\"; dkim-atps=neutral","Message-ID":"<d3d878ec-e48a-bed6-0799-c945f407a1dc@ideasonboard.com>","Date":"Fri, 22 Jul 2022 20:45:05 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera devel <libcamera-devel@lists.libcamera.org>","References":"<20220721121310.1286862-1-kieran.bingham@ideasonboard.com>\n\t<20220721121310.1286862-2-kieran.bingham@ideasonboard.com>","In-Reply-To":"<20220721121310.1286862-2-kieran.bingham@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [RFC PATCH 01/12] libcamera: request: Add\n\tsupport for error flags","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>","From":"Umang Jain via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Umang Jain <umang.jain@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24089,"web_url":"https://patchwork.libcamera.org/comment/24089/","msgid":"<20220725075425.qgoql6s5tv64umog@uno.localdomain>","date":"2022-07-25T07:54:25","subject":"Re: [libcamera-devel] [RFC PATCH 01/12] libcamera: request: Add\n\tsupport for error flags","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran\n\nOn Thu, Jul 21, 2022 at 01:12:59PM +0100, Kieran Bingham via libcamera-devel wrote:\n> From: Paul Elder <paul.elder@ideasonboard.com>\n>\n> Add error flags to the Request to indicate non-fatal errors.\n>\n> Applications should check the error() state of the Request to determine\n> if any fault occured while processing the request.\n>\n> Initially, a single error flag ControlError is added to allow a pipeline\n> handler to report a failure to set controls on a hardware device while\n> processing the request.\n>\n> ControlErrors occur when a Request was asked to set a control and the\n> pipeline handler attempted to do so, but it was rejected by the kernel.\n>\n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>\n> ---\n> v2:\n>  - Add documentation for Request::Private::setErrorFlags\n>\n> ---\n> v2\n>  - Extend documentation\n>\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>  include/libcamera/internal/request.h |  3 ++\n>  include/libcamera/request.h          |  9 ++++++\n>  src/libcamera/request.cpp            | 46 ++++++++++++++++++++++++++++\n>  3 files changed, 58 insertions(+)\n>\n> diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\n> index 9dadd6c60361..8e592272cfae 100644\n> --- a/include/libcamera/internal/request.h\n> +++ b/include/libcamera/internal/request.h\n> @@ -38,6 +38,7 @@ public:\n>  \tvoid complete();\n>  \tvoid cancel();\n>  \tvoid reuse();\n> +\tvoid setErrorFlags(ErrorFlags flags);\n\nPlease move after the 'prepared' signal with an empy line to match the\nprivate fields declaration order, where error_ follows the\nfence-related members.\n\n>\n>  \tvoid prepare(std::chrono::milliseconds timeout = 0ms);\n>  \tSignal<> prepared;\n> @@ -59,6 +60,8 @@ private:\n>  \tstd::unordered_set<FrameBuffer *> pending_;\n>  \tstd::map<FrameBuffer *, std::unique_ptr<EventNotifier>> notifiers_;\n>  \tstd::unique_ptr<Timer> timer_;\n> +\n> +\tErrorFlags error_;\n\nI wonder if s/Flags// almost everywhere\n\nIs this initialized ? Or just set in reuse() it doesn't seem to me\nthat reuse is called at class construction time\n\n>  };\n>\n>  } /* namespace libcamera */\n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index dffde1536cad..992629e11aa4 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -15,6 +15,7 @@\n>  #include <unordered_set>\n>\n>  #include <libcamera/base/class.h>\n> +#include <libcamera/base/flags.h>\n>  #include <libcamera/base/signal.h>\n>\n>  #include <libcamera/controls.h>\n> @@ -43,6 +44,13 @@ public:\n>  \t\tReuseBuffers = (1 << 0),\n>  \t};\n>\n> +\tenum ErrorFlag {\n\nLike here having this as ErrorId or similar\n\n> +\t\tNoError = 0,\n> +\t\tControlError = (1 << 0),\n> +\t};\n> +\n> +\tusing ErrorFlags = Flags<ErrorFlag>;\n\nand s/ErrorFlags/Errors/\n\n> +\n>  \tusing BufferMap = std::map<const Stream *, FrameBuffer *>;\n>\n>  \tRequest(Camera *camera, uint64_t cookie = 0);\n> @@ -60,6 +68,7 @@ public:\n>  \tuint32_t sequence() const;\n>  \tuint64_t cookie() const { return cookie_; }\n>  \tStatus status() const { return status_; }\n> +\tErrorFlags error() const;\n>\n>  \tbool hasPendingBuffers() const;\n>\n> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> index d2af1d2212ad..8b82757ea7e3 100644\n> --- a/src/libcamera/request.cpp\n> +++ b/src/libcamera/request.cpp\n> @@ -162,6 +162,7 @@ void Request::Private::cancel()\n>   */\n>  void Request::Private::reuse()\n>  {\n> +\terror_ = Request::NoError;\n>  \tsequence_ = 0;\n>  \tcancelled_ = false;\n>  \tprepared_ = false;\n> @@ -284,6 +285,21 @@ void Request::Private::notifierActivated(FrameBuffer *buffer)\n>  \temitPrepareCompleted();\n>  }\n>\n> +/**\n> + * \\brief Update the error flags of the Request\n> + * \\param[in] flags Flags to apply on the Request\n> + *\n> + * Apply \\a flags to the Request to report to the application when the Request\n> + * completes.\n> + *\n> + * Setting an Error flag does not cause a Request to fail, and once set it can\n> + * only be cleared by the application destroying the Request or calling reuse().\n> + */\n> +void Request::Private::setErrorFlags(ErrorFlags flags)\n> +{\n> +\terror_ |= flags;\n> +}\n> +\n>  void Request::Private::timeout()\n>  {\n>  \t/* A timeout can only happen if there are fences not yet signalled. */\n> @@ -318,6 +334,22 @@ void Request::Private::timeout()\n>   * Reuse the buffers that were previously added by addBuffer()\n>   */\n>\n> +/**\n> + * \\enum Request::ErrorFlag\n> + * Flags to report non-fatal errors\n> + * \\var Request::NoError\n> + * No error\n\nNo error. The Request completed succesfully and all its buffer contain\nvalid data.\n\n> + * \\var Request::ControlError\n> + * Control Error. At least on control was not able to be applied to the device.\n\ns/on/one\n\nIsn't \"was not able to be applied\" a bit convoluted ?\n\nAt lest one control was not correctly applied to the Camera.\n\n> + * The application should compare the metadata to the requested control values\n> + * to check which controls weren't applied.\n> + */\n> +\n> +/**\n> + * \\typedef Request::ErrorFlags\n> + * The error state of the request defined by \\a Request::ErrorFlag\n> + */\n> +\n>  /**\n>   * \\typedef Request::BufferMap\n>   * \\brief A map of Stream to FrameBuffer pointers\n> @@ -560,6 +592,20 @@ uint32_t Request::sequence() const\n>   * \\return The request completion status\n>   */\n>\n> +/**\n> + * \\brief Retrieve the error flags\n> + *\n> + * The request could complete with non-fatal error. The completion status will\n> + * indicate success. This function returns the non-fatal errors that the\n> + * request completed with\n> + *\n> + * \\return Flags of non-fatal errors that the request completed with\n\nI understand we have debated fatal vs non-fatal errors, but I would\nleave it out from here. As the error flags description doesn't mention\nthat the request is cancelled I don't think it's necessary to describe\nthis as non-fatal, mostly because there's not clear definition in the\ndocumentation of what fatal means.\n\nI would then remove non-fatal from the documentation and expand this a\nbit to make it clear this is separate from Request::Status.\n\nSpeaking of which, are errors meaningful when a request completes in\nCancelled state ? (to be eventually added below)\n\n\n/**\n * \\brief Retrieve the mask of error flags associated with a completed request\n *\n * The request could complete with errors, which indicate failures in\n * completing correctly parts of the request submitted by the application.\n *\n * The possible failure reasons are defined by the error flags defined\n * by Request::ErrorFlag and application are expected to retrieve the\n * mask of error flags by using this function before accessing the\n * buffers and data associated with a completed request.\n *\n * Error conditions reported through this function do not change the\n * request completion status retrieved through Request::status() which\n * indicates if the Request has been processed or not.\n *\n * \\return Mask of ErrorFlags that the request completed with\n */\n\nPlease note that the documentation of Request::status() mentions\n\"errors\" as well.\n\nIf we go towards a model where errors are reported through this\nfunction and the completion status is reported through \"status()\" the\ndocumentation there should be updated.\n\nIn example, if we want \"status\" to report if the Request has been\nprocessed or not (as a Cancelled request has not been processed at\nall, and that's the only failure status we have there) I would updated\nthe documentation with\n\n/*\n * \\fn Request::status()\n * \\brief Retrieve the request completion status\n *\n * The request status indicates whether the request has completed successfully\n * or has been cancelled before being processed.\n *\n * Requests are created with their status set to RequestPending. When\n * a Request is successfully processed and completed by the Camera its\n * status is set to RequestComplete. If a Request is cancelled before\n * being processed, in example because the Camera has been stopped\n * before the request is completed, its status is set to RequestCancelled.\n *\n * Successfully completed requests can complete with errors,\n * applications shall inspect the error mask returned by\n * Request::error() before accessing buffers and data associated with\n * a completed request.\n *\n * \\return The request completion status\n */\n\n\n> + */\n> +Request::ErrorFlags Request::error() const\n> +{\n> +\treturn _d()->error_;\n> +}\n> +\n>  /**\n>   * \\brief Check if a request has buffers yet to be completed\n>   *\n> --\n> 2.34.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 4F12CBE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 25 Jul 2022 07:54:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A1AEC63312;\n\tMon, 25 Jul 2022 09:54:28 +0200 (CEST)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8512E63309\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Jul 2022 09:54:27 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 0B8CE1BF208;\n\tMon, 25 Jul 2022 07:54:26 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658735668;\n\tbh=NSwbXzxRv5cdjjoz6j/RySosBfz7qTxlqfoSupaElDY=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=a6bZIGggyRl9uchZX2bpRB08HaE7zCEBUWvEXOpQJACa2fXtEZtAOYLSoBcG4rp4f\n\tnWWOUca63t/VXCVA1tb1JoxgvdIFNOUSce6vZJP+8TWq4gZKvX0KYFVeKC8p3Ne5XZ\n\tVmJ7N8Z+b7MQyQ7M4JUutxW4r9XA56bzLwRZKrsBO2wmvatAYCPvuQzxhY7thmBUOz\n\tX001G/RFc6VrIoU6HTCjEPszfTelilJZyhEZaSRjVYAnCPr/rNIhkk0ZdVtQvqhQVK\n\tFtCSyStIY55gzqFBVwKGrFS2v6XNGN4MoUKeBJyb8JbcffkDSQSNc/Ak7pZ0yfy5DG\n\tUMYvODSH/uoKQ==","Date":"Mon, 25 Jul 2022 09:54:25 +0200","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20220725075425.qgoql6s5tv64umog@uno.localdomain>","References":"<20220721121310.1286862-1-kieran.bingham@ideasonboard.com>\n\t<20220721121310.1286862-2-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220721121310.1286862-2-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 01/12] libcamera: request: Add\n\tsupport for error flags","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","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":24098,"web_url":"https://patchwork.libcamera.org/comment/24098/","msgid":"<165875272405.3981176.14427640190741424363@Monstersaurus>","date":"2022-07-25T12:38:44","subject":"Re: [libcamera-devel] [RFC PATCH 01/12] libcamera: request: Add\n\tsupport for error flags","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Umang Jain (2022-07-22 16:15:05)\n> Hi Kieran,\n> \n> On 7/21/22 17:42, Kieran Bingham via libcamera-devel wrote:\n> > From: Paul Elder <paul.elder@ideasonboard.com>\n> >\n> > Add error flags to the Request to indicate non-fatal errors.\n> >\n> > Applications should check the error() state of the Request to determine\n> > if any fault occured while processing the request.\n> >\n> > Initially, a single error flag ControlError is added to allow a pipeline\n> > handler to report a failure to set controls on a hardware device while\n> > processing the request.\n> >\n> > ControlErrors occur when a Request was asked to set a control and the\n> > pipeline handler attempted to do so, but it was rejected by the kernel.\n> >\n> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> >\n> > ---\n> > v2:\n> >   - Add documentation for Request::Private::setErrorFlags\n> >\n> > ---\n> > v2\n> >   - Extend documentation\n> >\n> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > ---\n> >   include/libcamera/internal/request.h |  3 ++\n> >   include/libcamera/request.h          |  9 ++++++\n> >   src/libcamera/request.cpp            | 46 ++++++++++++++++++++++++++++\n> >   3 files changed, 58 insertions(+)\n> >\n> > diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\n> > index 9dadd6c60361..8e592272cfae 100644\n> > --- a/include/libcamera/internal/request.h\n> > +++ b/include/libcamera/internal/request.h\n> > @@ -38,6 +38,7 @@ public:\n> >       void complete();\n> >       void cancel();\n> >       void reuse();\n> > +     void setErrorFlags(ErrorFlags flags);\n> >   \n> >       void prepare(std::chrono::milliseconds timeout = 0ms);\n> >       Signal<> prepared;\n> > @@ -59,6 +60,8 @@ private:\n> >       std::unordered_set<FrameBuffer *> pending_;\n> >       std::map<FrameBuffer *, std::unique_ptr<EventNotifier>> notifiers_;\n> >       std::unique_ptr<Timer> timer_;\n> > +\n> > +     ErrorFlags error_;\n> >   };\n> >   \n> >   } /* namespace libcamera */\n> > diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> > index dffde1536cad..992629e11aa4 100644\n> > --- a/include/libcamera/request.h\n> > +++ b/include/libcamera/request.h\n> > @@ -15,6 +15,7 @@\n> >   #include <unordered_set>\n> >   \n> >   #include <libcamera/base/class.h>\n> > +#include <libcamera/base/flags.h>\n> >   #include <libcamera/base/signal.h>\n> >   \n> >   #include <libcamera/controls.h>\n> > @@ -43,6 +44,13 @@ public:\n> >               ReuseBuffers = (1 << 0),\n> >       };\n> >   \n> > +     enum ErrorFlag {\n> > +             NoError = 0,\n> > +             ControlError = (1 << 0),\n> > +     };\n> > +\n> > +     using ErrorFlags = Flags<ErrorFlag>;\n> > +\n> >       using BufferMap = std::map<const Stream *, FrameBuffer *>;\n> >   \n> >       Request(Camera *camera, uint64_t cookie = 0);\n> > @@ -60,6 +68,7 @@ public:\n> >       uint32_t sequence() const;\n> >       uint64_t cookie() const { return cookie_; }\n> >       Status status() const { return status_; }\n> > +     ErrorFlags error() const;\n> >   \n> >       bool hasPendingBuffers() const;\n> >   \n> > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> > index d2af1d2212ad..8b82757ea7e3 100644\n> > --- a/src/libcamera/request.cpp\n> > +++ b/src/libcamera/request.cpp\n> > @@ -162,6 +162,7 @@ void Request::Private::cancel()\n> >    */\n> >   void Request::Private::reuse()\n> >   {\n> > +     error_ = Request::NoError;\n> >       sequence_ = 0;\n> >       cancelled_ = false;\n> >       prepared_ = false;\n> > @@ -284,6 +285,21 @@ void Request::Private::notifierActivated(FrameBuffer *buffer)\n> >       emitPrepareCompleted();\n> >   }\n> >   \n> > +/**\n> > + * \\brief Update the error flags of the Request\n> > + * \\param[in] flags Flags to apply on the Request\n> > + *\n> > + * Apply \\a flags to the Request to report to the application when the Request\n> > + * completes.\n> \n> \n> s/to report to the application/which will get reported to the application/\n\nAck.\n\n\n> \n> > + *\n> > + * Setting an Error flag does not cause a Request to fail, and once set it can\n> > + * only be cleared by the application destroying the Request or calling reuse().\n> > + */\n> > +void Request::Private::setErrorFlags(ErrorFlags flags)\n> > +{\n> > +     error_ |= flags;\n> > +}\n> > +\n> >   void Request::Private::timeout()\n> >   {\n> >       /* A timeout can only happen if there are fences not yet signalled. */\n> > @@ -318,6 +334,22 @@ void Request::Private::timeout()\n> >    * Reuse the buffers that were previously added by addBuffer()\n> >    */\n> >   \n> > +/**\n> > + * \\enum Request::ErrorFlag\n> > + * Flags to report non-fatal errors\n> > + * \\var Request::NoError\n> > + * No error\n> > + * \\var Request::ControlError\n> > + * Control Error. At least on control was not able to be applied to the device.\n> \n> \n> s/on/one/ maybe?\n\nAck.\n\n\n> \n> > + * The application should compare the metadata to the requested control values\n> > + * to check which controls weren't applied.\n> > + */\n> > +\n> > +/**\n> > + * \\typedef Request::ErrorFlags\n> > + * The error state of the request defined by \\a Request::ErrorFlag\n> > + */\n> > +\n> >   /**\n> >    * \\typedef Request::BufferMap\n> >    * \\brief A map of Stream to FrameBuffer pointers\n> > @@ -560,6 +592,20 @@ uint32_t Request::sequence() const\n> >    * \\return The request completion status\n> >    */\n> >   \n> > +/**\n> > + * \\brief Retrieve the error flags\n> > + *\n> > + * The request could complete with non-fatal error. The completion status will\n> > + * indicate success. This function returns the non-fatal errors that the\n> > + * request completed with\n> \n> \n> s/that the request completed with/with which the request was completed.\n\nAck, but I think Jacopo has comments here too.\n\n\n> \n> > + *\n> > + * \\return Flags of non-fatal errors that the request completed with\n> > + */\n> > +Request::ErrorFlags Request::error() const\n> > +{\n> > +     return _d()->error_;\n> > +}\n> > +\n> >   /**\n> >    * \\brief Check if a request has buffers yet to be completed\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 A47C0BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 25 Jul 2022 12:38:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2947E63312;\n\tMon, 25 Jul 2022 14:38:47 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 462F86330A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Jul 2022 14:38:46 +0200 (CEST)","from pendragon.ideasonboard.com\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 C73086DD;\n\tMon, 25 Jul 2022 14:38:45 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658752727;\n\tbh=BOITWmoyAtHsH11QUXfJJkxzYS2Xp+ihAZt3jE+pHPQ=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=G4NjT8k6Sr8MA/bo/Pj5ur330gVjHStKmHVfPl7QkjqOnk+ZJsggzXsupKJafTwOd\n\trzIxhIEpN+0zBWEHMlI6oUVzGqyqMuupXxtY+4UdwfkZjUexAMH1JiVG3yXcLxzjWP\n\twalY4bul7ecRPq9W2rr3PBwXHc3TnfxiCgTw5Q06PpTFj1UqN7wUE8ot1/vvrgLE2U\n\tJ5xV2Y5Yk6HSMDSFisLRvodG4K/8eaX/hQSOgGbP++qKDdLHrNaj85n1qBKiG/hRBX\n\tTOa8PXjtXj05mlc+bAMM3duW+vDYBcpMh/J7s8OmNOGKiixN8kX2DV8TiL7Hux/zMQ\n\t+Xb/OAP87KDdQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658752725;\n\tbh=BOITWmoyAtHsH11QUXfJJkxzYS2Xp+ihAZt3jE+pHPQ=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=sizvKqW02D1bh+66wU0VU2eQpx6QVmxZriNeHfM7JPoj8HKDxQpMbPg4DgdAWRFhd\n\ti5Rjv/885Uh5XJdXSaRMXzHJjgC8m4/PC6om76RRLYM2kM0NZUQT3ecwZUeYkpQBHx\n\t0XqwIEp0MKrJAkUeyb5rRoh4eLj9N+dhPNfOGS/g="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"sizvKqW0\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<d3d878ec-e48a-bed6-0799-c945f407a1dc@ideasonboard.com>","References":"<20220721121310.1286862-1-kieran.bingham@ideasonboard.com>\n\t<20220721121310.1286862-2-kieran.bingham@ideasonboard.com>\n\t<d3d878ec-e48a-bed6-0799-c945f407a1dc@ideasonboard.com>","To":"Umang Jain <umang.jain@ideasonboard.com>,\n\tlibcamera devel <libcamera-devel@lists.libcamera.org>","Date":"Mon, 25 Jul 2022 13:38:44 +0100","Message-ID":"<165875272405.3981176.14427640190741424363@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [RFC PATCH 01/12] libcamera: request: Add\n\tsupport for error flags","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>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24102,"web_url":"https://patchwork.libcamera.org/comment/24102/","msgid":"<165876105046.3981176.10018728905338546740@Monstersaurus>","date":"2022-07-25T14:57:30","subject":"Re: [libcamera-devel] [RFC PATCH 01/12] libcamera: request: Add\n\tsupport for error flags","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"@Quoting Jacopo Mondi (2022-07-25 08:54:25)\n> Hi Kieran\n> \n> On Thu, Jul 21, 2022 at 01:12:59PM +0100, Kieran Bingham via libcamera-devel wrote:\n> > From: Paul Elder <paul.elder@ideasonboard.com>\n> >\n> > Add error flags to the Request to indicate non-fatal errors.\n> >\n> > Applications should check the error() state of the Request to determine\n> > if any fault occured while processing the request.\n> >\n> > Initially, a single error flag ControlError is added to allow a pipeline\n> > handler to report a failure to set controls on a hardware device while\n> > processing the request.\n> >\n> > ControlErrors occur when a Request was asked to set a control and the\n> > pipeline handler attempted to do so, but it was rejected by the kernel.\n> >\n> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> >\n> > ---\n> > v2:\n> >  - Add documentation for Request::Private::setErrorFlags\n> >\n> > ---\n> > v2\n> >  - Extend documentation\n> >\n> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > ---\n> >  include/libcamera/internal/request.h |  3 ++\n> >  include/libcamera/request.h          |  9 ++++++\n> >  src/libcamera/request.cpp            | 46 ++++++++++++++++++++++++++++\n> >  3 files changed, 58 insertions(+)\n> >\n> > diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\n> > index 9dadd6c60361..8e592272cfae 100644\n> > --- a/include/libcamera/internal/request.h\n> > +++ b/include/libcamera/internal/request.h\n> > @@ -38,6 +38,7 @@ public:\n> >       void complete();\n> >       void cancel();\n> >       void reuse();\n> > +     void setErrorFlags(ErrorFlags flags);\n> \n> Please move after the 'prepared' signal with an empy line to match the\n> private fields declaration order, where error_ follows the\n> fence-related members.\n\nOk.\n\n\n> >\n> >       void prepare(std::chrono::milliseconds timeout = 0ms);\n> >       Signal<> prepared;\n> > @@ -59,6 +60,8 @@ private:\n> >       std::unordered_set<FrameBuffer *> pending_;\n> >       std::map<FrameBuffer *, std::unique_ptr<EventNotifier>> notifiers_;\n> >       std::unique_ptr<Timer> timer_;\n> > +\n> > +     ErrorFlags error_;\n> \n> I wonder if s/Flags// almost everywhere\n> \n> Is this initialized ? Or just set in reuse() it doesn't seem to me\n> that reuse is called at class construction time\n\nFlags<> are always initiliased to '0':\n\n https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/base/flags.cpp#n36\n https://git.libcamera.org/libcamera/libcamera.git/tree/include/libcamera/base/flags.h#n23\n\n\n> >  };\n> >\n> >  } /* namespace libcamera */\n> > diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> > index dffde1536cad..992629e11aa4 100644\n> > --- a/include/libcamera/request.h\n> > +++ b/include/libcamera/request.h\n> > @@ -15,6 +15,7 @@\n> >  #include <unordered_set>\n> >\n> >  #include <libcamera/base/class.h>\n> > +#include <libcamera/base/flags.h>\n> >  #include <libcamera/base/signal.h>\n> >\n> >  #include <libcamera/controls.h>\n> > @@ -43,6 +44,13 @@ public:\n> >               ReuseBuffers = (1 << 0),\n> >       };\n> >\n> > +     enum ErrorFlag {\n> \n> Like here having this as ErrorId or similar\n\nI don't mind changing Flag for Id, I hadn't given it much thought of the\nname itself.\n\n\n> \n> > +             NoError = 0,\n> > +             ControlError = (1 << 0),\n> > +     };\n> > +\n> > +     using ErrorFlags = Flags<ErrorFlag>;\n> \n> and s/ErrorFlags/Errors/\n\nIf we don't need to call them flags, I think that's fine yes.\n\n\n> \n> > +\n> >       using BufferMap = std::map<const Stream *, FrameBuffer *>;\n> >\n> >       Request(Camera *camera, uint64_t cookie = 0);\n> > @@ -60,6 +68,7 @@ public:\n> >       uint32_t sequence() const;\n> >       uint64_t cookie() const { return cookie_; }\n> >       Status status() const { return status_; }\n> > +     ErrorFlags error() const;\n> >\n> >       bool hasPendingBuffers() const;\n> >\n> > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> > index d2af1d2212ad..8b82757ea7e3 100644\n> > --- a/src/libcamera/request.cpp\n> > +++ b/src/libcamera/request.cpp\n> > @@ -162,6 +162,7 @@ void Request::Private::cancel()\n> >   */\n> >  void Request::Private::reuse()\n> >  {\n> > +     error_ = Request::NoError;\n> >       sequence_ = 0;\n> >       cancelled_ = false;\n> >       prepared_ = false;\n> > @@ -284,6 +285,21 @@ void Request::Private::notifierActivated(FrameBuffer *buffer)\n> >       emitPrepareCompleted();\n> >  }\n> >\n> > +/**\n> > + * \\brief Update the error flags of the Request\n> > + * \\param[in] flags Flags to apply on the Request\n> > + *\n> > + * Apply \\a flags to the Request to report to the application when the Request\n> > + * completes.\n> > + *\n> > + * Setting an Error flag does not cause a Request to fail, and once set it can\n> > + * only be cleared by the application destroying the Request or calling reuse().\n> > + */\n> > +void Request::Private::setErrorFlags(ErrorFlags flags)\n> > +{\n> > +     error_ |= flags;\n> > +}\n> > +\n> >  void Request::Private::timeout()\n> >  {\n> >       /* A timeout can only happen if there are fences not yet signalled. */\n> > @@ -318,6 +334,22 @@ void Request::Private::timeout()\n> >   * Reuse the buffers that were previously added by addBuffer()\n> >   */\n> >\n> > +/**\n> > + * \\enum Request::ErrorFlag\n> > + * Flags to report non-fatal errors\n> > + * \\var Request::NoError\n> > + * No error\n> \n> No error. The Request completed succesfully and all its buffer contain\n> valid data.\n\nYes, I think that's a good expansion.\n\n\n> \n> > + * \\var Request::ControlError\n> > + * Control Error. At least on control was not able to be applied to the device.\n> \n> s/on/one\n> \n> Isn't \"was not able to be applied\" a bit convoluted ?\n> \n> At lest one control was not correctly applied to the Camera.\n\nThe issue I have here, is that as I understand it - failing to set one\n- probably means failing to set more than one control. All following\ncontrols in the control list are in indetermined state, while previous\ncontrols (of the list) are set.\n\nEssentially - it means our overall state becomes a bit undefined. :-(\n\n\n \n> > + * The application should compare the metadata to the requested control values\n> > + * to check which controls weren't applied.\n> > + */\n> > +\n> > +/**\n> > + * \\typedef Request::ErrorFlags\n> > + * The error state of the request defined by \\a Request::ErrorFlag\n> > + */\n> > +\n> >  /**\n> >   * \\typedef Request::BufferMap\n> >   * \\brief A map of Stream to FrameBuffer pointers\n> > @@ -560,6 +592,20 @@ uint32_t Request::sequence() const\n> >   * \\return The request completion status\n> >   */\n> >\n> > +/**\n> > + * \\brief Retrieve the error flags\n> > + *\n> > + * The request could complete with non-fatal error. The completion status will\n> > + * indicate success. This function returns the non-fatal errors that the\n> > + * request completed with\n> > + *\n> > + * \\return Flags of non-fatal errors that the request completed with\n> \n> I understand we have debated fatal vs non-fatal errors, but I would\n> leave it out from here. As the error flags description doesn't mention\n> that the request is cancelled I don't think it's necessary to describe\n> this as non-fatal, mostly because there's not clear definition in the\n> documentation of what fatal means.\n> \n> I would then remove non-fatal from the documentation and expand this a\n> bit to make it clear this is separate from Request::Status.\n> \n> Speaking of which, are errors meaningful when a request completes in\n> Cancelled state ? (to be eventually added below)\n\n\nNo - if a request is cancelled, then the error flags are meaningless. As\nI udnerstand it currently Request state Cancelled - means do not requeue\nthe request. And do not even look into the request, it's all invalid.\n\n\n> /**\n>  * \\brief Retrieve the mask of error flags associated with a completed request\n>  *\n>  * The request could complete with errors, which indicate failures in\n>  * completing correctly parts of the request submitted by the application.\n>  *\n>  * The possible failure reasons are defined by the error flags defined\n>  * by Request::ErrorFlag and application are expected to retrieve the\n>  * mask of error flags by using this function before accessing the\n>  * buffers and data associated with a completed request.\n>  *\n>  * Error conditions reported through this function do not change the\n>  * request completion status retrieved through Request::status() which\n>  * indicates if the Request has been processed or not.\n>  *\n>  * \\return Mask of ErrorFlags that the request completed with\n>  */\n> \n> Please note that the documentation of Request::status() mentions\n> \"errors\" as well.\n> \n> If we go towards a model where errors are reported through this\n> function and the completion status is reported through \"status()\" the\n> documentation there should be updated.\n> \n> In example, if we want \"status\" to report if the Request has been\n> processed or not (as a Cancelled request has not been processed at\n> all, and that's the only failure status we have there) I would updated\n> the documentation with\n> \n> /*\n>  * \\fn Request::status()\n>  * \\brief Retrieve the request completion status\n>  *\n>  * The request status indicates whether the request has completed successfully\n>  * or has been cancelled before being processed.\n>  *\n>  * Requests are created with their status set to RequestPending. When\n>  * a Request is successfully processed and completed by the Camera its\n>  * status is set to RequestComplete. If a Request is cancelled before\n>  * being processed, in example because the Camera has been stopped\n>  * before the request is completed, its status is set to RequestCancelled.\n>  *\n>  * Successfully completed requests can complete with errors,\n>  * applications shall inspect the error mask returned by\n>  * Request::error() before accessing buffers and data associated with\n>  * a completed request.\n>  *\n>  * \\return The request completion status\n>  */\n\nThat sounds good too. (With a very small grammar fix to \n  \"/in example/for example\"\n\n> \n> \n> > + */\n> > +Request::ErrorFlags Request::error() const\n> > +{\n> > +     return _d()->error_;\n> > +}\n> > +\n> >  /**\n> >   * \\brief Check if a request has buffers yet to be completed\n> >   *\n> > --\n> > 2.34.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 28E82BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 25 Jul 2022 14:57:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D94FB63315;\n\tMon, 25 Jul 2022 16:57:35 +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 710726330E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Jul 2022 16:57:33 +0200 (CEST)","from pendragon.ideasonboard.com\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 E45066DD;\n\tMon, 25 Jul 2022 16:57:32 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658761055;\n\tbh=foeUG5TrCRbm0+nbfdqqsTcl7StzPRLMeXYl0qz4+yU=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=QVfVleDfkusJUV+0rawBv3s03Fs4L2g3FHTylKOYaqBnEiaeLGwWTJS9rsfuEMNwE\n\t6CkgfXmvZ2WvUK9k9xpHx7xcEJFBfq5Z2VBzzbEMXMUysZl3S2rJnEEO4h3ua4lyt3\n\tlZUNt8mUPEN0ZY+vfhQeev12uO2nR6/7NM7KzhvnzXPp+B5oioTKh/kMSUqRHvKFtd\n\tpWACDp2P7D8G3Y7tn5xZP/Ymz4rrbcZx1Xvv5YQB2Z36kNPTN9HL7NTVyUteSWisCs\n\tEqRw0oGwHaAGxMfKvVfPU/0eaNthI5KsZACJCbfhie9dfli3L8aReWQJP16wRpPNqr\n\tzd8/sqSjMhlMg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658761053;\n\tbh=foeUG5TrCRbm0+nbfdqqsTcl7StzPRLMeXYl0qz4+yU=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=WnYw0wIBcpBd1KaAIH+HdcrPiva1aN8cnh7rS8aIitlA07nlmGPjPEiXsbPbUakCZ\n\t74goCFsqCO1fS4z+eBJ6mHSWZ8SGCmKeccT/BZLW/XNYzlBg42r0VepiCOqXiSuLrK\n\tUztnBzqRCUwYMTwGzq8WZbdAJpm9SNeWVN0jdeHo="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"WnYw0wIB\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220725075425.qgoql6s5tv64umog@uno.localdomain>","References":"<20220721121310.1286862-1-kieran.bingham@ideasonboard.com>\n\t<20220721121310.1286862-2-kieran.bingham@ideasonboard.com>\n\t<20220725075425.qgoql6s5tv64umog@uno.localdomain>","To":"Jacopo Mondi <jacopo@jmondi.org>","Date":"Mon, 25 Jul 2022 15:57:30 +0100","Message-ID":"<165876105046.3981176.10018728905338546740@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [RFC PATCH 01/12] libcamera: request: Add\n\tsupport for error flags","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>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","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>"}}]