[{"id":2133,"web_url":"https://patchwork.libcamera.org/comment/2133/","msgid":"<20190703140402.GF5007@pendragon.ideasonboard.com>","date":"2019-07-03T14:04:02","subject":"Re: [libcamera-devel] [RFC PATCH v2 1/7] libcamera:\n\tipa_module_info: add license field","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Wed, Jul 03, 2019 at 05:00:01PM +0900, Paul Elder wrote:\n> Add a field to IPAModuleInfo to contain the license of the module.\n\nShould you briefly describe here what the license field will be used for\n? I haven't read the rest of the series, but I expect that libcamera\nwill use it to allow running open-source IPAs without process isolation\n*if* enabled by the user (any IPA should be possible to run in\nisolation, and libcamera would never allow running the closed-source\nones without isolation).\n\n> Update the dummy IPA and IPA test to conform to the new struct layout\n> \n> Signed-off-by: Paul Elder \n> ---\n> New patch in v2\n> - this replaces the isolate flag that was used in v1\n> \n> include/libcamera/ipa/ipa_module_info.h | 2 ++\n> src/ipa/ipa_dummy.cpp | 1 +\n> src/libcamera/ipa_module.cpp | 3 +++\n> test/ipa/ipa_test.cpp | 1 +\n> 4 files changed, 7 insertions(+)\n> \n> diff --git a/include/libcamera/ipa/ipa_module_info.h b/include/libcamera/ipa/ipa_module_info.h\n> index 585f753..39dca1a 100644\n> --- a/include/libcamera/ipa/ipa_module_info.h\n> +++ b/include/libcamera/ipa/ipa_module_info.h\n> @@ -18,6 +18,8 @@ struct IPAModuleInfo {\n> \tuint32_t pipelineVersion;\n> \tchar pipelineName[256];\n> \tchar name[256];\n> +\tchar license[64];\n> +\n\nUnneeded blank line.\n\n> } __attribute__((packed));\n> \n> extern \"C\" {\n> diff --git a/src/ipa/ipa_dummy.cpp b/src/ipa/ipa_dummy.cpp\n> index ee7a3a8..2e6ff71 100644\n> --- a/src/ipa/ipa_dummy.cpp\n> +++ b/src/ipa/ipa_dummy.cpp\n> @@ -34,6 +34,7 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> \t0,\n> \t\"PipelineHandlerVimc\",\n> \t\"Dummy IPA for Vimc\",\n> +\t\"LGPLv2.1\",\n> };\n> \n> IPAInterface *ipaCreate()\n> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\n> index d82ac69..f786c16 100644\n> --- a/src/libcamera/ipa_module.cpp\n> +++ b/src/libcamera/ipa_module.cpp\n> @@ -215,6 +215,9 @@ elfLoadSymbol(void *map, size_t soSize, const char *symbol)\n> *\n> * \\var IPAModuleInfo::name\n> * \\brief The name of the IPA module\n> + *\n> + * \\var IPAModuleInfo::license\n> + * \\brief License of the IPA module\n\nI think this requires more documentation. In particular you should\ndocumentation what license strings are allowed. One option could be to\nuse the SPDX license strings, available from https://spdx.org/licenses/.\nYou would then need to explicitly define what string should be used for\nproprietary licenses.\n\nThe longest license string currently defined in SPDX is 36 for\ncharacters long (BSD-3-Clause-No-Nuclear-License-2014), with the second\ncontender 32 characters long (BSD-3-Clause-No-Nuclear-Warranty)\n(slightly out of topic, but the concept of no nuclear warranty is\ninteresting :-)). With the use of license expressions\n(https://spdx.org/spdx-specification-21-web-version#h.jxpfx0ykyb60) this\ncould become much longer, but I think 64 is a safe value for now.\n\nAnother option would be to use a numerical identifier. We would then\nlikely restrict that to a few common license types, otherwise it would\nbecome a mess to handle. That would be a simpler mechanism, but not as\npowerful.\n\n> */\n> \n> /**\n> diff --git a/test/ipa/ipa_test.cpp b/test/ipa/ipa_test.cpp\n> index bbef069..4c51034 100644\n> --- a/test/ipa/ipa_test.cpp\n> +++ b/test/ipa/ipa_test.cpp\n> @@ -59,6 +59,7 @@ protected:\n> \t\t\t0,\n> \t\t\t\"PipelineHandlerVimc\",\n> \t\t\t\"Dummy IPA for Vimc\",\n> +\t\t\t\"LGPLv2.1\"\n\nThis (and the above other license above) would thus become\n\"LGPL-2.1-or-later\".\n\n> \t\t};\n> \n> \t\tcount += runTest(\"src/ipa/ipa_dummy.so\", testInfo);","headers":{"Return-Path":"","Received":["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 4DCEB60C23\n\tfor ;\n\tWed, 3 Jul 2019 16:04:23 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BB19224B;\n\tWed, 3 Jul 2019 16:04:22 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1562162662;\n\tbh=1+t27y37fHc+YbMydRnAahpvfQqZdTc+8EsI2DoDK0A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NFNC/Yk29ExYK667cnbAvL/P78JiDo8DLwanreTClrwATWctMVQ7KQhpgj3OfvqWW\n\tqjqcGVppcsN8vKJuGICtTmdvLaYdHan50lHtL33dtfycZO4suQmZ9a5nUfky3IJwu0\n\tr40metFeNRxC7uJmbAPVCsJohAX0Jb1vbHFxkBlU=","Date":"Wed, 3 Jul 2019 17:04:02 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190703140402.GF5007@pendragon.ideasonboard.com>","References":"<20190703080007.21376-1-paul.elder@ideasonboard.com>\n\t<20190703080007.21376-2-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190703080007.21376-2-paul.elder@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [RFC PATCH v2 1/7] libcamera:\n\tipa_module_info: add license field","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 03 Jul 2019 14:04:23 -0000"}},{"id":2145,"web_url":"https://patchwork.libcamera.org/comment/2145/","msgid":"<20190703233616.GC17912@bigcity.dyn.berto.se>","date":"2019-07-03T23:36:16","subject":"Re: [libcamera-devel] [RFC PATCH v2 1/7] libcamera:\n\tipa_module_info: add license field","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Paul,\n\nOn 2019-07-03 17:04:02 +0300, Laurent Pinchart wrote:\n> Hi Paul,\n> \n> Thank you for the patch.\n> \n> On Wed, Jul 03, 2019 at 05:00:01PM +0900, Paul Elder wrote:\n> > Add a field to IPAModuleInfo to contain the license of the module.\n> \n> Should you briefly describe here what the license field will be used for\n> ? I haven't read the rest of the series, but I expect that libcamera\n> will use it to allow running open-source IPAs without process isolation\n> *if* enabled by the user (any IPA should be possible to run in\n> isolation, and libcamera would never allow running the closed-source\n> ones without isolation).\n> \n> > Update the dummy IPA and IPA test to conform to the new struct layout\n> > \n> > Signed-off-by: Paul Elder \n> > ---\n> > New patch in v2\n> > - this replaces the isolate flag that was used in v1\n> > \n> > include/libcamera/ipa/ipa_module_info.h | 2 ++\n> > src/ipa/ipa_dummy.cpp | 1 +\n> > src/libcamera/ipa_module.cpp | 3 +++\n> > test/ipa/ipa_test.cpp | 1 +\n> > 4 files changed, 7 insertions(+)\n> > \n> > diff --git a/include/libcamera/ipa/ipa_module_info.h b/include/libcamera/ipa/ipa_module_info.h\n> > index 585f753..39dca1a 100644\n> > --- a/include/libcamera/ipa/ipa_module_info.h\n> > +++ b/include/libcamera/ipa/ipa_module_info.h\n> > @@ -18,6 +18,8 @@ struct IPAModuleInfo {\n> > \tuint32_t pipelineVersion;\n> > \tchar pipelineName[256];\n> > \tchar name[256];\n> > +\tchar license[64];\n> > +\n> \n> Unneeded blank line.\n> \n> > } __attribute__((packed));\n> > \n> > extern \"C\" {\n> > diff --git a/src/ipa/ipa_dummy.cpp b/src/ipa/ipa_dummy.cpp\n> > index ee7a3a8..2e6ff71 100644\n> > --- a/src/ipa/ipa_dummy.cpp\n> > +++ b/src/ipa/ipa_dummy.cpp\n> > @@ -34,6 +34,7 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> > \t0,\n> > \t\"PipelineHandlerVimc\",\n> > \t\"Dummy IPA for Vimc\",\n> > +\t\"LGPLv2.1\",\n> > };\n> > \n> > IPAInterface *ipaCreate()\n> > diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\n> > index d82ac69..f786c16 100644\n> > --- a/src/libcamera/ipa_module.cpp\n> > +++ b/src/libcamera/ipa_module.cpp\n> > @@ -215,6 +215,9 @@ elfLoadSymbol(void *map, size_t soSize, const char *symbol)\n> > *\n> > * \\var IPAModuleInfo::name\n> > * \\brief The name of the IPA module\n> > + *\n> > + * \\var IPAModuleInfo::license\n> > + * \\brief License of the IPA module\n> \n> I think this requires more documentation. In particular you should\n> documentation what license strings are allowed. One option could be to\n> use the SPDX license strings, available from https://spdx.org/licenses/.\n> You would then need to explicitly define what string should be used for\n> proprietary licenses.\n> \n> The longest license string currently defined in SPDX is 36 for\n> characters long (BSD-3-Clause-No-Nuclear-License-2014), with the second\n> contender 32 characters long (BSD-3-Clause-No-Nuclear-Warranty)\n> (slightly out of topic, but the concept of no nuclear warranty is\n> interesting :-)). With the use of license expressions\n> (https://spdx.org/spdx-specification-21-web-version#h.jxpfx0ykyb60) this\n> could become much longer, but I think 64 is a safe value for now.\n> \n> Another option would be to use a numerical identifier. We would then\n> likely restrict that to a few common license types, otherwise it would\n> become a mess to handle. That would be a simpler mechanism, but not as\n> powerful.\n\nWill not a list of string SPDX identifiers be just as messy to maintain? \nLibcamera would still need to keep an internal list of either strings or \nnumerical ids of licences which are OK to run without isolation.\n\nThe advantage I see with numerical ids is that the risk of typos in the\nlicensing \"field\" is eliminated, but I'm not so concerned that will be a \nproblem as the developer of a out-of-tree IPA would notice the typo \nrather quickly.\n\nI don't feel super strong about this issue but I'm tilting towards using \nSPDX strings in the licence field. And in libcamera start with a limited \nlist of licences we think will be common and be open to add more from \nthe SPDX list if needed down the road.\n\nAnother option is of course to have closed source IPAs set a \"closed \nsource bit\", possibly done automatically when downloading the IPA using \nRFC3514 ;-P\n\n> \n> > */\n> > \n> > /**\n> > diff --git a/test/ipa/ipa_test.cpp b/test/ipa/ipa_test.cpp\n> > index bbef069..4c51034 100644\n> > --- a/test/ipa/ipa_test.cpp\n> > +++ b/test/ipa/ipa_test.cpp\n> > @@ -59,6 +59,7 @@ protected:\n> > \t\t\t0,\n> > \t\t\t\"PipelineHandlerVimc\",\n> > \t\t\t\"Dummy IPA for Vimc\",\n> > +\t\t\t\"LGPLv2.1\"\n> \n> This (and the above other license above) would thus become\n> \"LGPL-2.1-or-later\".\n> \n> > \t\t};\n> > \n> > \t\tcount += runTest(\"src/ipa/ipa_dummy.so\", testInfo);\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lj1-x244.google.com (mail-lj1-x244.google.com\n\t[IPv6:2a00:1450:4864:20::244])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AACD260C01\n\tfor ;\n\tThu, 4 Jul 2019 01:36:18 +0200 (CEST)","by mail-lj1-x244.google.com with SMTP id x25so4264945ljh.2\n\tfor ;\n\tWed, 03 Jul 2019 16:36:18 -0700 (PDT)","from localhost (customer-145-14-112-32.stosn.net. [145.14.112.32])\n\tby smtp.gmail.com with ESMTPSA id\n\tt21sm593236lfl.17.2019.07.03.16.36.16\n\tfor \n\t(version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256);\n\tWed, 03 Jul 2019 16:36:16 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=e7QCdOZXXS+Q8F1ErYNwV2m1TmZzhJVKtu6kJ7l8C2Y=;\n\tb=fl7EEWJzcw0LCkVzZh5MTFX78O018hB7WhbYCCSpwd8ymIqchlM6ZvEkqV/N3n+yvu\n\tpHhygWae8vF5t/rqhiF8WkIqJyLA0wQb4eCJQg/KpkpL7zTcgBUlOv02c65mJOZGDW03\n\t7UJ88kWq1+kfpXShzmLGR6pxcfQ/Eed8GU0GYCyj7MRMivChd6Sv5r1G1uC1vmVUMwDr\n\txOHljkOLgHO77/wvcounK+13oOxNcZTAOVAbzrtW47QougRTR6X/0ByOwXgS7EY+T0dU\n\tgEuFnTZKgYLCC/vkYrZE3RpRwW0SRM3p4ZKPaOcBZTY/YSOpcD9xyfImQTx8Ac9sp14E\n\t1nZQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=e7QCdOZXXS+Q8F1ErYNwV2m1TmZzhJVKtu6kJ7l8C2Y=;\n\tb=Zg93Ug6QZKsdT5+ylsRb5yRBTI3BPpjxttEjE8pPPRSOkjakiVuCG1LeoFRclS9z/j\n\tnhLZAFIHz2DUjbSI+ANx2VVX/1w1gmDuJCVEgc6Sc6Ot+2e/yV/fMlRPxjMWMdS+Bgk4\n\twbI6ts9dzn575U8wgJyVLnz+RRpBXDkwWlm7XWStXWnGwhTEI1rtKm6VDnqCqN4DHrKF\n\t9gtVXCzhCBv9oGjF90WcsKQYVGja4XddFWnVvgo+UiAcvkz+GR0UUwiON8nkM00rtejb\n\tdAKwGpyuVkCqC0c/prV7QiLy8X66JCjyKgQCH0hsXmYL9pcpVaAMO/SHAuvoC1PeRMit\n\tiH/w==","X-Gm-Message-State":"APjAAAV4VwX0WYWv75sNNPm77ntaGm0ia+kIfWebLpEd+UED0i7GEcGh\n\t1hvdqv3Ix2vMJk2eN0r2E0qT5J9h+m4=","X-Google-Smtp-Source":"APXvYqxxFJS5xCnij+aayvZ0zXOq0JsZSNOGDnqBhngcGf9KkdbrFEcUcAYQfu00whsOg/jA+WnyFQ==","X-Received":"by 2002:a2e:9997:: with SMTP id w23mr3764832lji.45.1562196977675;\n\tWed, 03 Jul 2019 16:36:17 -0700 (PDT)","Date":"Thu, 4 Jul 2019 01:36:16 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190703233616.GC17912@bigcity.dyn.berto.se>","References":"<20190703080007.21376-1-paul.elder@ideasonboard.com>\n\t<20190703080007.21376-2-paul.elder@ideasonboard.com>\n\t<20190703140402.GF5007@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190703140402.GF5007@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [RFC PATCH v2 1/7] libcamera:\n\tipa_module_info: add license field","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 03 Jul 2019 23:36:18 -0000"}}]