[{"id":22359,"web_url":"https://patchwork.libcamera.org/comment/22359/","msgid":"","date":"2022-03-22T20:15:43","subject":"Re: [libcamera-devel] [PATCH v4 6/8] libcamera: v4l2_videodevice:\n\tDo not allow buffer queueing in stopping state","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Tue, Mar 22, 2022 at 09:22:55AM +0000, Naushir Patuck via libcamera-devel wrote:\n> If the device is in the process of being stopped (i.e. Stopping state), any\n> call to queueBuffer() must fail. This is to ensure the integrity of the buffer\n> queue, as it gets cleared at the end of streamOff.\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> src/libcamera/v4l2_videodevice.cpp | 8 ++++++++\n> 1 file changed, 8 insertions(+)\n> \n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index 9cea6a608660..1516be129d73 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -1491,6 +1491,9 @@ int V4L2VideoDevice::releaseBuffers()\n> * The best available V4L2 buffer is picked for \\a buffer using the V4L2 buffer\n> * cache.\n> *\n> + * Note that \\a queueBuffer will fail if the device is in the process of being\n\nIf you write it queueBuffer() doxygen should recognize the function call\nand generate proper links, you can drop the \\a. Same below.\n\n> + * stopped from a streaming state through the \\a streamOff function.\n> + *\n> * \\return 0 on success or a negative error code otherwise\n> */\n> int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)\n> @@ -1499,6 +1502,11 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)\n> \tstruct v4l2_buffer buf = {};\n> \tint ret;\n> \n> +\tif (state_ == State::Stopping) {\n> +\t\tLOG(V4L2, Error) << \"Device is in a stopping state.\";\n\nDoes this currently happen ? I assume it does, causing the unit test\nfailures in v3 of the series. Is it a bug on the application side, or a\nproblem elsewhere ? Are the cam, qcam and simple-cam applications\naffected ? We should not have any error message printed under normal\noperation conditions, so I'd like to at least know what needs to be\nfixed.\n\n> +\t\treturn -EPERM;\n\nI'm tempted to use -ESHUTDOWN as a more precise alternative. Any opinion\nfrom anyone ?\n\n> +\t}\n> +\n> \t/*\n> \t * Pipeline handlers should not requeue buffers after releasing the\n> \t * buffers on the device. Any occurence of this error should be fixed","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 8DC0BC0F1B\n\tfor ;\n\tTue, 22 Mar 2022 20:16:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DE49B604DB;\n\tTue, 22 Mar 2022 21:16:02 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 14191604C5\n\tfor ;\n\tTue, 22 Mar 2022 21:16:01 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7D0F5DFA;\n\tTue, 22 Mar 2022 21:16:00 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1647980162;\n\tbh=eKC/njPQCQ6GrjR5ZLU7PM6Jt7O2zNg584wOM4MJ+14=;\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=GUnGS14SJ1AADqGYolSPJlxU83XGooJd1+1sVdee4vqoQJx+XQroyFiCgfieYTe17\n\t26Wsw4nfztKWXXUTF3tmfRA2Dr+UxLqQD5vWDj6D3BMHzk5dZ4C/NUGcipimetWc1F\n\th+zIinrlBEOjgEOjzRCVrM2AvscXJFcLIBefbvgJEkMTHUvMYsZhUPaYWDpGUOrMwB\n\t7wr+t0GWGrLLsjjRHaCHePHuy+FuBoXcOuCVvYGWcG0GkrFR66VTLvG7g3bKxKuEyF\n\tmzwVyt3fTntvZnG6nQChpIY1t58BYw1cxWhq9IhD9Of52YDqyw1366NfQUEJKgx2qZ\n\tbbA1QVxa0+Crw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1647980160;\n\tbh=eKC/njPQCQ6GrjR5ZLU7PM6Jt7O2zNg584wOM4MJ+14=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IWrusF17la59ZgEQPpKH9XJC+Ha7CaZAFG8RnjxKHlEBZCr8xyPjbUvet+xN7AMfI\n\t+ugDixa4ZhxXMTr4J2kkazj+0r8vYRLoOBtodfUP5vyaeP1sDc8ua7Y5PIaw8ZQeMS\n\tfTPC0/aFyvfsQaeklp2rIh4WbDSdPIcndz84A4ZQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"IWrusF17\"; dkim-atps=neutral","Date":"Tue, 22 Mar 2022 22:15:43 +0200","To":"Naushir Patuck ","Message-ID":"","References":"<20220322092257.2713521-1-naush@raspberrypi.com>\n\t<20220322092257.2713521-7-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220322092257.2713521-7-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v4 6/8] libcamera: v4l2_videodevice:\n\tDo not allow buffer queueing in stopping state","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22385,"web_url":"https://patchwork.libcamera.org/comment/22385/","msgid":"","date":"2022-03-23T09:03:44","subject":"Re: [libcamera-devel] [PATCH v4 6/8] libcamera: v4l2_videodevice:\n\tDo not allow buffer queueing in stopping state","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThank you for your feedback.\n\nOn Tue, 22 Mar 2022 at 20:16, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Tue, Mar 22, 2022 at 09:22:55AM +0000, Naushir Patuck via\n> libcamera-devel wrote:\n> > If the device is in the process of being stopped (i.e. Stopping state),\n> any\n> > call to queueBuffer() must fail. This is to ensure the integrity of the\n> buffer\n> > queue, as it gets cleared at the end of streamOff.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > src/libcamera/v4l2_videodevice.cpp | 8 ++++++++\n> > 1 file changed, 8 insertions(+)\n> >\n> > diff --git a/src/libcamera/v4l2_videodevice.cpp\n> b/src/libcamera/v4l2_videodevice.cpp\n> > index 9cea6a608660..1516be129d73 100644\n> > --- a/src/libcamera/v4l2_videodevice.cpp\n> > +++ b/src/libcamera/v4l2_videodevice.cpp\n> > @@ -1491,6 +1491,9 @@ int V4L2VideoDevice::releaseBuffers()\n> > * The best available V4L2 buffer is picked for \\a buffer using the\n> V4L2 buffer\n> > * cache.\n> > *\n> > + * Note that \\a queueBuffer will fail if the device is in the process\n> of being\n>\n> If you write it queueBuffer() doxygen should recognize the function call\n> and generate proper links, you can drop the \\a. Same below.\n>\n> > + * stopped from a streaming state through the \\a streamOff function.\n> > + *\n> > * \\return 0 on success or a negative error code otherwise\n> > */\n> > int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)\n> > @@ -1499,6 +1502,11 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer\n> *buffer)\n> > struct v4l2_buffer buf = {};\n> > int ret;\n> >\n> > + if (state_ == State::Stopping) {\n> > + LOG(V4L2, Error) << \"Device is in a stopping state.\";\n>\n> Does this currently happen ? I assume it does, causing the unit test\n> failures in v3 of the series. Is it a bug on the application side, or a\n> problem elsewhere ? Are the cam, qcam and simple-cam applications\n> affected ? We should not have any error message printed under normal\n> operation conditions, so I'd like to at least know what needs to be\n> fixed.\n>\n\nThis does occur, but only in the v4l2_m2mdevice test (that I've found).\nApplications\nare not directly affected, as this is on the interface between the pipeline\nhandlers\nand the v4l2 device. The RPi pipeline handler does not re-queue buffers\ninto the\ndevice when we are stopping. I've had a brief look at the ipu3 and rkisp\nones, and\nI don't think they do either, but it would be good for someone else to\nconfirm.\n\nKieran and I briefly talked about what to do with the v4l2_m2mdevice test,\nit would\nbe an easy fix to correct the behavior and get rid of the above log\nmessages popping\nout with this change.\n\n\n> > + return -EPERM;\n>\n> I'm tempted to use -ESHUTDOWN as a more precise alternative. Any opinion\n> from anyone ?\n>\n\nI am fine with ESHUTDOWN if others think it is more appropriate.\n\nRegards,\nNaush\n\n\n>\n> > + }\n> > +\n> > /*\n> > * Pipeline handlers should not requeue buffers after releasing the\n> > * buffers on the device. Any occurence of this error should be\n> fixed\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 F3EBEBD80A\n\tfor ;\n\tWed, 23 Mar 2022 09:04:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4BCB5604D5;\n\tWed, 23 Mar 2022 10:04:03 +0100 (CET)","from mail-lf1-x12a.google.com (mail-lf1-x12a.google.com\n\t[IPv6:2a00:1450:4864:20::12a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 75E12601F8\n\tfor ;\n\tWed, 23 Mar 2022 10:04:01 +0100 (CET)","by mail-lf1-x12a.google.com with SMTP id 5so1585668lfp.1\n\tfor ;\n\tWed, 23 Mar 2022 02:04:01 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648026243;\n\tbh=eLJtd16YCx8HLUlFPL/Vd/00yHf8OuVb5kxSkxKFR6o=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=XniXQZHargoENNfGmkrBlPTMXSQ9QouItTe/3yhp9n+w3+r3hhhDb7vPZjZWy7i+B\n\tPGT/UdZBeZvCkJxstpkHhFGiY0ga3Aa/vGqL5LJ614Q74qv7gvuCMzxCEHOqzY/Hgl\n\taVoisqNRkLMkc/IBaf955tsHVNta05DPMdaP/CbBnSoCspF2EwngdMwpdOyZJSMgJ/\n\tTvctZx8FJDO9eTNAnokPhyD7GEHa9Gyl3yR4pdjNaMqjuQKdr7CsYEwYxbc/uV18hC\n\tfj0RitAZXbI084rgpc6kbDJWiP1ZRjreV6J7N8h21mySxluicSH2HlUSDp9xL/6uaF\n\tS9ObXxk42Q+fA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=We+gGBZ6kfWyCq+GT/gLibaLq7hJdozUii8uwzCgOT4=;\n\tb=odbFnPQEJJH9nVGaBwuuGpoZY85aDRhXlByns8iNb5RMkLRZvpWztwKgXAO0geRHeA\n\tN9/zg1wmDYa9n1UBF+IPbW2KDGB+/R1avzBFm+lF9ABilIVha5qZ9Sckum89od5jzvlV\n\t0FkeSJnBZyu1qIuf+kTCD1X8o0cENFnTJpZMGuEMsN9r1OMkzvQWuyMPIZBx+4e/txzY\n\tTiZiLXBmX14N8Lg/C/6OuVxMuMDZvMD8c//9lAKp53nStHahokRSy1QQOfx5TCmMYIc+\n\tZpjjh8aQB1FZOrI/fYKUBF1wHWmWvBJb47JBbqOHQvAOgMRGy+4RfK3Gpw4gEn2p6eB6\n\t8kDQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"odbFnPQE\"; dkim-atps=neutral","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=We+gGBZ6kfWyCq+GT/gLibaLq7hJdozUii8uwzCgOT4=;\n\tb=drIatZUmZ6v2vtaPre7raSHmvxsJI//35GJ7q+hFRbk6CfQLUGTFJbpJtCy1w3kBFo\n\tUNXotI/yJ5XhppjECeHcJRtAtDiRzD9H0D0j5S8tvI601KkMUnDqrbnfy/IbVsNK9OJo\n\tbayrPnOcq264UzTmMSORnoj4DrS4mzwx93tkOy3/YE60BDRAyoGoU7JZyDYCk2o7ST7c\n\tnDspPu/nNDyTjKvX6/IG9EJI8AeQXbHTjxUoDJbWekR4qJ+0V34e8cpTKzGFja3PJdSk\n\t2PgHkEbTDOezIke3WHkAnoMJSutiA1VEDZfZcbZu9KxiVSsT4SoL+RjfZbt2uRW1nqLU\n\t17Iw==","X-Gm-Message-State":"AOAM5327OH1lXKpgWWcfyUUgeys1yN/JaVt83oCUKHGYKRbmY2qgBaas\n\tNIO+LwQzZ4ZlG7GtLs2Wpx2nRzVkAitd7WXIX97oSA==","X-Google-Smtp-Source":"ABdhPJxeZRtCC/AA4QZZrky9V8lWe8IcYTluTVt5nWa+3GSuChawkGnuCl0/WVdXdwIZa1PtIm2c6jkToSwPKHRCUh4=","X-Received":"by 2002:a05:6512:304c:b0:44a:4ad4:4527 with SMTP id\n\tb12-20020a056512304c00b0044a4ad44527mr2348796lfb.161.1648026240574;\n\tWed, 23 Mar 2022 02:04:00 -0700 (PDT)","MIME-Version":"1.0","References":"<20220322092257.2713521-1-naush@raspberrypi.com>\n\t<20220322092257.2713521-7-naush@raspberrypi.com>\n\t","In-Reply-To":"","Date":"Wed, 23 Mar 2022 09:03:44 +0000","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000a6808b05dadf02b7\"","Subject":"Re: [libcamera-devel] [PATCH v4 6/8] libcamera: v4l2_videodevice:\n\tDo not allow buffer queueing in stopping state","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","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22391,"web_url":"https://patchwork.libcamera.org/comment/22391/","msgid":"<164802927640.2130830.13390103006059119069@Monstersaurus>","date":"2022-03-23T09:54:36","subject":"Re: [libcamera-devel] [PATCH v4 6/8] libcamera: v4l2_videodevice:\n\tDo not allow buffer queueing in stopping state","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck via libcamera-devel (2022-03-23 09:03:44)\n> Hi Laurent,\n> \n> Thank you for your feedback.\n> \n> On Tue, 22 Mar 2022 at 20:16, Laurent Pinchart <\n> laurent.pinchart@ideasonboard.com> wrote:\n> \n> > Hi Naush,\n> >\n> > Thank you for the patch.\n> >\n> > On Tue, Mar 22, 2022 at 09:22:55AM +0000, Naushir Patuck via\n> > libcamera-devel wrote:\n> > > If the device is in the process of being stopped (i.e. Stopping state),\n> > any\n> > > call to queueBuffer() must fail. This is to ensure the integrity of the\n> > buffer\n> > > queue, as it gets cleared at the end of streamOff.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > src/libcamera/v4l2_videodevice.cpp | 8 ++++++++\n> > > 1 file changed, 8 insertions(+)\n> > >\n> > > diff --git a/src/libcamera/v4l2_videodevice.cpp\n> > b/src/libcamera/v4l2_videodevice.cpp\n> > > index 9cea6a608660..1516be129d73 100644\n> > > --- a/src/libcamera/v4l2_videodevice.cpp\n> > > +++ b/src/libcamera/v4l2_videodevice.cpp\n> > > @@ -1491,6 +1491,9 @@ int V4L2VideoDevice::releaseBuffers()\n> > > * The best available V4L2 buffer is picked for \\a buffer using the\n> > V4L2 buffer\n> > > * cache.\n> > > *\n> > > + * Note that \\a queueBuffer will fail if the device is in the process\n> > of being\n> >\n> > If you write it queueBuffer() doxygen should recognize the function call\n> > and generate proper links, you can drop the \\a. Same below.\n> >\n> > > + * stopped from a streaming state through the \\a streamOff function.\n> > > + *\n> > > * \\return 0 on success or a negative error code otherwise\n> > > */\n> > > int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)\n> > > @@ -1499,6 +1502,11 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer\n> > *buffer)\n> > > struct v4l2_buffer buf = {};\n> > > int ret;\n> > >\n> > > + if (state_ == State::Stopping) {\n> > > + LOG(V4L2, Error) << \"Device is in a stopping state.\";\n> >\n> > Does this currently happen ? I assume it does, causing the unit test\n> > failures in v3 of the series. Is it a bug on the application side, or a\n> > problem elsewhere ? Are the cam, qcam and simple-cam applications\n> > affected ? We should not have any error message printed under normal\n> > operation conditions, so I'd like to at least know what needs to be\n> > fixed.\n> >\n> \n> This does occur, but only in the v4l2_m2mdevice test (that I've found).\n> Applications\n> are not directly affected, as this is on the interface between the pipeline\n> handlers\n> and the v4l2 device. The RPi pipeline handler does not re-queue buffers\n> into the\n> device when we are stopping. I've had a brief look at the ipu3 and rkisp\n> ones, and\n> I don't think they do either, but it would be good for someone else to\n> confirm.\n> \n> Kieran and I briefly talked about what to do with the v4l2_m2mdevice test,\n> it would\n> be an easy fix to correct the behavior and get rid of the above log\n> messages popping\n> out with this change.\n\nDoesn't it happen on the async capture test too?\n\nIndeed as I understand it - this isn't something that can happen from a\n'libcamera' application as it is already guarded by the stopping state\non the Camera state machine.\n\nBut ... it is an issue if someone uses the V4L2VideoDevice directly\n(such as our tests), and any future expansion of the V4L2VideoDevice\nclass - so I would rather see this fix/check here to be sure it doesn't\ncause issues with future users of the V4L2VideoDevice.\n\n\n> \n> \n> > > + return -EPERM;\n> >\n> > I'm tempted to use -ESHUTDOWN as a more precise alternative. Any opinion\n> > from anyone ?\n> >\n> \n> I am fine with ESHUTDOWN if others think it is more appropriate.\n\n ESHUTDOWN 108 Cannot send after transport endpoint shutdown\n\nSounds reasonable enough to me.\n\n\nReviewed-by: Kieran Bingham \n\n> \n> Regards,\n> Naush\n> \n> \n> >\n> > > + }\n> > > +\n> > > /*\n> > > * Pipeline handlers should not requeue buffers after releasing the\n> > > * buffers on the device. Any occurence of this error should be\n> > fixed\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart\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 A096FC0F1B\n\tfor ;\n\tWed, 23 Mar 2022 09:54:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5804D604D5;\n\tWed, 23 Mar 2022 10:54:40 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 301F8601F8\n\tfor ;\n\tWed, 23 Mar 2022 10:54:39 +0100 (CET)","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 D13E19DE;\n\tWed, 23 Mar 2022 10:54:38 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648029280;\n\tbh=zX9V8uDJI4T4coJcgwx5f0iPXRLXpRd3cpoiT++0an0=;\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=4KHbVwQKhpFYL1NstENZDqmTrii9H3JKQ1baF1OdfZw1FxuOqf2R3b6iEtlVOEYlL\n\teXOArS9s0zkLM9CSA0bX+M0o/JiAqbyTVvR1qUPEllMfILXOrpSb33h7DbGPuR6Pkm\n\tJSwKm+/cYitsaNBsbs/3cMuwtfPy7E+1Xi1O0l3TxDfFpr1wzXj9fgRzzI+gCW4CZF\n\tY9vzMbG1CQA+5Jco5ome8uSxta6o70BW/d01/pRuQBA6pKgOxe9iAzV8uxIZk1tgxY\n\tkzkNlXT6jlIGg8rBH3gblYIXakD4lhYPEePasQ8pdwZExjbyGMp/k4ELZCDtOE794R\n\tQz7Z8xX+6wDzw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648029278;\n\tbh=zX9V8uDJI4T4coJcgwx5f0iPXRLXpRd3cpoiT++0an0=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=BhOyL8COS4JUCku1Y4FK8xcZIrgxpDPoxzQXu4SBnMTs7f6721J+7i2hVQOnH43iD\n\trATTvLBwO4CwRhxOM1titTRFkm3KKxqaNWXFV2y5XzmvsuBIpmXXyT6eayuEkPBrMz\n\t2xQEzoRq7qzV1SIiXNqL19xCNvbwsGeOm3Ba1oH4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"BhOyL8CO\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20220322092257.2713521-1-naush@raspberrypi.com>\n\t<20220322092257.2713521-7-naush@raspberrypi.com>\n\t\n\t","To":"Laurent Pinchart ,\n\tNaushir Patuck ,\n\tNaushir Patuck via libcamera-devel ","Date":"Wed, 23 Mar 2022 09:54:36 +0000","Message-ID":"<164802927640.2130830.13390103006059119069@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v4 6/8] libcamera: v4l2_videodevice:\n\tDo not allow buffer queueing in stopping state","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22409,"web_url":"https://patchwork.libcamera.org/comment/22409/","msgid":"","date":"2022-03-23T14:31:07","subject":"Re: [libcamera-devel] [PATCH v4 6/8] libcamera: v4l2_videodevice:\n\tDo not allow buffer queueing in stopping state","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Wed, 23 Mar 2022 at 09:54, Kieran Bingham <\nkieran.bingham@ideasonboard.com> wrote:\n\n> Quoting Naushir Patuck via libcamera-devel (2022-03-23 09:03:44)\n> > Hi Laurent,\n> >\n> > Thank you for your feedback.\n> >\n> > On Tue, 22 Mar 2022 at 20:16, Laurent Pinchart <\n> > laurent.pinchart@ideasonboard.com> wrote:\n> >\n> > > Hi Naush,\n> > >\n> > > Thank you for the patch.\n> > >\n> > > On Tue, Mar 22, 2022 at 09:22:55AM +0000, Naushir Patuck via\n> > > libcamera-devel wrote:\n> > > > If the device is in the process of being stopped (i.e. Stopping\n> state),\n> > > any\n> > > > call to queueBuffer() must fail. This is to ensure the integrity of\n> the\n> > > buffer\n> > > > queue, as it gets cleared at the end of streamOff.\n> > > >\n> > > > Signed-off-by: Naushir Patuck \n> > > > ---\n> > > > src/libcamera/v4l2_videodevice.cpp | 8 ++++++++\n> > > > 1 file changed, 8 insertions(+)\n> > > >\n> > > > diff --git a/src/libcamera/v4l2_videodevice.cpp\n> > > b/src/libcamera/v4l2_videodevice.cpp\n> > > > index 9cea6a608660..1516be129d73 100644\n> > > > --- a/src/libcamera/v4l2_videodevice.cpp\n> > > > +++ b/src/libcamera/v4l2_videodevice.cpp\n> > > > @@ -1491,6 +1491,9 @@ int V4L2VideoDevice::releaseBuffers()\n> > > > * The best available V4L2 buffer is picked for \\a buffer using the\n> > > V4L2 buffer\n> > > > * cache.\n> > > > *\n> > > > + * Note that \\a queueBuffer will fail if the device is in the\n> process\n> > > of being\n> > >\n> > > If you write it queueBuffer() doxygen should recognize the function\n> call\n> > > and generate proper links, you can drop the \\a. Same below.\n> > >\n> > > > + * stopped from a streaming state through the \\a streamOff function.\n> > > > + *\n> > > > * \\return 0 on success or a negative error code otherwise\n> > > > */\n> > > > int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer)\n> > > > @@ -1499,6 +1502,11 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer\n> > > *buffer)\n> > > > struct v4l2_buffer buf = {};\n> > > > int ret;\n> > > >\n> > > > + if (state_ == State::Stopping) {\n> > > > + LOG(V4L2, Error) << \"Device is in a stopping state.\";\n> > >\n> > > Does this currently happen ? I assume it does, causing the unit test\n> > > failures in v3 of the series. Is it a bug on the application side, or a\n> > > problem elsewhere ? Are the cam, qcam and simple-cam applications\n> > > affected ? We should not have any error message printed under normal\n> > > operation conditions, so I'd like to at least know what needs to be\n> > > fixed.\n> > >\n> >\n> > This does occur, but only in the v4l2_m2mdevice test (that I've found).\n> > Applications\n> > are not directly affected, as this is on the interface between the\n> pipeline\n> > handlers\n> > and the v4l2 device. The RPi pipeline handler does not re-queue buffers\n> > into the\n> > device when we are stopping. I've had a brief look at the ipu3 and rkisp\n> > ones, and\n> > I don't think they do either, but it would be good for someone else to\n> > confirm.\n> >\n> > Kieran and I briefly talked about what to do with the v4l2_m2mdevice\n> test,\n> > it would\n> > be an easy fix to correct the behavior and get rid of the above log\n> > messages popping\n> > out with this change.\n>\n> Doesn't it happen on the async capture test too?\n>\n\nSorry, capture_async does also behave in the same way, so we do get\nlog messages on that run.\n\n\n>\n> Indeed as I understand it - this isn't something that can happen from a\n> 'libcamera' application as it is already guarded by the stopping state\n> on the Camera state machine.\n>\n> But ... it is an issue if someone uses the V4L2VideoDevice directly\n> (such as our tests), and any future expansion of the V4L2VideoDevice\n> class - so I would rather see this fix/check here to be sure it doesn't\n> cause issues with future users of the V4L2VideoDevice.\n>\n>\n> >\n> >\n> > > > + return -EPERM;\n> > >\n> > > I'm tempted to use -ESHUTDOWN as a more precise alternative. Any\n> opinion\n> > > from anyone ?\n> > >\n> >\n> > I am fine with ESHUTDOWN if others think it is more appropriate.\n>\n> ESHUTDOWN 108 Cannot send after transport endpoint shutdown\n>\n> Sounds reasonable enough to me.\n>\n>\n> Reviewed-by: Kieran Bingham \n>\n\nHappy to change to ESHUTDOWN. Do you want me to submit a new version,\nor can this (as well as the doxygen change suggested by Laurent) be fixed\nwhile\napplying? I think this patch needs one more R-B tag for the series to be\nmerged.\n\nRegards,\nNaush\n\n\n\n>\n> >\n> > Regards,\n> > Naush\n> >\n> >\n> > >\n> > > > + }\n> > > > +\n> > > > /*\n> > > > * Pipeline handlers should not requeue buffers after\n> releasing the\n> > > > * buffers on the device. Any occurence of this error should be\n> > > fixed\n> > >\n> > > --\n> > > Regards,\n> > >\n> > > Laurent Pinchart\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 9DEEFBD80A\n\tfor ;\n\tWed, 23 Mar 2022 14:31:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D1758604DA;\n\tWed, 23 Mar 2022 15:31:26 +0100 (CET)","from mail-lj1-x234.google.com (mail-lj1-x234.google.com\n\t[IPv6:2a00:1450:4864:20::234])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6883A604C4\n\tfor ;\n\tWed, 23 Mar 2022 15:31:24 +0100 (CET)","by mail-lj1-x234.google.com with SMTP id c15so2075451ljr.9\n\tfor ;\n\tWed, 23 Mar 2022 07:31:24 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648045886;\n\tbh=2oKDVwDYsBj0fJChabt0OjAtpDL6Dsb9syvmPhvbBcc=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=DHVBvWP35lE6MBopQf4BAsp9njXUt59xcWpJ0qPE2HMoH2zBjXFsdu2/x/7vfh9aV\n\t3/1lT2Ma5PqR91NR5tBvbPKxV1bq+GuiQSPsphpD5sSuQ+6cIvzDU+dSYjiH6/769l\n\tn6DvC3Pz2y/5kZl3Xx405tWZ5DYZlboYNygzVPakjIDStgYJZeNOJk1Utb62vxTE5H\n\th3Fl46Nd12A34gFtQKIM3Rs69p6byFNKQxw40AZ8WO+1hQuRsgS9HK3MEt0AZlYy/N\n\t3bZQLNzWdLDBcmFbxLUC86bMTK5j7mbzXRqEuUDWLTLGxc+jVMB4ZlcYxfSlpAij9F\n\t50CNKcNG909RA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=cGBax7DLlWc9jHeDh48p7kjHY9oqZV5ZZciCaDEVBzM=;\n\tb=EfhHW7c74J8pAaETVbTg190SFV15e/Pb1N1pi7i1d7j/SK8UkJKRxewweBzj8TXloY\n\tTD5gaCi+wqVlHauo1x17/qodfOEoKtyVv6qp1XzztiWcqdMzBvjjQytWb8gulEMVTqn6\n\tFL9EX/W+5KtV92pYFseVL99v1+F0+0R+UrzZnGUfQRf5wCS2zIzBa9+OBEHt6L6xjHtg\n\tjTC1NJTd4aAqnHwzXA1y9oQ7utuk4+DG/ma281d4sXnf03NwhBUArGu4icowCWBVLyWj\n\tdT01G0F/WibHIQuMxkdN1TUYT8jPwMBpxelE2lGr0EkRfxCekreE4WHGhvbPUvQatT42\n\tpAcw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"EfhHW7c7\"; dkim-atps=neutral","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=cGBax7DLlWc9jHeDh48p7kjHY9oqZV5ZZciCaDEVBzM=;\n\tb=jkpzPVq04OO6yzvBcNmQYwMWWsC01I4eKBUoKaSXGGrA5hZtot7uZf99YQKXw3shMZ\n\tO3aRjZVyF0mQHmHPvSEiUEKGW4AIKH5aU4AjpDZoFoEKi8cif6R+czeqJY6bYo2eH33f\n\tWbGQU3/CAcSg8p0yLgKPfk4K19caJcdgNzF35Nr9wgGDUX+0ujTKvLgrZFK7rdNeJecY\n\tZuoXAgDiyZnv5IXTFhaII/u+IXgCCMR7NvrcmlKFmD1O0nzV2XMf1INv7xtLJ15Xh3Gv\n\tA1VjX35C7cFagV7S8HNSWuuw/tNRD8QAABBSOVNpyvopFxKg6nD2Ra75rAs7LC3tmQzh\n\twlCQ==","X-Gm-Message-State":"AOAM533x3+gk974ElQ3wV+TXnHdxjWj33BRpm9iMwb5ZJYfENs9DgV9v\n\tB9PkuKWKnnYE0pgfRDj3WoddSMr1HRexdqu7U5LWwDSbE8I=","X-Google-Smtp-Source":"ABdhPJzPUyz/0KfImDGhYIROIe+4lHccxGzpZzncdMWKcUevCMwu4sY+2kXC8IWw57GQXvBbelWcMoD8WIbeZKQAghE=","X-Received":"by 2002:a2e:9d83:0:b0:246:2c6:79df with SMTP id\n\tc3-20020a2e9d83000000b0024602c679dfmr211051ljj.280.1648045883310;\n\tWed, 23 Mar 2022 07:31:23 -0700 (PDT)","MIME-Version":"1.0","References":"<20220322092257.2713521-1-naush@raspberrypi.com>\n\t<20220322092257.2713521-7-naush@raspberrypi.com>\n\t\n\t\n\t<164802927640.2130830.13390103006059119069@Monstersaurus>","In-Reply-To":"<164802927640.2130830.13390103006059119069@Monstersaurus>","Date":"Wed, 23 Mar 2022 14:31:07 +0000","Message-ID":"","To":"Kieran Bingham ","Content-Type":"multipart/alternative; boundary=\"00000000000072d6f705dae39572\"","Subject":"Re: [libcamera-devel] [PATCH v4 6/8] libcamera: v4l2_videodevice:\n\tDo not allow buffer queueing in stopping state","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","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"Naushir Patuck via libcamera-devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]