[{"id":16013,"web_url":"https://patchwork.libcamera.org/comment/16013/","msgid":"<bf9d7d8f-1a14-a36f-0066-0a8587ec8e75@ideasonboard.com>","date":"2021-03-29T10:27:54","subject":"Re: [libcamera-devel] [PATCH v2 3/3] Regard a request error in the\n\trequest completion","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Hiro,\n\nOn 29/03/2021 10:15, Hirokazu Honda wrote:\n> libcamera::Request contains an error value. Every\n> libcamera::Camera client should regards the error in a\n> request completion.\n> \n> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>\n> ---\n>  Documentation/guides/application-developer.rst | 2 ++\n>  src/cam/capture.cpp                            | 5 +++++\n>  src/gstreamer/gstlibcamerasrc.cpp              | 6 +++++-\n>  src/qcam/main_window.cpp                       | 4 ++++\n>  src/v4l2/v4l2_camera.cpp                       | 5 +++++\n>  5 files changed, 21 insertions(+), 1 deletion(-)\n> \n> diff --git a/Documentation/guides/application-developer.rst b/Documentation/guides/application-developer.rst\n> index e3430f03..e8a1a7b4 100644\n> --- a/Documentation/guides/application-developer.rst\n> +++ b/Documentation/guides/application-developer.rst\n> @@ -392,6 +392,8 @@ the `Request::Status`_ class enum documentation.\n>  \n>     if (request->status() == Request::RequestCancelled)\n>        return;\n> +   if (request->status() == Request::RequestError)\n> +      // handle error.\n\nEven if it's just documentation, we should wrap that // handle error. in\n{ } as it leaves the if () statement applying to whatever would happen\nnext otherwise...\n\n\n\n>  If the ``Request`` has completed successfully, applications can access the\n>  completed buffers using the ``Request::buffers()`` function, which returns a map\n> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> index 7b55fc67..3e9edf80 100644\n> --- a/src/cam/capture.cpp\n> +++ b/src/cam/capture.cpp\n> @@ -167,6 +167,11 @@ void Capture::requestComplete(Request *request)\n>  {\n>  \tif (request->status() == Request::RequestCancelled)\n>  \t\treturn;\n> +\t/* TODO: Handle an error correctly */\n> +\tif (request->status() == Request::RequestError) {\n> +\t\tstd::cout << \"Failed to capture request: \" << request->cookie();\n> +\t\treturn;\n\nI don't think we can let this return early here. It would cause us to\ndrop errorred frames, and then run out of active requests.\n\nIf a Request status is RequestError, then we need to decide if the frame\nis to be dropped or displayed regardless. (which is an application\nchoice, rather than a libcamera choice). That's fine as we're here in cam..\n\nAn application which wants to display only 'good' frames would likely\nchoose to not display this frame, but it would then need to make sure\nthat a new request would be queued still.\n\nFor cam, I think we *should* track errored frames, as they are important\nin testing.\n\nSo this should certainly call into Capture::processRequest(), and likely\nprint a notice saying that this frame was an error. And still save it\naccordingly...\n\nAlthough, I'm not sure what RequestError means yet.\nDoes it mean there was an error and the Request wasn't processed\ncompletely, or that it wasn't processed at all.\n\nPerhaps the use case for a buffer not completely being processed is\nalready handled by the buffer status...\n\n\n\n> +\t}\n>  \n>  \t/*\n>  \t * Defer processing of the completed request to the event loop, to avoid\n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 87246b40..1ecb9883 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -163,10 +163,14 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n>  \n>  \tg_return_if_fail(wrap->request_.get() == request);\n>  \n> -\tif ((request->status() == Request::RequestCancelled)) {\n> +\tif (request->status() == Request::RequestCancelled) {\n>  \t\tGST_DEBUG_OBJECT(src_, \"Request was cancelled\");\n>  \t\treturn;\n>  \t}\n> +\tif (request->status() == Request::RequestError) {\n> +\t\tGST_ERROR_OBJECT(src_, \"Request doesn't complete successfully\");\n\ns/doesn't/didn't/\n\nI haven't looked into how gstlibcamerasrc uses it's requests yet, but if\nthis leaks a request and causes the pipeline to stall that would be bad too.\n\n\n> +\t\treturn;\n> +\t}\n>  \n>  \tGstBuffer *buffer;\n>  \tfor (GstPad *srcpad : srcpads_) {\n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index 39d034de..1288bcd5 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -694,6 +694,10 @@ void MainWindow::requestComplete(Request *request)\n>  {\n>  \tif (request->status() == Request::RequestCancelled)\n>  \t\treturn;\n> +\tif (request->status() == Request::RequestError) {\n> +\t\tqCritical() << \"Request doesn't complete successfully\";\n\ns/doesn't/didn't/\n\nAnd the same concerns of course.\n\n> +\t\treturn;\n> +\t}\n>  \n>  \t/*\n>  \t * We're running in the libcamera thread context, expensive operations\n> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp\n> index 97825c71..d6b1d755 100644\n> --- a/src/v4l2/v4l2_camera.cpp\n> +++ b/src/v4l2/v4l2_camera.cpp\n> @@ -84,6 +84,11 @@ void V4L2Camera::requestComplete(Request *request)\n>  {\n>  \tif (request->status() == Request::RequestCancelled)\n>  \t\treturn;\n> +\tif (request->status() == Request::RequestError) {\n> +\t\tLOG(V4L2Compat, Error)\n> +\t\t\t<< \"Request doesn't complete successfully\";\n\nAnd same ;-)\n\n> +\t\treturn;\n> +\t}\n>  \n>  \t/* We only have one stream at the moment. */\n>  \tbufferLock_.lock();\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 7E462C32EA\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 29 Mar 2021 10:28:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 91D1B68785;\n\tMon, 29 Mar 2021 12:27:59 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C88E968782\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 29 Mar 2021 12:27:57 +0200 (CEST)","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 3E6A0503;\n\tMon, 29 Mar 2021 12:27:57 +0200 (CEST)"],"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=\"AKGjXFda\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1617013677;\n\tbh=5Ks2TUIY7nrYifxub5GnHuybZjUJhdC8wl1omxKX/cI=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=AKGjXFdaRTe51Mq2uWSUMF5kN7NfVAdMi4Cokc4rSMpoYMIYYBQyLmye6jbjcZ3oY\n\tZRtOhs5svZgd46/O3aHlP+j5Z9piIu5CFcqHkARZ39AYVjyAnA9Mw094ooFC2AphxQ\n\tysFafMF+F/lL3BIZZQ7Kdq4sHBx+XpPTDytnDsjg=","To":"Hirokazu Honda <hiroh@chromium.org>, libcamera-devel@lists.libcamera.org","References":"<20210329091552.157208-1-hiroh@chromium.org>\n\t<20210329091552.157208-4-hiroh@chromium.org>","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":"<bf9d7d8f-1a14-a36f-0066-0a8587ec8e75@ideasonboard.com>","Date":"Mon, 29 Mar 2021 11:27:54 +0100","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":"<20210329091552.157208-4-hiroh@chromium.org>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] Regard a request error in the\n\trequest completion","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","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":16101,"web_url":"https://patchwork.libcamera.org/comment/16101/","msgid":"<YGfEZ+g0/HK76hcd@pendragon.ideasonboard.com>","date":"2021-04-03T01:27:03","subject":"Re: [libcamera-devel] [PATCH v2 3/3] Regard a request error in the\n\trequest completion","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran and Hiro,\n\nOn Mon, Mar 29, 2021 at 11:27:54AM +0100, Kieran Bingham wrote:\n> On 29/03/2021 10:15, Hirokazu Honda wrote:\n> > libcamera::Request contains an error value. Every\n> > libcamera::Camera client should regards the error in a\n> > request completion.\n> > \n> > Signed-off-by: Hirokazu Honda <hiroh@chromium.org>\n> > ---\n> >  Documentation/guides/application-developer.rst | 2 ++\n> >  src/cam/capture.cpp                            | 5 +++++\n> >  src/gstreamer/gstlibcamerasrc.cpp              | 6 +++++-\n> >  src/qcam/main_window.cpp                       | 4 ++++\n> >  src/v4l2/v4l2_camera.cpp                       | 5 +++++\n> >  5 files changed, 21 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/Documentation/guides/application-developer.rst b/Documentation/guides/application-developer.rst\n> > index e3430f03..e8a1a7b4 100644\n> > --- a/Documentation/guides/application-developer.rst\n> > +++ b/Documentation/guides/application-developer.rst\n> > @@ -392,6 +392,8 @@ the `Request::Status`_ class enum documentation.\n> >  \n> >     if (request->status() == Request::RequestCancelled)\n> >        return;\n> > +   if (request->status() == Request::RequestError)\n> > +      // handle error.\n> \n> Even if it's just documentation, we should wrap that // handle error. in\n> { } as it leaves the if () statement applying to whatever would happen\n> next otherwise...\n> \n> >  If the ``Request`` has completed successfully, applications can access the\n> >  completed buffers using the ``Request::buffers()`` function, which returns a map\n> > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> > index 7b55fc67..3e9edf80 100644\n> > --- a/src/cam/capture.cpp\n> > +++ b/src/cam/capture.cpp\n> > @@ -167,6 +167,11 @@ void Capture::requestComplete(Request *request)\n> >  {\n> >  \tif (request->status() == Request::RequestCancelled)\n> >  \t\treturn;\n> > +\t/* TODO: Handle an error correctly */\n\nWe use the doxygen syntax, /* \\todo ... */\n\n> > +\tif (request->status() == Request::RequestError) {\n> > +\t\tstd::cout << \"Failed to capture request: \" << request->cookie();\n> > +\t\treturn;\n> \n> I don't think we can let this return early here. It would cause us to\n> drop errorred frames, and then run out of active requests.\n> \n> If a Request status is RequestError, then we need to decide if the frame\n> is to be dropped or displayed regardless. (which is an application\n> choice, rather than a libcamera choice). That's fine as we're here in cam..\n\nI think we need to take a step back, and reconsider the design of error\nreporting. We currently have a request status, which can only report\nsuccessful completion or full cancellation of the request, and we have a\nbuffer status that can report erroneous buffers (without any additional\ninformation). The two don't really need to interact together. With the\nintroduction of RequestError, this situation changes, and the\ninteraction of buffer status and request status needs to be\nreconsidered. I'd like to see a design proposal for that (as part of\npatch 2/3), that would consider the combination of statuses that are\nallowed, and what each of the possible combinations mean. For instance\n(starting brainstorming here), we could consider that RequestError\nsignals an error at the request level that means that no buffer have\nbeen written (all the buffer metadata would then be invalid), or we\ncould consider that any buffer that is erroneous would result in the\nrequest being marked with RequestError (in which case applications would\nneed to inspect individual buffers). This needs to be designed by\nconsidering use cases, including the error notification mechanism of the\nAndroid camera HAL, as well as ease of use for applications. We can also\nconsider adding additional error information (possibly as part of this\nseries, or later on top), to differentiate, for instance, between fatal\nerrors that indicate that the camera has become unusable, and errors\nthat are specific to a request but won't affect further requests (is\nthis something that can ever happen ?). This can be done with additional\ninformation (an error code maybe ?), or additional request statuses.\nFinally, if we decide to add error codes to requests, we may consider\nturning RequestCancel into an error code.\n\nThis is a bit of work, an RFC with a design proposal (or multiple\noptions to choose from) and an explanation of the design rationale and\nassociated use cases would help.\n\n> An application which wants to display only 'good' frames would likely\n> choose to not display this frame, but it would then need to make sure\n> that a new request would be queued still.\n> \n> For cam, I think we *should* track errored frames, as they are important\n> in testing.\n> \n> So this should certainly call into Capture::processRequest(), and likely\n> print a notice saying that this frame was an error. And still save it\n> accordingly...\n> \n> Although, I'm not sure what RequestError means yet.\n> Does it mean there was an error and the Request wasn't processed\n> completely, or that it wasn't processed at all.\n> \n> Perhaps the use case for a buffer not completely being processed is\n> already handled by the buffer status...\n> \n> > +\t}\n> >  \n> >  \t/*\n> >  \t * Defer processing of the completed request to the event loop, to avoid\n> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > index 87246b40..1ecb9883 100644\n> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > @@ -163,10 +163,14 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n> >  \n> >  \tg_return_if_fail(wrap->request_.get() == request);\n> >  \n> > -\tif ((request->status() == Request::RequestCancelled)) {\n> > +\tif (request->status() == Request::RequestCancelled) {\n> >  \t\tGST_DEBUG_OBJECT(src_, \"Request was cancelled\");\n> >  \t\treturn;\n> >  \t}\n> > +\tif (request->status() == Request::RequestError) {\n> > +\t\tGST_ERROR_OBJECT(src_, \"Request doesn't complete successfully\");\n> \n> s/doesn't/didn't/\n> \n> I haven't looked into how gstlibcamerasrc uses it's requests yet, but if\n> this leaks a request and causes the pipeline to stall that would be bad too.\n> \n> > +\t\treturn;\n> > +\t}\n> >  \n> >  \tGstBuffer *buffer;\n> >  \tfor (GstPad *srcpad : srcpads_) {\n> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> > index 39d034de..1288bcd5 100644\n> > --- a/src/qcam/main_window.cpp\n> > +++ b/src/qcam/main_window.cpp\n> > @@ -694,6 +694,10 @@ void MainWindow::requestComplete(Request *request)\n> >  {\n> >  \tif (request->status() == Request::RequestCancelled)\n> >  \t\treturn;\n> > +\tif (request->status() == Request::RequestError) {\n> > +\t\tqCritical() << \"Request doesn't complete successfully\";\n> \n> s/doesn't/didn't/\n> \n> And the same concerns of course.\n> \n> > +\t\treturn;\n> > +\t}\n> >  \n> >  \t/*\n> >  \t * We're running in the libcamera thread context, expensive operations\n> > diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp\n> > index 97825c71..d6b1d755 100644\n> > --- a/src/v4l2/v4l2_camera.cpp\n> > +++ b/src/v4l2/v4l2_camera.cpp\n> > @@ -84,6 +84,11 @@ void V4L2Camera::requestComplete(Request *request)\n> >  {\n> >  \tif (request->status() == Request::RequestCancelled)\n> >  \t\treturn;\n> > +\tif (request->status() == Request::RequestError) {\n> > +\t\tLOG(V4L2Compat, Error)\n> > +\t\t\t<< \"Request doesn't complete successfully\";\n> \n> And same ;-)\n> \n> > +\t\treturn;\n> > +\t}\n> >  \n> >  \t/* We only have one stream at the moment. */\n> >  \tbufferLock_.lock();","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 9C344BD695\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat,  3 Apr 2021 01:27:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 082BE602E6;\n\tSat,  3 Apr 2021 03:27:51 +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 055BD602CF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat,  3 Apr 2021 03:27:49 +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 666903D7;\n\tSat,  3 Apr 2021 03:27:48 +0200 (CEST)"],"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=\"Zb2qBfZB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1617413268;\n\tbh=rEO8RHXG93hAxnEJygrdA+dlaKyFcczj/zAzzATi2Hs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Zb2qBfZBYWXLjUNP3NPupG1rc2Dx+K7yEya4Gawf5tK8+iX53jnlQLj0lvO874WKr\n\t09Y7w8jXJCb6MhPuRoOVdtz3KG1cW69wL8Ui8LBcG41JbBx3t9HgX+c/Ut5UF4yJA6\n\tI92yV8qwwsQaYcPNGHMu1NXEShqL5F+YXdcvDRwA=","Date":"Sat, 3 Apr 2021 04:27:03 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<YGfEZ+g0/HK76hcd@pendragon.ideasonboard.com>","References":"<20210329091552.157208-1-hiroh@chromium.org>\n\t<20210329091552.157208-4-hiroh@chromium.org>\n\t<bf9d7d8f-1a14-a36f-0066-0a8587ec8e75@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<bf9d7d8f-1a14-a36f-0066-0a8587ec8e75@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] Regard a request error in the\n\trequest completion","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","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>"}}]