[PATCH v2 2/2] pipeline: simple: Fix matching with empty media graphs

[Kieran Bingham]

Quoting Paul Elder (2025-03-26 08:47:59)
> The match() function currently reports that it is not possible to create
> any cameras if it encounters an empty media graph.
> 
> Fix this by looping over all media graphs and only returning false when
> all of them fail to create a camera.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

I think that SoB only appeared because I might have sent this patch to
the list on your behalf in the past as
https://patchwork.libcamera.org/patch/20849/.

But there's no functional changes in here from me that I'm aware of.

Anyway,

Patch 1/2 in this series addresses Laurents comments on
https://patchwork.libcamera.org/patch/20849/, and I've seen this work on
a complex device so I think it's helpful to land it.


> ---
> Changes in v2:
> - prevent a mix of valid and invalid media devices from ending up in
>   mediaDevices_
> ---
>  src/libcamera/pipeline/simple/simple.cpp | 52 ++++++++++++++++--------
>  1 file changed, 35 insertions(+), 17 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 6e039bf35fc1..a6bdd6024a99 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -373,6 +373,9 @@ private:
>                 return static_cast<SimpleCameraData *>(camera->_d());
>         }
>  
> +       bool matchDevice(MediaDevice *media, const SimplePipelineInfo &info,
> +                        DeviceEnumerator *enumerator);
> +
>         std::vector<MediaEntity *> locateSensors(MediaDevice *media);
>         static int resetRoutingTable(V4L2Subdevice *subdev);
>  
> @@ -1532,25 +1535,13 @@ int SimplePipelineHandler::resetRoutingTable(V4L2Subdevice *subdev)
>         return 0;
>  }
>  
> -bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
> +bool SimplePipelineHandler::matchDevice(MediaDevice *media,
> +                                       const SimplePipelineInfo &info,
> +                                       DeviceEnumerator *enumerator)
>  {
> -       const SimplePipelineInfo *info = nullptr;
>         unsigned int numStreams = 1;
> -       MediaDevice *media;
> -
> -       for (const SimplePipelineInfo &inf : supportedDevices) {
> -               DeviceMatch dm(inf.driver);
> -               media = acquireMediaDevice(enumerator, dm);
> -               if (media) {
> -                       info = &inf;
> -                       break;
> -               }
> -       }
> -
> -       if (!media)
> -               return false;
>  
> -       for (const auto &[name, streams] : info->converters) {
> +       for (const auto &[name, streams] : info.converters) {
>                 DeviceMatch converterMatch(name);
>                 converter_ = acquireMediaDevice(enumerator, converterMatch);
>                 if (converter_) {
> @@ -1559,7 +1550,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
>                 }
>         }
>  
> -       swIspEnabled_ = info->swIspEnabled;
> +       swIspEnabled_ = info.swIspEnabled;
>  
>         /* Locate the sensors. */
>         std::vector<MediaEntity *> sensors = locateSensors(media);
> @@ -1678,6 +1669,33 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
>         return registered;
>  }
>  
> +bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
> +{
> +       MediaDevice *media;
> +
> +       for (const SimplePipelineInfo &inf : supportedDevices) {
> +               DeviceMatch dm(inf.driver);
> +               while ((media = acquireMediaDevice(enumerator, dm))) {
> +                       /*
> +                        * If match succeeds, return true to let match() be
> +                        * called again on a new instance of the pipeline
> +                        * handler. Otherwise keep looping until we do
> +                        * successfully match one (or run out).
> +                        */
> +                       if (matchDevice(media, inf, enumerator)) {
> +                               LOG(SimplePipeline, Debug)
> +                                       << "Matched on device: "
> +                                       << media->deviceNode();
> +                               return true;
> +                       }
> +               }
> +
> +               clearMediaDevices();

Hrm, I was about to add a tag, but now I'm doubting myself.

Should clearMediaDevices be here or in the loop above before the next
call to acquireMediaDevice() ?

Or does the functionality of acquireMediaDevice rely on the ones we
don't support already being acquired so we don't try to acquire them
again in an infinite loop ?

(i.e. - is the loop operator based on the enumerator, or something
imposed by the media devices?)



> +       }
> +
> +       return false;
> +}
> +
>  V4L2VideoDevice *SimplePipelineHandler::video(const MediaEntity *entity)
>  {
>         auto iter = entities_.find(entity);
> -- 
> 2.47.2
>