| 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
> > > >
Hi, Kieran As you mentioned, there's a similar patch https://patchwork.libcamera.org/patch/20849/, is there plan to merge it? Also I have already tested that 2 sensors work ok at same time. Are there other concerns? Thanks! BRs, Fang Hui > -----Original Message----- > From: Hui Fang > Sent: 2025年2月28日 16:39 > 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 > > 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' <mailto:kieran.bingham@ideasonboard.com>; > > 'libcamera-devel@lists.libcamera.org' > > <mailto:libcamera-devel@lists.libcamera.org> > > Cc: 'jacopo.mondi@ideasonboard.com' > <mailto:jacopo.mondi@ideasonboard.com>; > > 'mzamazal@redhat.com' <mailto:mzamazal@redhat.com>; > > 'dan.scally@ideasonboard.com' <mailto:dan.scally@ideasonboard.com>; Antoine > > Bouyer <mailto:antoine.bouyer@nxp.com>; Julien Vuillaumier > > <mailto: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%2Fpatc > > 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. > > > > 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' <mailto:kieran.bingham@ideasonboard.com>; > > > mailto:libcamera-devel@lists.libcamera.org > > > Cc: mailto:jacopo.mondi@ideasonboard.com; mailto:mzamazal@redhat.com; > > > mailto:dan.scally@ideasonboard.com; Antoine Bouyer > > <mailto:antoine.bouyer@nxp.com>; > > > Julien Vuillaumier <mailto: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 > > > pipe->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 <mailto:kieran.bingham@ideasonboard.com> > > > > Sent: 2025年2月28日 3:57 > > > > To: Hui Fang <mailto:hui.fang@nxp.com>; > > > > mailto:libcamera-devel@lists.libcamera.org > > > > Cc: mailto:jacopo.mondi@ideasonboard.com; mailto:mzamazal@redhat.com; > > > > mailto:dan.scally@ideasonboard.com; Hui Fang <mailto:hui.fang@nxp.com>; Antoine > > > > Bouyer <mailto: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 <mailto: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%2F > > > > pa > > > > 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 > > > > >
Although one pipeline handler for 2 sensors, the specific "Camera" is passed to SimplePipelineHandler's interfaces, so will not mess up if 2 cameras work at same time. BRs, Fang Hui
Quoting Hui Fang (2025-06-04 10:21:17) > Although one pipeline handler for 2 sensors, the specific "Camera" is passed to SimplePipelineHandler's interfaces, so will not mess up if 2 cameras work at same time. > > BRs, > Fang Hui > > ________________________________ > From: Hui Fang > Sent: Wednesday, June 4, 2025 5:16 PM > 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 > > Hi, Kieran > As you mentioned, there's a similar patch https://patchwork.libcamera.org/patch/20849/, is there plan to merge it? > Also I have already tested that 2 sensors work ok at same time. Are there other concerns? Thanks! > I think the patch referenced above is now superceeded by https://patchwork.libcamera.org/patch/23363/ Could you test/review that one please? If that solves your issue - a reviewed-by / tested by tag would help get it merged. -- Kieran > > BRs, > Fang Hui > > > -----Original Message----- > > From: Hui Fang > > Sent: 2025$BG/(B2$B7n(B28$BF|(B 16:39 > > 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 > > > > 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$BG/(B2$B7n(B28$BF|(B 15:43 > > > To: 'Kieran Bingham' <mailto:kieran.bingham@ideasonboard.com>; > > > 'libcamera-devel@lists.libcamera.org' > > > <mailto:libcamera-devel@lists.libcamera.org> > > > Cc: 'jacopo.mondi@ideasonboard.com' > > <mailto:jacopo.mondi@ideasonboard.com>; > > > 'mzamazal@redhat.com' <mailto:mzamazal@redhat.com>; > > > 'dan.scally@ideasonboard.com' <mailto:dan.scally@ideasonboard.com>; Antoine > > > Bouyer <mailto:antoine.bouyer@nxp.com>; Julien Vuillaumier > > > <mailto: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%2Fpatc > > > 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. > > > > > > 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$BG/(B2$B7n(B28$BF|(B 14:17 > > > > To: 'Kieran Bingham' <mailto:kieran.bingham@ideasonboard.com>; > > > > mailto:libcamera-devel@lists.libcamera.org > > > > Cc: mailto:jacopo.mondi@ideasonboard.com; mailto:mzamazal@redhat.com; > > > > mailto:dan.scally@ideasonboard.com; Antoine Bouyer > > > <mailto:antoine.bouyer@nxp.com>; > > > > Julien Vuillaumier <mailto: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 > > > > pipe->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 <mailto:kieran.bingham@ideasonboard.com> > > > > > Sent: 2025$BG/(B2$B7n(B28$BF|(B 3:57 > > > > > To: Hui Fang <mailto:hui.fang@nxp.com>; > > > > > mailto:libcamera-devel@lists.libcamera.org > > > > > Cc: mailto:jacopo.mondi@ideasonboard.com; mailto:mzamazal@redhat.com; > > > > > mailto:dan.scally@ideasonboard.com; Hui Fang <mailto:hui.fang@nxp.com>; Antoine > > > > > Bouyer <mailto: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 <mailto: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%2F > > > > > pa > > > > > 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 > > > > > >
Hi, Kieran Applied https://patchwork.libcamera.org/patch/23363/, tested ok on evk_8mq plugged 1 ov5640. evk_8mq:/data/cam-test # ls /dev/media* /dev/media0 /dev/media1 evk_8mq:/data/cam-test # evk_8mq:/data/cam-test # cam -l Available cameras: 1: 'ov5640' (/base/soc@0/bus@30800000/i2c@30a20000/ov5640_mipi2@3c) evk_8mq:/data/cam-test # evk_8mq:/data/cam-test # logcat 06-07 16:36:42.696 1637 1637 I LIBCAMERA: [0:01:43.676715045] [1637] WARN IPAManager ipa_manager.cpp:148 No IPA found in '/vendor/lib64/ipa' 06-07 16:36:42.697 1637 1637 I LIBCAMERA: [0:01:43.677268965] [1637] INFO Camera camera_manager.cpp:326 libcamera v0.5.0+349-b920c09f-dirty 06-07 16:36:42.700 1637 1638 I LIBCAMERA: [0:01:43.680738405] [1638] INFO SimplePipeline simple.cpp:1686 No sensor found for /dev/media0 06-07 16:36:42.702 1637 1638 I LIBCAMERA: [0:01:43.682392365] [1638] WARN CameraSensor camera_sensor_legacy.cpp:354 'ov5640 0-003c': Recommended V4L2 control 0x009a0922 not supported 06-07 16:36:42.702 1637 1638 I LIBCAMERA: [0:01:43.682487405] [1638] WARN CameraSensor camera_sensor_legacy.cpp:426 'ov5640 0-003c': The sensor kernel driver needs to be fixed 06-07 16:36:42.702 1637 1638 I LIBCAMERA: [0:01:43.682568405] [1638] WARN CameraSensor camera_sensor_legacy.cpp:428 'ov5640 0-003c': See Documentation/sensor_driver_requirements.rst in the libcamera sources for more information 06-07 16:36:42.703 1637 1638 I LIBCAMERA: [0:01:43.683572445] [1638] WARN CameraSensor camera_sensor_legacy.cpp:594 'ov5640 0-003c': Failed to retrieve the camera location 06-07 16:36:42.703 1637 1638 I LIBCAMERA: [0:01:43.683642885] [1638] WARN CameraSensor camera_sensor_legacy.cpp:616 'ov5640 0-003c': Rotation control not available, default to 0 degrees 06-07 16:36:42.703 1637 1638 I LIBCAMERA: [0:01:43.683768525] [1638] WARN CameraSensor camera_sensor_legacy.cpp:501 'ov5640 0-003c': No sensor delays found in static properties. Assuming unverified defaults. BRs, Fang Hui > -----Original Message----- > > ________________________________ > > From: Hui Fang > > Sent: Wednesday, June 4, 2025 5:16 PM > > 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 > > > > Hi, Kieran > > As you mentioned, there's a similar patch > 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%7Cc8b78686e66346ad844508dda41c3442%7C686ea1d3bc2b4c6fa > 92cd99c5c301635%7C0%7C0%7C638847163234281141%7CUnknown%7CT > WFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXa > W4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=FIU > DlBN5dNHNzTSwzr9kvoQMijd2CRKfJEVBr0umiL4%3D&reserved=0, is there > plan to merge it? > > Also I have already tested that 2 sensors work ok at same time. Are there > other concerns? Thanks! > > > > I think the patch referenced above is now superceeded by > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch > work.libcamera.org%2Fpatch%2F23363%2F&data=05%7C02%7Chui.fang%40 > nxp.com%7Cc8b78686e66346ad844508dda41c3442%7C686ea1d3bc2b4c6fa > 92cd99c5c301635%7C0%7C0%7C638847163234328382%7CUnknown%7CT > WFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXa > W4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=7%2 > Bv5ikttk1l8AqL6T6EGdF99YM7v3WP4IzFNH6NW3xw%3D&reserved=0 > > Could you test/review that one please? If that solves your issue - a > reviewed-by / tested by tag would help get it merged. > > -- > Kieran
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(-)