[EXT] Re: [PATCH] libcamera: simple: Check all media devices matched the pipeline
Hui Fang
hui.fang at nxp.com
Fri Feb 28 08:42:35 CET 2025
Checked, my patch is similar as https://patchwork.libcamera.org/patch/20849/.
For the case to use 2 cameras at same time, if the specific "Camera" is passed to SimplePipelineHandler's interfaces, maybe should ok?
Need test to verity.
BRs,
Fang Hui
> -----Original Message-----
> From: Hui Fang
> Sent: 2025年2月28日 14:17
> To: 'Kieran Bingham' <kieran.bingham at ideasonboard.com>;
> libcamera-devel at lists.libcamera.org
> Cc: jacopo.mondi at ideasonboard.com; mzamazal at redhat.com;
> dan.scally at ideasonboard.com; Antoine Bouyer <antoine.bouyer at nxp.com>;
> Julien Vuillaumier <julien.vuillaumier at nxp.com>
> Subject: RE: [EXT] Re: [PATCH] libcamera: simple: Check all media devices
> matched the pipeline
>
> 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://patc/
> > h
> >
> 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
> > >
More information about the libcamera-devel
mailing list