[{"id":30604,"web_url":"https://patchwork.libcamera.org/comment/30604/","msgid":"<948ca6cce07ac2ce09e20ea00ad402ae097a6a88.camel@ndufresne.ca>","date":"2024-08-05T21:02:04","subject":"Re: [PATCH 0/3] gstreamer: Generate controls from\n\tcontrol_ids_*.yaml files","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Hi,\n\nLe lundi 05 août 2024 à 11:28 +0200, Jaslo Ziska a écrit :\n> Hi everyone,\n> \n> in this patchset I implemented libcamerasrc properties which are generated from\n> the control_ids_*.yaml files.\n\nI'm very happy to see that someone found that time to work on this.\n\n> \n> The first commit removes the auto-focus-mode property which was already\n> implemented (it is added again in the next commit).\n> The second commit adds a Python script to generate C++ code capable of\n> interacting with the controls and adds the controls to libcamerasrc.\n> The third commit is just a small fix for the missing closing \"greater than\"\n> symbol in the author string I noticed.\n> \n> The gstlibcamera-controls.h file is taken from Nicolas' branch with the change\n> that I removed the enum with the control names from the class. Instead the enum\n> variants from libcamera::controls:: are now used directly.\n> The structure of the gstlibcamera-controls.cpp.in file is also taken from\n> Nicolas. The only change is that its content now gets generated by\n> gen-gst-controls.py\n> \n> The boilerplate of gen-gst-controls.py is mostly copied from gen-controls.py.\n> This is also where I have some questions:\n> \n> The definition of the ControlEnum and Control Python classes (and some helper\n> functions) is now duplicate code. Should this be handled differently somehow to\n> avoid the code duplication?\n> \n> I haven't added a copyright to gen-gst-controls.py yet as I am not sure because\n> I copied much of it from gen-controls.py.\n\nHaven't checked, but I would probably have copied the copyright from original\nand added mine under.\n\n> \n> Another small issue is that I haven't implemented the Rectangle type yet and\n> thus the controls using it won't show up. The reason for this is that there\n> is the AfWindows control which is an array of Rectangles. As the gstreamer\n> properties can only be glib types I wasn't sure what to do here: For a\n> single Rectangle you could use an array and make the entries (x, y, w, h)\n> but what about an array of Rectangles? Should it use an array with 4 * N\n> entries, so (x, y, w, h) for each value?\n\nThe type GstValueArray can nest other GValue types (was needed for\nGstValueFraction). So in theory you can have an array of of N rectangle array\n(size 4). In serialized form it would look like:\n\n  property=\"< <x1, y1, w1, h1>, <x2, y2, w2, h2>, ...>\"\n\nThe C API that let you assemble/deassemble this is pretty tedious though. We\nusually prefer not to always go through the de-serializer of course. If you look\naround, most of the time these are split in multiple properties, but it also\nmeans that your control can be inconsistent while its being set, so pretty\ndifficult to set at run-time.\n\nSo even though complex, I think its a fair approach since the only other option\nis to use a custom serialization/deserialzation and a string type property.\n\n> \n> At the moment the gstreamer properties all have zero (or the first enum value)\n> as a default and the minimum and maximum numeric values as minimum and maximum\n> values for numbers. Also all controls are defined as readable and writeable.\n> Because of this (maybe as a discussion) I have a small wish list of things I'd\n> like to see added in the control_ids_*.yaml files which would greatly improve\n> the gstreamer properties:\n> For enum and bool controls: the default value if available.\n\nWhile in GStreamer, we often see the implementation differ from the default\nafter moving the element to READY state (so we could just read it back later), I\nlike the implied consistency that having default (were that actually make sense)\nwould bring. I had the same impression where I first looked at it.\n\n> For numeric controls: the minimum, maximum and default value.\n> And for all controls: whether it is read-only, write-only or read-write.\n> \n> Best regards,\n> \n> Jaslo\n> \n> Jaslo Ziska (3):\n>   gstreamer: Remove auto-focus-mode property\n>   gstreamer: Generate controls from control_ids_*.yaml files\n>   gstreamer: Fix missing \"greater than\" symbol in author string\n\nI will have a look this week, thanks for working on this.\n\nNicolas\n\n> \n>  src/gstreamer/gstlibcamera-controls.cpp.in |  46 +++\n>  src/gstreamer/gstlibcamera-controls.h      |  36 ++\n>  src/gstreamer/gstlibcamerasrc.cpp          |  47 +--\n>  src/gstreamer/meson.build                  |  14 +\n>  utils/gen-gst-controls.py                  | 398 +++++++++++++++++++++\n>  utils/meson.build                          |   1 +\n>  6 files changed, 510 insertions(+), 32 deletions(-)\n>  create mode 100644 src/gstreamer/gstlibcamera-controls.cpp.in\n>  create mode 100644 src/gstreamer/gstlibcamera-controls.h\n>  create mode 100755 utils/gen-gst-controls.py\n> \n> --\n> 2.46.0","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 6EA65C323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  5 Aug 2024 21:02:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E378363381;\n\tMon,  5 Aug 2024 23:02:08 +0200 (CEST)","from mail-ot1-x335.google.com (mail-ot1-x335.google.com\n\t[IPv6:2607:f8b0:4864:20::335])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C8DC76195D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  5 Aug 2024 23:02:06 +0200 (CEST)","by mail-ot1-x335.google.com with SMTP id\n\t46e09a7af769-7093efbade6so37966a34.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 05 Aug 2024 14:02:06 -0700 (PDT)","from nicolas-tpx395.lan ([2606:6d00:15:820c::580])\n\tby smtp.gmail.com with ESMTPSA id\n\t6a1803df08f44-6bb9c83c771sm39115376d6.78.2024.08.05.14.02.04\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 05 Aug 2024 14:02:04 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ndufresne-ca.20230601.gappssmtp.com\n\theader.i=@ndufresne-ca.20230601.gappssmtp.com\n\theader.b=\"IHDKaHAW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20230601.gappssmtp.com; s=20230601; t=1722891725;\n\tx=1723496525; darn=lists.libcamera.org; \n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:to:from:subject:message-id:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=vmMfKsOxfloRJF6eIY4JYVgPNxbg3Gz0EAjGztjKg44=;\n\tb=IHDKaHAWJjdzTF/cdIQGcwArAjRDb/ZpQtUShpusoyBQKDZjyysdueMCbv4rZ3CM1B\n\tXdRuEjytc993QdgIjzQeGjheL2y58VtqCYbvNMmoRwRHvxnJi6Ynf45wTDY6C+SwZtx3\n\t4f1HpPmVe9bw0c0nZQN19yjeaO6cZIGi7uXZlL32+ql1KgKvyPg6oX4RE/c7wwoY9tU7\n\tjH0XBPNvxjeZR+wL+8QIaeGAimqoZ08TlMEUsRoCASe2X/BncJqLTrFDxgJH8yIRcr6x\n\tsiyzbOXmwYQjlGG4vZXJPJ0qrxIRwbdEoO3rBtBduNxs/nOBney7t6m1ywEAVvmdKo6P\n\tNPbQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1722891725; x=1723496525;\n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:to:from:subject:message-id:x-gm-message-state:from\n\t:to:cc:subject:date:message-id:reply-to;\n\tbh=vmMfKsOxfloRJF6eIY4JYVgPNxbg3Gz0EAjGztjKg44=;\n\tb=rWlUaGTEP3ora6AS3fgpD8zokQ51mI2nnh77SuQTN505m/pxDK00K3NqHLQVGDnC3E\n\tHrj/wZCzg/KrhA96o4rLU/asmceJJ1EXnW/Wn76OkoUnHhnp2Ir1iXopdO5/nmS2tCo2\n\tHE4g+3V6Uut9lLUpJGxD0Bv5ZTf806w2EunDuCGGbV53EJ6EbhHF6v5CfPL+RiJw/LcA\n\tZtEzsRZeW8wIe2joFTJ4nJddIiOcVgwQeAIU57tsJgaSUSmmF2qasYZBxaFbmjpLEe0S\n\t+udcuN47AndeNdiovhnBIRnBV+hK8FBzAif+45tvq7UZKqSz//HF0b0hYEElvDlTTMDj\n\txqfw==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCUwua+eXuNcvR61HnN1r+jhfk3P5J35Pv4Mq7HgeG90eSbg/Ujjy6t5iVCX07yeXmBYftyO+to+I8Tquj0oxhfUWiGzA5z2ghJgjs63ep1D+GSrcw==","X-Gm-Message-State":"AOJu0YxB7pW84wJ3rf8ZznosHD8ec179HwCuv41+/gZ6m6QokPG7ukGw\n\t2EM7yoRvJ6FuMOga9rUtky5JFA7+jLD3leUrKaxmOH9pXBIJJZHa6NDPD9B2eMBv6+cJWqVjw/e\n\to","X-Google-Smtp-Source":"AGHT+IGYkTgkFOR2pd0fATP6xtkjk0kZonBQCEb+I+e24aBKXw0HT2lbZGUB2RICT+U+kwbqDcdshQ==","X-Received":"by 2002:a05:6830:7303:b0:704:482e:216a with SMTP id\n\t46e09a7af769-709b3230244mr16194435a34.14.1722891725002; \n\tMon, 05 Aug 2024 14:02:05 -0700 (PDT)","Message-ID":"<948ca6cce07ac2ce09e20ea00ad402ae097a6a88.camel@ndufresne.ca>","Subject":"Re: [PATCH 0/3] gstreamer: Generate controls from\n\tcontrol_ids_*.yaml files","From":"Nicolas Dufresne <nicolas@ndufresne.ca>","To":"Jaslo Ziska <jaslo@ziska.de>, libcamera-devel@lists.libcamera.org","Date":"Mon, 05 Aug 2024 17:02:04 -0400","In-Reply-To":"<20240805100038.11972-1-jaslo@ziska.de>","References":"<20240805100038.11972-1-jaslo@ziska.de>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.52.3 (3.52.3-1.fc40) ","MIME-Version":"1.0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30632,"web_url":"https://patchwork.libcamera.org/comment/30632/","msgid":"<877cctd7my.fsf@ziska.de>","date":"2024-08-06T14:54:13","subject":"Re: [PATCH 0/3] gstreamer: Generate controls from\n\tcontrol_ids_*.yaml files","submitter":{"id":173,"url":"https://patchwork.libcamera.org/api/people/173/","name":"Jaslo Ziska","email":"jaslo@ziska.de"},"content":"Jaslo Ziska <jaslo@ziska.de> writes:\n> Hi everyone,\n>\n> in this patchset I implemented libcamerasrc properties which are \n> generated from\n> the control_ids_*.yaml files.\n>\n> The first commit removes the auto-focus-mode property which was \n> already\n> implemented (it is added again in the next commit).\n> The second commit adds a Python script to generate C++ code \n> capable of\n> interacting with the controls and adds the controls to \n> libcamerasrc.\n> The third commit is just a small fix for the missing closing \n> \"greater than\"\n> symbol in the author string I noticed.\n>\n> The gstlibcamera-controls.h file is taken from Nicolas' branch \n> with the change\n> that I removed the enum with the control names from the class. \n> Instead the enum\n> variants from libcamera::controls:: are now used directly.\n> The structure of the gstlibcamera-controls.cpp.in file is also \n> taken from\n> Nicolas. The only change is that its content now gets generated \n> by\n> gen-gst-controls.py\nI just noticed that it is not yet implemented to get the control \nvalues returned by libcamera::Request::metadata() (at the moment \nyou can only read back the controls which were already set). I \nwill add that in the next revision.\n\n>\n> The boilerplate of gen-gst-controls.py is mostly copied from \n> gen-controls.py.\n> This is also where I have some questions:\n>\n> The definition of the ControlEnum and Control Python classes \n> (and some helper\n> functions) is now duplicate code. Should this be handled \n> differently somehow to\n> avoid the code duplication?\n>\n> I haven't added a copyright to gen-gst-controls.py yet as I am \n> not sure because\n> I copied much of it from gen-controls.py.\n>\n> Another small issue is that I haven't implemented the Rectangle \n> type yet and\n> thus the controls using it won't show up. The reason for this is \n> that there\n> is the AfWindows control which is an array of Rectangles. As the \n> gstreamer\n> properties can only be glib types I wasn't sure what to do here: \n> For a\n> single Rectangle you could use an array and make the entries (x, \n> y, w, h)\n> but what about an array of Rectangles? Should it use an array \n> with 4 * N\n> entries, so (x, y, w, h) for each value?\n>\n> At the moment the gstreamer properties all have zero (or the \n> first enum value)\n> as a default and the minimum and maximum numeric values as \n> minimum and maximum\n> values for numbers. Also all controls are defined as readable \n> and writeable.\n> Because of this (maybe as a discussion) I have a small wish list \n> of things I'd\n> like to see added in the control_ids_*.yaml files which would \n> greatly improve\n> the gstreamer properties:\n> For enum and bool controls: the default value if available.\n> For numeric controls: the minimum, maximum and default value.\n> And for all controls: whether it is read-only, write-only or \n> read-write.\n>\n> Best regards,\n>\n> Jaslo\n>\n> Jaslo Ziska (3):\n>   gstreamer: Remove auto-focus-mode property\n>   gstreamer: Generate controls from control_ids_*.yaml files\n>   gstreamer: Fix missing \"greater than\" symbol in author string\n>\n>  src/gstreamer/gstlibcamera-controls.cpp.in |  46 +++\n>  src/gstreamer/gstlibcamera-controls.h      |  36 ++\n>  src/gstreamer/gstlibcamerasrc.cpp          |  47 +--\n>  src/gstreamer/meson.build                  |  14 +\n>  utils/gen-gst-controls.py                  | 398 \n>  +++++++++++++++++++++\n>  utils/meson.build                          |   1 +\n>  6 files changed, 510 insertions(+), 32 deletions(-)\n>  create mode 100644 src/gstreamer/gstlibcamera-controls.cpp.in\n>  create mode 100644 src/gstreamer/gstlibcamera-controls.h\n>  create mode 100755 utils/gen-gst-controls.py","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 AB358BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  6 Aug 2024 14:54:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CC7FA63393;\n\tTue,  6 Aug 2024 16:54:16 +0200 (CEST)","from mo4-p00-ob.smtp.rzone.de (mo4-p00-ob.smtp.rzone.de\n\t[81.169.146.162])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DCD086195D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  6 Aug 2024 16:54:14 +0200 (CEST)","from archlinux by smtp.strato.de (RZmta 51.1.0 AUTH)\n\twith ESMTPSA id zb9f0a076EsE5ff\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits))\n\t(Client did not present a certificate)\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 6 Aug 2024 16:54:14 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ziska.de header.i=@ziska.de header.b=\"n9KEtVJr\";\n\tdkim=permerror (0-bit key) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"i8mqyLZc\"; dkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1722956054; cv=none;\n\td=strato.com; s=strato-dkim-0002;\n\tb=CRS/FCKOH18T+oQ9ia1nvU9p05VKOSghVrr25W21SJb/Hgs8IGN0IrZtkzr388h7Dh\n\t11vcdPf7i6DWml1KIfriBrXwkYkZhYbp0eP1g/l5V6yuuPBbTltKS1rvMjF8jJjtQrvL\n\t9xiyeOpcJ1r1b/9WvWml7LlEaD3rGuYVf3qIush09ceE1spwJZ7RexRwOSr9TnLKutxj\n\tG8b92LsLdTB964BFH17EEd9t90H1PgtgUE4a6erAVBgKt8LNJ6HiLQJCR73RdT54kv35\n\tb50Fp+AXJZXwPP4L+/B6iQ2psmnpkWRutqReZcVLV8h5vtN6b6eg7f5HuaP3A0ZdIl6q\n\tE2qw==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; t=1722956054;\n\ts=strato-dkim-0002; d=strato.com;\n\th=Message-ID:Date:References:In-Reply-To:Subject:To:From:Cc:Date:From:\n\tSubject:Sender;\n\tbh=oVHHyYFe+JbHPQBo7rJ+SMAzvANLfFgD1XPJAqnxQMs=;\n\tb=Y5lc6vYUNQzz3nvHf2WUEjfKhrBwmK5+IRVIxPC1B2k7DENZ9EBlDqQOawsgxFfMpw\n\tLg584E1wueThtmO2zHDgxiazk+ePLDVp1DQ8rMzSp59cBCOw858h+4l+eHCuAJJYEbg5\n\t6IxuaXe6CoolTy34wX3lUgsjg3ZFShXiN0hQ2jvFKcI4BmhmDlaP8iK8CGZqD4wmCZSz\n\t5OLIVs3gGMn+V1g8J6D8C8KZFUsBFDMkZHtwwfpfJkw/tFYG0CWm4g0JyYSCaNKbavvN\n\t/KfLzFSBDW3DBHDDuCmvcJemkM6kl5Y2XWFPQsCcy3ijKLv2atist5D99RVoTCSE3yU0\n\tQSdQ==","ARC-Authentication-Results":"i=1; strato.com;\n    arc=none;\n    dkim=none","X-RZG-CLASS-ID":"mo00","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; t=1722956054;\n\ts=strato-dkim-0002; d=ziska.de;\n\th=Message-ID:Date:References:In-Reply-To:Subject:To:From:Cc:Date:From:\n\tSubject:Sender;\n\tbh=oVHHyYFe+JbHPQBo7rJ+SMAzvANLfFgD1XPJAqnxQMs=;\n\tb=n9KEtVJrNdIhdyvXlW0aPZRfd7dOXM38XyDI8aowSvuZ087k+aligfOt6DEKL2rgIo\n\tn34/8w0wAlpXiuPiNtO1sQqACTsVyydT6+pmGCucZha63YVDhoa35CPrHB+pVK8vwnxr\n\tpXahq7TRyAv1fK4AOZnUlH7dl6IdYRKE8CTWimorg4xAyd+PThYHfCLjopdeSlqk0C7S\n\tGRUG5SAW6cVDnay+V6xTmMag+kRZxF6ThWLgZOVDu5N/0V6qEpRAzVsHyuOHoTStU2JN\n\t764meX0pnw+qsXTVpzjNiPrCMslF2DRUFj9wJmZdgblPBMSEJTi3ahcBSRaIhcMnreaM\n\tcnGA==","v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1722956054;\n\ts=strato-dkim-0003; d=ziska.de;\n\th=Message-ID:Date:References:In-Reply-To:Subject:To:From:Cc:Date:From:\n\tSubject:Sender;\n\tbh=oVHHyYFe+JbHPQBo7rJ+SMAzvANLfFgD1XPJAqnxQMs=;\n\tb=i8mqyLZcEvAfkklWCc8u/xn0ZJKDz6ZAJ4n7ggouAnksScQSs3VaSbBs1/FFrgpsNm\n\ttZl09IchqxN5yukbvoCg=="],"X-RZG-AUTH":"\":Jm0XeU+IYfb0x77LHmrjN5Wlb7TBwusDqIM6Hizy8VdfzvKi4yoFC9cGiIq9APJapdDJIcSpwRa2wtm/dzX/daFv3UHJ\"","From":"Jaslo Ziska <jaslo@ziska.de>","To":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 0/3] gstreamer: Generate controls from\n\tcontrol_ids_*.yaml files","In-Reply-To":"<20240805100038.11972-1-jaslo@ziska.de> (Jaslo Ziska's message\n\tof \"Mon, 5 Aug 2024 11:28:35 +0200\")","References":"<20240805100038.11972-1-jaslo@ziska.de>","Date":"Tue, 06 Aug 2024 16:54:13 +0200","Message-ID":"<877cctd7my.fsf@ziska.de>","MIME-Version":"1.0","Content-Type":"text/plain; format=flowed","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30694,"web_url":"https://patchwork.libcamera.org/comment/30694/","msgid":"<20240807162129.GE8166@pendragon.ideasonboard.com>","date":"2024-08-07T16:21:29","subject":"Re: [PATCH 0/3] gstreamer: Generate controls from control_ids_*.yaml\n\tfiles","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Aug 05, 2024 at 05:02:04PM -0400, Nicolas Dufresne wrote:\n> Le lundi 05 août 2024 à 11:28 +0200, Jaslo Ziska a écrit :\n> > Hi everyone,\n> > \n> > in this patchset I implemented libcamerasrc properties which are generated from\n> > the control_ids_*.yaml files.\n> \n> I'm very happy to see that someone found that time to work on this.\n\nLikewise :-)\n\n> > The first commit removes the auto-focus-mode property which was already\n> > implemented (it is added again in the next commit).\n> > The second commit adds a Python script to generate C++ code capable of\n> > interacting with the controls and adds the controls to libcamerasrc.\n> > The third commit is just a small fix for the missing closing \"greater than\"\n> > symbol in the author string I noticed.\n> > \n> > The gstlibcamera-controls.h file is taken from Nicolas' branch with the change\n> > that I removed the enum with the control names from the class. Instead the enum\n> > variants from libcamera::controls:: are now used directly.\n> > The structure of the gstlibcamera-controls.cpp.in file is also taken from\n> > Nicolas. The only change is that its content now gets generated by\n> > gen-gst-controls.py\n> > \n> > The boilerplate of gen-gst-controls.py is mostly copied from gen-controls.py.\n> > This is also where I have some questions:\n> > \n> > The definition of the ControlEnum and Control Python classes (and some helper\n> > functions) is now duplicate code. Should this be handled differently somehow to\n> > avoid the code duplication?\n\nI would be nice to avoid code duplication if possible, yes. I'll review\nthe patch and comment there.\n\n> > I haven't added a copyright to gen-gst-controls.py yet as I am not sure because\n> > I copied much of it from gen-controls.py.\n> \n> Haven't checked, but I would probably have copied the copyright from original\n> and added mine under.\n\nThat's a good approach.\n\n> > Another small issue is that I haven't implemented the Rectangle type yet and\n> > thus the controls using it won't show up. The reason for this is that there\n> > is the AfWindows control which is an array of Rectangles. As the gstreamer\n> > properties can only be glib types I wasn't sure what to do here: For a\n> > single Rectangle you could use an array and make the entries (x, y, w, h)\n> > but what about an array of Rectangles? Should it use an array with 4 * N\n> > entries, so (x, y, w, h) for each value?\n> \n> The type GstValueArray can nest other GValue types (was needed for\n> GstValueFraction). So in theory you can have an array of of N rectangle array\n> (size 4). In serialized form it would look like:\n> \n>   property=\"< <x1, y1, w1, h1>, <x2, y2, w2, h2>, ...>\"\n> \n> The C API that let you assemble/deassemble this is pretty tedious though. We\n> usually prefer not to always go through the de-serializer of course. If you look\n> around, most of the time these are split in multiple properties, but it also\n> means that your control can be inconsistent while its being set, so pretty\n> difficult to set at run-time.\n> \n> So even though complex, I think its a fair approach since the only other option\n> is to use a custom serialization/deserialzation and a string type property.\n> \n> > At the moment the gstreamer properties all have zero (or the first enum value)\n> > as a default and the minimum and maximum numeric values as minimum and maximum\n> > values for numbers. Also all controls are defined as readable and writeable.\n> > Because of this (maybe as a discussion) I have a small wish list of things I'd\n> > like to see added in the control_ids_*.yaml files which would greatly improve\n> > the gstreamer properties:\n> > For enum and bool controls: the default value if available.\n> \n> While in GStreamer, we often see the implementation differ from the default\n> after moving the element to READY state (so we could just read it back later), I\n> like the implied consistency that having default (were that actually make sense)\n> would bring. I had the same impression where I first looked at it.\n\nThe problem is that the default value may be device-specific. For some\nenum controls, the set of supported values may even differ between\ndevices.\n\n> > For numeric controls: the minimum, maximum and default value.\n\nSame problem here, this is device-specific, so we can't provide that.\n\n> > And for all controls: whether it is read-only, write-only or read-write.\n\nThere we may be able to provide some information. I would need to check\nif a control defined as read-write in the spec could possibly be\nimplemented as read-only for some devices though.\n\n> > Jaslo Ziska (3):\n> >   gstreamer: Remove auto-focus-mode property\n> >   gstreamer: Generate controls from control_ids_*.yaml files\n> >   gstreamer: Fix missing \"greater than\" symbol in author string\n> \n> I will have a look this week, thanks for working on this.\n> \n> Nicolas\n> \n> > \n> >  src/gstreamer/gstlibcamera-controls.cpp.in |  46 +++\n> >  src/gstreamer/gstlibcamera-controls.h      |  36 ++\n> >  src/gstreamer/gstlibcamerasrc.cpp          |  47 +--\n> >  src/gstreamer/meson.build                  |  14 +\n> >  utils/gen-gst-controls.py                  | 398 +++++++++++++++++++++\n> >  utils/meson.build                          |   1 +\n> >  6 files changed, 510 insertions(+), 32 deletions(-)\n> >  create mode 100644 src/gstreamer/gstlibcamera-controls.cpp.in\n> >  create mode 100644 src/gstreamer/gstlibcamera-controls.h\n> >  create mode 100755 utils/gen-gst-controls.py","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 EDCABC323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  7 Aug 2024 16:21:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 02E356337F;\n\tWed,  7 Aug 2024 18:21:55 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E76236337E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  7 Aug 2024 18:21:52 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CE36D2EC;\n\tWed,  7 Aug 2024 18:20:59 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"p1zQ+KDs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1723047660;\n\tbh=l8hBT6sXbA3lMrbAPjw1Gr1NNqDawnnCLQiRN0xwIO0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=p1zQ+KDsKuUUq42flBQxj7K0wYx0sscFkS3aT+9Uc9oz9Pd8PnXAUC+kkRe4r8qhV\n\tsO2f++RjpWc0XiTJojymeOKYkHrqZXKtw3SbW9xlbS23al7t9LzomE2LKF3ca2dWc9\n\tZHGvr1zMIk7MABnTImDABPKhdB2+SNh68Y83i70U=","Date":"Wed, 7 Aug 2024 19:21:29 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Nicolas Dufresne <nicolas@ndufresne.ca>","Cc":"Jaslo Ziska <jaslo@ziska.de>, libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 0/3] gstreamer: Generate controls from control_ids_*.yaml\n\tfiles","Message-ID":"<20240807162129.GE8166@pendragon.ideasonboard.com>","References":"<20240805100038.11972-1-jaslo@ziska.de>\n\t<948ca6cce07ac2ce09e20ea00ad402ae097a6a88.camel@ndufresne.ca>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<948ca6cce07ac2ce09e20ea00ad402ae097a6a88.camel@ndufresne.ca>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30697,"web_url":"https://patchwork.libcamera.org/comment/30697/","msgid":"<a9d012dca8c20ad3a46648e25118d6de42b657a5.camel@ndufresne.ca>","date":"2024-08-07T20:42:51","subject":"Re: [PATCH 0/3] gstreamer: Generate controls from\n\tcontrol_ids_*.yaml files","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Hi Laurent,\n\nLe mercredi 07 août 2024 à 19:21 +0300, Laurent Pinchart a écrit :\n> On Mon, Aug 05, 2024 at 05:02:04PM -0400, Nicolas Dufresne wrote:\n> > Le lundi 05 août 2024 à 11:28 +0200, Jaslo Ziska a écrit :\n> > > Hi everyone,\n> > > \n> > > in this patchset I implemented libcamerasrc properties which are generated from\n> > > the control_ids_*.yaml files.\n> > \n> > I'm very happy to see that someone found that time to work on this.\n> \n> Likewise :-)\n> \n> > > The first commit removes the auto-focus-mode property which was already\n> > > implemented (it is added again in the next commit).\n> > > The second commit adds a Python script to generate C++ code capable of\n> > > interacting with the controls and adds the controls to libcamerasrc.\n> > > The third commit is just a small fix for the missing closing \"greater than\"\n> > > symbol in the author string I noticed.\n> > > \n> > > The gstlibcamera-controls.h file is taken from Nicolas' branch with the change\n> > > that I removed the enum with the control names from the class. Instead the enum\n> > > variants from libcamera::controls:: are now used directly.\n> > > The structure of the gstlibcamera-controls.cpp.in file is also taken from\n> > > Nicolas. The only change is that its content now gets generated by\n> > > gen-gst-controls.py\n> > > \n> > > The boilerplate of gen-gst-controls.py is mostly copied from gen-controls.py.\n> > > This is also where I have some questions:\n> > > \n> > > The definition of the ControlEnum and Control Python classes (and some helper\n> > > functions) is now duplicate code. Should this be handled differently somehow to\n> > > avoid the code duplication?\n> \n> I would be nice to avoid code duplication if possible, yes. I'll review\n> the patch and comment there.\n> \n> > > I haven't added a copyright to gen-gst-controls.py yet as I am not sure because\n> > > I copied much of it from gen-controls.py.\n> > \n> > Haven't checked, but I would probably have copied the copyright from original\n> > and added mine under.\n> \n> That's a good approach.\n> \n> > > Another small issue is that I haven't implemented the Rectangle type yet and\n> > > thus the controls using it won't show up. The reason for this is that there\n> > > is the AfWindows control which is an array of Rectangles. As the gstreamer\n> > > properties can only be glib types I wasn't sure what to do here: For a\n> > > single Rectangle you could use an array and make the entries (x, y, w, h)\n> > > but what about an array of Rectangles? Should it use an array with 4 * N\n> > > entries, so (x, y, w, h) for each value?\n> > \n> > The type GstValueArray can nest other GValue types (was needed for\n> > GstValueFraction). So in theory you can have an array of of N rectangle array\n> > (size 4). In serialized form it would look like:\n> > \n> >   property=\"< <x1, y1, w1, h1>, <x2, y2, w2, h2>, ...>\"\n> > \n> > The C API that let you assemble/deassemble this is pretty tedious though. We\n> > usually prefer not to always go through the de-serializer of course. If you look\n> > around, most of the time these are split in multiple properties, but it also\n> > means that your control can be inconsistent while its being set, so pretty\n> > difficult to set at run-time.\n> > \n> > So even though complex, I think its a fair approach since the only other option\n> > is to use a custom serialization/deserialzation and a string type property.\n> > \n> > > At the moment the gstreamer properties all have zero (or the first enum value)\n> > > as a default and the minimum and maximum numeric values as minimum and maximum\n> > > values for numbers. Also all controls are defined as readable and writeable.\n> > > Because of this (maybe as a discussion) I have a small wish list of things I'd\n> > > like to see added in the control_ids_*.yaml files which would greatly improve\n> > > the gstreamer properties:\n> > > For enum and bool controls: the default value if available.\n> > \n> > While in GStreamer, we often see the implementation differ from the default\n> > after moving the element to READY state (so we could just read it back later), I\n> > like the implied consistency that having default (were that actually make sense)\n> > would bring. I had the same impression where I first looked at it.\n> \n> The problem is that the default value may be device-specific. For some\n> enum controls, the set of supported values may even differ between\n> devices.\n> \n> > > For numeric controls: the minimum, maximum and default value.\n> \n> Same problem here, this is device-specific, so we can't provide that.\n\nThis isn't black and white, some of the controls have the range documented, but\nno machine parsable form to apply into GStreamer. For the other, there is common\nsense limits that could be applied.\n\nWe don't have a run-time mechanism for range in GStreamer properties (well in\nGOBject), but we can have a runtime check to provide developers sensible\nfeedback. If this becomes critical, we could introduce a property description (a\nproperty that provides a description of all the supported controls as seen at\nrun-time, as I would hope the range is known at runtime considering libcamera\napp don't usually have HW specific code). This is fairly easy using a\nGstStructure.\n\nIf though, you have HW specific controls that clearly don't allow for non-\nhardware aware application to use, I would strongly recommend to flag them in\nthe yaml and skip them in the generator. For these special things, I would\nrather add a signal that passes the libcamera object and the request, and let\nthe application modify it in the way it wants before the request is queued.\n\n> \n> > > And for all controls: whether it is read-only, write-only or read-write.\n> \n> There we may be able to provide some information. I would need to check\n> if a control defined as read-write in the spec could possibly be\n> implemented as read-only for some devices though.\n\nI've sent a breakdown reply using the output of gst-inspect earlier, hope this\nwill clarify what I mean. One thing to keep in mind, the range is for\ndocumentation and to assist developers, it is not critical and can be changed\nlater. It is clear that there is down-side of auto-generated controls, as it\ngives less room for GStreamer specific instructions to our users. But as I\nsuggest in my previous reply, we can in the generator have some addition and\nquirks.\n\n> \n> > > Jaslo Ziska (3):\n> > >   gstreamer: Remove auto-focus-mode property\n> > >   gstreamer: Generate controls from control_ids_*.yaml files\n> > >   gstreamer: Fix missing \"greater than\" symbol in author string\n> > \n> > I will have a look this week, thanks for working on this.\n> > \n> > Nicolas\n> > \n> > > \n> > >  src/gstreamer/gstlibcamera-controls.cpp.in |  46 +++\n> > >  src/gstreamer/gstlibcamera-controls.h      |  36 ++\n> > >  src/gstreamer/gstlibcamerasrc.cpp          |  47 +--\n> > >  src/gstreamer/meson.build                  |  14 +\n> > >  utils/gen-gst-controls.py                  | 398 +++++++++++++++++++++\n> > >  utils/meson.build                          |   1 +\n> > >  6 files changed, 510 insertions(+), 32 deletions(-)\n> > >  create mode 100644 src/gstreamer/gstlibcamera-controls.cpp.in\n> > >  create mode 100644 src/gstreamer/gstlibcamera-controls.h\n> > >  create mode 100755 utils/gen-gst-controls.py\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 0C29EBE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  7 Aug 2024 20:42:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1552B6338D;\n\tWed,  7 Aug 2024 22:42:55 +0200 (CEST)","from mail-qk1-x733.google.com (mail-qk1-x733.google.com\n\t[IPv6:2607:f8b0:4864:20::733])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9307B6337E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  7 Aug 2024 22:42:53 +0200 (CEST)","by mail-qk1-x733.google.com with SMTP id\n\taf79cd13be357-7a1d81dc0beso19134885a.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 07 Aug 2024 13:42:53 -0700 (PDT)","from nicolas-tpx395.lan ([2606:6d00:15:820c::580])\n\tby smtp.gmail.com with ESMTPSA id\n\taf79cd13be357-7a3785d0d0fsm92494285a.3.2024.08.07.13.42.51\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 07 Aug 2024 13:42:51 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ndufresne-ca.20230601.gappssmtp.com\n\theader.i=@ndufresne-ca.20230601.gappssmtp.com\n\theader.b=\"YyYBf9dq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20230601.gappssmtp.com; s=20230601; t=1723063372;\n\tx=1723668172; darn=lists.libcamera.org; \n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:cc:to:from:subject:message-id:from:to:cc:subject\n\t:date:message-id:reply-to;\n\tbh=/DM1B3dqQBmBdRVsikTxqgksX3wojqEeRURZcvjnSPw=;\n\tb=YyYBf9dqLEtrMwEP8cG1NhL1cBPoFI27hSAXGe/ovSwC9y0ni4UPbvo04Zn3A3VsR/\n\tJj4M1s/i6Tyc1k7nRJ0UzBC8JfspUjxMkBZSwn+eR8hhbnTnnetd5IzlyZfs5y1fweVH\n\t+YRw9DxB/JdElSHJwV3fCv/OlAJbQbdqWyw7AHS8bhwPpMP162b60fZQt5l6t0Udeq2f\n\t/A9o49zxwCTxrnL+8qIwQKj+v4nCBhzlM7w1Nrk2NDqxL59YU4uq6kKw9MVSXoVKF62D\n\te6jgAnE8aDOLmsT7DuPQFetDS23yot1I45UU1TSC7/MpzEpvfCY2jVXrS5xLlwlAUaG0\n\tASiQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1723063372; x=1723668172;\n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state\n\t:from:to:cc:subject:date:message-id:reply-to;\n\tbh=/DM1B3dqQBmBdRVsikTxqgksX3wojqEeRURZcvjnSPw=;\n\tb=QAdUueqP4BxpoJMPWMEaMKkm3s/Lr1QWheTYI0S9La3LgmwCk6FwmH5snnJcpQ/wiQ\n\tAJh0iQd5peOItLf56l9aQ1tJ6Rg9xOuujVYvfxaRF8+BIFDBKzfAOZBv7dRtsUEtvqfS\n\tqUVUMCRHCDJ3PKv2LlL+RL69B8pQTQKabtL4mGDBHHOxQSQtSuWwnQRNIONTHLP4Se3D\n\tuxRpgBWyBjr8J+dkl2N6NqlZLc8YYR1Go+1R2hYneQ+DDd6Ivnrl65pbJVenk19jLdpV\n\t59yEm8Rzz+gshSL+wv9JY/ajJlEuv7vS1fZ5AX9D3QZ2tB+0E3oHtQ55hwxJWf65ka9f\n\tk1NQ==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCXX9Y+47uWJMv4oq6heNO+jtSijZZ5JxbjGmjmMQXf0wrgg9a+bxmTUoHO4JqJ7pZbyA7lEqknEck1HcTzfZd3jqZCm+QIeivqKoz1cjN+2qXCdvQ==","X-Gm-Message-State":"AOJu0YwoXEbc4y+jEu/tjxqf1HR+waGZU1h+QU9kkSXfFFPR/W2+XseN\n\tkXkRD6ZndyGOJNzCmxaWDRzWFcF947hUdVpPvFy8euc8aoOkfgkHyAG+nL/vai+DdUn4WF/+8nT\n\tB","X-Google-Smtp-Source":"AGHT+IFlDBEOXw+BYOhzOTXQUd+HnpA87juxtmGCsKMsbWBwszz+dFIRwpAAnVFdfykjCD3s3getUQ==","X-Received":"by 2002:a05:620a:269a:b0:79f:78a:f46d with SMTP id\n\taf79cd13be357-7a34f009563mr2464867985a.65.1723063372275; \n\tWed, 07 Aug 2024 13:42:52 -0700 (PDT)","Message-ID":"<a9d012dca8c20ad3a46648e25118d6de42b657a5.camel@ndufresne.ca>","Subject":"Re: [PATCH 0/3] gstreamer: Generate controls from\n\tcontrol_ids_*.yaml files","From":"Nicolas Dufresne <nicolas@ndufresne.ca>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Jaslo Ziska <jaslo@ziska.de>, libcamera-devel@lists.libcamera.org","Date":"Wed, 07 Aug 2024 16:42:51 -0400","In-Reply-To":"<20240807162129.GE8166@pendragon.ideasonboard.com>","References":"<20240805100038.11972-1-jaslo@ziska.de>\n\t<948ca6cce07ac2ce09e20ea00ad402ae097a6a88.camel@ndufresne.ca>\n\t<20240807162129.GE8166@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.52.4 (3.52.4-1.fc40) ","MIME-Version":"1.0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30705,"web_url":"https://patchwork.libcamera.org/comment/30705/","msgid":"<87y157s16z.fsf@ziska.de>","date":"2024-08-08T11:28:52","subject":"Re: [PATCH 0/3] gstreamer: Generate controls from\n\tcontrol_ids_*.yaml files","submitter":{"id":173,"url":"https://patchwork.libcamera.org/api/people/173/","name":"Jaslo Ziska","email":"jaslo@ziska.de"},"content":"Hi Nicolas and Laurent,\n\nthanks for the reviews.\n\nNicolas Dufresne <nicolas@ndufresne.ca> writes:\n> Hi Laurent,\n>\n> Le mercredi 07 août 2024 à 19:21 +0300, Laurent Pinchart a \n> écrit :\n>> On Mon, Aug 05, 2024 at 05:02:04PM -0400, Nicolas Dufresne \n>> wrote:\n>> > Le lundi 05 août 2024 à 11:28 +0200, Jaslo Ziska a écrit :\n>> > > Hi everyone,\n>> > > \n>> > > in this patchset I implemented libcamerasrc properties \n>> > > which are generated from\n>> > > the control_ids_*.yaml files.\n>> > \n>> > I'm very happy to see that someone found that time to work on \n>> > this.\n>> \n>> Likewise :-)\n>> \n>> > > The first commit removes the auto-focus-mode property which \n>> > > was already\n>> > > implemented (it is added again in the next commit).\n>> > > The second commit adds a Python script to generate C++ code \n>> > > capable of\n>> > > interacting with the controls and adds the controls to \n>> > > libcamerasrc.\n>> > > The third commit is just a small fix for the missing \n>> > > closing \"greater than\"\n>> > > symbol in the author string I noticed.\n>> > > \n>> > > The gstlibcamera-controls.h file is taken from Nicolas' \n>> > > branch with the change\n>> > > that I removed the enum with the control names from the \n>> > > class. Instead the enum\n>> > > variants from libcamera::controls:: are now used directly.\n>> > > The structure of the gstlibcamera-controls.cpp.in file is \n>> > > also taken from\n>> > > Nicolas. The only change is that its content now gets \n>> > > generated by\n>> > > gen-gst-controls.py\n>> > > \n>> > > The boilerplate of gen-gst-controls.py is mostly copied \n>> > > from gen-controls.py.\n>> > > This is also where I have some questions:\n>> > > \n>> > > The definition of the ControlEnum and Control Python \n>> > > classes (and some helper\n>> > > functions) is now duplicate code. Should this be handled \n>> > > differently somehow to\n>> > > avoid the code duplication?\n>> \n>> I would be nice to avoid code duplication if possible, yes. \n>> I'll review\n>> the patch and comment there.\n>> \n>> > > I haven't added a copyright to gen-gst-controls.py yet as I \n>> > > am not sure because\n>> > > I copied much of it from gen-controls.py.\n>> > \n>> > Haven't checked, but I would probably have copied the \n>> > copyright from original\n>> > and added mine under.\n>> \n>> That's a good approach.\n>> \n>> > > Another small issue is that I haven't implemented the \n>> > > Rectangle type yet and\n>> > > thus the controls using it won't show up. The reason for \n>> > > this is that there\n>> > > is the AfWindows control which is an array of Rectangles. \n>> > > As the gstreamer\n>> > > properties can only be glib types I wasn't sure what to do \n>> > > here: For a\n>> > > single Rectangle you could use an array and make the \n>> > > entries (x, y, w, h)\n>> > > but what about an array of Rectangles? Should it use an \n>> > > array with 4 * N\n>> > > entries, so (x, y, w, h) for each value?\n>> > \n>> > The type GstValueArray can nest other GValue types (was \n>> > needed for\n>> > GstValueFraction). So in theory you can have an array of of N \n>> > rectangle array\n>> > (size 4). In serialized form it would look like:\n>> > \n>> >   property=\"< <x1, y1, w1, h1>, <x2, y2, w2, h2>, ...>\"\n>> > \n>> > The C API that let you assemble/deassemble this is pretty \n>> > tedious though. We\n>> > usually prefer not to always go through the de-serializer of \n>> > course. If you look\n>> > around, most of the time these are split in multiple \n>> > properties, but it also\n>> > means that your control can be inconsistent while its being \n>> > set, so pretty\n>> > difficult to set at run-time.\n>> > \n>> > So even though complex, I think its a fair approach since the \n>> > only other option\n>> > is to use a custom serialization/deserialzation and a string \n>> > type property.\n\nJust to be certain: you would suggest an array of arrays instead \nof an array of length 4N?\n\n>> > > At the moment the gstreamer properties all have zero (or \n>> > > the first enum value)\n>> > > as a default and the minimum and maximum numeric values as \n>> > > minimum and maximum\n>> > > values for numbers. Also all controls are defined as \n>> > > readable and writeable.\n>> > > Because of this (maybe as a discussion) I have a small wish \n>> > > list of things I'd\n>> > > like to see added in the control_ids_*.yaml files which \n>> > > would greatly improve\n>> > > the gstreamer properties:\n>> > > For enum and bool controls: the default value if available.\n>> > \n>> > While in GStreamer, we often see the implementation differ \n>> > from the default\n>> > after moving the element to READY state (so we could just \n>> > read it back later), I\n>> > like the implied consistency that having default (were that \n>> > actually make sense)\n>> > would bring. I had the same impression where I first looked \n>> > at it.\n>> \n>> The problem is that the default value may be device-specific. \n>> For some\n>> enum controls, the set of supported values may even differ \n>> between\n>> devices.\n>> \n>> > > For numeric controls: the minimum, maximum and default \n>> > > value.\n>> \n>> Same problem here, this is device-specific, so we can't provide \n>> that.\n>\n> This isn't black and white, some of the controls have the range \n> documented, but\n> no machine parsable form to apply into GStreamer. For the other, \n> there is common\n> sense limits that could be applied.\n\nSorry if it wasn't clear in the original text, that is exactly \nwhat I meant. Just to have limits where it is already written in \nthe documentation and limits where the values are just physically \nconstrained.\n\n> We don't have a run-time mechanism for range in GStreamer \n> properties (well in\n> GOBject), but we can have a runtime check to provide developers \n> sensible\n> feedback. If this becomes critical, we could introduce a \n> property description (a\n> property that provides a description of all the supported \n> controls as seen at\n> run-time, as I would hope the range is known at runtime \n> considering libcamera\n> app don't usually have HW specific code). This is fairly easy \n> using a\n> GstStructure.\n\nI think that's a good idea to get information about the camera \ncapabilities at runtime. Should I try to add something like this \nin the next revision?\n\n> If though, you have HW specific controls that clearly don't \n> allow for non-\n> hardware aware application to use, I would strongly recommend to \n> flag them in\n> the yaml and skip them in the generator. For these special \n> things, I would\n> rather add a signal that passes the libcamera object and the \n> request, and let\n> the application modify it in the way it wants before the request \n> is queued.\n\nI'm not sure about this, can't most controls theoretically be \nhardware specific? Then most controls would not show up as \ngstreamer properties. In my opinion I would rather have the \ncontrols show up like normal and then not be able to use them.\n\n>> > > And for all controls: whether it is read-only, write-only \n>> > > or read-write.\n>> \n>> There we may be able to provide some information. I would need \n>> to check\n>> if a control defined as read-write in the spec could possibly \n>> be\n>> implemented as read-only for some devices though.\n>\n> I've sent a breakdown reply using the output of gst-inspect \n> earlier, hope this\n> will clarify what I mean. One thing to keep in mind, the range \n> is for\n> documentation and to assist developers, it is not critical and \n> can be changed\n> later. It is clear that there is down-side of auto-generated \n> controls, as it\n> gives less room for GStreamer specific instructions to our \n> users. But as I\n> suggest in my previous reply, we can in the generator have some \n> addition and\n> quirks.\n>\n>> \n>> > > Jaslo Ziska (3):\n>> > >   gstreamer: Remove auto-focus-mode property\n>> > >   gstreamer: Generate controls from control_ids_*.yaml \n>> > >   files\n>> > >   gstreamer: Fix missing \"greater than\" symbol in author \n>> > >   string\n>> > \n>> > I will have a look this week, thanks for working on this.\n>> > \n>> > Nicolas\n>> > \n>> > > \n>> > >  src/gstreamer/gstlibcamera-controls.cpp.in |  46 +++\n>> > >  src/gstreamer/gstlibcamera-controls.h      |  36 ++\n>> > >  src/gstreamer/gstlibcamerasrc.cpp          |  47 +--\n>> > >  src/gstreamer/meson.build                  |  14 +\n>> > >  utils/gen-gst-controls.py                  | 398 \n>> > >  +++++++++++++++++++++\n>> > >  utils/meson.build                          |   1 +\n>> > >  6 files changed, 510 insertions(+), 32 deletions(-)\n>> > >  create mode 100644 \n>> > >  src/gstreamer/gstlibcamera-controls.cpp.in\n>> > >  create mode 100644 src/gstreamer/gstlibcamera-controls.h\n>> > >  create mode 100755 utils/gen-gst-controls.py\n\nBest regards,\n\nJaslo","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 7027CBE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  8 Aug 2024 11:28:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 173CD6337F;\n\tThu,  8 Aug 2024 13:28:56 +0200 (CEST)","from mo4-p00-ob.smtp.rzone.de (mo4-p00-ob.smtp.rzone.de\n\t[81.169.146.218])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7557C61955\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  8 Aug 2024 13:28:54 +0200 (CEST)","from archlinux by smtp.strato.de (RZmta 51.1.0 AUTH)\n\twith ESMTPSA id zb9f0a078BSqDCj\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits))\n\t(Client did not present a certificate);\n\tThu, 8 Aug 2024 13:28:52 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ziska.de header.i=@ziska.de header.b=\"YtoBwIal\";\n\tdkim=permerror (0-bit key) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"2FPhNg+3\"; dkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1723116533; cv=none;\n\td=strato.com; s=strato-dkim-0002;\n\tb=bVDCBAJ1u+uqTh/IFBUWlQ9llzIn8iJeHPATkwRHyR3mLYY2JYs69q3lKbzu4kFj8p\n\t0N+/5mUhyi+FH/381M5+huqJzZRhbwH66X/DD0F67MjNhWoIsAxtv9I00/BTX5LLYKjw\n\tpspIUVmWi8p6aCsWf1LQzE7POEQEVi4zfkh2rPO7Ypsek29R6ziOwTK4HzIfbhIT404p\n\t266lGsATNZvaMf17JxRTvPVPGKrSNZI+0AgFSkTbXNla56m+EoRHKHF3iNHd/x2XlVz+\n\tUP5HMUM/WXp/t964pzLSWJVmGvYX06lhN2Gt2pAVCZPDXS5vlra8sXr/D/z7C5U/gG9I\n\t1TPg==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; t=1723116533;\n\ts=strato-dkim-0002; d=strato.com;\n\th=Message-ID:Date:References:In-Reply-To:Subject:Cc:To:From:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=yBAKwBBGOk7/HiYrCHUIdSb27rIF3++qhtrevNCVdaI=;\n\tb=A4SaQPAKhaLiO8ZzT3nkqfeQnwTtX7vFVTPVNUeAL7RxHKxVSzpO4RQsjj3x/NXDIc\n\tKW7SncecFhZIp586SSBiXnvAk1LlXdEX3BMJ2i5e5ecane2SlrNjvPMe0K87ZQ2Otxls\n\t6CtOiPzgwXk33uuJyhL+GBkCNWirhFfbq6SdqSavysD+/qXPtGtaP0uPz7bscfvwW0Wl\n\tAm0GJBf5DqpJ6k6Q68rTX1bMyYfz7ojG/USDh7i5ifVfqh7k2yDrsdKR5z/10cKzf+2R\n\tz/bGzkYU7vNEGsdOpyq5BSIahrurSrD5VibyamMsEzpVAGM4yctt96QkYUWsj1PvU4pp\n\t3XkQ==","ARC-Authentication-Results":"i=1; strato.com;\n    arc=none;\n    dkim=none","X-RZG-CLASS-ID":"mo00","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; t=1723116532;\n\ts=strato-dkim-0002; d=ziska.de;\n\th=Message-ID:Date:References:In-Reply-To:Subject:Cc:To:From:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=yBAKwBBGOk7/HiYrCHUIdSb27rIF3++qhtrevNCVdaI=;\n\tb=YtoBwIalcLY02xDF6U3sUfnsI5KHoHsZ+efI6XGLtjt7uGsspN33rjD+aBkKfLXEIR\n\tGT6wutSYIf9rD6UfYxDculvZu6BN4DwGSDKcfYepKXZ6HL0DTJlKAysk8wY7A17jT1Zr\n\tOHMLToAOXPLWwna4qH3Q+HhRvYGaZ+KkHz932VntLsLFr6QMEjgYZ2S3pp3axamZro0g\n\taetLS/YP21YvDX27TFPEPDKcJIKNEgU1cjHFY/umqYmV/rLHg6Qc5ULdcM4U4A4OkJ8v\n\tcmLUtegsPtnRMmr87c2etoayrUuLSmOWSkS3qUkCXCxZRAMGzwaE3E0QPNd0Krz1LkPT\n\tAaiQ==","v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1723116532;\n\ts=strato-dkim-0003; d=ziska.de;\n\th=Message-ID:Date:References:In-Reply-To:Subject:Cc:To:From:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=yBAKwBBGOk7/HiYrCHUIdSb27rIF3++qhtrevNCVdaI=;\n\tb=2FPhNg+3Mzk8u10GJiXIhjW6l41WvS2Ovq3vwyGwq3XgiSYSVrOMhjVZ30GIkgBW3e\n\tWgw2xlKZUD7ttMDSRwDw=="],"X-RZG-AUTH":"\":Jm0XeU+IYfb0x77LHmrjN5Wlb7TBwusDqIM6Hizy8VdfzvKi4yoFC9cH04qwVvJa/DZEjmksR1QX+7v142b0MuXc5T7Z\"","From":"Jaslo Ziska <jaslo@ziska.de>","To":"Nicolas Dufresne <nicolas@ndufresne.ca>","Cc":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 0/3] gstreamer: Generate controls from\n\tcontrol_ids_*.yaml files","In-Reply-To":"<a9d012dca8c20ad3a46648e25118d6de42b657a5.camel@ndufresne.ca>\n\t(Nicolas Dufresne's message of \"Wed, 07 Aug 2024 16:42:51 -0400\")","References":"<20240805100038.11972-1-jaslo@ziska.de>\n\t<948ca6cce07ac2ce09e20ea00ad402ae097a6a88.camel@ndufresne.ca>\n\t<20240807162129.GE8166@pendragon.ideasonboard.com>\n\t<a9d012dca8c20ad3a46648e25118d6de42b657a5.camel@ndufresne.ca>","Date":"Thu, 08 Aug 2024 13:28:52 +0200","Message-ID":"<87y157s16z.fsf@ziska.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30709,"web_url":"https://patchwork.libcamera.org/comment/30709/","msgid":"<20240809112333.GI5833@pendragon.ideasonboard.com>","date":"2024-08-09T11:23:33","subject":"Re: [PATCH 0/3] gstreamer: Generate controls from control_ids_*.yaml\n\tfiles","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Aug 08, 2024 at 01:28:52PM +0200, Jaslo Ziska wrote:\n> Nicolas Dufresne writes:\n> > Le mercredi 07 août 2024 à 19:21 +0300, Laurent Pinchart a  écrit :\n> >> On Mon, Aug 05, 2024 at 05:02:04PM -0400, Nicolas Dufresne wrote:\n> >> > Le lundi 05 août 2024 à 11:28 +0200, Jaslo Ziska a écrit :\n> >> > > Hi everyone,\n> >> > > \n> >> > > in this patchset I implemented libcamerasrc properties \n> >> > > which are generated from\n> >> > > the control_ids_*.yaml files.\n> >> > \n> >> > I'm very happy to see that someone found that time to work on \n> >> > this.\n> >> \n> >> Likewise :-)\n> >> \n> >> > > The first commit removes the auto-focus-mode property which was already\n> >> > > implemented (it is added again in the next commit).\n> >> > > The second commit adds a Python script to generate C++ code capable of\n> >> > > interacting with the controls and adds the controls to libcamerasrc.\n> >> > > The third commit is just a small fix for the missing closing \"greater than\"\n> >> > > symbol in the author string I noticed.\n> >> > > \n> >> > > The gstlibcamera-controls.h file is taken from Nicolas' branch with the change\n> >> > > that I removed the enum with the control names from the class. Instead the enum\n> >> > > variants from libcamera::controls:: are now used directly.\n> >> > > The structure of the gstlibcamera-controls.cpp.in file is also taken from\n> >> > > Nicolas. The only change is that its content now gets generated by\n> >> > > gen-gst-controls.py\n> >> > > \n> >> > > The boilerplate of gen-gst-controls.py is mostly copied from gen-controls.py.\n> >> > > This is also where I have some questions:\n> >> > > \n> >> > > The definition of the ControlEnum and Control Python classes (and some helper\n> >> > > functions) is now duplicate code. Should this be handled differently somehow to\n> >> > > avoid the code duplication?\n> >> \n> >> I would be nice to avoid code duplication if possible, yes. I'll review\n> >> the patch and comment there.\n> >> \n> >> > > I haven't added a copyright to gen-gst-controls.py yet as I am not sure because\n> >> > > I copied much of it from gen-controls.py.\n> >> > \n> >> > Haven't checked, but I would probably have copied the copyright from original\n> >> > and added mine under.\n> >> \n> >> That's a good approach.\n> >> \n> >> > > Another small issue is that I haven't implemented the Rectangle type yet and\n> >> > > thus the controls using it won't show up. The reason for this is that there\n> >> > > is the AfWindows control which is an array of Rectangles. As the gstreamer\n> >> > > properties can only be glib types I wasn't sure what to do here: For a\n> >> > > single Rectangle you could use an array and make the entries (x, y, w, h)\n> >> > > but what about an array of Rectangles? Should it use an array with 4 * N\n> >> > > entries, so (x, y, w, h) for each value?\n> >> > \n> >> > The type GstValueArray can nest other GValue types (was needed for\n> >> > GstValueFraction). So in theory you can have an array of of N rectangle array\n> >> > (size 4). In serialized form it would look like:\n> >> > \n> >> >   property=\"< <x1, y1, w1, h1>, <x2, y2, w2, h2>, ...>\"\n> >> > \n> >> > The C API that let you assemble/deassemble this is pretty tedious though. We\n> >> > usually prefer not to always go through the de-serializer of course. If you look\n> >> > around, most of the time these are split in multiple properties, but it also\n> >> > means that your control can be inconsistent while its being set, so pretty\n> >> > difficult to set at run-time.\n> >> > \n> >> > So even though complex, I think its a fair approach since the only other option\n> >> > is to use a custom serialization/deserialzation and a string type property.\n> \n> Just to be certain: you would suggest an array of arrays instead \n> of an array of length 4N?\n> \n> >> > > At the moment the gstreamer properties all have zero (or the first enum value)\n> >> > > as a default and the minimum and maximum numeric values as minimum and maximum\n> >> > > values for numbers. Also all controls are defined as readable and writeable.\n> >> > > Because of this (maybe as a discussion) I have a small wish list of things I'd\n> >> > > like to see added in the control_ids_*.yaml files which would greatly improve\n> >> > > the gstreamer properties:\n> >> > > For enum and bool controls: the default value if available.\n> >> > \n> >> > While in GStreamer, we often see the implementation differ from the default\n> >> > after moving the element to READY state (so we could just read it back later), I\n> >> > like the implied consistency that having default (were that actually make sense)\n> >> > would bring. I had the same impression where I first looked at it.\n> >> \n> >> The problem is that the default value may be device-specific. For some\n> >> enum controls, the set of supported values may even differ between\n> >> devices.\n> >> \n> >> > > For numeric controls: the minimum, maximum and default value.\n> >> \n> >> Same problem here, this is device-specific, so we can't provide \n> >> that.\n> >\n> > This isn't black and white, some of the controls have the range documented, but\n> > no machine parsable form to apply into GStreamer. For the other, there is common\n> > sense limits that could be applied.\n> \n> Sorry if it wasn't clear in the original text, that is exactly \n> what I meant. Just to have limits where it is already written in \n> the documentation and limits where the values are just physically \n> constrained.\n\nI think that makes sense, we should add that. I'm not sure I would have\ntime to work on it in the near future though. If you want to give it a\ngo, I would suggest starting with a patch that indicates the read/write\ncapabilities of each control as it will likely be easier, and then\nmoving to address the min/max values.\n\n> > We don't have a run-time mechanism for range in GStreamer properties (well in\n> > GOBject), but we can have a runtime check to provide developers sensible\n> > feedback. If this becomes critical, we could introduce a property description (a\n> > property that provides a description of all the supported controls as seen at\n> > run-time, as I would hope the range is known at runtime considering libcamera\n> > app don't usually have HW specific code). This is fairly easy using a\n> > GstStructure.\n> \n> I think that's a good idea to get information about the camera \n> capabilities at runtime. Should I try to add something like this \n> in the next revision?\n> \n> > If though, you have HW specific controls that clearly don't allow for non-\n> > hardware aware application to use, I would strongly recommend to flag them in\n> > the yaml and skip them in the generator. For these special things, I would\n> > rather add a signal that passes the libcamera object and the request, and let\n> > the application modify it in the way it wants before the request is queued.\n> \n> I'm not sure about this, can't most controls theoretically be \n> hardware specific? Then most controls would not show up as \n> gstreamer properties. In my opinion I would rather have the \n> controls show up like normal and then not be able to use them.\n\nI don't have a strong opinion on this topic. Most of the vendor-specific\nand draft controls we have today are there because we haven't taken the\ntime necessary to properly standardize them, and that's something we\nshould address. We can expect some vendor controls to remain, while the\ndraft category will likely disappear.\n\n> >> > > And for all controls: whether it is read-only, write-only \n> >> > > or read-write.\n> >> \n> >> There we may be able to provide some information. I would need to check\n> >> if a control defined as read-write in the spec could possibly be\n> >> implemented as read-only for some devices though.\n> >\n> > I've sent a breakdown reply using the output of gst-inspect earlier, hope this\n> > will clarify what I mean. One thing to keep in mind, the range is for\n> > documentation and to assist developers, it is not critical and can be changed\n> > later.\n\nThanks for that clarification, I wasn't sure about it.\n\n> > It is clear that there is down-side of auto-generated controls, as it\n> > gives less room for GStreamer specific instructions to our users. But as I\n> > suggest in my previous reply, we can in the generator have some addition and\n> > quirks.\n\nAgreed.\n\n> >> > > Jaslo Ziska (3):\n> >> > >   gstreamer: Remove auto-focus-mode property\n> >> > >   gstreamer: Generate controls from control_ids_*.yaml files\n> >> > >   gstreamer: Fix missing \"greater than\" symbol in author string\n> >> > \n> >> > I will have a look this week, thanks for working on this.\n> >> > \n> >> > Nicolas\n> >> > \n> >> > > \n> >> > >  src/gstreamer/gstlibcamera-controls.cpp.in |  46 +++\n> >> > >  src/gstreamer/gstlibcamera-controls.h      |  36 ++\n> >> > >  src/gstreamer/gstlibcamerasrc.cpp          |  47 +--\n> >> > >  src/gstreamer/meson.build                  |  14 +\n> >> > >  utils/gen-gst-controls.py                  | 398 +++++++++++++++++++++\n> >> > >  utils/meson.build                          |   1 +\n> >> > >  6 files changed, 510 insertions(+), 32 deletions(-)\n> >> > >  create mode 100644 src/gstreamer/gstlibcamera-controls.cpp.in\n> >> > >  create mode 100644 src/gstreamer/gstlibcamera-controls.h\n> >> > >  create mode 100755 utils/gen-gst-controls.py","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 62FC0C323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  9 Aug 2024 11:24:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EE6EB6195D;\n\tFri,  9 Aug 2024 13:23:59 +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 D86AB61955\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  9 Aug 2024 13:23:57 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6E51A83F;\n\tFri,  9 Aug 2024 13:23:03 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"qsKCbKBM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1723202583;\n\tbh=LBw24U6ok18w6PhKO1JNEGGoDyQm6JebyL1ZnIcx6s4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qsKCbKBM1o/CaIZeKFLhwobbddCwNlGQ06SuUvMUwrtxlj9HgWy4wSHeZkFtQZP2V\n\tsbOyo4c5tq1U2rwCaqH38pVIpTKRynOGGUlAfD7DjQK9lsJrhJVzoBzGtDV6asVNA9\n\td6hh//oVGtIDKjspZ4B1emqhxnUk3ZL3bjRT7qds=","Date":"Fri, 9 Aug 2024 14:23:33 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jaslo Ziska <jaslo@ziska.de>","Cc":"Nicolas Dufresne <nicolas@ndufresne.ca>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 0/3] gstreamer: Generate controls from control_ids_*.yaml\n\tfiles","Message-ID":"<20240809112333.GI5833@pendragon.ideasonboard.com>","References":"<20240805100038.11972-1-jaslo@ziska.de>\n\t<948ca6cce07ac2ce09e20ea00ad402ae097a6a88.camel@ndufresne.ca>\n\t<20240807162129.GE8166@pendragon.ideasonboard.com>\n\t<a9d012dca8c20ad3a46648e25118d6de42b657a5.camel@ndufresne.ca>\n\t<87y157s16z.fsf@ziska.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<87y157s16z.fsf@ziska.de>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30800,"web_url":"https://patchwork.libcamera.org/comment/30800/","msgid":"<dd3b074eda378a1f1f75e6a21c34641302dfee82.camel@ndufresne.ca>","date":"2024-08-13T18:52:26","subject":"Re: [PATCH 0/3] gstreamer: Generate controls from\n\tcontrol_ids_*.yaml files","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Le jeudi 08 août 2024 à 13:28 +0200, Jaslo Ziska a écrit :\n> > > > So even though complex, I think its a fair approach since the \n> > > > only other option\n> > > > is to use a custom serialization/deserialzation and a string \n> > > > type property.\n> \n> Just to be certain: you would suggest an array of arrays instead \n> of an array of length 4N?\n\nFor Rectangle, an array of 4 seems justified. Array of Array would be more\nconvenient though for matrix, or array of \"pairs\" like the colors.\n\nNicolas","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 67749BDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 13 Aug 2024 18:52:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 56F1A633B9;\n\tTue, 13 Aug 2024 20:52:31 +0200 (CEST)","from mail-qt1-x834.google.com (mail-qt1-x834.google.com\n\t[IPv6:2607:f8b0:4864:20::834])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E413063398\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 13 Aug 2024 20:52:28 +0200 (CEST)","by mail-qt1-x834.google.com with SMTP id\n\td75a77b69052e-44fe6672297so32404791cf.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 13 Aug 2024 11:52:28 -0700 (PDT)","from nicolas-tpx395.lan ([2606:6d00:17:9cac::7a9])\n\tby smtp.gmail.com with ESMTPSA id\n\td75a77b69052e-4531c27176fsm34501351cf.77.2024.08.13.11.52.27\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 13 Aug 2024 11:52:27 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ndufresne-ca.20230601.gappssmtp.com\n\theader.i=@ndufresne-ca.20230601.gappssmtp.com\n\theader.b=\"2dbu2J1J\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20230601.gappssmtp.com; s=20230601; t=1723575148;\n\tx=1724179948; darn=lists.libcamera.org; \n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:cc:to:from:subject:message-id:from:to:cc:subject\n\t:date:message-id:reply-to;\n\tbh=aZve+vuC3R/S+v8CM1Od8kUZNTSM+mplbjqWvkeQB3w=;\n\tb=2dbu2J1J2n94ymlWaNspfhbpff570iUpnEq0PM+4vR7yxRrQ8cVQrqhEJ+mbh4tcve\n\tSm/Ux/IhixUrySij9+qdpvMFK1XJQ/hJ4myE0x5/8+DAkU1sWGttZ1HgMRJNv9SKYCO3\n\totiYWwX9y02zSY1pKCOdGu5mdvWXPBQCPfT6fNxS80HzUWIZpyytJ6aF+xGRsaMrfrom\n\t+A0kjTyGaIaS4S+neQCzUyWPRf6/Ho4OOIxBORVg0DXmLR/miv4umDiXkNovP7FZ5iFz\n\th0i1kFffy1Y/hwXAyWC4E0dhM81g9vwtB793fYW75eD1Hv4Rpn/D9/cEM0Oan4DUvFUu\n\tRL+A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1723575148; x=1724179948;\n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state\n\t:from:to:cc:subject:date:message-id:reply-to;\n\tbh=aZve+vuC3R/S+v8CM1Od8kUZNTSM+mplbjqWvkeQB3w=;\n\tb=BIekuNDG+jiHZ0eGWmXnCzyPS4v96gfqT13JtGY+icOLJlX84AyglmyCCoviKvgvzN\n\tTJdE9SOvfJNFkbsy7GWQRl4H1Be71hUfG7oe5oep1iBQmw306FRqIHvlpIym8/mO9/8J\n\tkv+BuSjWQHzlRMslzR6qH25GREUjMw8buqdjj9/R18Dfsone0zGI6fFcGNXwRZ66i68d\n\t9XZAIUSPpQzupwqtHu5IdQ/47/K0IiWH6/vNbpV+S0u3hKp/K3XU/na19dL0pa5HvNwx\n\txfqWrxRchtxykbOAFCr4nZOgf+fTKPEeUrYyWk6r5m3mrYTSob11pg1VE5jLBna+iEtW\n\tUN9g==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCVeXHcH82zB8URCi/XUsRi2r2bIbzQy/mv31i6SNuyBVKuQiOLjHPJUoUe1KxgvzN8v7fFW93nAGpRTQ5enYUVnU3NsQSoO2bWWNBdOMWpuj59gNA==","X-Gm-Message-State":"AOJu0YwVZlwXec1bcsZYWGjtsm8w66hj85vd+LuImsZZmVdQbAwN4Yiq\n\t/pETJPDYqgRjP8zel/oOEd0zXMrMCg6NYMWVfohQTui66lvpGHXwdkZ1uuXkjRw=","X-Google-Smtp-Source":"AGHT+IFFruGBR+exObphEXmJeYIAbJ7GD+etRlxOjFBF7qDjUSu1hFxSeO8GVuD3g6yjUDzgiOvW1g==","X-Received":"by 2002:a05:622a:5a06:b0:440:572f:391c with SMTP id\n\td75a77b69052e-4535bb0792emr3126051cf.24.1723575147682; \n\tTue, 13 Aug 2024 11:52:27 -0700 (PDT)","Message-ID":"<dd3b074eda378a1f1f75e6a21c34641302dfee82.camel@ndufresne.ca>","Subject":"Re: [PATCH 0/3] gstreamer: Generate controls from\n\tcontrol_ids_*.yaml files","From":"Nicolas Dufresne <nicolas@ndufresne.ca>","To":"Jaslo Ziska <jaslo@ziska.de>","Cc":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>, \n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 13 Aug 2024 14:52:26 -0400","In-Reply-To":"<87y157s16z.fsf@ziska.de>","References":"<20240805100038.11972-1-jaslo@ziska.de>\n\t<948ca6cce07ac2ce09e20ea00ad402ae097a6a88.camel@ndufresne.ca>\n\t<20240807162129.GE8166@pendragon.ideasonboard.com>\n\t<a9d012dca8c20ad3a46648e25118d6de42b657a5.camel@ndufresne.ca>\n\t<87y157s16z.fsf@ziska.de>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.52.4 (3.52.4-1.fc40) ","MIME-Version":"1.0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]