[libcamera-devel,v2] libcamera: device_enumerator: ensure deviceNode is not empty
diff mbox series

Message ID 20230609-empty-devnode-v2-1-82a11e74aafd@skidata.com
State Superseded
Headers show
Series
  • [libcamera-devel,v2] libcamera: device_enumerator: ensure deviceNode is not empty
Related show

Commit Message

Benjamin Bara June 9, 2023, 6:14 a.m. UTC
From: Benjamin Bara <benjamin.bara@skidata.com>

When activating both ISP nodes on the i.MX8MP, but only connecting one
camera sensor, libcamera aborts because it couldn't find the chosen
entity's device node:

[37:54:40.779902250] [3631] DEBUG DeviceEnumerator device_enumerator.cpp:252 Added device /dev/media1: rkisp1
[37:54:40.780196750] [3631] DEBUG DeviceEnumerator device_enumerator_udev.cpp:322 All dependencies for media device /dev/media0 found
[37:54:40.780237875] [3631] DEBUG DeviceEnumerator device_enumerator.cpp:252 Added device /dev/media0: rkisp1
[37:54:40.780505125] [3631] DEBUG Camera camera_manager.cpp:152 Found registered pipeline handler 'PipelineHandlerRkISP1'
[37:54:40.780599875] [3631] DEBUG DeviceEnumerator device_enumerator.cpp:312 Successful match for media device "rkisp1"
[37:54:40.780731375] [3631] ERROR V4L2 v4l2_device.cpp:93 'rkisp1_isp': Failed to open V4L2 device '': No such file or directory

Fix this by skipping empty device nodes:

[37:49:05.172672000] [3603] DEBUG DeviceEnumerator device_enumerator_udev.cpp:322 All dependencies for media device /dev/media1 found
[37:49:05.172720625] [3603] DEBUG DeviceEnumerator device_enumerator.cpp:256 Added device /dev/media1: rkisp1
[37:49:05.172973875] [3603] DEBUG DeviceEnumerator device_enumerator_udev.cpp:322 All dependencies for media device /dev/media0 found
[37:49:05.173012125] [3603] DEBUG DeviceEnumerator device_enumerator.cpp:256 Added device /dev/media0: rkisp1
[37:49:05.173281625] [3603] DEBUG Camera camera_manager.cpp:152 Found registered pipeline handler 'PipelineHandlerRkISP1'
[37:49:05.173376875] [3603] DEBUG DeviceEnumerator device_enumerator.cpp:107 Skip rkisp1_isp: no device node
[37:49:05.173414375] [3603] DEBUG DeviceEnumerator device_enumerator.cpp:316 Successful match for media device "rkisp1"
[37:49:05.173671250] [3603] DEBUG V4L2 v4l2_videodevice.cpp:632 /dev/video1[15:cap]: Opened device platform:rkisp1: rkisp1: rkisp1_stats
[37:49:05.173775125] [3603] DEBUG V4L2 v4l2_videodevice.cpp:632 /dev/video2[16:out]: Opened device platform:rkisp1: rkisp1: rkisp1_params
[37:49:05.173880500] [3603] DEBUG V4L2 v4l2_videodevice.cpp:632 /dev/video0[18:cap]: Opened device platform:rkisp1: rkisp1: rkisp1

Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com>
---
Hi!

Seems like something went wrong with the format of the log in V1 when
copying it.
---
Changes in v2:
- adapt commit message
- Link to v1: https://lists.libcamera.org/pipermail/libcamera-devel/2023-June/038181.html
---
 src/libcamera/device_enumerator.cpp | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)


---
base-commit: 0ee9339331c648232e87d2de2ccd5a92cc61cab2
change-id: 20230609-empty-devnode-36dd2647cde0

Best regards,

Comments

Kieran Bingham July 4, 2023, 10:16 p.m. UTC | #1
Quoting Benjamin Bara via libcamera-devel (2023-06-09 07:14:32)
> From: Benjamin Bara <benjamin.bara@skidata.com>
> 
> When activating both ISP nodes on the i.MX8MP, but only connecting one
> camera sensor, libcamera aborts because it couldn't find the chosen
> entity's device node:

This sounds annoying indeed. If there is a camera there - we should be
able to use it!


