[libcamera-devel] [PATCH] pipeline: simple: Rework the supportedDevices list

[Phi-bang Nguyen]

Hi Marco & Laurent,

On Wed, May 5, 2021 at 5:40 PM Marco Felsch <m.felsch at pengutronix.de> wrote:

> Hi,
>
> On 21-05-03 14:25, Phi-Bang Nguyen wrote:
> > There is a usecase that the same driver may go with a converter
> > on a platform and with another converter on another platform.
>
> What did you mean by "another converter on another platform"? What other
> convert should the platform take? IMHO if there are more than one
> convert than you should use a own pipeline handler.
>
>
Thank you for your comment.

The simple pipeline handler well fits our needs so we don't make a specific
pipeline handler.
Perhaps, it will be better if I describe with an example.

We have a driver named "mtk-seninf" that uses the "mtk-mdp" converter on
the MT8167 platform and the "mtk-mdp3" converter on the MT8183 platform.
With the current code, if I add two lines to the supportedDevices list as
below :

{ "mtk-seninf", "mtk-mdp", 3 },
 { "mtk-seninf", "mtk-mdp3", 3 },

On the MT8183 platform, it will not work because libcamera will find and
acquire the first entry : "mtk-seninf" together with "mtk-mdp" converter
and skip the rest.
So, that's the purpose of my patch.


Regards,
>   Marco
>

Best Regards,
Phi-Bang


> > Currently, the simple pipeline handler only takes the 1st driver
> > it can acquire in the supported devices list and skip the rest.
> > This makes it take the wrong converter for the above usecase.
> >
> > So, make the changes to support the above usecase.
> >
> > Signed-off-by: Phi-Bang Nguyen <pnguyen at baylibre.com>
> > ---
> >  src/libcamera/pipeline/simple/simple.cpp | 34 +++++++++++++++---------
> >  1 file changed, 22 insertions(+), 12 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp
> b/src/libcamera/pipeline/simple/simple.cpp
> > index f6095d38..9a572694 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -127,16 +127,19 @@ class SimplePipelineHandler;
> >
> >  struct SimplePipelineInfo {
> >       const char *driver;
> > -     const char *converter;
> > -     unsigned int numStreams;
> > +     /*
> > +      * Each converter in the list contains the name
> > +      * and the number of streams it supports.
> > +      */
> > +     std::vector<std::pair<const char *, unsigned int>> converters;
> >  };
> >
> >  namespace {
> >
> >  static const SimplePipelineInfo supportedDevices[] = {
> > -     { "imx7-csi", "pxp", 1 },
> > -     { "qcom-camss", nullptr, 1 },
> > -     { "sun6i-csi", nullptr, 1 },
> > +     { "imx7-csi", { { "pxp", 1 } } },
> > +     { "qcom-camss", { { nullptr, 1 } } },
> > +     { "sun6i-csi", { { nullptr, 1 } } },
> >  };
> >
> >  } /* namespace */
> > @@ -145,7 +148,7 @@ class SimpleCameraData : public CameraData
> >  {
> >  public:
> >       SimpleCameraData(SimplePipelineHandler *pipe,
> > -                      const SimplePipelineInfo *info,
> > +                      unsigned int numStreams,
> >                        MediaEntity *sensor);
> >
> >       bool isValid() const { return sensor_ != nullptr; }
> > @@ -266,9 +269,9 @@ private:
> >   */
> >
> >  SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
> > -                                const SimplePipelineInfo *info,
> > +                                unsigned int numStreams,
> >                                  MediaEntity *sensor)
> > -     : CameraData(pipe), streams_(info->numStreams)
> > +     : CameraData(pipe), streams_(numStreams)
> >  {
> >       int ret;
> >
> > @@ -934,6 +937,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator
> *enumerator)
> >  {
> >       const SimplePipelineInfo *info = nullptr;
> >       MediaDevice *converter = nullptr;
> > +     unsigned int numStreams = 1;
> >
> >       for (const SimplePipelineInfo &inf : supportedDevices) {
> >               DeviceMatch dm(inf.driver);
> > @@ -947,9 +951,15 @@ bool SimplePipelineHandler::match(DeviceEnumerator
> *enumerator)
> >       if (!media_)
> >               return false;
> >
> > -     if (info->converter) {
> > -             DeviceMatch converterMatch(info->converter);
> > -             converter = acquireMediaDevice(enumerator, converterMatch);
> > +     for (const auto &cvt : info->converters) {
> > +             if (cvt.first) {
> > +                     DeviceMatch converterMatch(cvt.first);
> > +                     converter = acquireMediaDevice(enumerator,
> converterMatch);
> > +                     if (converter) {
> > +                             numStreams = cvt.second;
> > +                             break;
> > +                     }
> > +             }
> >       }
> >
> >       /* Locate the sensors. */
> > @@ -983,7 +993,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator
> *enumerator)
> >
> >       for (MediaEntity *sensor : sensors) {
> >               std::unique_ptr<SimpleCameraData> data =
> > -                     std::make_unique<SimpleCameraData>(this, info,
> sensor);
> > +                     std::make_unique<SimpleCameraData>(this,
> numStreams, sensor);
> >               if (!data->isValid()) {
> >                       LOG(SimplePipeline, Error)
> >                               << "No valid pipeline for sensor '"
> > --
> > 2.25.1
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
> >
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210505/cf780b63/attachment-0001.htm>