[{"id":4654,"web_url":"https://patchwork.libcamera.org/comment/4654/","msgid":"<20200430004414.GT5956@pendragon.ideasonboard.com>","date":"2020-04-30T00:44:14","subject":"Re: [libcamera-devel] [RFC/PATCH 1/5] qcam: Check that camera can\n\tgenerate configuration from roles","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, Apr 30, 2020 at 02:36:00AM +0200, Niklas Söderlund wrote:\n> If the camera can not generate a configuration from the requested roles\n> it returns a nullptr which leads to a nullptr dereference. Fix this by\n> adding a check that the camera generated a configuration before trying\n> to access it.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> src/qcam/main_window.cpp | 4 ++++\n> 1 file changed, 4 insertions(+)\n> \n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index ee779728fc630da8..b683c2e00d317307 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -293,6 +293,10 @@ int MainWindow::startCapture()\n> \n> \t/* Configure the camera. */\n> \tconfig_ = camera_->generateConfiguration(roles);\n> +\tif (!config_) {\n> +\t\tqWarning() << \"Failed to generate configuration from roles\";\n> +\t\treturn -EINVAL;\n> +\t}\n\nThis is certainly better than a crash, so\n\nReviewed-by: Laurent Pinchart \n\nbut what happens in the GUI ?\n\n> \n> \tStreamConfiguration &cfg = config_->at(0);\n>","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 05AA16041C\n\tfor ;\n\tThu, 30 Apr 2020 02:44:17 +0200 (CEST)","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 E9FD5503;\n\tThu, 30 Apr 2020 02:44:15 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"rAm0JuZ2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1588207456;\n\tbh=Ura6NVVGlpxEtnCpeFJ8UDTlZNmmB/UHofpQ7FRjOrc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rAm0JuZ2MBFWbTGXMTdEVlcnQnqFpII5EjKn08K3kFauw9++9ZPUfJ0Gbfin/y09E\n\tkUtmz7/I8KRHPPubYnKVhikUa9WsQepO9gjz9V9kmFf4YTteSN0OT4MFqGOidvs6Oj\n\tclM773iJ6WR1yHYzASUTzQYnapZkMutSy8ANrM44=","Date":"Thu, 30 Apr 2020 03:44:14 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200430004414.GT5956@pendragon.ideasonboard.com>","References":"<20200430003604.2423018-1-niklas.soderlund@ragnatech.se>\n\t<20200430003604.2423018-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":"<20200430003604.2423018-2-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [RFC/PATCH 1/5] qcam: Check that camera can\n\tgenerate configuration from roles","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","X-List-Received-Date":"Thu, 30 Apr 2020 00:44:17 -0000"}},{"id":4691,"web_url":"https://patchwork.libcamera.org/comment/4691/","msgid":"<20200501020559.GA2351061@oden.dyn.berto.se>","date":"2020-05-01T02:05:59","subject":"Re: [libcamera-devel] [RFC/PATCH 1/5] qcam: Check that camera can\n\tgenerate configuration from roles","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 2020-04-30 03:44:14 +0300, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Thu, Apr 30, 2020 at 02:36:00AM +0200, Niklas Söderlund wrote:\n> > If the camera can not generate a configuration from the requested roles\n> > it returns a nullptr which leads to a nullptr dereference. Fix this by\n> > adding a check that the camera generated a configuration before trying\n> > to access it.\n> > \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > src/qcam/main_window.cpp | 4 ++++\n> > 1 file changed, 4 insertions(+)\n> > \n> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> > index ee779728fc630da8..b683c2e00d317307 100644\n> > --- a/src/qcam/main_window.cpp\n> > +++ b/src/qcam/main_window.cpp\n> > @@ -293,6 +293,10 @@ int MainWindow::startCapture()\n> > \n> > \t/* Configure the camera. */\n> > \tconfig_ = camera_->generateConfiguration(roles);\n> > +\tif (!config_) {\n> > +\t\tqWarning() << \"Failed to generate configuration from roles\";\n> > +\t\treturn -EINVAL;\n> > +\t}\n> \n> This is certainly better than a crash, so\n> \n> Reviewed-by: Laurent Pinchart \n> \n> but what happens in the GUI ?\n\nIt refuses to start to stream but does not lock-up, same as for the \nerror cases bellow this one where -EINVAL is already returned.\n\n> \n> > \n> > \tStreamConfiguration &cfg = config_->at(0);\n> > \n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4790D61469\n\tfor ;\n\tFri, 1 May 2020 04:06:02 +0200 (CEST)","by mail-lj1-x241.google.com with SMTP id u15so1464263ljd.3\n\tfor ;\n\tThu, 30 Apr 2020 19:06:02 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tw9sm1035544ljj.88.2020.04.30.19.06.00\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 30 Apr 2020 19:06:00 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com header.b=\"GiSN8BI7\"; \n\tdkim-atps=neutral","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\tbh=5/ME252jxNbhbYtExTp1JjMcKk7TDJxK9i4mzWoBUB4=;\n\tb=GiSN8BI7d42Au/JlA5/rZbkBzPqnBvMBcsS2z1E7AP9GFZymQRWBg7wMvuefOp7LpY\n\trkZQdH+XDIXKkJ2xSflsZAqS9DwvFxmDY2pJMHBHMVCQd/cluefJ6aD2iR2zpvdM4+B8\n\t4HO/SnD8j4wuJCACOiNwluVtW4nSUdqdlJLijUZGoLq4cyWPOM7n8u6G3Z1ldsyXrwm/\n\tqAXPgMIFk2++2BG7IXA2naWPpopwYdcb8gkRRBEaENw8RBZ9Th2gVZvrNA+R8EnAYDC/\n\te4CY85MsTPW5tQ4R5npnXU1ETewwQ4a51uB7tUyk2fj4z6fgBp0wzx0XocenJpBKu8BB\n\tqswA==","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;\n\tbh=5/ME252jxNbhbYtExTp1JjMcKk7TDJxK9i4mzWoBUB4=;\n\tb=RQz7lGTInWcQ0xpsamFkGiafuUwQ/4P2v/v1lQMtk/khRVQYPMjMlxUdGdxA3BywAl\n\t/fM5Zx8eq3JIPIRFbJ1yfxNaniAmLqrH5neXHko6T3NyzElxnsFocAefZPujTT0vL6yn\n\tgZzVheKCPvDW8SJBNBTj7EvQzTFCUYDgli7D1kyfhXMq7cPEPCmIQT6rG4OdbfhB7DWi\n\t9eRY/gQQRRMWrEd8tYBPpf2Yva3lmPZqQaiJqQw97srGWBFMCdv+AfbGjoDtlCGBR9CR\n\tO38p20mo0aMuZwTaO8ltcdfb1fnt+sR1cGxb3WEJ4ZHaCMzIKA2VgfFHd1OSRmeMklQN\n\tu56A==","X-Gm-Message-State":"AGi0PuYTIfezm7AJSuhv5goTzuaX5AaHf6RrqN/Pb0vFTovmCm57VxgA\n\t9yue+/hkXXEEQywrh+VyhMgiPLlOu04=","X-Google-Smtp-Source":"APiQypKBdR/vVS/eGGAlHdK/BBsSPZiKZcQGgDg4kU4Ph/b9Qk4ck8iKZdyHN4HHzO1IIBHJaqlJQg==","X-Received":"by 2002:a2e:90cd:: with SMTP id\n\to13mr1107181ljg.220.1588298761215; \n\tThu, 30 Apr 2020 19:06:01 -0700 (PDT)","Date":"Fri, 1 May 2020 04:05:59 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200501020559.GA2351061@oden.dyn.berto.se>","References":"<20200430003604.2423018-1-niklas.soderlund@ragnatech.se>\n\t<20200430003604.2423018-2-niklas.soderlund@ragnatech.se>\n\t<20200430004414.GT5956@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":"<20200430004414.GT5956@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC/PATCH 1/5] qcam: Check that camera can\n\tgenerate configuration from roles","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","X-List-Received-Date":"Fri, 01 May 2020 02:06:02 -0000"}},{"id":4696,"web_url":"https://patchwork.libcamera.org/comment/4696/","msgid":"<20200501114857.GB5951@pendragon.ideasonboard.com>","date":"2020-05-01T11:48:57","subject":"Re: [libcamera-devel] [RFC/PATCH 1/5] qcam: Check that camera can\n\tgenerate configuration from roles","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Fri, May 01, 2020 at 04:05:59AM +0200, Niklas Söderlund wrote:\n> On 2020-04-30 03:44:14 +0300, Laurent Pinchart wrote:\n> > On Thu, Apr 30, 2020 at 02:36:00AM +0200, Niklas Söderlund wrote:\n> > > If the camera can not generate a configuration from the requested roles\n> > > it returns a nullptr which leads to a nullptr dereference. Fix this by\n> > > adding a check that the camera generated a configuration before trying\n> > > to access it.\n> > > \n> > > Signed-off-by: Niklas Söderlund \n> > > ---\n> > > src/qcam/main_window.cpp | 4 ++++\n> > > 1 file changed, 4 insertions(+)\n> > > \n> > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> > > index ee779728fc630da8..b683c2e00d317307 100644\n> > > --- a/src/qcam/main_window.cpp\n> > > +++ b/src/qcam/main_window.cpp\n> > > @@ -293,6 +293,10 @@ int MainWindow::startCapture()\n> > > \n> > > \t/* Configure the camera. */\n> > > \tconfig_ = camera_->generateConfiguration(roles);\n> > > +\tif (!config_) {\n> > > +\t\tqWarning() << \"Failed to generate configuration from roles\";\n> > > +\t\treturn -EINVAL;\n> > > +\t}\n> > \n> > This is certainly better than a crash, so\n> > \n> > Reviewed-by: Laurent Pinchart \n> > \n> > but what happens in the GUI ?\n> \n> It refuses to start to stream but does not lock-up, same as for the \n> error cases bellow this one where -EINVAL is already returned.\n\nWe probably want to have a way to display messages in the GUI. Not\nsomething for this patch series of course. I was thinking of a zone at\nthe bottom of the window with some sort of log (or even a single message\nin a status bar).\n\n> > > \n> > > \tStreamConfiguration &cfg = config_->at(0);\n> > >","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 948FA603F3\n\tfor ;\n\tFri, 1 May 2020 13:49:00 +0200 (CEST)","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 EDC5572C;\n\tFri, 1 May 2020 13:48:59 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"qovlWgsh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1588333740;\n\tbh=JsOT0nk2tMBKjEiE2BggPHaFruVA/jWeEABgq11eJoo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qovlWgshOFGPJRy3DNIam8ogQdYpaEOmI1h37hvc+0mlaE6An7ZzSv9617tvAAcg7\n\tB+fJbBYH8UCoePipaMUykwuC0I3ddqaQbZD6vhgnEYVAfDpPXKlE/5417m+ecqLMz9\n\tQHgfSx6DKH2YRodmZGqXRFTvYcvJ6NTp40HVEjr0=","Date":"Fri, 1 May 2020 14:48:57 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200501114857.GB5951@pendragon.ideasonboard.com>","References":"<20200430003604.2423018-1-niklas.soderlund@ragnatech.se>\n\t<20200430003604.2423018-2-niklas.soderlund@ragnatech.se>\n\t<20200430004414.GT5956@pendragon.ideasonboard.com>\n\t<20200501020559.GA2351061@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200501020559.GA2351061@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [RFC/PATCH 1/5] qcam: Check that camera can\n\tgenerate configuration from roles","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","X-List-Received-Date":"Fri, 01 May 2020 11:49:00 -0000"}}]