[{"id":15308,"web_url":"https://patchwork.libcamera.org/comment/15308/","msgid":"<1b276ffa607edd42069984268d34ed6308543f66.camel@collabora.com>","date":"2021-02-24T18:33:17","subject":"Re: [libcamera-devel] [PATCH] src: gstreamer: use the streams of\n\tCameraConfiguration when allocating buffers","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le mercredi 24 février 2021 à 18:19 +0100, Dafna Hirschfeld a écrit :\n> Currently, when allocating buffers, the streams of\n> the Camera object are used. Instead the streams of\n> the CameraConfiguration object should be used.\n\nThe changes looks fine, but perhaps you could extend, I wrote this, still I\ndon't understand why enumerating the streams in the configuration would make any\ndifference after the camera has been configured.\n\nMinor comment, other GStreamer patches commit message was simply \"gst:\nDescription\", just mentioning for consistency, but perhaps there is new rules.\n\nReviewed-by: Nicolas Dufresne \n\n> \n> Signed-off-by: Dafna Hirschfeld \n> ---\n>  src/gstreamer/gstlibcameraallocator.cpp | 6 ++++--\n>  src/gstreamer/gstlibcameraallocator.h   | 4 +++-\n>  src/gstreamer/gstlibcamerasrc.cpp       | 2 +-\n>  3 files changed, 8 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/gstreamer/gstlibcameraallocator.cpp\n> b/src/gstreamer/gstlibcameraallocator.cpp\n> index 13c6b493..7bd8ba2d 100644\n> --- a/src/gstreamer/gstlibcameraallocator.cpp\n> +++ b/src/gstreamer/gstlibcameraallocator.cpp\n> @@ -183,13 +183,15 @@\n> gst_libcamera_allocator_class_init(GstLibcameraAllocatorClass *klass)\n>  }\n>  \n>  GstLibcameraAllocator *\n> -gst_libcamera_allocator_new(std::shared_ptr camera)\n> +gst_libcamera_allocator_new(std::shared_ptr camera,\n> +                           CameraConfiguration *config_)\n>  {\n>         auto *self =\n> GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR,\n>                                                           nullptr));\n>  \n>         self->fb_allocator = new FrameBufferAllocator(camera);\n> -       for (Stream *stream : camera->streams()) {\n> +       for (StreamConfiguration &streamCfg : *config_) {\n> +               Stream *stream = streamCfg.stream();\n>                 gint ret;\n>  \n>                 ret = self->fb_allocator->allocate(stream);\n> diff --git a/src/gstreamer/gstlibcameraallocator.h\n> b/src/gstreamer/gstlibcameraallocator.h\n> index befdcad6..f7fa6acd 100644\n> --- a/src/gstreamer/gstlibcameraallocator.h\n> +++ b/src/gstreamer/gstlibcameraallocator.h\n> @@ -13,12 +13,14 @@\n>  #include \n>  \n>  #include \n> +#include \n>  \n>  #define GST_TYPE_LIBCAMERA_ALLOCATOR gst_libcamera_allocator_get_type()\n>  G_DECLARE_FINAL_TYPE(GstLibcameraAllocator, gst_libcamera_allocator,\n>                      GST_LIBCAMERA, ALLOCATOR, GstDmaBufAllocator)\n>  \n> -GstLibcameraAllocator\n> *gst_libcamera_allocator_new(std::shared_ptr camera);\n> +GstLibcameraAllocator\n> *gst_libcamera_allocator_new(std::shared_ptr camera,\n> +                                                 \n> libcamera::CameraConfiguration *config_);\n>  \n>  bool gst_libcamera_allocator_prepare_buffer(GstLibcameraAllocator *self,\n>                                             libcamera::Stream *stream,\n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n> b/src/gstreamer/gstlibcamerasrc.cpp\n> index 636c14df..7b13667b 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -429,7 +429,7 @@ gst_libcamera_src_task_enter(GstTask *task,\n> [[maybe_unused]] GThread *thread,\n>                 return;\n>         }\n>  \n> -       self->allocator = gst_libcamera_allocator_new(state->cam_);\n> +       self->allocator = gst_libcamera_allocator_new(state->cam_, state-\n> >config_.get());\n>         if (!self->allocator) {\n>                 GST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT,\n>                                   (\"Failed to allocate memory\"),","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 29B7EBD1F1\n\tfor ;\n\tWed, 24 Feb 2021 18:33:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 70E2B68A3D;\n\tWed, 24 Feb 2021 19:33:29 +0100 (CET)","from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A30A368A1E\n\tfor ;\n\tWed, 24 Feb 2021 19:33:28 +0100 (CET)","from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: nicolas) with ESMTPSA id B5A7B1F45BFC"],"Message-ID":"<1b276ffa607edd42069984268d34ed6308543f66.camel@collabora.com>","From":"Nicolas Dufresne ","To":"Dafna Hirschfeld , \n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 24 Feb 2021 13:33:17 -0500","In-Reply-To":"<20210224171926.31470-1-dafna.hirschfeld@collabora.com>","References":"<20210224171926.31470-1-dafna.hirschfeld@collabora.com>","User-Agent":"Evolution 3.38.3 (3.38.3-1.fc33) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [PATCH] src: gstreamer: use the streams of\n\tCameraConfiguration when allocating buffers","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":"kernel@collabora.com","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15317,"web_url":"https://patchwork.libcamera.org/comment/15317/","msgid":"<5993a8b5-ced2-7473-02a0-6fa6d48f2e4e@collabora.com>","date":"2021-02-25T15:41:39","subject":"Re: [libcamera-devel] [PATCH] src: gstreamer: use the streams of\n\tCameraConfiguration when allocating buffers","submitter":{"id":46,"url":"https://patchwork.libcamera.org/api/people/46/","name":"Dafna Hirschfeld","email":"dafna.hirschfeld@collabora.com"},"content":"Hi,\n\nOn 24.02.21 19:33, Nicolas Dufresne wrote:\n> Le mercredi 24 février 2021 à 18:19 +0100, Dafna Hirschfeld a écrit :\n>> Currently, when allocating buffers, the streams of\n>> the Camera object are used. Instead the streams of\n>> the CameraConfiguration object should be used.\n> \n> The changes looks fine, but perhaps you could extend, I wrote this, still I\n> don't understand why enumerating the streams in the configuration would make any\n> difference after the camera has been configured.\n\nThe Camera holds a list of all available streams.\nFor rkisp1 those are mainpath and selfpath.\nThen only subset of those streams are chosen which are\nthe stream kept in the CameraConfiguration object.\nSpecifically using gstreamer with rkisp1 I got:\n\n[3:59:32.003159780] [2499] ERROR Allocator framebuffer_allocator.cpp:97 Stream is not part of /base/i2c@ff3d0000/sensor@10 active configuration\n\n> \n> Minor comment, other GStreamer patches commit message was simply \"gst:\n> Description\", just mentioning for consistency, but perhaps there is new rules.\n> \n\nIll send v2,\n\nThanks,\nDafna\n\n> Reviewed-by: Nicolas Dufresne \n> \n>>\n>> Signed-off-by: Dafna Hirschfeld \n>> ---\n>>  src/gstreamer/gstlibcameraallocator.cpp | 6 ++++--\n>>  src/gstreamer/gstlibcameraallocator.h   | 4 +++-\n>>  src/gstreamer/gstlibcamerasrc.cpp       | 2 +-\n>>  3 files changed, 8 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/src/gstreamer/gstlibcameraallocator.cpp\n>> b/src/gstreamer/gstlibcameraallocator.cpp\n>> index 13c6b493..7bd8ba2d 100644\n>> --- a/src/gstreamer/gstlibcameraallocator.cpp\n>> +++ b/src/gstreamer/gstlibcameraallocator.cpp\n>> @@ -183,13 +183,15 @@\n>> gst_libcamera_allocator_class_init(GstLibcameraAllocatorClass *klass)\n>>  }\n>> \n>>  GstLibcameraAllocator *\n>> -gst_libcamera_allocator_new(std::shared_ptr camera)\n>> +gst_libcamera_allocator_new(std::shared_ptr camera,\n>> +                           CameraConfiguration *config_)\n>>  {\n>>         auto *self =\n>> GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR,\n>>                                                           nullptr));\n>> \n>>         self->fb_allocator = new FrameBufferAllocator(camera);\n>> -       for (Stream *stream : camera->streams()) {\n>> +       for (StreamConfiguration &streamCfg : *config_) {\n>> +               Stream *stream = streamCfg.stream();\n>>                 gint ret;\n>> \n>>                 ret = self->fb_allocator->allocate(stream);\n>> diff --git a/src/gstreamer/gstlibcameraallocator.h\n>> b/src/gstreamer/gstlibcameraallocator.h\n>> index befdcad6..f7fa6acd 100644\n>> --- a/src/gstreamer/gstlibcameraallocator.h\n>> +++ b/src/gstreamer/gstlibcameraallocator.h\n>> @@ -13,12 +13,14 @@\n>>  #include \n>> \n>>  #include \n>> +#include \n>> \n>>  #define GST_TYPE_LIBCAMERA_ALLOCATOR gst_libcamera_allocator_get_type()\n>>  G_DECLARE_FINAL_TYPE(GstLibcameraAllocator, gst_libcamera_allocator,\n>>                      GST_LIBCAMERA, ALLOCATOR, GstDmaBufAllocator)\n>> \n>> -GstLibcameraAllocator\n>> *gst_libcamera_allocator_new(std::shared_ptr camera);\n>> +GstLibcameraAllocator\n>> *gst_libcamera_allocator_new(std::shared_ptr camera,\n>> +\n>> libcamera::CameraConfiguration *config_);\n>> \n>>  bool gst_libcamera_allocator_prepare_buffer(GstLibcameraAllocator *self,\n>>                                             libcamera::Stream *stream,\n>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n>> b/src/gstreamer/gstlibcamerasrc.cpp\n>> index 636c14df..7b13667b 100644\n>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>> @@ -429,7 +429,7 @@ gst_libcamera_src_task_enter(GstTask *task,\n>> [[maybe_unused]] GThread *thread,\n>>                 return;\n>>         }\n>> \n>> -       self->allocator = gst_libcamera_allocator_new(state->cam_);\n>> +       self->allocator = gst_libcamera_allocator_new(state->cam_, state-\n>>> config_.get());\n>>         if (!self->allocator) {\n>>                 GST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT,\n>>                                   (\"Failed to allocate memory\"),\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 76BD9BD1F1\n\tfor ;\n\tThu, 25 Feb 2021 15:41:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ED29468A4B;\n\tThu, 25 Feb 2021 16:41:42 +0100 (CET)","from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 699E6602EE\n\tfor ;\n\tThu, 25 Feb 2021 16:41:41 +0100 (CET)","from [IPv6:2003:c7:cf1c:ce00:c495:f7eb:403f:1d15]\n\t(p200300c7cf1cce00c495f7eb403f1d15.dip0.t-ipconnect.de\n\t[IPv6:2003:c7:cf1c:ce00:c495:f7eb:403f:1d15])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits))\n\t(No client certificate requested) (Authenticated sender: dafna)\n\tby bhuna.collabora.co.uk (Postfix) with ESMTPSA id F1F111F460D7;\n\tThu, 25 Feb 2021 15:41:40 +0000 (GMT)"],"To":"Nicolas Dufresne ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210224171926.31470-1-dafna.hirschfeld@collabora.com>\n\t<1b276ffa607edd42069984268d34ed6308543f66.camel@collabora.com>","From":"Dafna Hirschfeld ","Message-ID":"<5993a8b5-ced2-7473-02a0-6fa6d48f2e4e@collabora.com>","Date":"Thu, 25 Feb 2021 16:41:39 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<1b276ffa607edd42069984268d34ed6308543f66.camel@collabora.com>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] src: gstreamer: use the streams of\n\tCameraConfiguration when allocating buffers","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":"kernel@collabora.com","Content-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15321,"web_url":"https://patchwork.libcamera.org/comment/15321/","msgid":"<83721b2d16b62aa7728f0b0fb7f8a58e5c6d3adc.camel@collabora.com>","date":"2021-02-25T18:30:57","subject":"Re: [libcamera-devel] [PATCH] src: gstreamer: use the streams of\n\tCameraConfiguration when allocating buffers","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le jeudi 25 février 2021 à 16:41 +0100, Dafna Hirschfeld a écrit :\n> Hi,\n> \n> On 24.02.21 19:33, Nicolas Dufresne wrote:\n> > Le mercredi 24 février 2021 à 18:19 +0100, Dafna Hirschfeld a écrit :\n> > > Currently, when allocating buffers, the streams of\n> > > the Camera object are used. Instead the streams of\n> > > the CameraConfiguration object should be used.\n> > \n> > The changes looks fine, but perhaps you could extend, I wrote this, still I\n> > don't understand why enumerating the streams in the configuration would make\n> > any\n> > difference after the camera has been configured.\n> \n> The Camera holds a list of all available streams.\n> For rkisp1 those are mainpath and selfpath.\n> Then only subset of those streams are chosen which are\n> the stream kept in the CameraConfiguration object.\n> Specifically using gstreamer with rkisp1 I got:\n> \n> [3:59:32.003159780] [2499] ERROR Allocator framebuffer_allocator.cpp:97 Stream\n> is not part of /base/i2c@ff3d0000/sensor@10 active configuration\n> \n> > \n> > Minor comment, other GStreamer patches commit message was simply \"gst:\n> > Description\", just mentioning for consistency, but perhaps there is new\n> > rules.\n> > \n> \n> Ill send v2,\n\nThanks for the explanation, and my review by still holds ;-P\n\n> \n> Thanks,\n> Dafna\n> \n> > Reviewed-by: Nicolas Dufresne \n> > \n> > > \n> > > Signed-off-by: Dafna Hirschfeld \n> > > ---\n> > >   src/gstreamer/gstlibcameraallocator.cpp | 6 ++++--\n> > >   src/gstreamer/gstlibcameraallocator.h   | 4 +++-\n> > >   src/gstreamer/gstlibcamerasrc.cpp       | 2 +-\n> > >   3 files changed, 8 insertions(+), 4 deletions(-)\n> > > \n> > > diff --git a/src/gstreamer/gstlibcameraallocator.cpp\n> > > b/src/gstreamer/gstlibcameraallocator.cpp\n> > > index 13c6b493..7bd8ba2d 100644\n> > > --- a/src/gstreamer/gstlibcameraallocator.cpp\n> > > +++ b/src/gstreamer/gstlibcameraallocator.cpp\n> > > @@ -183,13 +183,15 @@\n> > > gst_libcamera_allocator_class_init(GstLibcameraAllocatorClass *klass)\n> > >   }\n> > >   \n> > >   GstLibcameraAllocator *\n> > > -gst_libcamera_allocator_new(std::shared_ptr camera)\n> > > +gst_libcamera_allocator_new(std::shared_ptr camera,\n> > > +                           CameraConfiguration *config_)\n> > >   {\n> > >          auto *self =\n> > > GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR,\n> > >                                                            nullptr));\n> > >   \n> > >          self->fb_allocator = new FrameBufferAllocator(camera);\n> > > -       for (Stream *stream : camera->streams()) {\n> > > +       for (StreamConfiguration &streamCfg : *config_) {\n> > > +               Stream *stream = streamCfg.stream();\n> > >                  gint ret;\n> > >   \n> > >                  ret = self->fb_allocator->allocate(stream);\n> > > diff --git a/src/gstreamer/gstlibcameraallocator.h\n> > > b/src/gstreamer/gstlibcameraallocator.h\n> > > index befdcad6..f7fa6acd 100644\n> > > --- a/src/gstreamer/gstlibcameraallocator.h\n> > > +++ b/src/gstreamer/gstlibcameraallocator.h\n> > > @@ -13,12 +13,14 @@\n> > >   #include \n> > >   \n> > >   #include \n> > > +#include \n> > >   \n> > >   #define GST_TYPE_LIBCAMERA_ALLOCATOR gst_libcamera_allocator_get_type()\n> > >   G_DECLARE_FINAL_TYPE(GstLibcameraAllocator, gst_libcamera_allocator,\n> > >                       GST_LIBCAMERA, ALLOCATOR, GstDmaBufAllocator)\n> > >   \n> > > -GstLibcameraAllocator\n> > > *gst_libcamera_allocator_new(std::shared_ptr camera);\n> > > +GstLibcameraAllocator\n> > > *gst_libcamera_allocator_new(std::shared_ptr camera,\n> > > +\n> > > libcamera::CameraConfiguration *config_);\n> > >   \n> > >   bool gst_libcamera_allocator_prepare_buffer(GstLibcameraAllocator *self,\n> > >                                              libcamera::Stream *stream,\n> > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n> > > b/src/gstreamer/gstlibcamerasrc.cpp\n> > > index 636c14df..7b13667b 100644\n> > > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > > @@ -429,7 +429,7 @@ gst_libcamera_src_task_enter(GstTask *task,\n> > > [[maybe_unused]] GThread *thread,\n> > >                  return;\n> > >          }\n> > >   \n> > > -       self->allocator = gst_libcamera_allocator_new(state->cam_);\n> > > +       self->allocator = gst_libcamera_allocator_new(state->cam_, state-\n> > > > config_.get());\n> > >          if (!self->allocator) {\n> > >                  GST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT,\n> > >                                    (\"Failed to allocate memory\"),\n> > \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 8E581BD808\n\tfor ;\n\tThu, 25 Feb 2021 18:31:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F378C68A5C;\n\tThu, 25 Feb 2021 19:31:07 +0100 (CET)","from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3182868A51\n\tfor ;\n\tThu, 25 Feb 2021 19:31:07 +0100 (CET)","from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: nicolas) with ESMTPSA id 5E35B1F463C8"],"Message-ID":"<83721b2d16b62aa7728f0b0fb7f8a58e5c6d3adc.camel@collabora.com>","From":"Nicolas Dufresne ","To":"Dafna Hirschfeld , \n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 25 Feb 2021 13:30:57 -0500","In-Reply-To":"<5993a8b5-ced2-7473-02a0-6fa6d48f2e4e@collabora.com>","References":"<20210224171926.31470-1-dafna.hirschfeld@collabora.com>\n\t<1b276ffa607edd42069984268d34ed6308543f66.camel@collabora.com>\n\t<5993a8b5-ced2-7473-02a0-6fa6d48f2e4e@collabora.com>","User-Agent":"Evolution 3.38.3 (3.38.3-1.fc33) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [PATCH] src: gstreamer: use the streams of\n\tCameraConfiguration when allocating buffers","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":"kernel@collabora.com","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15682,"web_url":"https://patchwork.libcamera.org/comment/15682/","msgid":"","date":"2021-03-14T22:35:14","subject":"Re: [libcamera-devel] [PATCH] src: gstreamer: use the streams of\n\tCameraConfiguration when allocating buffers","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Dafna,\n\nThank you for the patch.\n\nOn Thu, Feb 25, 2021 at 04:41:39PM +0100, Dafna Hirschfeld wrote:\n> On 24.02.21 19:33, Nicolas Dufresne wrote:\n> > Le mercredi 24 février 2021 à 18:19 +0100, Dafna Hirschfeld a écrit :\n> >> Currently, when allocating buffers, the streams of\n> >> the Camera object are used. Instead the streams of\n> >> the CameraConfiguration object should be used.\n> > \n> > The changes looks fine, but perhaps you could extend, I wrote this, still I\n> > don't understand why enumerating the streams in the configuration would make any\n> > difference after the camera has been configured.\n> \n> The Camera holds a list of all available streams.\n> For rkisp1 those are mainpath and selfpath.\n> Then only subset of those streams are chosen which are\n> the stream kept in the CameraConfiguration object.\n> Specifically using gstreamer with rkisp1 I got:\n> \n> [3:59:32.003159780] [2499] ERROR Allocator framebuffer_allocator.cpp:97 Stream is not part of /base/i2c@ff3d0000/sensor@10 active configuration\n> \n> > Minor comment, other GStreamer patches commit message was simply \"gst:\n> > Description\", just mentioning for consistency, but perhaps there is new rules.\n> \n> Ill send v2,\n\nDo you still plan to send a v2 ?\n\n> > Reviewed-by: Nicolas Dufresne \n> >>\n> >> Signed-off-by: Dafna Hirschfeld \n> >> ---\n> >>  src/gstreamer/gstlibcameraallocator.cpp | 6 ++++--\n> >>  src/gstreamer/gstlibcameraallocator.h   | 4 +++-\n> >>  src/gstreamer/gstlibcamerasrc.cpp       | 2 +-\n> >>  3 files changed, 8 insertions(+), 4 deletions(-)\n> >>\n> >> diff --git a/src/gstreamer/gstlibcameraallocator.cpp\n> >> b/src/gstreamer/gstlibcameraallocator.cpp\n> >> index 13c6b493..7bd8ba2d 100644\n> >> --- a/src/gstreamer/gstlibcameraallocator.cpp\n> >> +++ b/src/gstreamer/gstlibcameraallocator.cpp\n> >> @@ -183,13 +183,15 @@ gst_libcamera_allocator_class_init(GstLibcameraAllocatorClass *klass)\n> >>  }\n> >> \n> >>  GstLibcameraAllocator *\n> >> -gst_libcamera_allocator_new(std::shared_ptr camera)\n> >> +gst_libcamera_allocator_new(std::shared_ptr camera,\n> >> +                           CameraConfiguration *config_)\n> >>  {\n> >>         auto *self = GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR,\n> >>                                                           nullptr));\n> >> \n> >>         self->fb_allocator = new FrameBufferAllocator(camera);\n> >> -       for (Stream *stream : camera->streams()) {\n> >> +       for (StreamConfiguration &streamCfg : *config_) {\n> >> +               Stream *stream = streamCfg.stream();\n> >>                 gint ret;\n> >> \n> >>                 ret = self->fb_allocator->allocate(stream);\n> >> diff --git a/src/gstreamer/gstlibcameraallocator.h\n> >> b/src/gstreamer/gstlibcameraallocator.h\n> >> index befdcad6..f7fa6acd 100644\n> >> --- a/src/gstreamer/gstlibcameraallocator.h\n> >> +++ b/src/gstreamer/gstlibcameraallocator.h\n> >> @@ -13,12 +13,14 @@\n> >>  #include \n> >> \n> >>  #include \n> >> +#include \n\nAlphabetical order please :-)\n\nWith this addressed, and an updated commit message,\n\nReviewed-by: Laurent Pinchart \n\n> >> \n> >>  #define GST_TYPE_LIBCAMERA_ALLOCATOR gst_libcamera_allocator_get_type()\n> >>  G_DECLARE_FINAL_TYPE(GstLibcameraAllocator, gst_libcamera_allocator,\n> >>                      GST_LIBCAMERA, ALLOCATOR, GstDmaBufAllocator)\n> >> \n> >> -GstLibcameraAllocator *gst_libcamera_allocator_new(std::shared_ptr camera);\n> >> + GstLibcameraAllocator *gst_libcamera_allocator_new(std::shared_ptr camera,\n> >> +\t\t\t\t\t\t libcamera::CameraConfiguration *config_);\n> >> \n> >>  bool gst_libcamera_allocator_prepare_buffer(GstLibcameraAllocator *self,\n> >>                                             libcamera::Stream *stream,\n> >> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n> >> b/src/gstreamer/gstlibcamerasrc.cpp\n> >> index 636c14df..7b13667b 100644\n> >> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> >> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> >> @@ -429,7 +429,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> >>                 return;\n> >>         }\n> >> \n> >> -       self->allocator = gst_libcamera_allocator_new(state->cam_);\n> >> +       self->allocator = gst_libcamera_allocator_new(state->cam_, state->config_.get());\n> >>         if (!self->allocator) {\n> >>                 GST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT,\n> >>                                   (\"Failed to allocate memory\"),","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 3FEFDBD80E\n\tfor ;\n\tSun, 14 Mar 2021 22:35:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A689160850;\n\tSun, 14 Mar 2021 23:35:51 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5EDA3602E3\n\tfor ;\n\tSun, 14 Mar 2021 23:35:50 +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 C4C493E6;\n\tSun, 14 Mar 2021 23:35:49 +0100 (CET)"],"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=\"urp/yyFb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615761350;\n\tbh=KvapulqD8fMwHhm+4jAluw6uzVTLBRzSdiL3AmQfNnY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=urp/yyFblEqDeyPTu5FedJAhFMxGys0PZ/IyfB3nu15P96VqnS2XPP62pfbhwsnM5\n\tABPGVwS6Rq5pu0X5QrsCxWeZTxxEUTJW3SHRIMzgj74p7S7Ylr84Qi+qt1XyRxH7b1\n\tE1KxDo3t5HMXXexgGg6AAHyKnxQAUstZLUDkAYs0=","Date":"Mon, 15 Mar 2021 00:35:14 +0200","From":"Laurent Pinchart ","To":"Dafna Hirschfeld ","Message-ID":"","References":"<20210224171926.31470-1-dafna.hirschfeld@collabora.com>\n\t<1b276ffa607edd42069984268d34ed6308543f66.camel@collabora.com>\n\t<5993a8b5-ced2-7473-02a0-6fa6d48f2e4e@collabora.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<5993a8b5-ced2-7473-02a0-6fa6d48f2e4e@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH] src: gstreamer: use the streams of\n\tCameraConfiguration when allocating buffers","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":"Nicolas Dufresne ,\n\tlibcamera-devel@lists.libcamera.org, kernel@collabora.com","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15684,"web_url":"https://patchwork.libcamera.org/comment/15684/","msgid":"","date":"2021-03-15T00:12:22","subject":"Re: [libcamera-devel] [PATCH] src: gstreamer: use the streams of\n\tCameraConfiguration when allocating buffers","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Dafna,\n\nOn Mon, Mar 15, 2021 at 12:35:15AM +0200, Laurent Pinchart wrote:\n> On Thu, Feb 25, 2021 at 04:41:39PM +0100, Dafna Hirschfeld wrote:\n> > On 24.02.21 19:33, Nicolas Dufresne wrote:\n> > > Le mercredi 24 février 2021 à 18:19 +0100, Dafna Hirschfeld a écrit :\n> > >> Currently, when allocating buffers, the streams of\n> > >> the Camera object are used. Instead the streams of\n> > >> the CameraConfiguration object should be used.\n> > > \n> > > The changes looks fine, but perhaps you could extend, I wrote this, still I\n> > > don't understand why enumerating the streams in the configuration would make any\n> > > difference after the camera has been configured.\n> > \n> > The Camera holds a list of all available streams.\n> > For rkisp1 those are mainpath and selfpath.\n> > Then only subset of those streams are chosen which are\n> > the stream kept in the CameraConfiguration object.\n> > Specifically using gstreamer with rkisp1 I got:\n> > \n> > [3:59:32.003159780] [2499] ERROR Allocator framebuffer_allocator.cpp:97 Stream is not part of /base/i2c@ff3d0000/sensor@10 active configuration\n> > \n> > > Minor comment, other GStreamer patches commit message was simply \"gst:\n> > > Description\", just mentioning for consistency, but perhaps there is new rules.\n> > \n> > Ill send v2,\n> \n> Do you still plan to send a v2 ?\n\nI see you've sent it already, sorry about that.\n\n> > > Reviewed-by: Nicolas Dufresne \n> > >>\n> > >> Signed-off-by: Dafna Hirschfeld \n> > >> ---\n> > >>  src/gstreamer/gstlibcameraallocator.cpp | 6 ++++--\n> > >>  src/gstreamer/gstlibcameraallocator.h   | 4 +++-\n> > >>  src/gstreamer/gstlibcamerasrc.cpp       | 2 +-\n> > >>  3 files changed, 8 insertions(+), 4 deletions(-)\n> > >>\n> > >> diff --git a/src/gstreamer/gstlibcameraallocator.cpp\n> > >> b/src/gstreamer/gstlibcameraallocator.cpp\n> > >> index 13c6b493..7bd8ba2d 100644\n> > >> --- a/src/gstreamer/gstlibcameraallocator.cpp\n> > >> +++ b/src/gstreamer/gstlibcameraallocator.cpp\n> > >> @@ -183,13 +183,15 @@ gst_libcamera_allocator_class_init(GstLibcameraAllocatorClass *klass)\n> > >>  }\n> > >> \n> > >>  GstLibcameraAllocator *\n> > >> -gst_libcamera_allocator_new(std::shared_ptr camera)\n> > >> +gst_libcamera_allocator_new(std::shared_ptr camera,\n> > >> +                           CameraConfiguration *config_)\n> > >>  {\n> > >>         auto *self = GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR,\n> > >>                                                           nullptr));\n> > >> \n> > >>         self->fb_allocator = new FrameBufferAllocator(camera);\n> > >> -       for (Stream *stream : camera->streams()) {\n> > >> +       for (StreamConfiguration &streamCfg : *config_) {\n> > >> +               Stream *stream = streamCfg.stream();\n> > >>                 gint ret;\n> > >> \n> > >>                 ret = self->fb_allocator->allocate(stream);\n> > >> diff --git a/src/gstreamer/gstlibcameraallocator.h\n> > >> b/src/gstreamer/gstlibcameraallocator.h\n> > >> index befdcad6..f7fa6acd 100644\n> > >> --- a/src/gstreamer/gstlibcameraallocator.h\n> > >> +++ b/src/gstreamer/gstlibcameraallocator.h\n> > >> @@ -13,12 +13,14 @@\n> > >>  #include \n> > >> \n> > >>  #include \n> > >> +#include \n> \n> Alphabetical order please :-)\n> \n> With this addressed, and an updated commit message,\n> \n> Reviewed-by: Laurent Pinchart \n> \n> > >> \n> > >>  #define GST_TYPE_LIBCAMERA_ALLOCATOR gst_libcamera_allocator_get_type()\n> > >>  G_DECLARE_FINAL_TYPE(GstLibcameraAllocator, gst_libcamera_allocator,\n> > >>                      GST_LIBCAMERA, ALLOCATOR, GstDmaBufAllocator)\n> > >> \n> > >> -GstLibcameraAllocator *gst_libcamera_allocator_new(std::shared_ptr camera);\n> > >> + GstLibcameraAllocator *gst_libcamera_allocator_new(std::shared_ptr camera,\n> > >> +\t\t\t\t\t\t libcamera::CameraConfiguration *config_);\n> > >> \n> > >>  bool gst_libcamera_allocator_prepare_buffer(GstLibcameraAllocator *self,\n> > >>                                             libcamera::Stream *stream,\n> > >> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n> > >> b/src/gstreamer/gstlibcamerasrc.cpp\n> > >> index 636c14df..7b13667b 100644\n> > >> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > >> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > >> @@ -429,7 +429,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> > >>                 return;\n> > >>         }\n> > >> \n> > >> -       self->allocator = gst_libcamera_allocator_new(state->cam_);\n> > >> +       self->allocator = gst_libcamera_allocator_new(state->cam_, state->config_.get());\n> > >>         if (!self->allocator) {\n> > >>                 GST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT,\n> > >>                                   (\"Failed to allocate memory\"),","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 B6A5BBD80E\n\tfor ;\n\tMon, 15 Mar 2021 00:12:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 305936084E;\n\tMon, 15 Mar 2021 01:12:59 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 71C80602E0\n\tfor ;\n\tMon, 15 Mar 2021 01:12:58 +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 969C7908;\n\tMon, 15 Mar 2021 01:12:57 +0100 (CET)"],"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=\"NM5fDXzt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615767177;\n\tbh=1VKRNxaDXCqHD3H1IXCEOpPSaHsrq3VHpO0QTWkdv10=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NM5fDXztboX53DLXgclVOjzbNkRqf3Hj7FXax36zv332r5hzsqqfmSI7dmcNb0UPG\n\tn1kaAK/Vqbi/p8cXi60wijKeTn875Rhoky2n9Gd4PWegWdfU4zGZqqFJSOuMgUhKGB\n\taxiXpPpHiXBJvaDrRmdALJ1TL9gHXHeHHp11Cvkc=","Date":"Mon, 15 Mar 2021 02:12:22 +0200","From":"Laurent Pinchart ","To":"Dafna Hirschfeld ","Message-ID":"","References":"<20210224171926.31470-1-dafna.hirschfeld@collabora.com>\n\t<1b276ffa607edd42069984268d34ed6308543f66.camel@collabora.com>\n\t<5993a8b5-ced2-7473-02a0-6fa6d48f2e4e@collabora.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] src: gstreamer: use the streams of\n\tCameraConfiguration when allocating buffers","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":"Nicolas Dufresne ,\n\tlibcamera-devel@lists.libcamera.org, kernel@collabora.com","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]