[{"id":953,"web_url":"https://patchwork.libcamera.org/comment/953/","msgid":"<20190228171437.GC14557@pendragon.ideasonboard.com>","date":"2019-02-28T17:14:37","subject":"Re: [libcamera-devel] [PATCH v2 1/4] cam: fix order camera is\n\toperated on","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Thu, Feb 28, 2019 at 03:16:23AM +0100, Niklas Söderlund wrote:\n> Upcoming enforcing of order the camera shall be operate on is not\n> compatible with the cam utility. Requests shall be queued after the\n> camera is started, not before.\n\nMy comment on v1 hasn't been addressed. Could you at least add a \\todo\nin the state machine patch to remind us to document camera start timings\n? I'm still not sure that preventing requests from being queued before\ncalling start() will not come back to bite us later, but for now I can't\nthink of a good alternative, so\n\nReviewed-by: Laurent Pinchart \n\n(with the added todo in 3/4)\n\n> Signed-off-by: Niklas Söderlund \n> ---\n> src/cam/main.cpp | 18 +++++++++++-------\n> 1 file changed, 11 insertions(+), 7 deletions(-)\n> \n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index 4c2df583fe8e99b7..8df8844c33a2d052 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -133,6 +133,7 @@ static int capture()\n> \tint ret;\n> \n> \tstd::vector streams = camera->streams();\n> +\tstd::vector requests;\n> \n> \tret = configureStreams(camera.get(), streams);\n> \tif (ret < 0) {\n> @@ -169,21 +170,24 @@ static int capture()\n> \t\t\tgoto out;\n> \t\t}\n> \n> -\t\tret = camera->queueRequest(request);\n> -\t\tif (ret < 0) {\n> -\t\t\tstd::cerr << \"Can't queue request\" << std::endl;\n> -\t\t\tgoto out;\n> -\t\t}\n> +\t\trequests.push_back(request);\n> \t}\n> \n> -\tstd::cout << \"Capture until user interrupts by SIGINT\" << std::endl;\n> -\n> \tret = camera->start();\n> \tif (ret) {\n> \t\tstd::cout << \"Failed to start capture\" << std::endl;\n> \t\tgoto out;\n> \t}\n> \n> +\tfor (Request *request : requests) {\n> +\t\tret = camera->queueRequest(request);\n> +\t\tif (ret < 0) {\n> +\t\t\tstd::cerr << \"Can't queue request\" << std::endl;\n> +\t\t\tgoto out;\n> +\t\t}\n> +\t}\n> +\n> +\tstd::cout << \"Capture until user interrupts by SIGINT\" << std::endl;\n> \tret = loop->exec();\n> \n> \tret = camera->stop();","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0886A610B6\n\tfor ;\n\tThu, 28 Feb 2019 18:14:43 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 73D7049;\n\tThu, 28 Feb 2019 18:14:42 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1551374082;\n\tbh=vhlikskx45Iwqdzslw+n1DlZNnoQI8Ln7OfB8POE1Yw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iNkloUkry3Qjp/Un9Q/CBrZrFIFtVQiKwHcJhZNzkpxvOg+HKt077Wdof/hPqa6oa\n\tEREFEh8wjbqtEAHoPB81qMOhJMWtgCtxdeE/IJWJEtRbQ+PalWAEak3rNc+xKGZ5zR\n\tzugXYCWCmyFw6e3gqQXETE8cg4dPgjC1vlol3ugs=","Date":"Thu, 28 Feb 2019 19:14:37 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190228171437.GC14557@pendragon.ideasonboard.com>","References":"<20190228021626.15062-1-niklas.soderlund@ragnatech.se>\n\t<20190228021626.15062-2-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190228021626.15062-2-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 1/4] cam: fix order camera is\n\toperated on","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Thu, 28 Feb 2019 17:14:43 -0000"}},{"id":962,"web_url":"https://patchwork.libcamera.org/comment/962/","msgid":"<20190228173839.GK899@bigcity.dyn.berto.se>","date":"2019-02-28T17:38:39","subject":"Re: [libcamera-devel] [PATCH v2 1/4] cam: fix order camera is\n\toperated on","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your feedback.\n\nOn 2019-02-28 19:14:37 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Thu, Feb 28, 2019 at 03:16:23AM +0100, Niklas Söderlund wrote:\n> > Upcoming enforcing of order the camera shall be operate on is not\n> > compatible with the cam utility. Requests shall be queued after the\n> > camera is started, not before.\n> \n> My comment on v1 hasn't been addressed. Could you at least add a \\todo\n> in the state machine patch to remind us to document camera start timings\n> ? I'm still not sure that preventing requests from being queued before\n> calling start() will not come back to bite us later, but for now I can't\n> think of a good alternative, so\n\nI will add a \\todo to 3/4, sorry for missing this. How about?\n\n * \\todo Add documentation for camera star timings. What exactly do the\n * camera expect the pipeline handler to do when start() is called.\n\nI agree we might need to relax the criteria for when requests can be \nqueued in the future. For now I think this is the right thing to do and \nwe can change it later if we discover a need for it.\n\n> \n> Reviewed-by: Laurent Pinchart \n> \n> (with the added todo in 3/4)\n> \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > src/cam/main.cpp | 18 +++++++++++-------\n> > 1 file changed, 11 insertions(+), 7 deletions(-)\n> > \n> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > index 4c2df583fe8e99b7..8df8844c33a2d052 100644\n> > --- a/src/cam/main.cpp\n> > +++ b/src/cam/main.cpp\n> > @@ -133,6 +133,7 @@ static int capture()\n> > \tint ret;\n> > \n> > \tstd::vector streams = camera->streams();\n> > +\tstd::vector requests;\n> > \n> > \tret = configureStreams(camera.get(), streams);\n> > \tif (ret < 0) {\n> > @@ -169,21 +170,24 @@ static int capture()\n> > \t\t\tgoto out;\n> > \t\t}\n> > \n> > -\t\tret = camera->queueRequest(request);\n> > -\t\tif (ret < 0) {\n> > -\t\t\tstd::cerr << \"Can't queue request\" << std::endl;\n> > -\t\t\tgoto out;\n> > -\t\t}\n> > +\t\trequests.push_back(request);\n> > \t}\n> > \n> > -\tstd::cout << \"Capture until user interrupts by SIGINT\" << std::endl;\n> > -\n> > \tret = camera->start();\n> > \tif (ret) {\n> > \t\tstd::cout << \"Failed to start capture\" << std::endl;\n> > \t\tgoto out;\n> > \t}\n> > \n> > +\tfor (Request *request : requests) {\n> > +\t\tret = camera->queueRequest(request);\n> > +\t\tif (ret < 0) {\n> > +\t\t\tstd::cerr << \"Can't queue request\" << std::endl;\n> > +\t\t\tgoto out;\n> > +\t\t}\n> > +\t}\n> > +\n> > +\tstd::cout << \"Capture until user interrupts by SIGINT\" << std::endl;\n> > \tret = loop->exec();\n> > \n> > \tret = camera->stop();\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lf1-x141.google.com (mail-lf1-x141.google.com\n\t[IPv6:2a00:1450:4864:20::141])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 305A7610B6\n\tfor ;\n\tThu, 28 Feb 2019 18:38:41 +0100 (CET)","by mail-lf1-x141.google.com with SMTP id j1so15880102lfb.10\n\tfor ;\n\tThu, 28 Feb 2019 09:38:41 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tf9sm4153308ljc.56.2019.02.28.09.38.39\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tThu, 28 Feb 2019 09:38:39 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=dUK0XJP7rKeypd/oOUCzhSSwwRQPksnjMEc16WX75Dg=;\n\tb=CaqVEsBFrqmxZ4CXs/7uzPp8n6wGrOAf20hB8KMRz7K2CR4779NuZATLW1mUFZtqt4\n\tr59nu5B13jjgJQ4STIDXrBLHDEcoGvSMeYrywxMa8/BV058MIjOMbuvtsfff5+Uw7oLX\n\tMEobjEq/4974ZvtJT9UgA4hGeUIWKu3OI8tnBMw1EYvw6pbf09tbEFFNL44q57A+XiZg\n\t5zaBLNa8he94IPs4SjcMFSAUrhZTtIeAvdxJMjUSl/RG0VIH3Jwjp3UHQzKoUGVPCSo9\n\tKrGobZf/Yc3DsBotV1rqQY9/2LzYPEZWjDvk3aSDg2bd9SsIvgpRv0clFbxYd3pzUQ1G\n\tak+A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=dUK0XJP7rKeypd/oOUCzhSSwwRQPksnjMEc16WX75Dg=;\n\tb=Z+xAFxS2LsvKtcdXRm+TBoDtB3wzZNwLBB55Xj+AftCkS+e8+LXa9mnRF5DhI+ogaX\n\tbOUVSmyYAfx0MXR2km7hxYTJDnPQwWEukrLmBi6U8RJCnHoOVwMc7QTGeqzMTawMBcVt\n\tUtS6trn5O21Gd9Dp5/9j/f0U/D+VLIqWZd9gACR+e2MNsOoKH31EbMVCyzh/WyA7eeK/\n\tB2pHJUOPfGO0a//bBELbQE01U+Ppm35Vd6v+aNTDL4uIQIfO5KGZ5YkxAmnRgX/w/5Ki\n\tAocdvI6i01pifz2LE/Znwse8ysoKjEVtB13bhyRVW24TFixAxoX8BOsTWV9zMHfuxnjG\n\tTP1Q==","X-Gm-Message-State":"APjAAAWCNouggmr4R3Av2ceOSlLS6BhDUL/mdYIQ50OBaL1rfwcPUPo4\n\t7bWjEANEyFFAmnoD5FFt3VodQw==","X-Google-Smtp-Source":"APXvYqx2ZPCcpBz9vZm8fgHNaLB5yBt/JBM8znf1GG2M6RSx+rXyWQ8dV20zTon0DG4Zmc7hPdARuA==","X-Received":"by 2002:a19:655e:: with SMTP id c30mr341465lfj.121.1551375520463;\n\tThu, 28 Feb 2019 09:38:40 -0800 (PST)","Date":"Thu, 28 Feb 2019 18:38:39 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190228173839.GK899@bigcity.dyn.berto.se>","References":"<20190228021626.15062-1-niklas.soderlund@ragnatech.se>\n\t<20190228021626.15062-2-niklas.soderlund@ragnatech.se>\n\t<20190228171437.GC14557@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190228171437.GC14557@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 1/4] cam: fix order camera is\n\toperated on","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Thu, 28 Feb 2019 17:38:41 -0000"}},{"id":963,"web_url":"https://patchwork.libcamera.org/comment/963/","msgid":"<20190228175130.GG7811@pendragon.ideasonboard.com>","date":"2019-02-28T17:51:30","subject":"Re: [libcamera-devel] [PATCH v2 1/4] cam: fix order camera is\n\toperated on","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Thu, Feb 28, 2019 at 06:38:39PM +0100, Niklas Söderlund wrote:\n> On 2019-02-28 19:14:37 +0200, Laurent Pinchart wrote:\n> > On Thu, Feb 28, 2019 at 03:16:23AM +0100, Niklas Söderlund wrote:\n> >> Upcoming enforcing of order the camera shall be operate on is not\n> >> compatible with the cam utility. Requests shall be queued after the\n> >> camera is started, not before.\n> > \n> > My comment on v1 hasn't been addressed. Could you at least add a \\todo\n> > in the state machine patch to remind us to document camera start timings\n> > ? I'm still not sure that preventing requests from being queued before\n> > calling start() will not come back to bite us later, but for now I can't\n> > think of a good alternative, so\n> \n> I will add a \\todo to 3/4, sorry for missing this. How about?\n> \n> * \\todo Add documentation for camera star timings. What exactly do the\n> * camera expect the pipeline handler to do when start() is called.\n\ns/star/start/\ns/do the/does the/\ns/called./called?/\n\n> I agree we might need to relax the criteria for when requests can be \n> queued in the future. For now I think this is the right thing to do and \n> we can change it later if we discover a need for it.\n> \n> > Reviewed-by: Laurent Pinchart \n> > \n> > (with the added todo in 3/4)\n> > \n> >> Signed-off-by: Niklas Söderlund \n> >> ---\n> >> src/cam/main.cpp | 18 +++++++++++-------\n> >> 1 file changed, 11 insertions(+), 7 deletions(-)\n> >> \n> >> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> >> index 4c2df583fe8e99b7..8df8844c33a2d052 100644\n> >> --- a/src/cam/main.cpp\n> >> +++ b/src/cam/main.cpp\n> >> @@ -133,6 +133,7 @@ static int capture()\n> >> \tint ret;\n> >> \n> >> \tstd::vector streams = camera->streams();\n> >> +\tstd::vector requests;\n> >> \n> >> \tret = configureStreams(camera.get(), streams);\n> >> \tif (ret < 0) {\n> >> @@ -169,21 +170,24 @@ static int capture()\n> >> \t\t\tgoto out;\n> >> \t\t}\n> >> \n> >> -\t\tret = camera->queueRequest(request);\n> >> -\t\tif (ret < 0) {\n> >> -\t\t\tstd::cerr << \"Can't queue request\" << std::endl;\n> >> -\t\t\tgoto out;\n> >> -\t\t}\n> >> +\t\trequests.push_back(request);\n> >> \t}\n> >> \n> >> -\tstd::cout << \"Capture until user interrupts by SIGINT\" << std::endl;\n> >> -\n> >> \tret = camera->start();\n> >> \tif (ret) {\n> >> \t\tstd::cout << \"Failed to start capture\" << std::endl;\n> >> \t\tgoto out;\n> >> \t}\n> >> \n> >> +\tfor (Request *request : requests) {\n> >> +\t\tret = camera->queueRequest(request);\n> >> +\t\tif (ret < 0) {\n> >> +\t\t\tstd::cerr << \"Can't queue request\" << std::endl;\n> >> +\t\t\tgoto out;\n> >> +\t\t}\n> >> +\t}\n> >> +\n> >> +\tstd::cout << \"Capture until user interrupts by SIGINT\" << std::endl;\n> >> \tret = loop->exec();\n> >> \n> >> \tret = camera->stop();","headers":{"Return-Path":"","Received":["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 59D58610B6\n\tfor ;\n\tThu, 28 Feb 2019 18:51:36 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C1CAF49;\n\tThu, 28 Feb 2019 18:51:35 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1551376296;\n\tbh=IpGrfWkV4dup5Ruiap0OkAiQY3gvtpA708OYfGNTQ8k=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pHCHADC04kvth+1mL5eJ58AbP1ua2iFcWdD6tYafkGxOtgF1xrC/TEbjAzg4dhlD6\n\ta+oX/ckECyQV67HFC29iNbP6XLMP1CkzwAshc8qvoK3kHzmmM175RsdFTJUGrDwfcR\n\tXotVQBhev5+R+GLRQXeEvJkgJwEv7gWkuXq+4cE0=","Date":"Thu, 28 Feb 2019 19:51:30 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190228175130.GG7811@pendragon.ideasonboard.com>","References":"<20190228021626.15062-1-niklas.soderlund@ragnatech.se>\n\t<20190228021626.15062-2-niklas.soderlund@ragnatech.se>\n\t<20190228171437.GC14557@pendragon.ideasonboard.com>\n\t<20190228173839.GK899@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190228173839.GK899@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 1/4] cam: fix order camera is\n\toperated on","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Thu, 28 Feb 2019 17:51:36 -0000"}}]