Patch Detail
Show a patch.
GET /api/1.1/patches/19062/?format=api
{ "id": 19062, "url": "https://patchwork.libcamera.org/api/1.1/patches/19062/?format=api", "web_url": "https://patchwork.libcamera.org/patch/19062/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20230920151921.31273-3-andrey.konovalov@linaro.org>", "date": "2023-09-20T15:19:19", "name": "[libcamera-devel,v2,2/4] libcamera: converter: make using MediaDevice optional for the Converter", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "9c2fee6d2c47cffc2a6a1a6ac584ce316ae5282a", "submitter": { "id": 25, "url": "https://patchwork.libcamera.org/api/1.1/people/25/?format=api", "name": "Andrey Konovalov", "email": "andrey.konovalov@linaro.org" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/19062/mbox/", "series": [ { "id": 4033, "url": "https://patchwork.libcamera.org/api/1.1/series/4033/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4033", "date": "2023-09-20T15:19:17", "name": "libcamera: converter: generalize Converter to remove MediaDevice dependency", "version": 2, "mbox": "https://patchwork.libcamera.org/series/4033/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/19062/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/19062/checks/", "tags": {}, "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 97E5BBE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 20 Sep 2023 15:19:51 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 42E3F6294A;\n\tWed, 20 Sep 2023 17:19:51 +0200 (CEST)", "from mail-ej1-x62d.google.com (mail-ej1-x62d.google.com\n\t[IPv6:2a00:1450:4864:20::62d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8B4B160388\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 20 Sep 2023 17:19:49 +0200 (CEST)", "by mail-ej1-x62d.google.com with SMTP id\n\ta640c23a62f3a-9ae2cc4d17eso253302166b.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 20 Sep 2023 08:19:49 -0700 (PDT)", "from Lat-5310.. ([87.116.162.81]) by smtp.gmail.com with ESMTPSA id\n\th24-20020a170906829800b009ae0042e48bsm5376736ejx.5.2023.09.20.08.19.48\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 20 Sep 2023 08:19:48 -0700 (PDT)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1695223191;\n\tbh=2Njc0O7Ob11wQMCP+v1HytL2/9nJoKyaxnVb/jvHmfg=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=QhzD/HGig1H/Cf7vGHYBf6br3hIaqoRINKnqsHhWd7TxSUwPHgRkn30ijiOBtTise\n\tsIiljWKF3ZFYlF50msUqv83LA7SZVE1L2Jzyn1WCpav/CnlFaDUXzUNQVKo1/G/GFh\n\t9yYT5mvT/nMdZgGEWL+hapD/UkzE0mTPypXF+ZHcuoIu+YgPouan3JhUFpDXKYDZ/t\n\tjhK5iZeBUZdGVn2CqRCB4xRc63Jbxkf/rFCAe0+ytVajl6KkpD5usjHWtR8soAS57C\n\ti9YWbO0jzEzQ9e5aW55zFMv5K4g8nbm0G9EcBJCyxCxouf4YRVndevJY9Hm3veUw/y\n\t+SWuq0qDWznWw==", "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=linaro.org; s=google; t=1695223189; x=1695827989;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:mime-version:references:in-reply-to\n\t:message-id:date:subject:cc:to:from:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=G2NSitUIDldnJJ4gfjSXMsL6xctreIOQIZGGhC40Bg8=;\n\tb=FueLl9o+/N1flf2eAKoPGKyzUWRF3ZhgaTMh+5nN4RsoDU80mkpIEffmOqZFjnGl0Z\n\tSRGfZfpaniO3s64jy8xh2QaUh/lmedrFRlxR2iqfeytQoYFwIgG232WCIEIQedlwa2Zr\n\tScANqUg7bGgzpblkb5rNDVL1/hYWM5qPg06QslqaWayLmTTlvQ0x4yQzsi8wpiXBu+3W\n\trcfPyoJ70cgT8WOalvwjYVjhkN5KfAPu0reihdBwLIkEsNlsy9Jkjlx0ykZDP2pyk688\n\tEOGsBmdYevfcySMl5D0PmkULgtdeG5EEB36mGntAU0sWjBiVXhAkNkoHdGdTUjJMwivQ\n\trFrA==" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=linaro.org\n\theader.i=@linaro.org header.b=\"FueLl9o+\"; \n\tdkim-atps=neutral", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1695223189; x=1695827989;\n\th=content-transfer-encoding:mime-version:references:in-reply-to\n\t:message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=G2NSitUIDldnJJ4gfjSXMsL6xctreIOQIZGGhC40Bg8=;\n\tb=KY1hnmdAVyjy5g4B/tFqqzZU3mZAEnaGRg1CaOZzZnlJL/KfqsmVEw7XF7pyXen6lX\n\tj6+CKKLFXzwyUI200uofpx0nrSEe9zKyKYje0Tvvwy+LmxpElQJuk/gq93j1os2O7KTi\n\tZmxa+W21ppy0WBrdgHZdQZ1BbWivFlqZwDOVzpb5HKnF8LlptJeu6C7sIqXVzl/V+wV9\n\t0N8Y6bTwygDvkMYfAT1LXge88w9E1VHW4IqCBG3FDCHG6IPN1jLnJNSNDhuP10lPXEnm\n\tFF4/EutpRkJXfcQOyzk4W5s+RRru1nE2NUAO/Da8DkMUr46OpZKcm+kw8x8Z5yw/Y3qf\n\tdqIQ==", "X-Gm-Message-State": "AOJu0YzWJIbC7N99KgB0r9x4n2RggYa4bDcWd4xBZztbbyHx8jECL778\n\tH6WUdyal3gjbcnVP4wgBzjf/MXoej2qE6JnT0YI=", "X-Google-Smtp-Source": "AGHT+IE2yHFzE7K0dLQIlqXKpFP1T9/giIIdTYwlTsnZp4hiB06RWm2LyfhVb1R2RSvGiil0w3GdqA==", "X-Received": "by 2002:a17:906:1d2:b0:9a1:cbe4:d029 with SMTP id\n\t18-20020a17090601d200b009a1cbe4d029mr2283360ejj.74.1695223189052; \n\tWed, 20 Sep 2023 08:19:49 -0700 (PDT)", "To": "libcamera-devel@lists.libcamera.org", "Date": "Wed, 20 Sep 2023 18:19:19 +0300", "Message-Id": "<20230920151921.31273-3-andrey.konovalov@linaro.org>", "X-Mailer": "git-send-email 2.34.1", "In-Reply-To": "<20230920151921.31273-1-andrey.konovalov@linaro.org>", "References": "<20230920151921.31273-1-andrey.konovalov@linaro.org>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "7bit", "Subject": "[libcamera-devel] [PATCH v2 2/4] libcamera: converter: make using\n\tMediaDevice optional for the Converter", "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>", "From": "Andrey Konovalov via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>", "Reply-To": "Andrey Konovalov <andrey.konovalov@linaro.org>", "Cc": "jacopo.mondi@ideasonboard.com, bryan.odonoghue@linaro.org,\n\tsrinivas.kandagatla@linaro.org", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Make Converter a bit more generic by making pointer to MediaDevice\nan optional argument for Converter::Converter(),\nConverterFactoryBase::create(), ConverterFactoryBase::createInstance(),\nand ConverterFactory::createInstance() functions.\n\nInstead of the MediaDevice driver name, use a generic string to match\nagainst the convertor factory name and its compatibles list. For\nMediaDevice based converters this string will be the MediaDevice driver\nname as before.\n\nSigned-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>\n---\n include/libcamera/internal/converter.h | 9 ++--\n src/libcamera/converter.cpp | 53 ++++++++++++++----------\n src/libcamera/pipeline/simple/simple.cpp | 33 +++++++++++----\n 3 files changed, 60 insertions(+), 35 deletions(-)", "diff": "diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h\nindex 834ec5ab..d0c70296 100644\n--- a/include/libcamera/internal/converter.h\n+++ b/include/libcamera/internal/converter.h\n@@ -2,6 +2,7 @@\n /*\n * Copyright (C) 2020, Laurent Pinchart\n * Copyright 2022 NXP\n+ * Copyright 2023, Linaro Ltd\n *\n * converter.h - Generic format converter interface\n */\n@@ -31,7 +32,7 @@ struct StreamConfiguration;\n class Converter\n {\n public:\n-\tConverter(MediaDevice *media);\n+\tConverter(MediaDevice *media = nullptr);\n \tvirtual ~Converter();\n \n \tvirtual int loadConfiguration(const std::string &filename) = 0;\n@@ -72,7 +73,7 @@ public:\n \n \tconst std::vector<std::string> &compatibles() const { return compatibles_; }\n \n-\tstatic std::unique_ptr<Converter> create(MediaDevice *media);\n+\tstatic std::unique_ptr<Converter> create(std::string name, MediaDevice *media = nullptr);\n \tstatic std::vector<ConverterFactoryBase *> &factories();\n \tstatic std::vector<std::string> names();\n \n@@ -81,7 +82,7 @@ private:\n \n \tstatic void registerType(ConverterFactoryBase *factory);\n \n-\tvirtual std::unique_ptr<Converter> createInstance(MediaDevice *media) const = 0;\n+\tvirtual std::unique_ptr<Converter> createInstance(MediaDevice *media = nullptr) const = 0;\n \n \tstd::string name_;\n \tstd::vector<std::string> compatibles_;\n@@ -96,7 +97,7 @@ public:\n \t{\n \t}\n \n-\tstd::unique_ptr<Converter> createInstance(MediaDevice *media) const override\n+\tstd::unique_ptr<Converter> createInstance(MediaDevice *media = nullptr) const override\n \t{\n \t\treturn std::make_unique<_Converter>(media);\n \t}\ndiff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp\nindex 466f8421..5070eabc 100644\n--- a/src/libcamera/converter.cpp\n+++ b/src/libcamera/converter.cpp\n@@ -1,6 +1,7 @@\n /* SPDX-License-Identifier: LGPL-2.1-or-later */\n /*\n * Copyright 2022 NXP\n+ * Copyright 2023 Linaro Ltd\n *\n * converter.cpp - Generic format converter interface\n */\n@@ -13,8 +14,6 @@\n \n #include \"libcamera/internal/media_device.h\"\n \n-#include \"linux/media.h\"\n-\n /**\n * \\file internal/converter.h\n * \\brief Abstract converter\n@@ -38,26 +37,28 @@ LOG_DEFINE_CATEGORY(Converter)\n \n /**\n * \\brief Construct a Converter instance\n- * \\param[in] media The media device implementing the converter\n+ * \\param[in] media The media device implementing the converter (optional)\n *\n * This searches for the entity implementing the data streaming function in the\n * media graph entities and use its device node as the converter device node.\n */\n Converter::Converter(MediaDevice *media)\n {\n-\tconst std::vector<MediaEntity *> &entities = media->entities();\n-\tauto it = std::find_if(entities.begin(), entities.end(),\n-\t\t\t [](MediaEntity *entity) {\n-\t\t\t\t return entity->function() == MEDIA_ENT_F_IO_V4L;\n-\t\t\t });\n-\tif (it == entities.end()) {\n-\t\tLOG(Converter, Error)\n-\t\t\t<< \"No entity suitable for implementing a converter in \"\n-\t\t\t<< media->driver() << \" entities list.\";\n-\t\treturn;\n+\tif (media) {\n+\t\tconst std::vector<MediaEntity *> &entities = media->entities();\n+\t\tauto it = std::find_if(entities.begin(), entities.end(),\n+\t\t\t\t [](MediaEntity *entity) {\n+\t\t\t\t\t return entity->function() == MEDIA_ENT_F_IO_V4L;\n+\t\t\t\t });\n+\t\tif (it == entities.end()) {\n+\t\t\tLOG(Converter, Error)\n+\t\t\t\t<< \"No entity suitable for implementing a converter in \"\n+\t\t\t\t<< media->driver() << \" entities list.\";\n+\t\t\treturn;\n+\t\t}\n+\n+\t\tdeviceNode_ = (*it)->deviceNode();\n \t}\n-\n-\tdeviceNode_ = (*it)->deviceNode();\n }\n \n Converter::~Converter()\n@@ -162,7 +163,8 @@ Converter::~Converter()\n /**\n * \\fn Converter::deviceNode()\n * \\brief The converter device node attribute accessor\n- * \\return The converter device node string\n+ * \\return The converter device node string. If there is no device node for\n+ * the converter returnes an empty string.\n */\n \n /**\n@@ -203,8 +205,13 @@ ConverterFactoryBase::ConverterFactoryBase(const std::string name, std::initiali\n */\n \n /**\n- * \\brief Create an instance of the converter corresponding to the media device\n- * \\param[in] media the media device to create the converter for\n+ * \\brief Create an instance of the converter corresponding to the converter\n+ * name\n+ * \\param[in] name the name of the converter to create\n+ * \\param[in] media the media device to create the converter for (optional)\n+ *\n+ * The converter \\a name must match the name of the converter factory, or one\n+ * of its compatibles.\n *\n * \\return A unique pointer to a new instance of the converter subclass\n * corresponding to the media device. The converter is created by the factory\n@@ -212,22 +219,22 @@ ConverterFactoryBase::ConverterFactoryBase(const std::string name, std::initiali\n * If the media device driver name doesn't match anything a null pointer is\n * returned.\n */\n-std::unique_ptr<Converter> ConverterFactoryBase::create(MediaDevice *media)\n+std::unique_ptr<Converter> ConverterFactoryBase::create(std::string name, MediaDevice *media)\n {\n \tconst std::vector<ConverterFactoryBase *> &factories =\n \t\tConverterFactoryBase::factories();\n \n \tfor (const ConverterFactoryBase *factory : factories) {\n \t\tconst std::vector<std::string> &compatibles = factory->compatibles();\n-\t\tauto it = std::find(compatibles.begin(), compatibles.end(), media->driver());\n+\t\tauto it = std::find(compatibles.begin(), compatibles.end(), name);\n \n-\t\tif (it == compatibles.end() && media->driver() != factory->name_)\n+\t\tif (it == compatibles.end() && name != factory->name_)\n \t\t\tcontinue;\n \n \t\tLOG(Converter, Debug)\n \t\t\t<< \"Creating converter from \"\n \t\t\t<< factory->name_ << \" factory with \"\n-\t\t\t<< (it == compatibles.end() ? \"no\" : media->driver()) << \" alias.\";\n+\t\t\t<< (it == compatibles.end() ? \"no\" : name) << \" alias.\";\n \n \t\tstd::unique_ptr<Converter> converter = factory->createInstance(media);\n \t\tif (converter->isValid())\n@@ -318,7 +325,7 @@ std::vector<ConverterFactoryBase *> &ConverterFactoryBase::factories()\n /**\n * \\fn ConverterFactory::createInstance() const\n * \\brief Create an instance of the Converter corresponding to the factory\n- * \\param[in] media Media device pointer\n+ * \\param[in] media Media device pointer (optional)\n * \\return A unique pointer to a newly constructed instance of the Converter\n * subclass corresponding to the factory\n */\ndiff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\nindex 05ba76bc..3f1b523a 100644\n--- a/src/libcamera/pipeline/simple/simple.cpp\n+++ b/src/libcamera/pipeline/simple/simple.cpp\n@@ -178,20 +178,26 @@ LOG_DEFINE_CATEGORY(SimplePipeline)\n \n class SimplePipelineHandler;\n \n+enum class ConverterFlag {\n+\tNoFlag = 0,\n+\tMediaDevice = (1 << 0),\n+};\n+using ConverterFlags = Flags<ConverterFlag>;\n+\n struct SimplePipelineInfo {\n \tconst char *driver;\n \t/*\n \t * Each converter in the list contains the name\n \t * and the number of streams it supports.\n \t */\n-\tstd::vector<std::pair<const char *, unsigned int>> converters;\n+\tstd::vector<std::tuple<ConverterFlags, const char *, unsigned int>> converters;\n };\n \n namespace {\n \n static const SimplePipelineInfo supportedDevices[] = {\n \t{ \"dcmipp\", {} },\n-\t{ \"imx7-csi\", { { \"pxp\", 1 } } },\n+\t{ \"imx7-csi\", { { ConverterFlag::MediaDevice, \"pxp\", 1 } } },\n \t{ \"j721e-csi2rx\", {} },\n \t{ \"mxc-isi\", {} },\n \t{ \"qcom-camss\", {} },\n@@ -330,6 +336,7 @@ public:\n \n \tV4L2VideoDevice *video(const MediaEntity *entity);\n \tV4L2Subdevice *subdev(const MediaEntity *entity);\n+\tconst char *converterName() { return converterName_; }\n \tMediaDevice *converter() { return converter_; }\n \n protected:\n@@ -358,6 +365,7 @@ private:\n \tMediaDevice *media_;\n \tstd::map<const MediaEntity *, EntityData> entities_;\n \n+\tconst char *converterName_;\n \tMediaDevice *converter_;\n };\n \n@@ -496,8 +504,10 @@ int SimpleCameraData::init()\n \n \t/* Open the converter, if any. */\n \tMediaDevice *converter = pipe->converter();\n-\tif (converter) {\n-\t\tconverter_ = ConverterFactoryBase::create(converter);\n+\tconst char *converterName = pipe->converterName();\n+\tif (converterName) {\n+\t\tLOG(SimplePipeline, Info) << \"Creating converter '\" << converterName << \"'\";\n+\t\tconverter_ = ConverterFactoryBase::create(std::string(converterName), converter);\n \t\tif (!converter_) {\n \t\t\tLOG(SimplePipeline, Warning)\n \t\t\t\t<< \"Failed to create converter, disabling format conversion\";\n@@ -1409,10 +1419,17 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n \tif (!media_)\n \t\treturn false;\n \n-\tfor (const auto &[name, streams] : info->converters) {\n-\t\tDeviceMatch converterMatch(name);\n-\t\tconverter_ = acquireMediaDevice(enumerator, converterMatch);\n-\t\tif (converter_) {\n+\tfor (const auto &[flags, name, streams] : info->converters) {\n+\t\tif (flags & ConverterFlag::MediaDevice) {\n+\t\t\tDeviceMatch converterMatch(name);\n+\t\t\tconverter_ = acquireMediaDevice(enumerator, converterMatch);\n+\t\t\tif (converter_) {\n+\t\t\t\tconverterName_ = name;\n+\t\t\t\tnumStreams = streams;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t} else {\n+\t\t\tconverterName_ = name;\n \t\t\tnumStreams = streams;\n \t\t\tbreak;\n \t\t}\n", "prefixes": [ "libcamera-devel", "v2", "2/4" ] }