Closed kitakar5525 closed 4 years ago
Yeah we're doing similar work. Here's my debugging prints (only the last call to setupLink()
:
[1:44:31.702099031] [12553] INFO MediaDevice media_device.cpp:806 /dev/media1[ipu3-imgu]: Source: ipu3-imgu 0[devnode: /dev/v4l-subdev4], Sink: ipu3-imgu 0 output[devnode: /dev/video6]
[1:44:31.702139553] [12553] INFO MediaDevice media_device.cpp:810 /dev/media1[ipu3-imgu]: Existing link flags: 3
[1:44:31.702177422] [12553] INFO MediaDevice media_device.cpp:813 /dev/media1[ipu3-imgu]: Requested link flags: 1
[1:44:31.702212204] [12553] INFO MediaDevice media_device.cpp:816 /dev/media1[ipu3-imgu]: File descriptor: 6
[1:44:31.702276285] [12553] INFO MediaDevice media_device.cpp:838 /dev/media1[ipu3-imgu]: points to: /dev/media12212204] [12553] INFO MediaDevice
[1:44:31.702308314] [12553] ERROR MediaDevice media_device.cpp:846 /dev/media1[ipu3-imgu]: Failed to setup link: Invalid argument
It's failing at the call to ioctl
:
int ret = ioctl(fd_, MEDIA_IOC_SETUP_LINK, &linkDesc);
if (ret) {
ret = -errno;
LOG(MediaDevice, Error)
<< "Failed to setup link: "
<< strerror(-ret);
return ret;
The link is flagged as Immutable already, but the flag it's setting is just an enable flag so that should be ok. So we need to figure out why it fails. I tried fetching the link to get the existing flags like this:
MediaLink *existinglink = link(nlink->source(), nlink->sink());
/* print existing links */
LOG(MediaDevice, Info)
<< " Existing link flags: " << existinglink->flags();
As you can see in my debug prints above, that reports a value of 3, I.E. 00000011. Flags are here in media.h:
#define MEDIA_LNK_FL_ENABLED (1 << 0)
#define MEDIA_LNK_FL_IMMUTABLE (1 << 1)
#define MEDIA_LNK_FL_DYNAMIC (1 << 2)
So, I think it's trying to enable an already enabled link. Not sure if that should matter. Alternatively, the call I make to the link
function doesn't look like it performs an ioctl
to get that link, so possibly it's mistaken in thinking that that exists. Might try using MEDIA_IOC_ENUM_LINKS to check for sure.
I wrote a reply but I'd made a mistake so I deleted it; sorry if you get a confusing email :P
I wrote some code to check the link exists via ioctl()
calls:
The debug output from cam
tells me that the link it's trying is between entity 1 and entity 19 of /dev/media1
, so checking entity 1 with my code shows:
djrscally@djrscally-MIIX-510-12ISK:~/Coding/miix-510-cameras/utils$ ./debug /dev/media1 1
Media device info
driver: ipu3-imgu, model: ipu3-imgu, serial: , bus_info: PCI:0000:00:05.0
Listing entities
num entities: 12, num interfaces: 12, num pads: 20, num links: 22
Entity found: ipu3-imgu 0 (1)
Entity found: ipu3-imgu 0 input (7)
Entity found: ipu3-imgu 0 parameters (13)
Entity found: ipu3-imgu 0 output (19)
Entity found: ipu3-imgu 0 viewfinder (25)
Entity found: ipu3-imgu 0 3a stat (31)
Entity found: ipu3-imgu 1 (37)
Entity found: ipu3-imgu 1 input (43)
Entity found: ipu3-imgu 1 parameters (49)
Entity found: ipu3-imgu 1 output (55)
Entity found: ipu3-imgu 1 viewfinder (61)
Entity found: ipu3-imgu 1 3a stat (67)
Checking entity ipu3-imgu 0: num_pads: 5, num outbound links: 3
source id 1[2] -> sink id 19[0]
source id 1[3] -> sink id 25[0]
source id 1[4] -> sink id 31[0]
so there is a link ipu3-imgu 0:2 -> ipu3-imgu 0 output:0
. I tried setting that link with media-ctl -d /dev/media1 -l "'ipu3-imgu 0':2 -> 'ipu3-imgu 0 output':0[1]"
- that command completes silently, but also doesn't fix the problem. That's quite annoying though, because theoretically that's identical to the code the cam
program is running. Given it exists, I guess the cam
program must be failing because it's trying to set the configuration when it's flagged as immutable. The documentation does specify that:
Links marked with the IMMUTABLE link flag can not be enabled or disabled.
Need to do a bit of reading to see how to de-set that flag, or maybe just modify libcamera to do nothing when the link is IMMUTABLE.
EDIT:
I tried checking the value of IMMUTABLE in the existing link before allowing the call to ioctl
, and that does indeed move past this problem:
if ((existinglink->flags() & 0x02) == 0) { /* only proceed if the IMMUTABLE bit is 0 */
int ret = ioctl(fd_, MEDIA_IOC_SETUP_LINK, &linkDesc);
if (ret) {
ret = -errno;
LOG(MediaDevice, Error)
<< "Failed to setup link: "
<< strerror(-ret)
<< " " << ret << " " << -errno;
return ret;
}
}
and running cam -c1 -C
now gives me:
[3:30:08.361731863] [15695] INFO MediaDevice media_device.cpp:806 /dev/media1[ipu3-imgu]: Source: ipu3-imgu 0 (1)devnode: /dev/v4l-subdev4
[3:30:08.361769021] [15695] INFO MediaDevice media_device.cpp:809 /dev/media1[ipu3-imgu]: Sink: ipu3-imgu 0 output (19)devnode: /dev/video6
[3:30:08.361817554] [15695] INFO MediaDevice media_device.cpp:812 /dev/media1[ipu3-imgu]: Existing link flags: 3
[3:30:08.361855222] [15695] INFO MediaDevice media_device.cpp:815 /dev/media1[ipu3-imgu]: Requested link flags: 1
[3:30:08.361896008] [15695] INFO MediaDevice media_device.cpp:818 /dev/media1[ipu3-imgu]: File descriptor: 6
[3:30:08.361980637] [15695] INFO MediaDevice media_device.cpp:840 /dev/media1[ipu3-imgu]: points to: /dev/media11896008] [15695] INFO MediaDevice
[3:30:08.362042073] [15695] INFO MediaDevice media_device.cpp:806 /dev/media1[ipu3-imgu]: Source: ipu3-imgu 0 (1)devnode: /dev/v4l-subdev4
[3:30:08.362146878] [15695] INFO MediaDevice media_device.cpp:809 /dev/media1[ipu3-imgu]: Sink: ipu3-imgu 0 viewfinder (25)devnode: /dev/video7
[3:30:08.362177526] [15695] INFO MediaDevice media_device.cpp:812 /dev/media1[ipu3-imgu]: Existing link flags: 0
[3:30:08.362227043] [15695] INFO MediaDevice media_device.cpp:815 /dev/media1[ipu3-imgu]: Requested link flags: 1
[3:30:08.362264799] [15695] INFO MediaDevice media_device.cpp:818 /dev/media1[ipu3-imgu]: File descriptor: 6
[3:30:08.362353125] [15695] INFO MediaDevice media_device.cpp:840 /dev/media1[ipu3-imgu]: points to: /dev/media12264799] [15695] INFO MediaDevice
[3:30:08.362400190] [15695] ERROR MediaDevice media_device.cpp:857 /dev/media1[ipu3-imgu]: Link established.
[3:30:08.362464674] [15695] INFO MediaDevice media_device.cpp:806 /dev/media1[ipu3-imgu]: Source: ipu3-imgu 0 (1)devnode: /dev/v4l-subdev4
[3:30:08.362515959] [15695] INFO MediaDevice media_device.cpp:809 /dev/media1[ipu3-imgu]: Sink: ipu3-imgu 0 3a stat (31)devnode: /dev/video8
[3:30:08.362554941] [15695] INFO MediaDevice media_device.cpp:812 /dev/media1[ipu3-imgu]: Existing link flags: 0
[3:30:08.362596076] [15695] INFO MediaDevice media_device.cpp:815 /dev/media1[ipu3-imgu]: Requested link flags: 1
[3:30:08.362635785] [15695] INFO MediaDevice media_device.cpp:818 /dev/media1[ipu3-imgu]: File descriptor: 6
[3:30:08.362716704] [15695] INFO MediaDevice media_device.cpp:840 /dev/media1[ipu3-imgu]: points to: /dev/media12635785] [15695] INFO MediaDevice
[3:30:08.362762532] [15695] ERROR MediaDevice media_device.cpp:857 /dev/media1[ipu3-imgu]: Link established.
Capture until user interrupts by SIGINT
[3:30:08.472019356] [15695] ERROR V4L2 v4l2_videodevice.cpp:1441 /dev/video1[cap]: Failed to queue buffer 0: Broken pipe
[3:30:08.516397665] [15695] ERROR V4L2 v4l2_videodevice.cpp:1441 /dev/video1[cap]: Failed to queue buffer 1: Broken pipe
[3:30:08.557717224] [15695] ERROR V4L2 v4l2_videodevice.cpp:1441 /dev/video1[cap]: Failed to queue buffer 2: Broken pipe
[3:30:08.562433084] [15695] ERROR V4L2 v4l2_videodevice.cpp:1441 /dev/video1[cap]: Failed to queue buffer 3: Broken pipe
Now this issue is resolved by your patch https://github.com/linux-surface/linux-surface/issues/91#issuecomment-679124112. Thanks! Closing this issue.
My pleasure! But note that after review by the libcamera guys, the patch did actually get changed to this:
The setupLink function fails (ioctl returns EINVAL) when it passes the
MEDIA_LNK_FL_ENABLE flag to a link that is already flagged with
MEDIA_LNK_FL_ENABLE and MEDIA_LNK_FL_IMMUTABLE. Contrast to media-ctl's
equivalent media_setup_link() which ORs the new flags with the existing
values. This patch modifies the behaviour of setupLink() to behave the
same as media_setup_link() in media-ctl.
Signed-off-by: Dan Scally <djrscally@gmail.com>
---
Changelog:
v3 - Moved the change to MediaLink::setEnabled()
v2 - Simplified by removing the call to link() to fetch a link that's
already passed as a parameter to the function.
src/libcamera/media_object.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp
index ce77a72..a2e6a0d 100644
--- a/src/libcamera/media_object.cpp
+++ b/src/libcamera/media_object.cpp
@@ -115,7 +115,8 @@ LOG_DECLARE_CATEGORY(MediaDevice)
*/
int MediaLink::setEnabled(bool enable)
{
- unsigned int flags = enable ? MEDIA_LNK_FL_ENABLED : 0;
+ unsigned int flags = (flags_ & ~MEDIA_LNK_FL_ENABLED)
+ | (enable ? MEDIA_LNK_FL_ENABLED : 0);
int ret = dev_->setupLink(this, flags);
if (ret)
--
2.25.1
It's just moving the flags setting to the function that calls the setupLink()
that we originally changed, because without that there's a chance it gets undone later.
Thanks!
Current status is, ov5693 isn't working yet.
libcamera
can't get images outputting thatFailed to setup link: Invalid argument
.0001-media_device-add-debug-print-in-setupLink.patch
```diff From a96cb7ece0050727e04bec30844d73156e0864c8 Mon Sep 17 00:00:00 2001 From: Tsuchiya Yuto
Date: Sat, 22 Aug 2020 23:42:53 +0900
Subject: [PATCH] media_device: add debug print in setupLink()
Signed-off-by: Tsuchiya Yuto
---
src/libcamera/media_device.cpp | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
index de18d57..e12db5e 100644
--- a/src/libcamera/media_device.cpp
+++ b/src/libcamera/media_device.cpp
@@ -802,9 +802,26 @@ int MediaDevice::setupLink(const MediaLink *link, unsigned int flags)
LOG(MediaDevice, Error)
<< "Failed to setup link: "
<< strerror(-ret);
+ LOG(MediaDevice, Error)
+ << "DEBUG: linkDesc.source.entity: " << linkDesc.source.entity;
+ LOG(MediaDevice, Error)
+ << "DEBUG: linkDesc.source.index: " << linkDesc.source.index;
+ LOG(MediaDevice, Error)
+ << "DEBUG: linkDesc.sink.entity: " << linkDesc.sink.entity;
+ LOG(MediaDevice, Error)
+ << "DEBUG: linkDesc.sink.index: " << linkDesc.sink.index;
return ret;
}
+ LOG(MediaDevice, Debug)
+ << "DEBUG: linkDesc.source.entity: " << linkDesc.source.entity;
+ LOG(MediaDevice, Debug)
+ << "DEBUG: linkDesc.source.index: " << linkDesc.source.index;
+ LOG(MediaDevice, Debug)
+ << "DEBUG: linkDesc.sink.entity: " << linkDesc.sink.entity;
+ LOG(MediaDevice, Debug)
+ << "DEBUG: linkDesc.sink.index: " << linkDesc.sink.index;
+
LOG(MediaDevice, Debug)
<< source->entity()->name() << "["
<< source->index() << "] -> "
--
2.28.0
```
Lines with
DEBUG:
are added by the above patch.Below is the full debug output with
LIBCAMERA_LOG_LEVELS="*:DEBUG"
: