[PATCH v3 1/1] pipeline: simple: Fix matching with empty media graphs

[Kieran Bingham]

Quoting Paul Elder (2025-05-13 14:37:51)
> 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.
> 
> It is worth noting that an issue does exist when on a partial match that
> ends in an invalid match, any media devices that were acquired will stay
> acquired. This is not a new issue though, as any acquired media devices
> in general are not released until pipeline handler deconstruction. This
> requires a rework of how we do matching and pipeline handler
> construction, so it is captured in a comment.
> 
> In the meantime, this fix fixes a problem without increasing the net
> number of problems.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> ---
> Changes in v3:
> - removed clearMediaDevices()
>   - the reason is elaborated on in both the commit message and the todo
>     comment

I think this is the best we can do until we have a rework of the
DeviceEnumerator...


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> 
> Changes in v2:
> - added clearMediaDevices()
> ---
>  src/libcamera/pipeline/simple/simple.cpp | 62 +++++++++++++++++-------
>  1 file changed, 45 insertions(+), 17 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index efb07051b..4323472e1 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -427,6 +427,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);
>  
> @@ -1660,25 +1663,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_) {
> @@ -1687,7 +1678,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
>                 }
>         }
>  
> -       swIspEnabled_ = info->swIspEnabled;
> +       swIspEnabled_ = info.swIspEnabled;
>  
>         /* Locate the sensors. */
>         std::vector<MediaEntity *> sensors = locateSensors(media);
> @@ -1806,6 +1797,43 @@ 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;
> +                       }
> +
> +                       /*
> +                        * \todo We need to clear the list of media devices
> +                        * that we've already acquired in the event that we
> +                        * fail to create a camera. This requires a rework of
> +                        * DeviceEnumerator, or even how we create pipelines
> +                        * handlers. This is because at the moment acquired
> +                        * media devices are only released on pipeline handler
> +                        * deconstruction, and if we release them any earlier
> +                        * then DeviceEnumerator::search() will keep returning
> +                        * the same media devices.
> +                        */
> +               }
> +       }
> +
> +       return false;
> +}
> +
>  V4L2VideoDevice *SimplePipelineHandler::video(const MediaEntity *entity)
>  {
>         auto iter = entities_.find(entity);
> -- 
> 2.39.2
>