[EXT] Re: [PATCH] libcamera: simple: Check all media devices matched the pipeline

[Hui Fang]

Hi, Bingham
Thanks for your reminding.

The patch is to fix below issue:
wevk_8mq has 2 camera slots, if just 1 slot plug sensor, if may mis-discovered.
There's 2 media deivces, suggest mediaA(with sensor), mediaB(no sensor). If mediaB is in the 1st place (depends of readdir()) of the enumerator,
pipe->match(enumerator_.get() return false and mediaA has no chance to be discovered.

Ref code in camera_manager.cpp	
void CameraManager::Private::pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory)
{
	CameraManager *const o = LIBCAMERA_O_PTR();

	/* Provide as many matching pipelines as possible. */
	while (1) {
		std::shared_ptr<PipelineHandler> pipe = factory->create(o);
		if (!pipe->match(enumerator_.get()))
			break;

		LOG(Camera, Debug)
			<< "Pipeline handler \"" << factory->name()
			<< "\" matched";
	}
}


I tested on android, APP/CTS just switch back/front camera, no case to use 2 cameras at same time.
Since the specific "Camera" is register to CameraManager and passed to SimplePipelineHandler, so no issue.

But if there's case to use 2 cameras at same time, should have issue.

Maybe the best resolution is in driver, don't create a media device if no sensor plugged in the media graph.


BRs,
Fang Hui

> -----Original Message-----
> From: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Sent: 2025年2月28日 3:57
> To: Hui Fang <hui.fang at nxp.com>; libcamera-devel at lists.libcamera.org
> Cc: jacopo.mondi at ideasonboard.com; mzamazal at redhat.com;
> dan.scally at ideasonboard.com; Hui Fang <hui.fang at nxp.com>; Antoine
> Bouyer <antoine.bouyer at nxp.com>
> Subject: [EXT] Re: [PATCH] libcamera: simple: Check all media devices
> matched the pipeline
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> Hi Fang,
> 
> Quoting Fang Hui (2025-02-25 02:10:23)
> > The current implementation of SimplePipelineHandler::match() will
> > return false once meet an un-complete media device such as no sensor
> > plugged. Thus the rest media devices will lost the chance to be discovered.
> > To discover all the cameras present, this change instanciates the
> > cameras discovery procedure for each media device that contains a
> supported camera port.
> >
> > Signed-off-by: Fang Hui <hui.fang at nxp.com>
> > ---
> >  src/libcamera/pipeline/simple/simple.cpp | 50
> > +++++++++++++++++-------
> >  1 file changed, 35 insertions(+), 15 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp
> > b/src/libcamera/pipeline/simple/simple.cpp
> > index 6e039bf3..134f0a07 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -379,6 +379,8 @@ private:
> >         const MediaPad *acquirePipeline(SimpleCameraData *data);
> >         void releasePipeline(SimpleCameraData *data);
> >
> > +       bool matchMedia(DeviceEnumerator *enumerator, const
> > + SimplePipelineInfo *info, MediaDevice *media);
> > +
> >         std::map<const MediaEntity *, EntityData> entities_;
> >
> >         MediaDevice *converter_;
> > @@ -1532,23 +1534,9 @@ int
> SimplePipelineHandler::resetRoutingTable(V4L2Subdevice *subdev)
> >         return 0;
> >  }
> >
> > -bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
> > +bool SimplePipelineHandler::matchMedia(DeviceEnumerator *enumerator,
> > +const SimplePipelineInfo *info, MediaDevice *media)
> >  {
> > -       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) {
> >                 DeviceMatch converterMatch(name); @@ -1678,6
> +1666,38
> > @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
> >         return registered;
> >  }
> >
> > +bool SimplePipelineHandler::match(DeviceEnumerator *enumerator) {
> > +       MediaDevice *media;
> > +       std::map<MediaDevice *, const SimplePipelineInfo *>
> > +mediaDevices;
> > +
> > +       for (const SimplePipelineInfo &inf : supportedDevices) {
> > +               LOG(SimplePipeline, Debug) << "check simple pipeline "
> << inf.driver;
> > +               DeviceMatch dm(inf.driver);
> > +
> > +               do {
> > +                       media = acquireMediaDevice(enumerator,
> dm);
> > +                       if (media) {
> > +                               mediaDevices[media] = &inf;
> > +                               LOG(SimplePipeline, Debug) <<
> "found media " << media->deviceNode();
> > +                       }
> > +               } while (media);
> > +       }
> > +
> > +       bool matched = false;
> > +
> > +       for (auto it = mediaDevices.begin(); it != mediaDevices.end(); it++)
> {
> > +               MediaDevice *media = it->first;
> > +               const SimplePipelineInfo *info = it->second;
> > +               LOG(SimplePipeline, Debug)
> > +                       << "call matchMedia for pipeline "
> > +                       << info->driver << ", media " <<
> media->deviceNode();
> > +               matched |= matchMedia(enumerator, info, media);
> > +       }
> 
> I'm not 100% sure I understand the implications here. I also tried to tackle this
> topic at
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> work.libcamera.org%2Fpatch%2F20849%2F&data=05%7C02%7Chui.fang%40
> nxp.com%7C7df77e1abad34fc20d4908dd5768dfd0%7C686ea1d3bc2b4c6fa9
> 2cd99c5c301635%7C0%7C0%7C638762830140008274%7CUnknown%7CTW
> FpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW
> 4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=Y1vZOi
> FLPsKslaQ9QjnsgWzAXfVdy6NZ%2BpZc1Gedl%2Fo%3D&reserved=0, and I
> see your version differs slightly that first you will try to find all compatible
> graphs, but then match each of the successfully acquired ones.
> 
> But I think this will be problematic as you will then register (or try to register)
> multiple graphs in a single pipeline handler ?
> 
> Have you checked what happens here if there are more than one simple
> pipeline handler supported in the system ?
> 
> 
> 
> > +
> > +       return matched;
> > +}
> > +
> >  V4L2VideoDevice *SimplePipelineHandler::video(const MediaEntity
> > *entity)  {
> >         auto iter = entities_.find(entity);
> > --
> > 2.25.1
> >