Patch Detail
Show a patch.
GET /api/1.1/patches/19096/?format=api
{ "id": 19096, "url": "https://patchwork.libcamera.org/api/1.1/patches/19096/?format=api", "web_url": "https://patchwork.libcamera.org/patch/19096/", "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": "<20230928185537.20178-4-andrey.konovalov@linaro.org>", "date": "2023-09-28T18:55:35", "name": "[libcamera-devel,v3,3/5] libcamera: converter: make using MediaDevice optional for the Converter", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "93705d058d5ab07bbe8a810a0be6709c81bb4b46", "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/19096/mbox/", "series": [ { "id": 4043, "url": "https://patchwork.libcamera.org/api/1.1/series/4043/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4043", "date": "2023-09-28T18:55:32", "name": "libcamera: converter: generalize Converter to remove MediaDevice dependency", "version": 3, "mbox": "https://patchwork.libcamera.org/series/4043/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/19096/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/19096/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 48F46C0F2A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 28 Sep 2023 18:57:08 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 035DC62964;\n\tThu, 28 Sep 2023 20:57:08 +0200 (CEST)", "from mail-wm1-x336.google.com (mail-wm1-x336.google.com\n\t[IPv6:2a00:1450:4864:20::336])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1F68F61DE4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 Sep 2023 20:57:06 +0200 (CEST)", "by mail-wm1-x336.google.com with SMTP id\n\t5b1f17b1804b1-4053c6f0db8so127753205e9.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 Sep 2023 11:57:06 -0700 (PDT)", "from Lat-5310.. ([87.116.164.210]) by smtp.gmail.com with ESMTPSA\n\tid\n\tu1-20020adfed41000000b003247d3e5d99sm890842wro.55.2023.09.28.11.57.05\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 28 Sep 2023 11:57:05 -0700 (PDT)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1695927428;\n\tbh=uXauu6ouhtLjZWNP5XLgaZ9jTggEIKauHcBLGkaqxEg=;\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=gIdAGRB73oQ7mxGuGl4sgkEIlVT7OK0/+mKkZPeOugeRkA29hu62vyRtRtzrgHxbs\n\tgb8pQy8daWxx7sChwNFXRZ15hYRN37BGViWiWg0r0n0LBpHj/9mGUDSBW3S/VYK1nJ\n\tKBbpD2h3pm8Yi94PJ398dzMPNPFjbfcm6rUG/WN0TmwUrcHM13szTLL8akAk7mvpIe\n\tzulQTsDXjiztrQl+TZk5DgsGurtQRrsfkRVf1LxVfpPHdXSOT3UOP9xjXxjXzh3Z36\n\tJzp6uqSObwa3veAq4ZS8nRoUaMzvxYDI5s6QisUivjBnlY+F9z+kVeqh7sxG/Kjrdb\n\tdhSZ31cNwT3/g==", "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=linaro.org; s=google; t=1695927426; x=1696532226;\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=OI2VKm75pLdX9VEIRplvrxvoZJu4os9OOVGF8JqDQLs=;\n\tb=obCZG18Pn03mD+IZsxqPiocC6b5ZxUVdVZRyeLnw7NDAIdivwBBN27YfItr0WK4zXH\n\tDVKZjivsIZW00x8ZMFlkHUkaBBp7Pr5P258ITZ3g3S0J1funs27TRq6OaVQ+QU1U81jH\n\tsMyiyPHV3NechlKumqeCgxMO+jVl6YlKDvFiIdsRXIXtIgwxlQWFlkdMsoUdQausHnqE\n\tFBZzNK21t9xbi/8OZ7s4OiyB6I3uEcn5d07GJ0wlasDBEYWu9Hqd32DfSVfnTpLB4yBJ\n\tERSLgyfimLHwQkiF0k/yS8y2ak1FXpeHCEfeSr45D1I7iwoDIUNE91Ka+SnS8plisJOv\n\thqBQ==" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=linaro.org\n\theader.i=@linaro.org header.b=\"obCZG18P\"; \n\tdkim-atps=neutral", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1695927426; x=1696532226;\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=OI2VKm75pLdX9VEIRplvrxvoZJu4os9OOVGF8JqDQLs=;\n\tb=qEvmZDBfH1JvA+Fvhwa9h2ezZ67KCKXWMXJfF0Bq1SiNLoLp/TjGmFZ2qebPU49+3N\n\tpQxiydAqmKZNj2Jd2fNsPvrY40eOo8ahZ12vPx4IXfpQVX+WTwkKq/Pc44/KH9WA1neH\n\t1h01n6SU2Mk8/KzlUOh/6uW7SenMECS9acVs1oR5YCCq/xu4gj6V5cmSUTWQMhPL3l0M\n\tTks+N6UKlwXdMrCmV40GPMHqkyqB2SvD1sDqdHXK2QDO5vXNYI75yixyDSJaUVvXr96j\n\t9JwBa9tzYjGahCs22OFgkhqMPBsXWYM0a1ewyQ8/bET0ZOJT5a+ZgRaA2CiMFdRyVlx9\n\tenfQ==", "X-Gm-Message-State": "AOJu0YwbTgBWoFLKP00GHg1xACqJy/2HPKtEi36jV90nmi0cjQ9rm7uy\n\tLiLjacEk7naxvnWZIJbdSB5d+gWbzcnLb/5+eds=", "X-Google-Smtp-Source": "AGHT+IGYx3+YrFp7aVlrfHrSPnkd6eNYVE62jov0aEMuObOwddr266S8uoBS/0YrPNIy+5t6ZE1QxQ==", "X-Received": "by 2002:a05:6000:136a:b0:317:1b08:b317 with SMTP id\n\tq10-20020a056000136a00b003171b08b317mr1929996wrz.6.1695927425719; \n\tThu, 28 Sep 2023 11:57:05 -0700 (PDT)", "To": "libcamera-devel@lists.libcamera.org", "Date": "Thu, 28 Sep 2023 21:55:35 +0300", "Message-Id": "<20230928185537.20178-4-andrey.konovalov@linaro.org>", "X-Mailer": "git-send-email 2.34.1", "In-Reply-To": "<20230928185537.20178-1-andrey.konovalov@linaro.org>", "References": "<20230928185537.20178-1-andrey.konovalov@linaro.org>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "7bit", "Subject": "[libcamera-devel] [PATCH v3 3/5] 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 to prepare for\nadding support for coverters not backed by a media device.\n\nInstead of the MediaDevice driver name, use a generic string to match\nagainst the converter 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 | 40 +++++++++++++++---------\n src/libcamera/pipeline/simple/simple.cpp | 39 ++++++++++++++++-------\n 3 files changed, 58 insertions(+), 30 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 aca4fbc7..fe073bf2 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@@ -36,13 +37,16 @@ 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+\tif (!media)\n+\t\treturn;\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@@ -160,7 +164,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 returns an empty string.\n */\n \n /**\n@@ -201,32 +206,37 @@ 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. For media device based converters the converter \\a name\n+ * is the media device driver name.\n+ *\n+ * \\return A unique pointer to a new instance of the converter subclass.\n+ * The converter is created by matching the factory name or any of its\n+ * compatible aliases with the converter name.\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 matching\n- * the factory name or any of its compatible aliases with the media device\n- * driver name.\n- * If the media device driver name doesn't match anything a null pointer is\n- * returned.\n+ * If the converter name doesn't match anything a null pointer is 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@@ -317,7 +327,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..c7da700b 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@@ -495,9 +503,10 @@ int SimpleCameraData::init()\n \tint ret;\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), pipe->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,13 +1418,21 @@ 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-\t\t\tnumStreams = streams;\n-\t\t\tbreak;\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\tLOG(SimplePipeline, Debug)\n+\t\t\t\t\t<< \"Failed to acquire converter media device '\"\n+\t\t\t\t\t<< name << \"'\";\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t}\n+\n+\t\tconverterName_ = name;\n+\t\tnumStreams = streams;\n+\t\tbreak;\n \t}\n \n \t/* Locate the sensors. */\n", "prefixes": [ "libcamera-devel", "v3", "3/5" ] }