[{"id":31424,"web_url":"https://patchwork.libcamera.org/comment/31424/","msgid":"","date":"2024-09-26T20:25:37","subject":"Re: [PATCH v13 6/7] libcamera: virtual: Read config and register\n\tcameras based on the config","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2024. szeptember 26., csütörtök 18:29 keltezéssel, Harvey Yang írta:\n\n> This patch introduces the configuration file for Virtual Pipeline\n> Handler. The config file is written in yaml, and the format is\n> documented in `README.md`.\n> \n> The config file will define the camera with IDs, supported formats and\n> image sources, etc. In the default config file, only Test Patterns are\n> used. Developers can use real images loading if desired.\n> \n> Signed-off-by: Konami Shu \n> Co-developed-by: Harvey Yang \n> Co-developed-by: Yunke Cao \n> Co-developed-by: Tomasz Figa \n> ---\n> [...]\n> +int Parser::parseFrameGenerator(const YamlObject &cameraConfigData, VirtualCameraData *data)\n> +{\n> +\tconst std::string testPatternKey = \"test_pattern\";\n> +\tconst std::string framesKey = \"frames\";\n> +\tif (cameraConfigData.contains(testPatternKey)) {\n> +\t\tif (cameraConfigData.contains(framesKey)) {\n> +\t\t\tLOG(Virtual, Error) << \"A camera should use either \"\n> +\t\t\t\t\t << testPatternKey << \" or \" << framesKey;\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\n> +\t\tauto testPattern = cameraConfigData[testPatternKey].get(\"\");\n> +\n> +\t\tif (testPattern == \"bars\") {\n> +\t\t\tdata->config_.frame = TestPattern::ColorBars;\n> +\t\t} else if (testPattern == \"lines\") {\n> +\t\t\tdata->config_.frame = TestPattern::DiagonalLines;\n> +\t\t} else {\n> +\t\t\tLOG(Virtual, Debug) << \"Test pattern: \" << testPattern\n> +\t\t\t\t\t << \" is not supported\";\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tconst YamlObject &frames = cameraConfigData[framesKey];\n> +\n> +\t/* When there is no frames provided in the config file, use color bar test pattern */\n> +\tif (!frames) {\n> +\t\tdata->config_.frame = TestPattern::ColorBars;\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tif (!frames.isDictionary()) {\n> +\t\tLOG(Virtual, Error) << \"'frames' is not a dictionary.\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tauto path = frames[\"path\"].get();\n> +\n> +\tif (!path) {\n> +\t\tLOG(Virtual, Error) << \"Test pattern or path should be specified.\";\n> +\t\treturn -EINVAL;\n> +\t} else if (auto ext = std::filesystem::path(*path).extension();\n> +\t\t ext == \".jpg\" || ext == \".jpeg\") {\n> +\t\tdata->config_.frame = ImageFrames{ *path, std::nullopt };\n> +\t} else if (std::filesystem::is_directory(std::filesystem::symlink_status(*path))) {\n> +\t\tusing std::filesystem::directory_iterator;\n> +\t\tunsigned int numOfFiles = std::distance(directory_iterator(*path), directory_iterator{});\n> +\t\tif (numOfFiles == 0) {\n> +\t\t\tLOG(Virtual, Error) << \"Empty directory\";\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\t\tdata->config_.frame = ImageFrames{ *path, numOfFiles };\n> +\t} else {\n> +\t\tLOG(Virtual, Error) << \"Frame: \" << *path << \" is not supported\";\n> +\t\treturn -EINVAL;\n> +\t}\n> [...]\n\nI wanted to reflect a bit on this part. I think the `ImageFrames` type can be made\nmore easily understandable as follows:\n\n struct ImageFrames {\n std::vector files;\n };\n\nand then the above part would become something like this:\n\n if (!path) {\n LOG(Virtual, Error) << \"Test pattern or path should be specified.\";\n return -EINVAL;\n }\n\n std::vector files;\n\n switch (std::filesystem::symlink_status(*path).type()) {\n case std::filesystem::file_type::regular:\n files.push_back(*path);\n break;\n case std::filesystem::file_type::directory:\n for (const auto& dentry : std::filesystem::directory_iterator { *path })\n if (dentry.is_regular_file())\n files.push_back(dentry.path());\n std::sort(files.begin(), files.end(), [](const auto& a, const auto& b) {\n return ::strverscmp(a.c_str(), b.c_str()) < 0;\n });\n if (files.empty()) {\n LOG(Virtual, Error) << \"Directory has no files: \" << *path;\n return -EINVAL;\n }\n break;\n default:\n LOG(Virtual, Error) << \"Frame: \" << *path << \" is not supported\";\n return -EINVAL;\n }\n \n data->config_.frame = ImageFrames { std::move(files) };\n\nSo there would simply be a list of files. I think this is simple, and does not\nneed any special code for the \"single file\" scenario when using an `ImageFrames`\nobject.\n\nNote, that the files in the dictionary are sorted using `strverscmp()`, so\nframe-99 should be placed before frame-100, as one would expect.\n\nAlso notice that there is no extension checking anymore, as I don't believe\nthat to be too useful since it does not guarantee anything, and only jpeg files\nare supported, so whether or not the file can be read will be found out when\nthe program tries to load them.\n\nAny thoughts?\n\n\nRegards,\nBarnabás Pőcze","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 81A54C3257\n\tfor ;\n\tThu, 26 Sep 2024 20:25:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3BE196350F;\n\tThu, 26 Sep 2024 22:25:45 +0200 (CEST)","from mail-40133.protonmail.ch (mail-40133.protonmail.ch\n\t[185.70.40.133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BDFA8618DA\n\tfor ;\n\tThu, 26 Sep 2024 22:25:43 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"uuXxGaC6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1727382342; x=1727641542;\n\tbh=jtzqzzUWQz6JA0o6l799hD5GHlZGqEVLoYZMaPTVIWw=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=uuXxGaC67T2kupUd/TZQrpHNszipIfdBbV+r2raNads7wuvtVJzdazoMfRanQTQs7\n\tXOHaov+2Mf8Lbp4/hgTFQNN2f1UKbfRXx90dk0nwTQknvPPyVl5H9m7+6/u/Xqoaq2\n\tkZ0PtDvmgxqsI2SiqP8qBag8JNEP+UWoiISU0KQ68TnW+Sz50hzstQCd6pVmCVu27R\n\trMLbaiK3fybZk4sNtflCUFHGvGuMIRXKqv0ICmcPev8OzgT9L/BeICjUCnHdhkThM4\n\ttXxOM+rf89tLNmb3TXL6PRRzySdvR59ASc6Pctdr/QhnkxsZ+9R3MDL4bBt/xqssGM\n\tsuEbqRW4+WWNQ==","Date":"Thu, 26 Sep 2024 20:25:37 +0000","To":"Harvey Yang ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org, Konami Shu ,\n\tYunke Cao , Tomasz Figa ","Subject":"Re: [PATCH v13 6/7] libcamera: virtual: Read config and register\n\tcameras based on the config","Message-ID":"","In-Reply-To":"<20240926163141.1593026-7-chenghaoyang@google.com>","References":"<20240926163141.1593026-1-chenghaoyang@google.com>\n\t<20240926163141.1593026-7-chenghaoyang@google.com>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"369587821cbafbfbf257eba68a7b71bbaf7f8d25","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31459,"web_url":"https://patchwork.libcamera.org/comment/31459/","msgid":"","date":"2024-09-30T06:35:09","subject":"Re: [PATCH v13 6/7] libcamera: virtual: Read config and register\n\tcameras based on the config","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Barnabás,\n\nOn Fri, Sep 27, 2024 at 4:25 AM Barnabás Pőcze wrote:\n>\n> Hi\n>\n>\n> 2024. szeptember 26., csütörtök 18:29 keltezéssel, Harvey Yang írta:\n>\n> > This patch introduces the configuration file for Virtual Pipeline\n> > Handler. The config file is written in yaml, and the format is\n> > documented in `README.md`.\n> >\n> > The config file will define the camera with IDs, supported formats and\n> > image sources, etc. In the default config file, only Test Patterns are\n> > used. Developers can use real images loading if desired.\n> >\n> > Signed-off-by: Konami Shu \n> > Co-developed-by: Harvey Yang \n> > Co-developed-by: Yunke Cao \n> > Co-developed-by: Tomasz Figa \n> > ---\n> > [...]\n> > +int Parser::parseFrameGenerator(const YamlObject &cameraConfigData, VirtualCameraData *data)\n> > +{\n> > + const std::string testPatternKey = \"test_pattern\";\n> > + const std::string framesKey = \"frames\";\n> > + if (cameraConfigData.contains(testPatternKey)) {\n> > + if (cameraConfigData.contains(framesKey)) {\n> > + LOG(Virtual, Error) << \"A camera should use either \"\n> > + << testPatternKey << \" or \" << framesKey;\n> > + return -EINVAL;\n> > + }\n> > +\n> > + auto testPattern = cameraConfigData[testPatternKey].get(\"\");\n> > +\n> > + if (testPattern == \"bars\") {\n> > + data->config_.frame = TestPattern::ColorBars;\n> > + } else if (testPattern == \"lines\") {\n> > + data->config_.frame = TestPattern::DiagonalLines;\n> > + } else {\n> > + LOG(Virtual, Debug) << \"Test pattern: \" << testPattern\n> > + << \" is not supported\";\n> > + return -EINVAL;\n> > + }\n> > +\n> > + return 0;\n> > + }\n> > +\n> > + const YamlObject &frames = cameraConfigData[framesKey];\n> > +\n> > + /* When there is no frames provided in the config file, use color bar test pattern */\n> > + if (!frames) {\n> > + data->config_.frame = TestPattern::ColorBars;\n> > + return 0;\n> > + }\n> > +\n> > + if (!frames.isDictionary()) {\n> > + LOG(Virtual, Error) << \"'frames' is not a dictionary.\";\n> > + return -EINVAL;\n> > + }\n> > +\n> > + auto path = frames[\"path\"].get();\n> > +\n> > + if (!path) {\n> > + LOG(Virtual, Error) << \"Test pattern or path should be specified.\";\n> > + return -EINVAL;\n> > + } else if (auto ext = std::filesystem::path(*path).extension();\n> > + ext == \".jpg\" || ext == \".jpeg\") {\n> > + data->config_.frame = ImageFrames{ *path, std::nullopt };\n> > + } else if (std::filesystem::is_directory(std::filesystem::symlink_status(*path))) {\n> > + using std::filesystem::directory_iterator;\n> > + unsigned int numOfFiles = std::distance(directory_iterator(*path), directory_iterator{});\n> > + if (numOfFiles == 0) {\n> > + LOG(Virtual, Error) << \"Empty directory\";\n> > + return -EINVAL;\n> > + }\n> > + data->config_.frame = ImageFrames{ *path, numOfFiles };\n> > + } else {\n> > + LOG(Virtual, Error) << \"Frame: \" << *path << \" is not supported\";\n> > + return -EINVAL;\n> > + }\n> > [...]\n>\n> I wanted to reflect a bit on this part. I think the `ImageFrames` type can be made\n> more easily understandable as follows:\n>\n> struct ImageFrames {\n> std::vector files;\n> };\n>\n> and then the above part would become something like this:\n>\n> if (!path) {\n> LOG(Virtual, Error) << \"Test pattern or path should be specified.\";\n> return -EINVAL;\n> }\n>\n> std::vector files;\n>\n> switch (std::filesystem::symlink_status(*path).type()) {\n> case std::filesystem::file_type::regular:\n> files.push_back(*path);\n> break;\n> case std::filesystem::file_type::directory:\n> for (const auto& dentry : std::filesystem::directory_iterator { *path })\n> if (dentry.is_regular_file())\n> files.push_back(dentry.path());\n> std::sort(files.begin(), files.end(), [](const auto& a, const auto& b) {\n> return ::strverscmp(a.c_str(), b.c_str()) < 0;\n> });\n> if (files.empty()) {\n> LOG(Virtual, Error) << \"Directory has no files: \" << *path;\n> return -EINVAL;\n> }\n> break;\n> default:\n> LOG(Virtual, Error) << \"Frame: \" << *path << \" is not supported\";\n> return -EINVAL;\n> }\n>\n> data->config_.frame = ImageFrames { std::move(files) };\n>\n> So there would simply be a list of files. I think this is simple, and does not\n> need any special code for the \"single file\" scenario when using an `ImageFrames`\n> object.\n>\n> Note, that the files in the dictionary are sorted using `strverscmp()`, so\n> frame-99 should be placed before frame-100, as one would expect.\n\nYeah this makes sense to me. IIUC, It also allows other file names, instead\nof using numbers as the index though.\n\n>\n> Also notice that there is no extension checking anymore, as I don't believe\n> that to be too useful since it does not guarantee anything, and only jpeg files\n> are supported, so whether or not the file can be read will be found out when\n> the program tries to load them.\n\nYeah that's true. Mentioning it in README.md should be enough.\n\n>\n> Any thoughts?\n\nMostly adopted.\n\nThanks!\nHarvey\n\n>\n>\n> Regards,\n> Barnabás Pőcze","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 AB798C3257\n\tfor ;\n\tMon, 30 Sep 2024 06:35:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 591A863518;\n\tMon, 30 Sep 2024 08:35:24 +0200 (CEST)","from mail-lj1-x22a.google.com (mail-lj1-x22a.google.com\n\t[IPv6:2a00:1450:4864:20::22a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1BC5762C92\n\tfor ;\n\tMon, 30 Sep 2024 08:35:22 +0200 (CEST)","by mail-lj1-x22a.google.com with SMTP id\n\t38308e7fff4ca-2fad15b3eeeso2848711fa.2\n\tfor ;\n\tSun, 29 Sep 2024 23:35:22 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"RpJ0h/EZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1727678121; x=1728282921;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=bzISSBbx6gMLJE0+dqAigGVaC4/fBCZc2y/G1/feUfg=;\n\tb=RpJ0h/EZETPpU4JekLvKQRDCa49iwEAkR3Z3RrAzCGuUAv5M6c7PfNd1+BfppzwybV\n\t7LiTQtiVfNAxuP64x7tj8MevbOucMJYn5ivLNp/hLE7KOWaqK7UQl6dT1anPuIDS+eeq\n\tFpfCPwyq+PpVeX+btMYDyjbCm+FARJJIo2To4=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1727678121; x=1728282921;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=bzISSBbx6gMLJE0+dqAigGVaC4/fBCZc2y/G1/feUfg=;\n\tb=hEdI6MeL9QidW699zQpsQlUYEfYOMbHvu9e346StQ86sCU7gpKFDfe7pAOnrDDKeZ/\n\tRig7VXbwWhZ8XROz1/Jb3iuCjyZgkLaAlZaH+mZoIuqdAhGRe2fgiyCppKCKTyTl89bB\n\tPCyM8oQF7gvGDJa978DCgh7Lka1eKtOCvbXjfXquiHwH9fLdpAwiyumrN5D5sQcpdyY6\n\tDpVORAuTVR/p1XpRrdlneDPvDqvTg+AhW4EJ2Dxd5p3q6vAsZ5AF3nKlQN6aCHh5bqiw\n\tWkLhRR9FEfM/FaMlA0WAa7VXZGgukALLTuhRVGMR5U0oUxD86D+ioUeL0dU8fN4YV158\n\tc2zw==","X-Gm-Message-State":"AOJu0YxVNGRokTgWSxi26IAeSwRDi/RX0G5oslFwh9gYAF5mDShVQzcZ\n\tAKsG6hupv5eP0ksORglxbVXHkPEMmxUSqaC6yooZihn4XYGHKxmO40fQdVsVETd06gFvC1coJof\n\tMjjvdLJWYnRqRoq1dJWecBpkxc1y6D6Kw4WI8GcawX0fnoMY=","X-Google-Smtp-Source":"AGHT+IEz/lxL49SmwbOSCGs9JKHb3ZMcjatBOa2FAAhjd1iYBP1sjQHj8N4rgSLFiWPlfX7SH/GxOjVBBY3jg1IOztY=","X-Received":"by 2002:a2e:a583:0:b0:2fa:c6b3:bf27 with SMTP id\n\t38308e7fff4ca-2fac6b3c4d4mr13462841fa.6.1727678121134;\n\tSun, 29 Sep 2024 23:35:21 -0700 (PDT)","MIME-Version":"1.0","References":"<20240926163141.1593026-1-chenghaoyang@google.com>\n\t<20240926163141.1593026-7-chenghaoyang@google.com>\n\t","In-Reply-To":"","From":"Cheng-Hao Yang ","Date":"Mon, 30 Sep 2024 14:35:09 +0800","Message-ID":"","Subject":"Re: [PATCH v13 6/7] libcamera: virtual: Read config and register\n\tcameras based on the config","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org, Konami Shu , \n\tYunke Cao , Tomasz Figa ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]