[{"id":16774,"web_url":"https://patchwork.libcamera.org/comment/16774/","msgid":"<20210505154051.jw6dx2gcmrio35u6@pengutronix.de>","date":"2021-05-05T15:40:52","subject":"Re: [libcamera-devel] [PATCH] pipeline: simple: Rework the\n\tsupportedDevices list","submitter":{"id":89,"url":"https://patchwork.libcamera.org/api/people/89/","name":"Marco Felsch","email":"m.felsch@pengutronix.de"},"content":"Hi,\n\nOn 21-05-03 14:25, Phi-Bang Nguyen wrote:\n> There is a usecase that the same driver may go with a converter\n> on a platform and with another converter on another platform.\n\nWhat did you mean by \"another converter on another platform\"? What other\nconvert should the platform take? IMHO if there are more than one\nconvert than you should use a own pipeline handler.\n\nRegards,\n  Marco\n\n> Currently, the simple pipeline handler only takes the 1st driver\n> it can acquire in the supported devices list and skip the rest.\n> This makes it take the wrong converter for the above usecase.\n> \n> So, make the changes to support the above usecase.\n> \n> Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>\n> ---\n>  src/libcamera/pipeline/simple/simple.cpp | 34 +++++++++++++++---------\n>  1 file changed, 22 insertions(+), 12 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index f6095d38..9a572694 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -127,16 +127,19 @@ class SimplePipelineHandler;\n>  \n>  struct SimplePipelineInfo {\n>  \tconst char *driver;\n> -\tconst char *converter;\n> -\tunsigned int numStreams;\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>  };\n>  \n>  namespace {\n>  \n>  static const SimplePipelineInfo supportedDevices[] = {\n> -\t{ \"imx7-csi\", \"pxp\", 1 },\n> -\t{ \"qcom-camss\", nullptr, 1 },\n> -\t{ \"sun6i-csi\", nullptr, 1 },\n> +\t{ \"imx7-csi\", { { \"pxp\", 1 } } },\n> +\t{ \"qcom-camss\", { { nullptr, 1 } } },\n> +\t{ \"sun6i-csi\", { { nullptr, 1 } } },\n>  };\n>  \n>  } /* namespace */\n> @@ -145,7 +148,7 @@ class SimpleCameraData : public CameraData\n>  {\n>  public:\n>  \tSimpleCameraData(SimplePipelineHandler *pipe,\n> -\t\t\t const SimplePipelineInfo *info,\n> +\t\t\t unsigned int numStreams,\n>  \t\t\t MediaEntity *sensor);\n>  \n>  \tbool isValid() const { return sensor_ != nullptr; }\n> @@ -266,9 +269,9 @@ private:\n>   */\n>  \n>  SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> -\t\t\t\t   const SimplePipelineInfo *info,\n> +\t\t\t\t   unsigned int numStreams,\n>  \t\t\t\t   MediaEntity *sensor)\n> -\t: CameraData(pipe), streams_(info->numStreams)\n> +\t: CameraData(pipe), streams_(numStreams)\n>  {\n>  \tint ret;\n>  \n> @@ -934,6 +937,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n>  {\n>  \tconst SimplePipelineInfo *info = nullptr;\n>  \tMediaDevice *converter = nullptr;\n> +\tunsigned int numStreams = 1;\n>  \n>  \tfor (const SimplePipelineInfo &inf : supportedDevices) {\n>  \t\tDeviceMatch dm(inf.driver);\n> @@ -947,9 +951,15 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n>  \tif (!media_)\n>  \t\treturn false;\n>  \n> -\tif (info->converter) {\n> -\t\tDeviceMatch converterMatch(info->converter);\n> -\t\tconverter = acquireMediaDevice(enumerator, converterMatch);\n> +\tfor (const auto &cvt : info->converters) {\n> +\t\tif (cvt.first) {\n> +\t\t\tDeviceMatch converterMatch(cvt.first);\n> +\t\t\tconverter = acquireMediaDevice(enumerator, converterMatch);\n> +\t\t\tif (converter) {\n> +\t\t\t\tnumStreams = cvt.second;\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t}\n>  \t}\n>  \n>  \t/* Locate the sensors. */\n> @@ -983,7 +993,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n>  \n>  \tfor (MediaEntity *sensor : sensors) {\n>  \t\tstd::unique_ptr<SimpleCameraData> data =\n> -\t\t\tstd::make_unique<SimpleCameraData>(this, info, sensor);\n> +\t\t\tstd::make_unique<SimpleCameraData>(this, numStreams, sensor);\n>  \t\tif (!data->isValid()) {\n>  \t\t\tLOG(SimplePipeline, Error)\n>  \t\t\t\t<< \"No valid pipeline for sensor '\"\n> -- \n> 2.25.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel\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 3D68ABDE79\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  5 May 2021 15:40:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C3AC6602C0;\n\tWed,  5 May 2021 17:40:53 +0200 (CEST)","from metis.ext.pengutronix.de (metis.ext.pengutronix.de\n\t[IPv6:2001:67c:670:201:290:27ff:fe1d:cc33])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D151C602BD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  5 May 2021 17:40:52 +0200 (CEST)","from pty.hi.pengutronix.de ([2001:67c:670:100:1d::c5])\n\tby metis.ext.pengutronix.de with esmtps\n\t(TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92)\n\t(envelope-from <mfe@pengutronix.de>)\n\tid 1leJds-0005Jp-Ea; Wed, 05 May 2021 17:40:52 +0200","from mfe by pty.hi.pengutronix.de with local (Exim 4.89)\n\t(envelope-from <mfe@pengutronix.de>)\n\tid 1leJds-0006Az-14; Wed, 05 May 2021 17:40:52 +0200"],"Date":"Wed, 5 May 2021 17:40:52 +0200","From":"Marco Felsch <m.felsch@pengutronix.de>","To":"Phi-Bang Nguyen <pnguyen@baylibre.com>","Message-ID":"<20210505154051.jw6dx2gcmrio35u6@pengutronix.de>","References":"<20210503122518.745088-1-pnguyen@baylibre.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210503122518.745088-1-pnguyen@baylibre.com>","X-Sent-From":"Pengutronix Hildesheim","X-URL":"http://www.pengutronix.de/","X-IRC":"#ptxdist @freenode","X-Accept-Language":"de,en","X-Accept-Content-Type":"text/plain","X-Uptime":"17:38:02 up 154 days, 5:44, 49 users, load average: 0.14, 0.12, \n\t0.12","User-Agent":"NeoMutt/20170113 (1.7.2)","X-SA-Exim-Connect-IP":"2001:67c:670:100:1d::c5","X-SA-Exim-Mail-From":"mfe@pengutronix.de","X-SA-Exim-Scanned":"No (on metis.ext.pengutronix.de);\n\tSAEximRunCond expanded to false","X-PTX-Original-Recipient":"libcamera-devel@lists.libcamera.org","Subject":"Re: [libcamera-devel] [PATCH] pipeline: simple: Rework the\n\tsupportedDevices list","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":16776,"web_url":"https://patchwork.libcamera.org/comment/16776/","msgid":"<CAO4hSriGP99ZH92XX+f7eqKP67ke9kgnuvOsKcST6-2uQYpcjw@mail.gmail.com>","date":"2021-05-05T16:20:29","subject":"Re: [libcamera-devel] [PATCH] pipeline: simple: Rework the\n\tsupportedDevices list","submitter":{"id":79,"url":"https://patchwork.libcamera.org/api/people/79/","name":"Phi-bang Nguyen","email":"pnguyen@baylibre.com"},"content":"Hi Marco & Laurent,\n\nOn Wed, May 5, 2021 at 5:40 PM Marco Felsch <m.felsch@pengutronix.de> wrote:\n\n> Hi,\n>\n> On 21-05-03 14:25, Phi-Bang Nguyen wrote:\n> > There is a usecase that the same driver may go with a converter\n> > on a platform and with another converter on another platform.\n>\n> What did you mean by \"another converter on another platform\"? What other\n> convert should the platform take? IMHO if there are more than one\n> convert than you should use a own pipeline handler.\n>\n>\nThank you for your comment.\n\nThe simple pipeline handler well fits our needs so we don't make a specific\npipeline handler.\nPerhaps, it will be better if I describe with an example.\n\nWe have a driver named \"mtk-seninf\" that uses the \"mtk-mdp\" converter on\nthe MT8167 platform and the \"mtk-mdp3\" converter on the MT8183 platform.\nWith the current code, if I add two lines to the supportedDevices list as\nbelow :\n\n{ \"mtk-seninf\", \"mtk-mdp\", 3 },\n { \"mtk-seninf\", \"mtk-mdp3\", 3 },\n\nOn the MT8183 platform, it will not work because libcamera will find and\nacquire the first entry : \"mtk-seninf\" together with \"mtk-mdp\" converter\nand skip the rest.\nSo, that's the purpose of my patch.\n\n\nRegards,\n>   Marco\n>\n\nBest Regards,\nPhi-Bang\n\n\n> > Currently, the simple pipeline handler only takes the 1st driver\n> > it can acquire in the supported devices list and skip the rest.\n> > This makes it take the wrong converter for the above usecase.\n> >\n> > So, make the changes to support the above usecase.\n> >\n> > Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>\n> > ---\n> >  src/libcamera/pipeline/simple/simple.cpp | 34 +++++++++++++++---------\n> >  1 file changed, 22 insertions(+), 12 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp\n> b/src/libcamera/pipeline/simple/simple.cpp\n> > index f6095d38..9a572694 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -127,16 +127,19 @@ class SimplePipelineHandler;\n> >\n> >  struct SimplePipelineInfo {\n> >       const char *driver;\n> > -     const char *converter;\n> > -     unsigned int numStreams;\n> > +     /*\n> > +      * Each converter in the list contains the name\n> > +      * and the number of streams it supports.\n> > +      */\n> > +     std::vector<std::pair<const char *, unsigned int>> converters;\n> >  };\n> >\n> >  namespace {\n> >\n> >  static const SimplePipelineInfo supportedDevices[] = {\n> > -     { \"imx7-csi\", \"pxp\", 1 },\n> > -     { \"qcom-camss\", nullptr, 1 },\n> > -     { \"sun6i-csi\", nullptr, 1 },\n> > +     { \"imx7-csi\", { { \"pxp\", 1 } } },\n> > +     { \"qcom-camss\", { { nullptr, 1 } } },\n> > +     { \"sun6i-csi\", { { nullptr, 1 } } },\n> >  };\n> >\n> >  } /* namespace */\n> > @@ -145,7 +148,7 @@ class SimpleCameraData : public CameraData\n> >  {\n> >  public:\n> >       SimpleCameraData(SimplePipelineHandler *pipe,\n> > -                      const SimplePipelineInfo *info,\n> > +                      unsigned int numStreams,\n> >                        MediaEntity *sensor);\n> >\n> >       bool isValid() const { return sensor_ != nullptr; }\n> > @@ -266,9 +269,9 @@ private:\n> >   */\n> >\n> >  SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> > -                                const SimplePipelineInfo *info,\n> > +                                unsigned int numStreams,\n> >                                  MediaEntity *sensor)\n> > -     : CameraData(pipe), streams_(info->numStreams)\n> > +     : CameraData(pipe), streams_(numStreams)\n> >  {\n> >       int ret;\n> >\n> > @@ -934,6 +937,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator\n> *enumerator)\n> >  {\n> >       const SimplePipelineInfo *info = nullptr;\n> >       MediaDevice *converter = nullptr;\n> > +     unsigned int numStreams = 1;\n> >\n> >       for (const SimplePipelineInfo &inf : supportedDevices) {\n> >               DeviceMatch dm(inf.driver);\n> > @@ -947,9 +951,15 @@ bool SimplePipelineHandler::match(DeviceEnumerator\n> *enumerator)\n> >       if (!media_)\n> >               return false;\n> >\n> > -     if (info->converter) {\n> > -             DeviceMatch converterMatch(info->converter);\n> > -             converter = acquireMediaDevice(enumerator, converterMatch);\n> > +     for (const auto &cvt : info->converters) {\n> > +             if (cvt.first) {\n> > +                     DeviceMatch converterMatch(cvt.first);\n> > +                     converter = acquireMediaDevice(enumerator,\n> converterMatch);\n> > +                     if (converter) {\n> > +                             numStreams = cvt.second;\n> > +                             break;\n> > +                     }\n> > +             }\n> >       }\n> >\n> >       /* Locate the sensors. */\n> > @@ -983,7 +993,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator\n> *enumerator)\n> >\n> >       for (MediaEntity *sensor : sensors) {\n> >               std::unique_ptr<SimpleCameraData> data =\n> > -                     std::make_unique<SimpleCameraData>(this, info,\n> sensor);\n> > +                     std::make_unique<SimpleCameraData>(this,\n> numStreams, sensor);\n> >               if (!data->isValid()) {\n> >                       LOG(SimplePipeline, Error)\n> >                               << \"No valid pipeline for sensor '\"\n> > --\n> > 2.25.1\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n> >\n>\n> --\n> Pengutronix e.K.                           |                             |\n> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |\n> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |\n> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |\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 A8EC7BDE79\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  5 May 2021 16:20:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0D32468915;\n\tWed,  5 May 2021 18:20:44 +0200 (CEST)","from mail-il1-x12b.google.com (mail-il1-x12b.google.com\n\t[IPv6:2607:f8b0:4864:20::12b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B8070602BD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  5 May 2021 18:20:41 +0200 (CEST)","by mail-il1-x12b.google.com with SMTP id e2so2224617ilr.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 05 May 2021 09:20:41 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=baylibre-com.20150623.gappssmtp.com\n\theader.i=@baylibre-com.20150623.gappssmtp.com\n\theader.b=\"pgjr/7cp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=baylibre-com.20150623.gappssmtp.com; s=20150623;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=lCJMKYlaI2QweoZJKxy3mtyZFNwCky5OWHFMrjR7rzg=;\n\tb=pgjr/7cpIuJzI7J5pPJDit1F7t14ywkI1oZ2y8MHF3lWbSgj6d0a/mTeoMy/2AFB0r\n\tXy97BfujDmkVSPlsWcqmH21yFSbii7AJ+yZ7ArfEoiGx0eGtUQjVkZywvIRkEnJ2JOxL\n\tAW+rkije70hp8DUn7phHqY602ciEGFE0iJDGm4OJkzgPRNWGCOGOZEX7YfHcw7FQalrV\n\tRXPvgFI7pfaTZ78Q12rEXVC9DO/JxXxjpOFRO1yk+i++J/gUMZH3I4fjHVTeSx9gss/y\n\tU93hWFg1Tzy7mOkpSNp5SvCbmSZ/uTvuOziDM3bDgp1QsUNkBVpgpx66/5tYg9It9E+M\n\tOZcA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=lCJMKYlaI2QweoZJKxy3mtyZFNwCky5OWHFMrjR7rzg=;\n\tb=ivAYn+EYfNE0xdFsY8i6zEjTUygNRs2euQY7jcB1yJ1+dcX6bJst+3eSq2Fi5jR14f\n\tG9dkmlW3GkHGq5lTL+VAHNKO7uLoKlP4FitrgTvXAclLPau9OoiCzxAZJ/dh/28D6OfW\n\t6lYmpjf4UXxOLaSsVRlkeztNNFImlxc6vRGE3RUrXK/1spuMDI2uyQm95uTKEs9quxa/\n\tBE0BE37GQYSz6iToofHjnPQ0FPrvdjfTE4VZ3WSJioUAibPGa5/Oh0HItCctg5HwlMrB\n\tQU0P6op8xGgJZqZkhHb6R4mbSjjwVjvyxg29ooLSL8DuZnb1JFd3wtR5GGbBxeLmR5/p\n\tyd0g==","X-Gm-Message-State":"AOAM531vEch3cyVxnvqo9t7ua1rt5cSF4MskG3oX2eIYVOBEya5SBiQI\n\t+fRFuW0n6fK0xPXa8TJP2ej0kf5Iy27lqQQIKi5rPA==","X-Google-Smtp-Source":"ABdhPJyo3lf2nA3fDMIljuTigIAdDhbjBinRAscfskYBXaoW6M4mcwyxmpsTsumJil/YHR45XWpBXDp+7VNbLMIy5KE=","X-Received":"by 2002:a92:c0d0:: with SMTP id\n\tt16mr26273367ilf.257.1620231640609; \n\tWed, 05 May 2021 09:20:40 -0700 (PDT)","MIME-Version":"1.0","References":"<20210503122518.745088-1-pnguyen@baylibre.com>\n\t<20210505154051.jw6dx2gcmrio35u6@pengutronix.de>","In-Reply-To":"<20210505154051.jw6dx2gcmrio35u6@pengutronix.de>","From":"Phi-bang Nguyen <pnguyen@baylibre.com>","Date":"Wed, 5 May 2021 18:20:29 +0200","Message-ID":"<CAO4hSriGP99ZH92XX+f7eqKP67ke9kgnuvOsKcST6-2uQYpcjw@mail.gmail.com>","To":"Marco Felsch <m.felsch@pengutronix.de>","Subject":"Re: [libcamera-devel] [PATCH] pipeline: simple: Rework the\n\tsupportedDevices list","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"multipart/mixed;\n\tboundary=\"===============2544272417187874330==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":16778,"web_url":"https://patchwork.libcamera.org/comment/16778/","msgid":"<YJMP6sVZrbAPgIMw@pendragon.ideasonboard.com>","date":"2021-05-05T21:36:42","subject":"Re: [libcamera-devel] [PATCH] pipeline: simple: Rework the\n\tsupportedDevices list","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Phi-Bang,\n\nOn Wed, May 05, 2021 at 06:20:29PM +0200, Phi-bang Nguyen wrote:\n> On Wed, May 5, 2021 at 5:40 PM Marco Felsch wrote:\n> > On 21-05-03 14:25, Phi-Bang Nguyen wrote:\n> > > There is a usecase that the same driver may go with a converter\n> > > on a platform and with another converter on another platform.\n> >\n> > What did you mean by \"another converter on another platform\"? What other\n> > convert should the platform take? IMHO if there are more than one\n> > convert than you should use a own pipeline handler.\n>\n> Thank you for your comment.\n> \n> The simple pipeline handler well fits our needs so we don't make a specific\n> pipeline handler.\n> Perhaps, it will be better if I describe with an example.\n> \n> We have a driver named \"mtk-seninf\" that uses the \"mtk-mdp\" converter on\n> the MT8167 platform and the \"mtk-mdp3\" converter on the MT8183 platform.\n> With the current code, if I add two lines to the supportedDevices list as\n> below :\n> \n> { \"mtk-seninf\", \"mtk-mdp\", 3 },\n>  { \"mtk-seninf\", \"mtk-mdp3\", 3 },\n> \n> On the MT8183 platform, it will not work because libcamera will find and\n> acquire the first entry : \"mtk-seninf\" together with \"mtk-mdp\" converter\n> and skip the rest.\n> So, that's the purpose of my patch.\n\nMakes sense to me.\n\n> > > Currently, the simple pipeline handler only takes the 1st driver\n> > > it can acquire in the supported devices list and skip the rest.\n> > > This makes it take the wrong converter for the above usecase.\n> > >\n> > > So, make the changes to support the above usecase.\n> > >\n> > > Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>\n> > > ---\n> > >  src/libcamera/pipeline/simple/simple.cpp | 34 +++++++++++++++---------\n> > >  1 file changed, 22 insertions(+), 12 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > > index f6095d38..9a572694 100644\n> > > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > > @@ -127,16 +127,19 @@ class SimplePipelineHandler;\n> > >\n> > >  struct SimplePipelineInfo {\n> > >       const char *driver;\n> > > -     const char *converter;\n> > > -     unsigned int numStreams;\n> > > +     /*\n> > > +      * Each converter in the list contains the name\n> > > +      * and the number of streams it supports.\n> > > +      */\n> > > +     std::vector<std::pair<const char *, unsigned int>> converters;\n> > >  };\n> > >\n> > >  namespace {\n> > >\n> > >  static const SimplePipelineInfo supportedDevices[] = {\n> > > -     { \"imx7-csi\", \"pxp\", 1 },\n> > > -     { \"qcom-camss\", nullptr, 1 },\n> > > -     { \"sun6i-csi\", nullptr, 1 },\n> > > +     { \"imx7-csi\", { { \"pxp\", 1 } } },\n> > > +     { \"qcom-camss\", { { nullptr, 1 } } },\n> > > +     { \"sun6i-csi\", { { nullptr, 1 } } },\n\nThere's no need to add an empty entry to the vector, you can write\n\n     { \"qcom-camss\", { } },\n     { \"sun6i-csi\", { } },\n\n> > >  };\n> > >\n> > >  } /* namespace */\n> > > @@ -145,7 +148,7 @@ class SimpleCameraData : public CameraData\n> > >  {\n> > >  public:\n> > >       SimpleCameraData(SimplePipelineHandler *pipe,\n> > > -                      const SimplePipelineInfo *info,\n> > > +                      unsigned int numStreams,\n> > >                        MediaEntity *sensor);\n> > >\n> > >       bool isValid() const { return sensor_ != nullptr; }\n> > > @@ -266,9 +269,9 @@ private:\n> > >   */\n> > >\n> > >  SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> > > -                                const SimplePipelineInfo *info,\n> > > +                                unsigned int numStreams,\n> > >                                  MediaEntity *sensor)\n> > > -     : CameraData(pipe), streams_(info->numStreams)\n> > > +     : CameraData(pipe), streams_(numStreams)\n> > >  {\n> > >       int ret;\n> > >\n> > > @@ -934,6 +937,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> > >  {\n> > >       const SimplePipelineInfo *info = nullptr;\n> > >       MediaDevice *converter = nullptr;\n> > > +     unsigned int numStreams = 1;\n> > >\n> > >       for (const SimplePipelineInfo &inf : supportedDevices) {\n> > >               DeviceMatch dm(inf.driver);\n> > > @@ -947,9 +951,15 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> > >       if (!media_)\n> > >               return false;\n> > >\n> > > -     if (info->converter) {\n> > > -             DeviceMatch converterMatch(info->converter);\n> > > -             converter = acquireMediaDevice(enumerator, converterMatch);\n> > > +     for (const auto &cvt : info->converters) {\n\nI'd write\n\n\tfor (const auto &[name, streams] : info->converters) {\n\nThe code will be more explicit as you can use name and streams instead\nof cvt.first and cvt.second.\n\n> > > +             if (cvt.first) {\n\nThis condition can be dropped if the empty vector entries are removed.\n\nOther than that, the patch looks good to me. Could you test those\nchanges and send a v2 ?\n\n> > > +                     DeviceMatch converterMatch(cvt.first);\n> > > +                     converter = acquireMediaDevice(enumerator, converterMatch);\n> > > +                     if (converter) {\n> > > +                             numStreams = cvt.second;\n> > > +                             break;\n> > > +                     }\n> > > +             }\n> > >       }\n> > >\n> > >       /* Locate the sensors. */\n> > > @@ -983,7 +993,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> > >\n> > >       for (MediaEntity *sensor : sensors) {\n> > >               std::unique_ptr<SimpleCameraData> data =\n> > > -                     std::make_unique<SimpleCameraData>(this, info, sensor);\n> > > +                     std::make_unique<SimpleCameraData>(this, numStreams, sensor);\n> > >               if (!data->isValid()) {\n> > >                       LOG(SimplePipeline, Error)\n> > >                               << \"No valid pipeline for sensor '\"","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 BEC62BDE79\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  5 May 2021 21:36:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1C99668915;\n\tWed,  5 May 2021 23:36:49 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9E71F602BD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  5 May 2021 23:36:47 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 16F71549;\n\tWed,  5 May 2021 23:36:47 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"c4ehXbnH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1620250607;\n\tbh=iCA06C8vnJlu3aicgyy0DH9vI7UxmVfbClU93Mulxpg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=c4ehXbnH5HZlqRU5Qu7UR5swsearrivaCjKepKccQheDxP4k0/uCpBLt1kISs9G3B\n\ttSiKu+RMTqJeiq6RhoO5tgvs6i4g76qycy0/6zgSzXpdbBo7osp62B6anM7mxTsJ6o\n\t3ojt7KzgDqrAM6afBLZtjxcmOlVtMsXobfxK4iWI=","Date":"Thu, 6 May 2021 00:36:42 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Phi-bang Nguyen <pnguyen@baylibre.com>","Message-ID":"<YJMP6sVZrbAPgIMw@pendragon.ideasonboard.com>","References":"<20210503122518.745088-1-pnguyen@baylibre.com>\n\t<20210505154051.jw6dx2gcmrio35u6@pengutronix.de>\n\t<CAO4hSriGP99ZH92XX+f7eqKP67ke9kgnuvOsKcST6-2uQYpcjw@mail.gmail.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<CAO4hSriGP99ZH92XX+f7eqKP67ke9kgnuvOsKcST6-2uQYpcjw@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH] pipeline: simple: Rework the\n\tsupportedDevices list","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":16792,"web_url":"https://patchwork.libcamera.org/comment/16792/","msgid":"<20210506072706.cbf2w7cxjtu64osl@pengutronix.de>","date":"2021-05-06T07:27:06","subject":"Re: [libcamera-devel] [PATCH] pipeline: simple: Rework the\n\tsupportedDevices list","submitter":{"id":89,"url":"https://patchwork.libcamera.org/api/people/89/","name":"Marco Felsch","email":"m.felsch@pengutronix.de"},"content":"Hi Laurent, Phi-Bang,\n\nOn 21-05-06 00:36, Laurent Pinchart wrote:\n> Hi Phi-Bang,\n> \n> On Wed, May 05, 2021 at 06:20:29PM +0200, Phi-bang Nguyen wrote:\n> > On Wed, May 5, 2021 at 5:40 PM Marco Felsch wrote:\n> > > On 21-05-03 14:25, Phi-Bang Nguyen wrote:\n> > > > There is a usecase that the same driver may go with a converter\n> > > > on a platform and with another converter on another platform.\n> > >\n> > > What did you mean by \"another converter on another platform\"? What other\n> > > convert should the platform take? IMHO if there are more than one\n> > > convert than you should use a own pipeline handler.\n> >\n> > Thank you for your comment.\n> > \n> > The simple pipeline handler well fits our needs so we don't make a specific\n> > pipeline handler.\n> > Perhaps, it will be better if I describe with an example.\n> > \n> > We have a driver named \"mtk-seninf\" that uses the \"mtk-mdp\" converter on\n> > the MT8167 platform and the \"mtk-mdp3\" converter on the MT8183 platform.\n> > With the current code, if I add two lines to the supportedDevices list as\n> > below :\n> > \n> > { \"mtk-seninf\", \"mtk-mdp\", 3 },\n> >  { \"mtk-seninf\", \"mtk-mdp3\", 3 },\n> > \n> > On the MT8183 platform, it will not work because libcamera will find and\n> > acquire the first entry : \"mtk-seninf\" together with \"mtk-mdp\" converter\n> > and skip the rest.\n> > So, that's the purpose of my patch.\n> \n> Makes sense to me.\n\nAfter that explanation to me as well, should we add this example to the\ncommit message?\n\nRegards,\n  Marco\n\n> \n> > > > Currently, the simple pipeline handler only takes the 1st driver\n> > > > it can acquire in the supported devices list and skip the rest.\n> > > > This makes it take the wrong converter for the above usecase.\n> > > >\n> > > > So, make the changes to support the above usecase.\n> > > >\n> > > > Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>\n> > > > ---\n> > > >  src/libcamera/pipeline/simple/simple.cpp | 34 +++++++++++++++---------\n> > > >  1 file changed, 22 insertions(+), 12 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > > > index f6095d38..9a572694 100644\n> > > > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > > > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > > > @@ -127,16 +127,19 @@ class SimplePipelineHandler;\n> > > >\n> > > >  struct SimplePipelineInfo {\n> > > >       const char *driver;\n> > > > -     const char *converter;\n> > > > -     unsigned int numStreams;\n> > > > +     /*\n> > > > +      * Each converter in the list contains the name\n> > > > +      * and the number of streams it supports.\n> > > > +      */\n> > > > +     std::vector<std::pair<const char *, unsigned int>> converters;\n> > > >  };\n> > > >\n> > > >  namespace {\n> > > >\n> > > >  static const SimplePipelineInfo supportedDevices[] = {\n> > > > -     { \"imx7-csi\", \"pxp\", 1 },\n> > > > -     { \"qcom-camss\", nullptr, 1 },\n> > > > -     { \"sun6i-csi\", nullptr, 1 },\n> > > > +     { \"imx7-csi\", { { \"pxp\", 1 } } },\n> > > > +     { \"qcom-camss\", { { nullptr, 1 } } },\n> > > > +     { \"sun6i-csi\", { { nullptr, 1 } } },\n> \n> There's no need to add an empty entry to the vector, you can write\n> \n>      { \"qcom-camss\", { } },\n>      { \"sun6i-csi\", { } },\n> \n> > > >  };\n> > > >\n> > > >  } /* namespace */\n> > > > @@ -145,7 +148,7 @@ class SimpleCameraData : public CameraData\n> > > >  {\n> > > >  public:\n> > > >       SimpleCameraData(SimplePipelineHandler *pipe,\n> > > > -                      const SimplePipelineInfo *info,\n> > > > +                      unsigned int numStreams,\n> > > >                        MediaEntity *sensor);\n> > > >\n> > > >       bool isValid() const { return sensor_ != nullptr; }\n> > > > @@ -266,9 +269,9 @@ private:\n> > > >   */\n> > > >\n> > > >  SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> > > > -                                const SimplePipelineInfo *info,\n> > > > +                                unsigned int numStreams,\n> > > >                                  MediaEntity *sensor)\n> > > > -     : CameraData(pipe), streams_(info->numStreams)\n> > > > +     : CameraData(pipe), streams_(numStreams)\n> > > >  {\n> > > >       int ret;\n> > > >\n> > > > @@ -934,6 +937,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> > > >  {\n> > > >       const SimplePipelineInfo *info = nullptr;\n> > > >       MediaDevice *converter = nullptr;\n> > > > +     unsigned int numStreams = 1;\n> > > >\n> > > >       for (const SimplePipelineInfo &inf : supportedDevices) {\n> > > >               DeviceMatch dm(inf.driver);\n> > > > @@ -947,9 +951,15 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> > > >       if (!media_)\n> > > >               return false;\n> > > >\n> > > > -     if (info->converter) {\n> > > > -             DeviceMatch converterMatch(info->converter);\n> > > > -             converter = acquireMediaDevice(enumerator, converterMatch);\n> > > > +     for (const auto &cvt : info->converters) {\n> \n> I'd write\n> \n> \tfor (const auto &[name, streams] : info->converters) {\n> \n> The code will be more explicit as you can use name and streams instead\n> of cvt.first and cvt.second.\n> \n> > > > +             if (cvt.first) {\n> \n> This condition can be dropped if the empty vector entries are removed.\n> \n> Other than that, the patch looks good to me. Could you test those\n> changes and send a v2 ?\n> \n> > > > +                     DeviceMatch converterMatch(cvt.first);\n> > > > +                     converter = acquireMediaDevice(enumerator, converterMatch);\n> > > > +                     if (converter) {\n> > > > +                             numStreams = cvt.second;\n> > > > +                             break;\n> > > > +                     }\n> > > > +             }\n> > > >       }\n> > > >\n> > > >       /* Locate the sensors. */\n> > > > @@ -983,7 +993,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> > > >\n> > > >       for (MediaEntity *sensor : sensors) {\n> > > >               std::unique_ptr<SimpleCameraData> data =\n> > > > -                     std::make_unique<SimpleCameraData>(this, info, sensor);\n> > > > +                     std::make_unique<SimpleCameraData>(this, numStreams, sensor);\n> > > >               if (!data->isValid()) {\n> > > >                       LOG(SimplePipeline, Error)\n> > > >                               << \"No valid pipeline for sensor '\"\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart\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 E1B1EBDE7F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  6 May 2021 07:27:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5D9B668909;\n\tThu,  6 May 2021 09:27:10 +0200 (CEST)","from metis.ext.pengutronix.de (metis.ext.pengutronix.de\n\t[IPv6:2001:67c:670:201:290:27ff:fe1d:cc33])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 015C268909\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  6 May 2021 09:27:08 +0200 (CEST)","from pty.hi.pengutronix.de ([2001:67c:670:100:1d::c5])\n\tby metis.ext.pengutronix.de with esmtps\n\t(TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92)\n\t(envelope-from <mfe@pengutronix.de>)\n\tid 1leYPb-0001qo-79; Thu, 06 May 2021 09:27:07 +0200","from mfe by pty.hi.pengutronix.de with local (Exim 4.89)\n\t(envelope-from <mfe@pengutronix.de>)\n\tid 1leYPa-0005nc-IG; Thu, 06 May 2021 09:27:06 +0200"],"Date":"Thu, 6 May 2021 09:27:06 +0200","From":"Marco Felsch <m.felsch@pengutronix.de>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20210506072706.cbf2w7cxjtu64osl@pengutronix.de>","References":"<20210503122518.745088-1-pnguyen@baylibre.com>\n\t<20210505154051.jw6dx2gcmrio35u6@pengutronix.de>\n\t<CAO4hSriGP99ZH92XX+f7eqKP67ke9kgnuvOsKcST6-2uQYpcjw@mail.gmail.com>\n\t<YJMP6sVZrbAPgIMw@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<YJMP6sVZrbAPgIMw@pendragon.ideasonboard.com>","X-Sent-From":"Pengutronix Hildesheim","X-URL":"http://www.pengutronix.de/","X-IRC":"#ptxdist @freenode","X-Accept-Language":"de,en","X-Accept-Content-Type":"text/plain","X-Uptime":"09:25:42 up 154 days, 21:32, 45 users, load average: 0.10, 0.07, \n\t0.08","User-Agent":"NeoMutt/20170113 (1.7.2)","X-SA-Exim-Connect-IP":"2001:67c:670:100:1d::c5","X-SA-Exim-Mail-From":"mfe@pengutronix.de","X-SA-Exim-Scanned":"No (on metis.ext.pengutronix.de);\n\tSAEximRunCond expanded to false","X-PTX-Original-Recipient":"libcamera-devel@lists.libcamera.org","Subject":"Re: [libcamera-devel] [PATCH] pipeline: simple: Rework the\n\tsupportedDevices list","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":16814,"web_url":"https://patchwork.libcamera.org/comment/16814/","msgid":"<CAO4hSrg9HvTnRYk7iG-8urFHvP6L7OAdsN+=bnexK68eURS7ag@mail.gmail.com>","date":"2021-05-06T18:12:43","subject":"Re: [libcamera-devel] [PATCH] pipeline: simple: Rework the\n\tsupportedDevices list","submitter":{"id":79,"url":"https://patchwork.libcamera.org/api/people/79/","name":"Phi-bang Nguyen","email":"pnguyen@baylibre.com"},"content":"Hi Laurent,\n\nOn Thu, May 6, 2021 at 9:27 AM Marco Felsch <m.felsch@pengutronix.de> wrote:\n\n> Hi Laurent, Phi-Bang,\n>\n> On 21-05-06 00:36, Laurent Pinchart wrote:\n> > Hi Phi-Bang,\n> >\n> > On Wed, May 05, 2021 at 06:20:29PM +0200, Phi-bang Nguyen wrote:\n> > > On Wed, May 5, 2021 at 5:40 PM Marco Felsch wrote:\n> > > > On 21-05-03 14:25, Phi-Bang Nguyen wrote:\n> > > > > There is a usecase that the same driver may go with a converter\n> > > > > on a platform and with another converter on another platform.\n> > > >\n> > > > What did you mean by \"another converter on another platform\"? What\n> other\n> > > > convert should the platform take? IMHO if there are more than one\n> > > > convert than you should use a own pipeline handler.\n> > >\n> > > Thank you for your comment.\n> > >\n> > > The simple pipeline handler well fits our needs so we don't make a\n> specific\n> > > pipeline handler.\n> > > Perhaps, it will be better if I describe with an example.\n> > >\n> > > We have a driver named \"mtk-seninf\" that uses the \"mtk-mdp\" converter\n> on\n> > > the MT8167 platform and the \"mtk-mdp3\" converter on the MT8183\n> platform.\n> > > With the current code, if I add two lines to the supportedDevices list\n> as\n> > > below :\n> > >\n> > > { \"mtk-seninf\", \"mtk-mdp\", 3 },\n> > >  { \"mtk-seninf\", \"mtk-mdp3\", 3 },\n> > >\n> > > On the MT8183 platform, it will not work because libcamera will find\n> and\n> > > acquire the first entry : \"mtk-seninf\" together with \"mtk-mdp\"\n> converter\n> > > and skip the rest.\n> > > So, that's the purpose of my patch.\n> >\n> > Makes sense to me.\n>\n> After that explanation to me as well, should we add this example to the\n> commit message?\n>\n> Regards,\n>   Marco\n>\n> >\n> > > > > Currently, the simple pipeline handler only takes the 1st driver\n> > > > > it can acquire in the supported devices list and skip the rest.\n> > > > > This makes it take the wrong converter for the above usecase.\n> > > > >\n> > > > > So, make the changes to support the above usecase.\n> > > > >\n> > > > > Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>\n> > > > > ---\n> > > > >  src/libcamera/pipeline/simple/simple.cpp | 34\n> +++++++++++++++---------\n> > > > >  1 file changed, 22 insertions(+), 12 deletions(-)\n> > > > >\n> > > > > diff --git a/src/libcamera/pipeline/simple/simple.cpp\n> b/src/libcamera/pipeline/simple/simple.cpp\n> > > > > index f6095d38..9a572694 100644\n> > > > > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > > > > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > > > > @@ -127,16 +127,19 @@ class SimplePipelineHandler;\n> > > > >\n> > > > >  struct SimplePipelineInfo {\n> > > > >       const char *driver;\n> > > > > -     const char *converter;\n> > > > > -     unsigned int numStreams;\n> > > > > +     /*\n> > > > > +      * Each converter in the list contains the name\n> > > > > +      * and the number of streams it supports.\n> > > > > +      */\n> > > > > +     std::vector<std::pair<const char *, unsigned int>>\n> converters;\n> > > > >  };\n> > > > >\n> > > > >  namespace {\n> > > > >\n> > > > >  static const SimplePipelineInfo supportedDevices[] = {\n> > > > > -     { \"imx7-csi\", \"pxp\", 1 },\n> > > > > -     { \"qcom-camss\", nullptr, 1 },\n> > > > > -     { \"sun6i-csi\", nullptr, 1 },\n> > > > > +     { \"imx7-csi\", { { \"pxp\", 1 } } },\n> > > > > +     { \"qcom-camss\", { { nullptr, 1 } } },\n> > > > > +     { \"sun6i-csi\", { { nullptr, 1 } } },\n> >\n> > There's no need to add an empty entry to the vector, you can write\n> >\n> >      { \"qcom-camss\", { } },\n> >      { \"sun6i-csi\", { } },\n> >\n> > > > >  };\n> > > > >\n> > > > >  } /* namespace */\n> > > > > @@ -145,7 +148,7 @@ class SimpleCameraData : public CameraData\n> > > > >  {\n> > > > >  public:\n> > > > >       SimpleCameraData(SimplePipelineHandler *pipe,\n> > > > > -                      const SimplePipelineInfo *info,\n> > > > > +                      unsigned int numStreams,\n> > > > >                        MediaEntity *sensor);\n> > > > >\n> > > > >       bool isValid() const { return sensor_ != nullptr; }\n> > > > > @@ -266,9 +269,9 @@ private:\n> > > > >   */\n> > > > >\n> > > > >  SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> > > > > -                                const SimplePipelineInfo *info,\n> > > > > +                                unsigned int numStreams,\n> > > > >                                  MediaEntity *sensor)\n> > > > > -     : CameraData(pipe), streams_(info->numStreams)\n> > > > > +     : CameraData(pipe), streams_(numStreams)\n> > > > >  {\n> > > > >       int ret;\n> > > > >\n> > > > > @@ -934,6 +937,7 @@ bool\n> SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> > > > >  {\n> > > > >       const SimplePipelineInfo *info = nullptr;\n> > > > >       MediaDevice *converter = nullptr;\n> > > > > +     unsigned int numStreams = 1;\n> > > > >\n> > > > >       for (const SimplePipelineInfo &inf : supportedDevices) {\n> > > > >               DeviceMatch dm(inf.driver);\n> > > > > @@ -947,9 +951,15 @@ bool\n> SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> > > > >       if (!media_)\n> > > > >               return false;\n> > > > >\n> > > > > -     if (info->converter) {\n> > > > > -             DeviceMatch converterMatch(info->converter);\n> > > > > -             converter = acquireMediaDevice(enumerator,\n> converterMatch);\n> > > > > +     for (const auto &cvt : info->converters) {\n> >\n> > I'd write\n> >\n> >       for (const auto &[name, streams] : info->converters) {\n> >\n> > The code will be more explicit as you can use name and streams instead\n> > of cvt.first and cvt.second.\n> >\n> > > > > +             if (cvt.first) {\n> >\n> > This condition can be dropped if the empty vector entries are removed.\n> >\n> > Other than that, the patch looks good to me. Could you test those\n> > changes and send a v2 ?\n>\n> Thank you ! I made the suggested changes, tested it and pushed v2.\nI also reworked the commit message.\n\nRegards,\nPhi-Bang\n\n> > > > +                     DeviceMatch converterMatch(cvt.first);\n> > > > > +                     converter = acquireMediaDevice(enumerator,\n> converterMatch);\n> > > > > +                     if (converter) {\n> > > > > +                             numStreams = cvt.second;\n> > > > > +                             break;\n> > > > > +                     }\n> > > > > +             }\n> > > > >       }\n> > > > >\n> > > > >       /* Locate the sensors. */\n> > > > > @@ -983,7 +993,7 @@ bool\n> SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> > > > >\n> > > > >       for (MediaEntity *sensor : sensors) {\n> > > > >               std::unique_ptr<SimpleCameraData> data =\n> > > > > -                     std::make_unique<SimpleCameraData>(this,\n> info, sensor);\n> > > > > +                     std::make_unique<SimpleCameraData>(this,\n> numStreams, sensor);\n> > > > >               if (!data->isValid()) {\n> > > > >                       LOG(SimplePipeline, Error)\n> > > > >                               << \"No valid pipeline for sensor '\"\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart\n> >\n>\n> --\n> Pengutronix e.K.                           |                             |\n> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |\n> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |\n> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |\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 39DC8BF825\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  6 May 2021 18:12:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A98906890C;\n\tThu,  6 May 2021 20:12:57 +0200 (CEST)","from mail-io1-xd2c.google.com (mail-io1-xd2c.google.com\n\t[IPv6:2607:f8b0:4864:20::d2c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CCE2968901\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  6 May 2021 20:12:55 +0200 (CEST)","by mail-io1-xd2c.google.com with SMTP id a11so5683754ioo.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 06 May 2021 11:12:55 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=baylibre-com.20150623.gappssmtp.com\n\theader.i=@baylibre-com.20150623.gappssmtp.com\n\theader.b=\"q3E9tr0l\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=baylibre-com.20150623.gappssmtp.com; s=20150623;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=esYF9Uf0l86QW7xO7/pF+ebSvh6bAN0x8B8lfhz8lAg=;\n\tb=q3E9tr0lIRuEGe0PB+Hn1CdihTzBZZDAGGl2+IaXH+1jsr1cpCBcdg1hzRRZuu/do1\n\t0LDtmkQvHUGXoJtLULqh7NF8o9RNYlG5G+ru9MbJOPNve4KfI78y4tMsErmEL4sPGT5r\n\tzRBFua+1dZOcQDpkILzW/WmJwOnp8esDjc8XDnwVzDckXAXEqwspVJvj8CWzzk/2XiNe\n\tuTjk0cM701XM4Zpi0ZtAsMud2VAYHqrXabEmc9kgmE8B6c10Pp8R98fEoNgqakT48ypF\n\tlXOKiBxN5urKE2kvfPMcNtXCt0as3iDH+6xtIWloIiIycRsQ3r30vFTKV4o4QRHWZHJ+\n\tDakw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=esYF9Uf0l86QW7xO7/pF+ebSvh6bAN0x8B8lfhz8lAg=;\n\tb=Zbxw4n3qHyTe3LARb2gbDI3FHQIbc3TlU53KwHG5wcM/XaENwEqCr2Y92tFS+Ju6wC\n\tjIs8dPRTEitEkGyspu20c0KK4INcpXXAMs/S75nergysoD5e92f6xMzOWcz3yorvcw0U\n\tNOHxQqYz5Fo0rv5n+sJNEDN20ulyGtGr5VtDrp8z+BU8hA7wgOSSsUvj1NR36Z5ue/tb\n\tIJF4Ed1RpWADj1M/0RSWSi/5JSG/xMclhR0uwMueFxdkQctnSh+cLoJZoUFS2bAA4xZE\n\tBAotE36JapezPpxNdoEk5moMVbsaBHQ2Fak7DJCKfGVwEwIDHqu+Fv++TPa0JF1f7zYo\n\tM1xA==","X-Gm-Message-State":"AOAM530q7vBDBQXs6iw8n66YGbcmu/0MkBJ9ShpCp10vaCewFy6e+QdL\n\t4IBozE2/4Ylwvav/VkhDWPyr0vodhWkBwSkgJM65JA==","X-Google-Smtp-Source":"ABdhPJz+sUBPu9WFRZeAJnlmn1MdFwCCAK+X7EPVsrI74V8txJAeaWnRhAu66bLSMLZKasu+L+DwlvynN28VwfNVDLM=","X-Received":"by 2002:a02:b717:: with SMTP id\n\tg23mr5324937jam.109.1620324774669; \n\tThu, 06 May 2021 11:12:54 -0700 (PDT)","MIME-Version":"1.0","References":"<20210503122518.745088-1-pnguyen@baylibre.com>\n\t<20210505154051.jw6dx2gcmrio35u6@pengutronix.de>\n\t<CAO4hSriGP99ZH92XX+f7eqKP67ke9kgnuvOsKcST6-2uQYpcjw@mail.gmail.com>\n\t<YJMP6sVZrbAPgIMw@pendragon.ideasonboard.com>\n\t<20210506072706.cbf2w7cxjtu64osl@pengutronix.de>","In-Reply-To":"<20210506072706.cbf2w7cxjtu64osl@pengutronix.de>","From":"Phi-bang Nguyen <pnguyen@baylibre.com>","Date":"Thu, 6 May 2021 20:12:43 +0200","Message-ID":"<CAO4hSrg9HvTnRYk7iG-8urFHvP6L7OAdsN+=bnexK68eURS7ag@mail.gmail.com>","To":"Marco Felsch <m.felsch@pengutronix.de>","Subject":"Re: [libcamera-devel] [PATCH] pipeline: simple: Rework the\n\tsupportedDevices list","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"multipart/mixed;\n\tboundary=\"===============8004417778084935634==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]