[{"id":20225,"web_url":"https://patchwork.libcamera.org/comment/20225/","msgid":"","date":"2021-10-15T02:02:08","subject":"Re: [libcamera-devel] [PATCH] android: Re-order out-of-order\n\tcompletion path","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Thu, Oct 14, 2021 at 12:37:57PM +0200, Jacopo Mondi wrote:\n> When the camera HAL detects an out-of-order completion of a request, it\n> sends to the camera framework a CAMERA3_MSG_ERROR_DEVICE error.\n> \n> Such error not only forces the service to close the camera as prescribed\n> by the camera3 specification, but in some implementation (specifically\n> the ChromeOS one) it causes the camera service to abort and exit.\n> \n> This prevents any error messages to be printed by libcamera, as the\n\ns/to be printed/from being printed/\n\n> library gets terminated before getting to that point, and also hides the\n> printout of error messages that lead to out-of-order completion, making\n> it impossible to get from the output log what happened.\n> \n> Move the call to notifyError() at the end of the error path and demote\n> the error message to LogLevels::Error from Fatal to let the service\n> implementation decide how to handle CAMERA3_MSG_ERROR_DEVICE errors.\n> \n> Before this patch, when waiting on a fence fails and the capture\n> request is not queued to the Camera, we get an out-of-order completion\n> but no backtrace. With this patch applied the error path is visible:\n> \n> ERROR HAL camera_worker.cpp:122 Failed waiting for fence: 82: Timer expired\n> ERROR HAL camera_device.cpp:1110 '\\_SB_.PCI0.I2C2.CAM0': Out-of-order completion for request 0x00007e6de4004c70\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_device.cpp | 9 +++++----\n> 1 file changed, 5 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 901867105085..f60297e13c8b 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1104,16 +1104,17 @@ void CameraDevice::requestComplete(Request *request)\n> \tif (descriptor->request_->cookie() != request->cookie()) {\n> \t\t/*\n> \t\t * \\todo Clarify if the Camera has to be closed on\n> -\t\t * ERROR_DEVICE and possibly demote the Fatal to simple\n> -\t\t * Error.\n> +\t\t * ERROR_DEVICE.\n> \t\t */\n> -\t\tnotifyError(0, nullptr, CAMERA3_MSG_ERROR_DEVICE);\n> -\t\tLOG(HAL, Fatal)\n> +\t\tLOG(HAL, Error)\n> \t\t\t<< \"Out-of-order completion for request \"\n> \t\t\t<< utils::hex(request->cookie());\n> \n> \t\tMutexLocker descriptorsLock(descriptorsMutex_);\n> \t\tdescriptors_.pop();\n> +\n> +\t\tnotifyError(0, nullptr, CAMERA3_MSG_ERROR_DEVICE);\n\nGiven that we've removed the descriptor at the head of the queue, which\ndoesn't correspond to the request, from this point onwards all hell will\nlikely break loose, possibly even in the close() path. It's probably not\nworth than a Fatal though, so\n\nReviewed-by: Laurent Pinchart \n\n> +\n> \t\treturn;\n> \t}\n>","headers":{"Return-Path":"","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 EF055C324C\n\tfor ;\n\tFri, 15 Oct 2021 02:02:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 790DE68F50;\n\tFri, 15 Oct 2021 04:02:25 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8196A60239\n\tfor ;\n\tFri, 15 Oct 2021 04:02:24 +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 ECE712E3;\n\tFri, 15 Oct 2021 04:02:23 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"gqbZ+cQv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634263344;\n\tbh=NYoNJ8McV5J/AymD65Tz7Oe7O5sVhC1LKo5xHb0F+u8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gqbZ+cQvH7+cpbPLzPsZbDEVUI2X06Kuqx05ZvpvbqE2DYKQfCQJ5tgWp+cnQd/8q\n\tj5abPQnbc30U88BVJVVkhy0WF2+N1pttob1c82FKmjprlLTaY27Qv5N6S4X55sgmwf\n\tkQGTtvnOhbe0oZghG7HR6u3hN2L+RzwXo0p/UeqA=","Date":"Fri, 15 Oct 2021 05:02:08 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20211014103757.21161-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211014103757.21161-1-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH] android: Re-order out-of-order\n\tcompletion path","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20227,"web_url":"https://patchwork.libcamera.org/comment/20227/","msgid":"","date":"2021-10-15T03:17:32","subject":"Re: [libcamera-devel] [PATCH] android: Re-order out-of-order\n\tcompletion path","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hello,\n\nOn 10/15/21 7:32 AM, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Thu, Oct 14, 2021 at 12:37:57PM +0200, Jacopo Mondi wrote:\n>> When the camera HAL detects an out-of-order completion of a request, it\n>> sends to the camera framework a CAMERA3_MSG_ERROR_DEVICE error.\n>>\n>> Such error not only forces the service to close the camera as prescribed\n>> by the camera3 specification, but in some implementation (specifically\n>> the ChromeOS one) it causes the camera service to abort and exit.\n>>\n>> This prevents any error messages to be printed by libcamera, as the\n> s/to be printed/from being printed/\n>\n>> library gets terminated before getting to that point, and also hides the\n>> printout of error messages that lead to out-of-order completion, making\n>> it impossible to get from the output log what happened.\n>>\n>> Move the call to notifyError() at the end of the error path and demote\n>> the error message to LogLevels::Error from Fatal to let the service\n>> implementation decide how to handle CAMERA3_MSG_ERROR_DEVICE errors.\n>>\n>> Before this patch, when waiting on a fence fails and the capture\n>> request is not queued to the Camera, we get an out-of-order completion\n>> but no backtrace. With this patch applied the error path is visible:\n>>\n>> ERROR HAL camera_worker.cpp:122 Failed waiting for fence: 82: Timer expired\n>> ERROR HAL camera_device.cpp:1110 '\\_SB_.PCI0.I2C2.CAM0': Out-of-order completion for request 0x00007e6de4004c70\n>>\n>> Signed-off-by: Jacopo Mondi \n>> ---\n>> src/android/camera_device.cpp | 9 +++++----\n>> 1 file changed, 5 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>> index 901867105085..f60297e13c8b 100644\n>> --- a/src/android/camera_device.cpp\n>> +++ b/src/android/camera_device.cpp\n>> @@ -1104,16 +1104,17 @@ void CameraDevice::requestComplete(Request *request)\n>> \tif (descriptor->request_->cookie() != request->cookie()) {\n>> \t\t/*\n>> \t\t * \\todo Clarify if the Camera has to be closed on\n>> -\t\t * ERROR_DEVICE and possibly demote the Fatal to simple\n>> -\t\t * Error.\n>> +\t\t * ERROR_DEVICE.\n>> \t\t */\n>> -\t\tnotifyError(0, nullptr, CAMERA3_MSG_ERROR_DEVICE);\n>> -\t\tLOG(HAL, Fatal)\n>> +\t\tLOG(HAL, Error)\n>> \t\t\t<< \"Out-of-order completion for request \"\n>> \t\t\t<< utils::hex(request->cookie());\n>> \n>> \t\tMutexLocker descriptorsLock(descriptorsMutex_);\n>> \t\tdescriptors_.pop();\n>> +\n>> +\t\tnotifyError(0, nullptr, CAMERA3_MSG_ERROR_DEVICE);\n> Given that we've removed the descriptor at the head of the queue, which\n> doesn't correspond to the request, from this point onwards all hell will\n> likely break loose, possibly even in the close() path. It's probably not\n\n\nCan we use this as an error message, \" All hell broke loose, Run!!!\" ;-)\n\n> worth than a Fatal though, so\n>\n> Reviewed-by: Laurent Pinchart \n\n\nReviewed-by: Umang Jain \n\n>\n>> +\n>> \t\treturn;\n>> \t}\n>>","headers":{"Return-Path":"","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 E0483C324C\n\tfor ;\n\tFri, 15 Oct 2021 03:17:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5826F68F50;\n\tFri, 15 Oct 2021 05:17:39 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 23AD960239\n\tfor ;\n\tFri, 15 Oct 2021 05:17:38 +0200 (CEST)","from [192.168.1.106] (unknown [103.251.226.115])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E35B72E3;\n\tFri, 15 Oct 2021 05:17:36 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"iRQltKrL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634267857;\n\tbh=jzwmk85CZUF2y3emgvzk7d0/DuE2xRsItQ3I068/NLc=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=iRQltKrL2ckEdPI+cvaNv3BYTyPyo9X+xVwjpbKbXD/TptR64SdNXWEWhXSk+aS5u\n\tdkHSJTXQvz81xl+KgflJxVKBfeesceXcIUOAo0PWzIVrhF2j6CFS7JBtRTFd8jeFGU\n\tdS1q5lfOl0o65PNczIaRuzXBlDnEe/2lHpCBCEvA=","To":"Laurent Pinchart ,\n\tJacopo Mondi ","References":"<20211014103757.21161-1-jacopo@jmondi.org>\n\t","From":"Umang Jain ","Message-ID":"","Date":"Fri, 15 Oct 2021 08:47:32 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] android: Re-order out-of-order\n\tcompletion path","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20287,"web_url":"https://patchwork.libcamera.org/comment/20287/","msgid":"","date":"2021-10-19T04:00:50","subject":"Re: [libcamera-devel] [PATCH] android: Re-order out-of-order\n\tcompletion path","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Jacopo, thank you for the patch.\n\nOn Fri, Oct 15, 2021 at 12:17 PM Umang Jain wrote:\n>\n> Hello,\n>\n> On 10/15/21 7:32 AM, Laurent Pinchart wrote:\n> > Hi Jacopo,\n> >\n> > Thank you for the patch.\n> >\n> > On Thu, Oct 14, 2021 at 12:37:57PM +0200, Jacopo Mondi wrote:\n> >> When the camera HAL detects an out-of-order completion of a request, it\n> >> sends to the camera framework a CAMERA3_MSG_ERROR_DEVICE error.\n> >>\n> >> Such error not only forces the service to close the camera as prescribed\n> >> by the camera3 specification, but in some implementation (specifically\n> >> the ChromeOS one) it causes the camera service to abort and exit.\n> >>\n> >> This prevents any error messages to be printed by libcamera, as the\n> > s/to be printed/from being printed/\n> >\n> >> library gets terminated before getting to that point, and also hides the\n> >> printout of error messages that lead to out-of-order completion, making\n> >> it impossible to get from the output log what happened.\n> >>\n> >> Move the call to notifyError() at the end of the error path and demote\n> >> the error message to LogLevels::Error from Fatal to let the service\n> >> implementation decide how to handle CAMERA3_MSG_ERROR_DEVICE errors.\n> >>\n> >> Before this patch, when waiting on a fence fails and the capture\n> >> request is not queued to the Camera, we get an out-of-order completion\n> >> but no backtrace. With this patch applied the error path is visible:\n\nIs this a bug in libcamera? Will we need to fix this, or intended behavior?\n\n> >>\n> >> ERROR HAL camera_worker.cpp:122 Failed waiting for fence: 82: Timer expired\n> >> ERROR HAL camera_device.cpp:1110 '\\_SB_.PCI0.I2C2.CAM0': Out-of-order completion for request 0x00007e6de4004c70\n> >>\n> >> Signed-off-by: Jacopo Mondi \n> >> ---\n> >> src/android/camera_device.cpp | 9 +++++----\n> >> 1 file changed, 5 insertions(+), 4 deletions(-)\n> >>\n> >> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> >> index 901867105085..f60297e13c8b 100644\n> >> --- a/src/android/camera_device.cpp\n> >> +++ b/src/android/camera_device.cpp\n> >> @@ -1104,16 +1104,17 @@ void CameraDevice::requestComplete(Request *request)\n> >> if (descriptor->request_->cookie() != request->cookie()) {\n> >> /*\n> >> * \\todo Clarify if the Camera has to be closed on\n> >> - * ERROR_DEVICE and possibly demote the Fatal to simple\n> >> - * Error.\n> >> + * ERROR_DEVICE.\n> >> */\n> >> - notifyError(0, nullptr, CAMERA3_MSG_ERROR_DEVICE);\n> >> - LOG(HAL, Fatal)\n> >> + LOG(HAL, Error)\n> >> << \"Out-of-order completion for request \"\n> >> << utils::hex(request->cookie());\n> >>\n> >> MutexLocker descriptorsLock(descriptorsMutex_);\n> >> descriptors_.pop();\n> >> +\n> >> + notifyError(0, nullptr, CAMERA3_MSG_ERROR_DEVICE);\n> > Given that we've removed the descriptor at the head of the queue, which\n> > doesn't correspond to the request, from this point onwards all hell will\n> > likely break loose, possibly even in the close() path. It's probably not\n>\n>\n> Can we use this as an error message, \" All hell broke loose, Run!!!\" ;-)\n>\n> > worth than a Fatal though, so\n> >\n> > Reviewed-by: Laurent Pinchart \n>\n>\n> Reviewed-by: Umang Jain \n>\n\nReviewed-by: Hirokazu Honda \n> >\n> >> +\n> >> return;\n> >> }\n> >>","headers":{"Return-Path":"","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 7B124C324C\n\tfor ;\n\tTue, 19 Oct 2021 04:01:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D98B268F5C;\n\tTue, 19 Oct 2021 06:01:03 +0200 (CEST)","from mail-ed1-x535.google.com (mail-ed1-x535.google.com\n\t[IPv6:2a00:1450:4864:20::535])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A603260126\n\tfor ;\n\tTue, 19 Oct 2021 06:01:02 +0200 (CEST)","by mail-ed1-x535.google.com with SMTP id a25so7436705edx.8\n\tfor ;\n\tMon, 18 Oct 2021 21:01:02 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"UgX+oiMU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=cATR9/JhGvq2W9iNoyQnn3gjN/lMFR5SBc+2695zixQ=;\n\tb=UgX+oiMUUOwb9bIlGM/Pc7soW6P/z0uf4z/DW6IXaIKWUSYllkZi8UoCu3EYPrsWKr\n\t3GPOfj+V22xkTR8wgbwPAHsR5WmOFpBJmErJ1/8EWSuDg01ZAWgVs1ZjDxOtVxo8JpUU\n\tL26fE/5nTOwkKhtKr8dO4s70eVyDMaL7Kf6n0=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=cATR9/JhGvq2W9iNoyQnn3gjN/lMFR5SBc+2695zixQ=;\n\tb=mAdg+eZaQo4qTQmjV/2WLyCAuk6vnAUQ27U2Fl63mpDMOeXDdFG+zS5ZIykySlHnhE\n\t+Vwi6rKiaKNgnhnVIr8aNSrJALURm/zs66GFmJxtKOezee6Jl9Q+C9NywRFsHonC0prL\n\t4yOWlYEgnAHFPnrcFJZ7i02entdk7FEFuCy272HmDsjHSY+xEMJukOHjL82/pnUxgofl\n\tRIYBze+2YNDAoiMEzpADNZCLqwhY2wa8zOPyZZWCINGEGCT/GtD9tPnADct4n4s6q5Fy\n\tVI6/Lz6jL8hunuRXRVtGfqaQd7ckeW7uKvLr4imBl24LONwv7E903J806JjCAguBdhnp\n\t0cQA==","X-Gm-Message-State":"AOAM5305jY2uG3e4HPlroL1mK5PfnlCiW384GV6epVeJfD3glkc6E30t\n\thRvOwYmj/zpi9feRb6E5VOorIGtNFzHR81ZwozfRqudFDFQ=","X-Google-Smtp-Source":"ABdhPJxAM/WVs7fzHT3+AxdM++wyLGnt6u9nAsSR6Yq9A6vTcGtog4KX4OQrmJkpID3ef+/aVL/CIKysrOdql/RMkis=","X-Received":"by 2002:a05:6402:51c6:: with SMTP id\n\tr6mr47746018edd.276.1634616062299; \n\tMon, 18 Oct 2021 21:01:02 -0700 (PDT)","MIME-Version":"1.0","References":"<20211014103757.21161-1-jacopo@jmondi.org>\n\t\n\t","In-Reply-To":"","From":"Hirokazu Honda ","Date":"Tue, 19 Oct 2021 13:00:50 +0900","Message-ID":"","To":"Umang Jain ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] android: Re-order out-of-order\n\tcompletion path","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20294,"web_url":"https://patchwork.libcamera.org/comment/20294/","msgid":"","date":"2021-10-19T06:42:05","subject":"Re: [libcamera-devel] [PATCH] android: Re-order out-of-order\n\tcompletion path","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hiro,\n\nOn Tue, Oct 19, 2021 at 01:00:50PM +0900, Hirokazu Honda wrote:\n> On Fri, Oct 15, 2021 at 12:17 PM Umang Jain wrote:\n> > On 10/15/21 7:32 AM, Laurent Pinchart wrote:\n> > > On Thu, Oct 14, 2021 at 12:37:57PM +0200, Jacopo Mondi wrote:\n> > >> When the camera HAL detects an out-of-order completion of a request, it\n> > >> sends to the camera framework a CAMERA3_MSG_ERROR_DEVICE error.\n> > >>\n> > >> Such error not only forces the service to close the camera as prescribed\n> > >> by the camera3 specification, but in some implementation (specifically\n> > >> the ChromeOS one) it causes the camera service to abort and exit.\n> > >>\n> > >> This prevents any error messages to be printed by libcamera, as the\n> > > s/to be printed/from being printed/\n> > >\n> > >> library gets terminated before getting to that point, and also hides the\n> > >> printout of error messages that lead to out-of-order completion, making\n> > >> it impossible to get from the output log what happened.\n> > >>\n> > >> Move the call to notifyError() at the end of the error path and demote\n> > >> the error message to LogLevels::Error from Fatal to let the service\n> > >> implementation decide how to handle CAMERA3_MSG_ERROR_DEVICE errors.\n> > >>\n> > >> Before this patch, when waiting on a fence fails and the capture\n> > >> request is not queued to the Camera, we get an out-of-order completion\n> > >> but no backtrace. With this patch applied the error path is visible:\n> \n> Is this a bug in libcamera? Will we need to fix this, or intended behavior?\n\nThe timeout is caused by the fence not being signalled for 300ms, which\nseems to be a bug in Chrome OS. The HAL should recover gracefully from\nthis condition, Jacopo is working on it.\n\n> > >> ERROR HAL camera_worker.cpp:122 Failed waiting for fence: 82: Timer expired\n> > >> ERROR HAL camera_device.cpp:1110 '\\_SB_.PCI0.I2C2.CAM0': Out-of-order completion for request 0x00007e6de4004c70\n> > >>\n> > >> Signed-off-by: Jacopo Mondi \n> > >> ---\n> > >> src/android/camera_device.cpp | 9 +++++----\n> > >> 1 file changed, 5 insertions(+), 4 deletions(-)\n> > >>\n> > >> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > >> index 901867105085..f60297e13c8b 100644\n> > >> --- a/src/android/camera_device.cpp\n> > >> +++ b/src/android/camera_device.cpp\n> > >> @@ -1104,16 +1104,17 @@ void CameraDevice::requestComplete(Request *request)\n> > >> if (descriptor->request_->cookie() != request->cookie()) {\n> > >> /*\n> > >> * \\todo Clarify if the Camera has to be closed on\n> > >> - * ERROR_DEVICE and possibly demote the Fatal to simple\n> > >> - * Error.\n> > >> + * ERROR_DEVICE.\n> > >> */\n> > >> - notifyError(0, nullptr, CAMERA3_MSG_ERROR_DEVICE);\n> > >> - LOG(HAL, Fatal)\n> > >> + LOG(HAL, Error)\n> > >> << \"Out-of-order completion for request \"\n> > >> << utils::hex(request->cookie());\n> > >>\n> > >> MutexLocker descriptorsLock(descriptorsMutex_);\n> > >> descriptors_.pop();\n> > >> +\n> > >> + notifyError(0, nullptr, CAMERA3_MSG_ERROR_DEVICE);\n> > >\n> > > Given that we've removed the descriptor at the head of the queue, which\n> > > doesn't correspond to the request, from this point onwards all hell will\n> > > likely break loose, possibly even in the close() path. It's probably not\n> >\n> > Can we use this as an error message, \" All hell broke loose, Run!!!\" ;-)\n> >\n> > > worth than a Fatal though, so\n> > >\n> > > Reviewed-by: Laurent Pinchart \n> >\n> > Reviewed-by: Umang Jain \n> \n> Reviewed-by: Hirokazu Honda \n>\n> > >> +\n> > >> return;\n> > >> }\n> > >>","headers":{"Return-Path":"","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 265FAC324C\n\tfor ;\n\tTue, 19 Oct 2021 06:42:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7110F68F5B;\n\tTue, 19 Oct 2021 08:42:25 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1EE1560126\n\tfor ;\n\tTue, 19 Oct 2021 08:42:24 +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 7401E12A;\n\tTue, 19 Oct 2021 08:42:23 +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=\"BpdBlWm6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634625743;\n\tbh=H+QJ1SG1gzs6OGsMsC8SUzF33efMQCSQY8hm2Du6wb4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BpdBlWm69dqN1RmhGTEmFgIocoiztoHbHBib9bVFktS7UZ1M5kDdPK2TScir7ezhp\n\t00e2JutHZ23tGlEzSsbXH1/fbUBX2vJEQN4eEDlHuw5ohbnZame1th+iyz7GDV0ZU9\n\tmL7sCiBsZOXacsA75Yy+ICi/cjwl+YbNYao7Vmb8=","Date":"Tue, 19 Oct 2021 09:42:05 +0300","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20211014103757.21161-1-jacopo@jmondi.org>\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] android: Re-order out-of-order\n\tcompletion path","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]