> 
> [37:54:40.779902250] [3631] DEBUG DeviceEnumerator device_enumerator.cpp:252 Added device /dev/media1: rkisp1
> [37:54:40.780196750] [3631] DEBUG DeviceEnumerator device_enumerator_udev.cpp:322 All dependencies for media device /dev/media0 found
> [37:54:40.780237875] [3631] DEBUG DeviceEnumerator device_enumerator.cpp:252 Added device /dev/media0: rkisp1
> [37:54:40.780505125] [3631] DEBUG Camera camera_manager.cpp:152 Found registered pipeline handler 'PipelineHandlerRkISP1'
> [37:54:40.780599875] [3631] DEBUG DeviceEnumerator device_enumerator.cpp:312 Successful match for media device "rkisp1"
> [37:54:40.780731375] [3631] ERROR V4L2 v4l2_device.cpp:93 'rkisp1_isp': Failed to open V4L2 device '': No such file or directory
> 
> Fix this by skipping empty device nodes:
> 
> [37:49:05.172672000] [3603] DEBUG DeviceEnumerator device_enumerator_udev.cpp:322 All dependencies for media device /dev/media1 found
> [37:49:05.172720625] [3603] DEBUG DeviceEnumerator device_enumerator.cpp:256 Added device /dev/media1: rkisp1
> [37:49:05.172973875] [3603] DEBUG DeviceEnumerator device_enumerator_udev.cpp:322 All dependencies for media device /dev/media0 found
> [37:49:05.173012125] [3603] DEBUG DeviceEnumerator device_enumerator.cpp:256 Added device /dev/media0: rkisp1
> [37:49:05.173281625] [3603] DEBUG Camera camera_manager.cpp:152 Found registered pipeline handler 'PipelineHandlerRkISP1'
> [37:49:05.173376875] [3603] DEBUG DeviceEnumerator device_enumerator.cpp:107 Skip rkisp1_isp: no device node
> [37:49:05.173414375] [3603] DEBUG DeviceEnumerator device_enumerator.cpp:316 Successful match for media device "rkisp1"
> [37:49:05.173671250] [3603] DEBUG V4L2 v4l2_videodevice.cpp:632 /dev/video1[15:cap]: Opened device platform:rkisp1: rkisp1: rkisp1_stats
> [37:49:05.173775125] [3603] DEBUG V4L2 v4l2_videodevice.cpp:632 /dev/video2[16:out]: Opened device platform:rkisp1: rkisp1: rkisp1_params
> [37:49:05.173880500] [3603] DEBUG V4L2 v4l2_videodevice.cpp:632 /dev/video0[18:cap]: Opened device platform:rkisp1: rkisp1: rkisp1
> 
> Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com>
> ---
> Hi!
> 
> Seems like something went wrong with the format of the log in V1 when
> copying it.
> ---
> Changes in v2:
> - adapt commit message
> - Link to v1: https://lists.libcamera.org/pipermail/libcamera-devel/2023-June/038181.html
> ---
>  src/libcamera/device_enumerator.cpp | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
> index f2e055de..f2d943d1 100644
> --- a/src/libcamera/device_enumerator.cpp
> +++ b/src/libcamera/device_enumerator.cpp
> @@ -100,9 +100,13 @@ bool DeviceMatch::match(const MediaDevice *device) const
>                 bool found = false;
>  
>                 for (const MediaEntity *entity : device->entities()) {
> -                       if (name == entity->name()) {
> +                       if (name == entity->name() && !entity->deviceNode().empty()) {
>                                 found = true;
>                                 break;
> +                       } else if (name == entity->name()) {
> +                               LOG(DeviceEnumerator, Debug)
> +                                       << "Skip " << entity->name()
> +                                       << ": no device node";

I think I'd be tempted to merge these two like this:

			if (name == entity->name()) {
				if (!entity->deviceNode().empty()) {
					found = true;
					break;
				} else {
					LOG(DeviceEnumerator, Debug)
						<< "Skip " << entity->name()
						<< ": no device node";
				}
			}


Just because that doesn't duplicate the if (name == entity->name())

This seems reasonable to me though - but it would help to see a media
graph to see what it's handling here.

Could you share the output of :

  media-ctl -d /dev/media0 --print-dot

please? (Where /dev/media0 might be different on your device)

My only worry is to be sure that this will be applicable for all
platforms in the same way without breaking anything but it seems
reasonable at the moment so:


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


>                         }
>                 }
>  
> 
> ---
> base-commit: 0ee9339331c648232e87d2de2ccd5a92cc61cab2
> change-id: 20230609-empty-devnode-36dd2647cde0
> 
> Best regards,
> -- 
> Benjamin Bara <benjamin.bara@skidata.com>
>

Patch
diff mbox series

diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
index f2e055de..f2d943d1 100644
--- a/src/libcamera/device_enumerator.cpp
+++ b/src/libcamera/device_enumerator.cpp
@@ -100,9 +100,13 @@  bool DeviceMatch::match(const MediaDevice *device) const
 		bool found = false;
 
 		for (const MediaEntity *entity : device->entities()) {
-			if (name == entity->name()) {
+			if (name == entity->name() && !entity->deviceNode().empty()) {
 				found = true;
 				break;
+			} else if (name == entity->name()) {
+				LOG(DeviceEnumerator, Debug)
+					<< "Skip " << entity->name()
+					<< ": no device node";
 			}
 		}