| Message ID | 20250225021023.2112783-1-hui.fang@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
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@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://patchwork.libcamera.org/patch/20849/, 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 >
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@ideasonboard.com>
> Sent: 2025年2月28日 3:57
> To: Hui Fang <hui.fang@nxp.com>; libcamera-devel@lists.libcamera.org
> Cc: jacopo.mondi@ideasonboard.com; mzamazal@redhat.com;
> dan.scally@ideasonboard.com; Hui Fang <hui.fang@nxp.com>; Antoine
> Bouyer <antoine.bouyer@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@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
> >
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@ideasonboard.com>; > libcamera-devel@lists.libcamera.org > Cc: jacopo.mondi@ideasonboard.com; mzamazal@redhat.com; > dan.scally@ideasonboard.com; Antoine Bouyer <antoine.bouyer@nxp.com>; > Julien Vuillaumier <julien.vuillaumier@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@ideasonboard.com> > > Sent: 2025年2月28日 3:57 > > To: Hui Fang <hui.fang@nxp.com>; libcamera-devel@lists.libcamera.org > > Cc: jacopo.mondi@ideasonboard.com; mzamazal@redhat.com; > > dan.scally@ideasonboard.com; Hui Fang <hui.fang@nxp.com>; Antoine > > Bouyer <antoine.bouyer@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@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 > > >
Use "cam" to capture from 2 cameras at same time, ok.
export W=1920
export H=1080
cam \
--camera 1 \
--stream width=${W},height=${H},pixelformat=YUYV \
--file=frame-#.yuv \
--capture=5 \
--camera 2 \
--stream width=${W},height=${H},pixelformat=YUYV \
--file=frame-#.yuv \
--capture=5
Using camera /base/soc@0/bus@30800000/i2c@30a20000/ov5640_mipi2@3c as cam0
Using camera /base/soc@0/bus@30800000/i2c@30a30000/ov5640_mipi@3c as cam1
cam0: Capture 5 frames
cam1: Capture 5 frames
17475.769932 (0.00 fps) cam0-stream0 seq: 000000 bytesused: 4147200
17475.784579 (0.00 fps) cam1-stream0 seq: 000000 bytesused: 4147200
17475.817831 (30.07 fps) cam1-stream0 seq: 000001 bytesused: 4147200
17475.836441 (15.04 fps) cam0-stream0 seq: 000002 bytesused: 4147200
17475.851084 (30.07 fps) cam1-stream0 seq: 000002 bytesused: 4147200
17475.869695 (30.07 fps) cam0-stream0 seq: 000003 bytesused: 4147200
17475.884338 (30.07 fps) cam1-stream0 seq: 000003 bytesused: 4147200
17475.902949 (30.07 fps) cam0-stream0 seq: 000004 bytesused: 4147200
17475.936202 (30.07 fps) cam0-stream0 seq: 000005 bytesused: 4147200
17475.984099 (10.02 fps) cam1-stream0 seq: 000006 bytesused: 4147200
BRs,
Fang Hui
> -----Original Message-----
> From: Hui Fang
> Sent: 2025年2月28日 15:43
> To: 'Kieran Bingham' <kieran.bingham@ideasonboard.com>;
> 'libcamera-devel@lists.libcamera.org' <libcamera-devel@lists.libcamera.org>
> Cc: 'jacopo.mondi@ideasonboard.com' <jacopo.mondi@ideasonboard.com>;
> 'mzamazal@redhat.com' <mzamazal@redhat.com>;
> 'dan.scally@ideasonboard.com' <dan.scally@ideasonboard.com>; Antoine
> Bouyer <antoine.bouyer@nxp.com>; Julien Vuillaumier
> <julien.vuillaumier@nxp.com>
> Subject: RE: [EXT] Re: [PATCH] libcamera: simple: Check all media devices
> matched the pipeline
>
> Checked, my patch is similar as
> 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.
>
> 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@ideasonboard.com>;
> > libcamera-devel@lists.libcamera.org
> > Cc: jacopo.mondi@ideasonboard.com; mzamazal@redhat.com;
> > dan.scally@ideasonboard.com; Antoine Bouyer
> <antoine.bouyer@nxp.com>;
> > Julien Vuillaumier <julien.vuillaumier@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
> > pipe->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@ideasonboard.com>
> > > Sent: 2025年2月28日 3:57
> > > To: Hui Fang <hui.fang@nxp.com>; libcamera-devel@lists.libcamera.org
> > > Cc: jacopo.mondi@ideasonboard.com; mzamazal@redhat.com;
> > > dan.scally@ideasonboard.com; Hui Fang <hui.fang@nxp.com>; Antoine
> > > Bouyer <antoine.bouyer@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@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%2Fpa
> > > tc
> > > 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
> > > >
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); + } + + return matched; +} + V4L2VideoDevice *SimplePipelineHandler::video(const MediaEntity *entity) { auto iter = entities_.find(entity);
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@nxp.com> --- src/libcamera/pipeline/simple/simple.cpp | 50 +++++++++++++++++------- 1 file changed, 35 insertions(+), 15 deletions(-)