[{"id":24970,"web_url":"https://patchwork.libcamera.org/comment/24970/","msgid":"<52bef396-39a4-d542-d478-58d510ea3540@ideasonboard.com>","date":"2022-09-12T10:24:18","subject":"Re: [libcamera-devel] [PATCH v4 1/2] gstreamer: Configure the\n\tcamera before exposing the caps.","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Rishi,\n\nOn 9/12/22 3:26 PM, Rishikesh Donadkar wrote:\n> In libcamerasrc capabilities are exposed on the src pad before\n> configuring the camera. To add support to control and\n> negotiate the framerate, the controls::FrameDuration needs to be\n> bound checked between the min/max values that camera can support\n> and later the applied framerate needs to be exposed along with\n> resolutions and colorimetry through caps. Valid bounds of the\n> controls::FrameDuration cannot be known before the\n> configuration of the camera.\n>\n> Shift the camera::configure() before the for loop that is exposing\n> resolutions, colorimetry to the GStreamer through a new CAPS event.\n> Through this we can know the valid bounds of the FrameDuration and\n> clam the frame_duration accordingly before applying to the camera.\n> Through this caps can be exposed without a need of additional new\n> CAPS event for framerate.\n\nAs we introduce framerate only in 2/2, hence, we should _not_ construct \na framerate centric commit message.\n\nYou can however mention it briefly with stating that it makes sense to \nconfigure the camera first(before exposing) as  the stream configuration \nand controls values are available, required to expose the new caps. \nFramerate is one such example.\n\nWould you be able to re-frame the commit message again based on above ?\n\n> Signed-off-by: Rishikesh Donadkar \n> ---\n> src/gstreamer/gstlibcamerasrc.cpp | 18 +++++++++---------\n> 1 file changed, 9 insertions(+), 9 deletions(-)\n>\n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 16d70fea..1ee1d08a 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -515,6 +515,15 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> \t\tgoto done;\n> \t}\n> \n> +\tret = state->cam_->configure(state->config_.get());\n> +\tif (ret) {\n> +\t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> +\t\t\t\t (\"Failed to configure camera: %s\", g_strerror(-ret)),\n> +\t\t\t\t (\"Camera::configure() failed with error code %i\", ret));\n> +\t\tgst_task_stop(task);\n> +\t\treturn;\n\nI think what you need here is:\n\n     flow_ret = GST_FLOW_NOT_NEGOTIATED;\n     goto done;\n\ninstead of return; similar to what happens when stream configuration is \nnot found to be valid (just above)\n\n> +\t}\n> +\n> \t/*\n> \t * Regardless if it has been modified, create clean caps and push the\n> \t * caps event. Downstream will decide if the caps are acceptable.\n> @@ -535,15 +544,6 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> \t\tgst_pad_push_event(srcpad, gst_event_new_segment(&segment));\n> \t}\n> \n> -\tret = state->cam_->configure(state->config_.get());\n> -\tif (ret) {\n> -\t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> -\t\t\t\t (\"Failed to configure camera: %s\", g_strerror(-ret)),\n> -\t\t\t\t (\"Camera::configure() failed with error code %i\", ret));\n> -\t\tgst_task_stop(task);\n> -\t\treturn;\n> -\t}\n> -\n> \tself->allocator = gst_libcamera_allocator_new(state->cam_, state->config_.get());\n> \tif (!self->allocator) {\n> \t\tGST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT,","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 7E1E9C0DA4\n\tfor ;\n\tMon, 12 Sep 2022 10:24:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C1E8161F8F;\n\tMon, 12 Sep 2022 12:24:27 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 59C44609A6\n\tfor ;\n\tMon, 12 Sep 2022 12:24:26 +0200 (CEST)","from [IPV6:2401:4900:1f3e:50f3:a89c:93ec:bcd7:8b90] (unknown\n\t[IPv6:2401:4900:1f3e:50f3:a89c:93ec:bcd7:8b90])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8E44659D;\n\tMon, 12 Sep 2022 12:24:24 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1662978267;\n\tbh=793FFyXuey8j/D1XmJQQrH/zLPxiuw/t1c1/1CiluuA=;\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=3+NrrV8Jo26VKcv0W8mtTHm5xaKukPNfszQo9k/RfdTrgSKFQIVLfLWnE0V8Hyot/\n\tsGfcJDB9VDAYvmKsKteV/DfiD6etB7JZ69Duq8BPnuXS4j+rkTgDJdQlMl5b+krgpV\n\tubpM7fbavF8HeQieW7qTKNgFylkQTlCQ/f5JpcpKm6la6LdHH3UI3qP2a8GTZSl4X7\n\tq+A5ciLvVSdsUho2OYqp0H3sceLzCEACdXcKT1YHRbbxNHUdcy97VAHY1/haVTUY2K\n\tjrsnCvbLY5iFJLxH/0BOeqYViRL3YCzD0GeMntFwAxRkQ//V+FWxiU70BcRGpn3sQc\n\toUOgahCK1MmlA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1662978265;\n\tbh=793FFyXuey8j/D1XmJQQrH/zLPxiuw/t1c1/1CiluuA=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=YAs4HgTGmKMFwKH+cxe81H5rjFaURu9K5NfHHJ6JgM/3W5KuTZDxTWFtSNupkQ9LC\n\tmjab9oNH3C12hA8KBFIc2QzHaSMPLWMAwEkNqfflYH9OKyXSxjWhnP648AU2TRezsT\n\tHihpCkAAy6RyIPGJRYuiUGVB1bdMwVWR9kDlk8GQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"YAs4HgTG\"; dkim-atps=neutral","Message-ID":"<52bef396-39a4-d542-d478-58d510ea3540@ideasonboard.com>","Date":"Mon, 12 Sep 2022 15:54:18 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.12.0","Content-Language":"en-US","To":"Rishikesh Donadkar ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220912095656.19013-1-rishikeshdonadkar@gmail.com>\n\t<20220912095656.19013-2-rishikeshdonadkar@gmail.com>","In-Reply-To":"<20220912095656.19013-2-rishikeshdonadkar@gmail.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v4 1/2] gstreamer: Configure the\n\tcamera before exposing the caps.","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":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Cc":"nicolas.dufresne@collabora.com, vedantparanjape160201@gmail.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24972,"web_url":"https://patchwork.libcamera.org/comment/24972/","msgid":"","date":"2022-09-12T12:25:58","subject":"Re: [libcamera-devel] [PATCH v4 1/2] gstreamer: Configure the\n\tcamera before exposing the caps.","submitter":{"id":118,"url":"https://patchwork.libcamera.org/api/people/118/","name":"Rishikesh Donadkar","email":"rishikeshdonadkar@gmail.com"},"content":"> As we introduce framerate only in 2/2, hence, we should _not_ construct\n> a framerate centric commit message.\n>\n> You can however mention it briefly with stating that it makes sense to\n> configure the camera first(before exposing) as the stream configuration\n> and controls values are available, required to expose the new caps.\n> Framerate is one such example.\n>\n> Would you be able to re-frame the commit message again based on above ?\nYes\n>\n> I think what you need here is:\n>\n> flow_ret = GST_FLOW_NOT_NEGOTIATED;\n> goto done;\n>\n> instead of return; similar to what happens when stream configuration is\n> not found to be valid (just above)\nNoted.\n\n\nOn Mon, Sep 12, 2022 at 3:54 PM Umang Jain wrote:\n>\n> Hi Rishi,\n>\n> On 9/12/22 3:26 PM, Rishikesh Donadkar wrote:\n> > In libcamerasrc capabilities are exposed on the src pad before\n> > configuring the camera. To add support to control and\n> > negotiate the framerate, the controls::FrameDuration needs to be\n> > bound checked between the min/max values that camera can support\n> > and later the applied framerate needs to be exposed along with\n> > resolutions and colorimetry through caps. Valid bounds of the\n> > controls::FrameDuration cannot be known before the\n> > configuration of the camera.\n> >\n> > Shift the camera::configure() before the for loop that is exposing\n> > resolutions, colorimetry to the GStreamer through a new CAPS event.\n> > Through this we can know the valid bounds of the FrameDuration and\n> > clam the frame_duration accordingly before applying to the camera.\n> > Through this caps can be exposed without a need of additional new\n> > CAPS event for framerate.\n>\n> As we introduce framerate only in 2/2, hence, we should _not_ construct\n> a framerate centric commit message.\n>\n> You can however mention it briefly with stating that it makes sense to\n> configure the camera first(before exposing) as the stream configuration\n> and controls values are available, required to expose the new caps.\n> Framerate is one such example.\n>\n> Would you be able to re-frame the commit message again based on above ?\n>\n> > Signed-off-by: Rishikesh Donadkar \n> > ---\n> > src/gstreamer/gstlibcamerasrc.cpp | 18 +++++++++---------\n> > 1 file changed, 9 insertions(+), 9 deletions(-)\n> >\n> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > index 16d70fea..1ee1d08a 100644\n> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > @@ -515,6 +515,15 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> > goto done;\n> > }\n> >\n> > + ret = state->cam_->configure(state->config_.get());\n> > + if (ret) {\n> > + GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> > + (\"Failed to configure camera: %s\", g_strerror(-ret)),\n> > + (\"Camera::configure() failed with error code %i\", ret));\n> > + gst_task_stop(task);\n> > + return;\n>\n> I think what you need here is:\n>\n> flow_ret = GST_FLOW_NOT_NEGOTIATED;\n> goto done;\n>\n> instead of return; similar to what happens when stream configuration is\n> not found to be valid (just above)\n>\n> > + }\n> > +\n> > /*\n> > * Regardless if it has been modified, create clean caps and push the\n> > * caps event. Downstream will decide if the caps are acceptable.\n> > @@ -535,15 +544,6 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> > gst_pad_push_event(srcpad, gst_event_new_segment(&segment));\n> > }\n> >\n> > - ret = state->cam_->configure(state->config_.get());\n> > - if (ret) {\n> > - GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> > - (\"Failed to configure camera: %s\", g_strerror(-ret)),\n> > - (\"Camera::configure() failed with error code %i\", ret));\n> > - gst_task_stop(task);\n> > - return;\n> > - }\n> > -\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>","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 689E9C0DA4\n\tfor ;\n\tMon, 12 Sep 2022 12:26:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9E62061F91;\n\tMon, 12 Sep 2022 14:26:12 +0200 (CEST)","from mail-vs1-xe2b.google.com (mail-vs1-xe2b.google.com\n\t[IPv6:2607:f8b0:4864:20::e2b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 05BCF609A6\n\tfor ;\n\tMon, 12 Sep 2022 14:26:11 +0200 (CEST)","by mail-vs1-xe2b.google.com with SMTP id m65so8802823vsc.1\n\tfor ;\n\tMon, 12 Sep 2022 05:26:10 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1662985572;\n\tbh=7OF8yXj8B/cOD2ppE8tvaSwxmeAqmUuzb8HVHFZ4cQ4=;\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=XMWjr3aTUpXhByGySYzs/dAHHH+JqmRI+V37aIsKg7jpHVzulhEAEDp+xiLOvRf1P\n\txlarqyFXoVueOa/caaf9uQHkcXxEgGX5VNDqoB4vEo8RIiwkqt8T9nv0WSX2xWoNko\n\tRGBIfSJVgGg3gd2uA9GIQKXMfXbQj0sLctHBwlHmpxC2LmV+yi4rEoOJwb0le7A/7s\n\tiSXBHJaQp5BlMUIBBVATutUtmdWypTamK6gTc70qzdPlaJ9QgkLNaaaZ1sYVbtWUe+\n\t0kMlcfMeq+ZXwu89J7YA5Pgm1KujyIJvaPfxee53aqCgYnSHVczp9l7Ow3wWXQVBGN\n\tq9+yP+H69kvhQ==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date;\n\tbh=FtYBTb1kF0Je8/jvCs6ei98T4WPRSC7Bi7Y/DebC60U=;\n\tb=C+Zb1kw6P9fLTQOkfIeLUzM1WaFonp6eVctJ78PGs+4rufz3Kk7Vrg16W1BzmzpKKa\n\tEfUZ2raimmOeXtygN070NDaqSiNNZCyJM1ZOCWCFVyN4odfra0aNJsCygpMsmjn4Dz2K\n\tY2/BSTotX9GqpiLNa9WCp4uvYyP2hSCtBpga6otKgEzEWlgKhsobVXqSkmVvJb/ztzJW\n\tgm8fN/D9FUXTv0egOOz/hwrWjEyIOSFhxRpqGUtrzhslO1b4l/8TrT3LUYK/IMqPIik1\n\tJn/ILgi8cAMpj7pZHATXFnZbu1acaisSTsUY8VpJf+qX2xkfrlETwXsEFFxuBPOTasYN\n\tzanA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"C+Zb1kw6\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date;\n\tbh=FtYBTb1kF0Je8/jvCs6ei98T4WPRSC7Bi7Y/DebC60U=;\n\tb=zSjUso7p5ekBm5yBV1h3hg29LoBsrQ7jO1URvhfxYJALyj/7Gsuto/STanXGc4w6pX\n\tLQglrql7oQjbQyhJuJRBxqHA6JEHt6qDh9tzeSOS3hMurVeRxHpQIZ7ji+RmmBGGYBcy\n\tFKW+hzj9sZ+KOXGg2jn3eLY667tN2ucdRahc4NjaGwZmGAmvXvtuzbvWB2kf8lfIK/CK\n\t1X0ILxJg8YJsGL/RaYp6K5X+YDIovZTXqR6142F7LitAyS6uNwNolxnOTU6oW4fSL+By\n\tg8MLeWyXzmM9u/UO4siGnvbehcTP6KJlVM+vs5qdqx6c3h/bU2+gk8LZpMvHYJPOZf8J\n\tREYQ==","X-Gm-Message-State":"ACgBeo0oTgfQ/RLtiA8YI266ZNhDfiw3aiZRn7yBJrUGMYvWLfuZKtdp\n\t43baqEuNmGCWGuODNZ/r/X1e5VyV4NCtxbrzxWo=","X-Google-Smtp-Source":"AA6agR41oOPh5pFFs0sY7eJSZeg8yb472kkT4zM/irKRs+YyjNJ/d+IX5wxQHN1grGQzR3y9qCwrLrGDbGXP1PsJYpE=","X-Received":"by 2002:a05:6102:14b:b0:398:2e7c:4780 with SMTP id\n\ta11-20020a056102014b00b003982e7c4780mr6514492vsr.72.1662985569786;\n\tMon, 12 Sep 2022 05:26:09 -0700 (PDT)","MIME-Version":"1.0","References":"<20220912095656.19013-1-rishikeshdonadkar@gmail.com>\n\t<20220912095656.19013-2-rishikeshdonadkar@gmail.com>\n\t<52bef396-39a4-d542-d478-58d510ea3540@ideasonboard.com>","In-Reply-To":"<52bef396-39a4-d542-d478-58d510ea3540@ideasonboard.com>","Date":"Mon, 12 Sep 2022 17:55:58 +0530","Message-ID":"","To":"Umang Jain ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v4 1/2] gstreamer: Configure the\n\tcamera before exposing the caps.","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":"Rishikesh Donadkar via libcamera-devel\n\t","Reply-To":"Rishikesh Donadkar ","Cc":"libcamera-devel@lists.libcamera.org, vedantparanjape160201@gmail.com,\n\tnicolas.dufresne@collabora.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]