[{"id":20018,"web_url":"https://patchwork.libcamera.org/comment/20018/","msgid":"","date":"2021-10-01T16:36:23","subject":"Re: [libcamera-devel] [PATCH 1/2] gstreamer: Check if Stream\n\tconfigurations were generated correctly","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Le vendredi 01 octobre 2021 à 18:10 +0200, Javier Martinez Canillas a écrit :\n> Currently, the gst_libcamera_device_new() function assumes that a call to\n> Camera::generateConfiguration() will always succeed, but that may not be\n> the case and the return value must to be checked.\n> \n> Otherwise, could lead to a NULL pointer deference if the pipeline handler\n *this* dereference\n> failed to generate a configuration for the VideoRecording stream role.\n> \n> Signed-off-by: Javier Martinez Canillas \n\nReviewed-by: Nicolas Dufresne \n\n> ---\n> \n> src/gstreamer/gstlibcameraprovider.cpp | 14 +++++++++++++-\n> 1 file changed, 13 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\n> index aee6f9a2be2..6eb0a0eb0c4 100644\n> --- a/src/gstreamer/gstlibcameraprovider.cpp\n> +++ b/src/gstreamer/gstlibcameraprovider.cpp\n> @@ -132,6 +132,10 @@ gst_libcamera_device_new(const std::shared_ptr &camera)\n> \n> \troles.push_back(StreamRole::VideoRecording);\n> \tstd::unique_ptr config = camera->generateConfiguration(roles);\n> +\tif (!config || config->size() != roles.size()) {\n> +\t\tGST_ERROR(\"Failed to generate a default configuration for %s\", name);\n> +\t\treturn nullptr;\n> +\t}\n> \n> \tfor (const StreamConfiguration &stream_cfg : *config) {\n> \t\tGstCaps *sub_caps = gst_libcamera_stream_formats_to_caps(stream_cfg.formats());\n> @@ -189,8 +193,16 @@ gst_libcamera_provider_probe(GstDeviceProvider *provider)\n> \n> \tfor (const std::shared_ptr &camera : cm->cameras()) {\n> \t\tGST_INFO_OBJECT(self, \"Found camera '%s'\", camera->id().c_str());\n> +\n> +\t\tGstDevice *dev = gst_libcamera_device_new(camera);\n> +\t\tif (!dev) {\n> +\t\t\tGST_ERROR_OBJECT(self, \"Failed to add camera '%s'\",\n> +\t\t\t\t\t camera->id().c_str());\n> +\t\t\treturn nullptr;\n> +\t\t}\n> +\n> \t\tdevices = g_list_append(devices,\n> -\t\t\t\t\tg_object_ref_sink(gst_libcamera_device_new(camera)));\n> +\t\t\t\t\tg_object_ref_sink(dev));\n> \t}\n> \n> \treturn devices;","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 10B2DC3243\n\tfor ;\n\tFri, 1 Oct 2021 16:36:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 79237691AA;\n\tFri, 1 Oct 2021 18:36:28 +0200 (CEST)","from mail-io1-xd35.google.com (mail-io1-xd35.google.com\n\t[IPv6:2607:f8b0:4864:20::d35])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 72E81691A6\n\tfor ;\n\tFri, 1 Oct 2021 18:36:27 +0200 (CEST)","by mail-io1-xd35.google.com with SMTP id i62so12361155ioa.6\n\tfor ;\n\tFri, 01 Oct 2021 09:36:27 -0700 (PDT)","from nicolas-tpx395.localdomain (mtl.collabora.ca. [66.171.169.34])\n\tby smtp.gmail.com with ESMTPSA id\n\tt16sm3889916ilm.68.2021.10.01.09.36.25\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 01 Oct 2021 09:36:25 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ndufresne-ca.20210112.gappssmtp.com\n\theader.i=@ndufresne-ca.20210112.gappssmtp.com\n\theader.b=\"ivUik+rR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20210112.gappssmtp.com; s=20210112;\n\th=message-id:subject:from:to:date:in-reply-to:references:user-agent\n\t:mime-version:content-transfer-encoding;\n\tbh=/O6HJRLltVIU0piZ5I7btuYQDWZauMqEAC5gIiCBhXo=;\n\tb=ivUik+rR43pINs44MYecmyLb4tP6q9MzakVFy4X0O1k83lMgGg8ycueMBwmPun+lFU\n\t071hbAqnb/eYpnXiLvIxROJ0UZ+HDkONHhZqWTr0s0xBjZQ6SCIHS8GR6G4RV5scK6Yg\n\t7hygUW02T02xTYgPAp68DG24K49ZEw1f6Vo8BzF3mha4RxcGiw5lflgtvcatqhUomDrf\n\tiicRWd9LCevtHVTTEh79IE2nb8E3rGOeEJxJuOnDV4esLtTJ5tymiu3YLoQTtVxlmg1t\n\teJV4oeG36NO9qBWSrsezBOKGTmC+4dZ9+1EaHSLzPzxhmusefACyB1TSi4zMSt/KhgRD\n\tjUOQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:message-id:subject:from:to:date:in-reply-to\n\t:references:user-agent:mime-version:content-transfer-encoding;\n\tbh=/O6HJRLltVIU0piZ5I7btuYQDWZauMqEAC5gIiCBhXo=;\n\tb=4lPXPr9B+ec2rqegNMEfl+rVmC8k3EgEoU16wBKYL/Ak4CxaypsJ5PdM3dgQ87LOie\n\t3Z7T17cz/3+PEohFPEJ2bW1WeCWUjmNCHr41/NAIJ0Z9G3PuyLjQKdP20FhH27e2Cr9A\n\tGWJ2sFwSJYxcXC1KhlY1ds15V/xntl1bw2Np7NUiqfzL96cITdRICPA/0tySZnvdl15D\n\th8IUxXOL6tG94uKXLR5tHtQEGPy/ObTxtSofFyl31zQ5/q5KxJiWcyARrXqSNq+3C+co\n\ttdEnfdNGOk7SifR6X5/J5MwNB4PfeIgV1iHsl3XA2bDUJoUrcRzWkYIrfk3vfdS/gYyw\n\t6uGQ==","X-Gm-Message-State":"AOAM532N1QTQmeQEuS9p++BMkar0i3mICFBc/jEZkKbN9g9pLJOI1Gjs\n\tZLl+DYpWf6DL3JCdWnJl43MiZA==","X-Google-Smtp-Source":"ABdhPJwYCW+2ZqgjgTbukQGzz5Jvi5pjzOYxBhjGy8hHXg8jGRLq1w9kWjr7GOEg37bUl6RJOiyQSg==","X-Received":"by 2002:a5e:9314:: with SMTP id\n\tk20mr7525093iom.136.1633106185999; \n\tFri, 01 Oct 2021 09:36:25 -0700 (PDT)","Message-ID":"","From":"Nicolas Dufresne ","To":"Javier Martinez Canillas , \n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 01 Oct 2021 12:36:23 -0400","In-Reply-To":"<20211001161100.1405576-1-javierm@redhat.com>","References":"<20211001161100.1405576-1-javierm@redhat.com>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.40.4 (3.40.4-1.fc34) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 1/2] gstreamer: Check if Stream\n\tconfigurations were generated correctly","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20123,"web_url":"https://patchwork.libcamera.org/comment/20123/","msgid":"","date":"2021-10-12T02:38:59","subject":"Re: [libcamera-devel] [PATCH 1/2] gstreamer: Check if Stream\n\tconfigurations were generated correctly","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Javier,\n\nThank you for the patch.\n\nOn Fri, Oct 01, 2021 at 12:36:23PM -0400, Nicolas Dufresne wrote:\n> Le vendredi 01 octobre 2021 à 18:10 +0200, Javier Martinez Canillas a écrit :\n> > Currently, the gst_libcamera_device_new() function assumes that a call to\n> > Camera::generateConfiguration() will always succeed, but that may not be\n> > the case and the return value must to be checked.\n> > \n> > Otherwise, could lead to a NULL pointer deference if the pipeline handler\n> *this* dereference\n\nI'll fix this when applying.\n\n> > failed to generate a configuration for the VideoRecording stream role.\n> > \n> > Signed-off-by: Javier Martinez Canillas \n> \n> Reviewed-by: Nicolas Dufresne \n\nReviewed-by: Laurent Pinchart \n\n> > ---\n> > \n> > src/gstreamer/gstlibcameraprovider.cpp | 14 +++++++++++++-\n> > 1 file changed, 13 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\n> > index aee6f9a2be2..6eb0a0eb0c4 100644\n> > --- a/src/gstreamer/gstlibcameraprovider.cpp\n> > +++ b/src/gstreamer/gstlibcameraprovider.cpp\n> > @@ -132,6 +132,10 @@ gst_libcamera_device_new(const std::shared_ptr &camera)\n> > \n> > \troles.push_back(StreamRole::VideoRecording);\n> > \tstd::unique_ptr config = camera->generateConfiguration(roles);\n> > +\tif (!config || config->size() != roles.size()) {\n> > +\t\tGST_ERROR(\"Failed to generate a default configuration for %s\", name);\n> > +\t\treturn nullptr;\n> > +\t}\n> > \n> > \tfor (const StreamConfiguration &stream_cfg : *config) {\n> > \t\tGstCaps *sub_caps = gst_libcamera_stream_formats_to_caps(stream_cfg.formats());\n> > @@ -189,8 +193,16 @@ gst_libcamera_provider_probe(GstDeviceProvider *provider)\n> > \n> > \tfor (const std::shared_ptr &camera : cm->cameras()) {\n> > \t\tGST_INFO_OBJECT(self, \"Found camera '%s'\", camera->id().c_str());\n> > +\n> > +\t\tGstDevice *dev = gst_libcamera_device_new(camera);\n> > +\t\tif (!dev) {\n> > +\t\t\tGST_ERROR_OBJECT(self, \"Failed to add camera '%s'\",\n> > +\t\t\t\t\t camera->id().c_str());\n> > +\t\t\treturn nullptr;\n> > +\t\t}\n> > +\n> > \t\tdevices = g_list_append(devices,\n> > -\t\t\t\t\tg_object_ref_sink(gst_libcamera_device_new(camera)));\n> > +\t\t\t\t\tg_object_ref_sink(dev));\n> > \t}\n> > \n> > \treturn devices;","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 B95B2BDC71\n\tfor ;\n\tTue, 12 Oct 2021 02:39:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2F7BE68F4A;\n\tTue, 12 Oct 2021 04:39:14 +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 166586023A\n\tfor ;\n\tTue, 12 Oct 2021 04:39:13 +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 8711FF1;\n\tTue, 12 Oct 2021 04:39:12 +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=\"TjyhErJf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634006352;\n\tbh=jAvvJ0UKwB3RI4hBIV3aaMhUSUJBbInjp7syO+fHEck=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TjyhErJf5wzuq+kivuaGpMcOkDcm2d415AmXDoZmhvJ7taTT0RUvuAm3eM2y9u8Sj\n\twSEvZZUop99YRzmjTsPHCC5G/RpPxx45sm+EA8Kn1da4J+690asVqt9+T+zkvl0MLi\n\tJigHJhoxT8dlVAZ+nBzH6ORwSiSnPpfAZ8m1sPYI=","Date":"Tue, 12 Oct 2021 05:38:59 +0300","From":"Laurent Pinchart ","To":"Javier Martinez Canillas ","Message-ID":"","References":"<20211001161100.1405576-1-javierm@redhat.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 1/2] gstreamer: Check if Stream\n\tconfigurations were generated correctly","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":20125,"web_url":"https://patchwork.libcamera.org/comment/20125/","msgid":"","date":"2021-10-12T02:47:28","subject":"Re: [libcamera-devel] [PATCH 1/2] gstreamer: Check if Stream\n\tconfigurations were generated correctly","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Oct 12, 2021 at 05:39:00AM +0300, Laurent Pinchart wrote:\n> Hi Javier,\n> \n> Thank you for the patch.\n> \n> On Fri, Oct 01, 2021 at 12:36:23PM -0400, Nicolas Dufresne wrote:\n> > Le vendredi 01 octobre 2021 à 18:10 +0200, Javier Martinez Canillas a écrit :\n> > > Currently, the gst_libcamera_device_new() function assumes that a call to\n> > > Camera::generateConfiguration() will always succeed, but that may not be\n> > > the case and the return value must to be checked.\n> > > \n> > > Otherwise, could lead to a NULL pointer deference if the pipeline handler\n> > *this* dereference\n> \n> I'll fix this when applying.\n\nAnd I'll of course push v2, not v1.\n\n> > > failed to generate a configuration for the VideoRecording stream role.\n> > > \n> > > Signed-off-by: Javier Martinez Canillas \n> > \n> > Reviewed-by: Nicolas Dufresne \n> \n> Reviewed-by: Laurent Pinchart \n> \n> > > ---\n> > > \n> > > src/gstreamer/gstlibcameraprovider.cpp | 14 +++++++++++++-\n> > > 1 file changed, 13 insertions(+), 1 deletion(-)\n> > > \n> > > diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\n> > > index aee6f9a2be2..6eb0a0eb0c4 100644\n> > > --- a/src/gstreamer/gstlibcameraprovider.cpp\n> > > +++ b/src/gstreamer/gstlibcameraprovider.cpp\n> > > @@ -132,6 +132,10 @@ gst_libcamera_device_new(const std::shared_ptr &camera)\n> > > \n> > > \troles.push_back(StreamRole::VideoRecording);\n> > > \tstd::unique_ptr config = camera->generateConfiguration(roles);\n> > > +\tif (!config || config->size() != roles.size()) {\n> > > +\t\tGST_ERROR(\"Failed to generate a default configuration for %s\", name);\n> > > +\t\treturn nullptr;\n> > > +\t}\n> > > \n> > > \tfor (const StreamConfiguration &stream_cfg : *config) {\n> > > \t\tGstCaps *sub_caps = gst_libcamera_stream_formats_to_caps(stream_cfg.formats());\n> > > @@ -189,8 +193,16 @@ gst_libcamera_provider_probe(GstDeviceProvider *provider)\n> > > \n> > > \tfor (const std::shared_ptr &camera : cm->cameras()) {\n> > > \t\tGST_INFO_OBJECT(self, \"Found camera '%s'\", camera->id().c_str());\n> > > +\n> > > +\t\tGstDevice *dev = gst_libcamera_device_new(camera);\n> > > +\t\tif (!dev) {\n> > > +\t\t\tGST_ERROR_OBJECT(self, \"Failed to add camera '%s'\",\n> > > +\t\t\t\t\t camera->id().c_str());\n> > > +\t\t\treturn nullptr;\n> > > +\t\t}\n> > > +\n> > > \t\tdevices = g_list_append(devices,\n> > > -\t\t\t\t\tg_object_ref_sink(gst_libcamera_device_new(camera)));\n> > > +\t\t\t\t\tg_object_ref_sink(dev));\n> > > \t}\n> > > \n> > > \treturn devices;","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 68059BDC71\n\tfor ;\n\tTue, 12 Oct 2021 02:47:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CAA3668F4A;\n\tTue, 12 Oct 2021 04:47:43 +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 4AF576023A\n\tfor ;\n\tTue, 12 Oct 2021 04:47:42 +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 BA9A08C4;\n\tTue, 12 Oct 2021 04:47:41 +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=\"s398JAx5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634006861;\n\tbh=CD99JSu9RiImtkgfqNWvqRgQUnkAwIb+djBNCHtwA8U=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=s398JAx5xSxoU6PIFn1PNcsOu1v+O5RyZykRAxqsdkTwsyPD0V/jdZNNZW1nElFFd\n\tzlbTjvTvp+/AmxQq5KlLtjzEBbdlKRrRO5nPMSa3KKNxhT+tnBrSJPZ0O4mgqPRtY8\n\tS3zx7Z9o4CcuZ0scPBYlesEjoZqQ+iCw9HHM/IeM=","Date":"Tue, 12 Oct 2021 05:47:28 +0300","From":"Laurent Pinchart ","To":"Javier Martinez Canillas ","Message-ID":"","References":"<20211001161100.1405576-1-javierm@redhat.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 1/2] gstreamer: Check if Stream\n\tconfigurations were generated correctly","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\" "}}]