v4l-helpers: Don't close the fd in {}_s_fd
PATCH 1/2 v4l-helpers: Don't close the fd in {}_s_fd
When creating a second node via copy or assignment:
node2 = node
The node being assigned to, i.e. node2, obtains the fd. This causes a later call to node2.media_open to close() the fd, thus unintendenly closing the original node fd, via the call path (e.g. for media devices):
node2.media_open
v4l_media_open
v4l_media_s_fdSimilar call paths apply for other device types. Fix this by removing the close in xxx_s_fd.
--- utils/common/v4l-helpers.h | 9 --------- 1 file changed, 9 deletions(-)
utils/common/v4l-helpers.h +0 -9
@@ -444,9 +444,6 @@ static inline int v4l_s_fd(struct v4l_fd *f, int fd, const char *devname, bool dstruct v4l2_queryctrl qc;struct v4l2_selection sel; -if (f->fd >=0 ) - f->close (f); - f->fd = fd; f->direct = direct;if (fd <0 )
@@ -492,9 +489,6 @@ static inline int v4l_open(struct v4l_fd *f, const char *devname, bool non_blockstatic inline int v4l_subdev_s_fd (struct v4l_fd *f,int fd,const char *devname) { -if (f->fd >=0 ) - f->close (f); - f->fd = fd; f->direct = false;if (fd <0 )
@@ -525,9 +519,6 @@ static inline int v4l_subdev_open(struct v4l_fd *f, const char *devname, bool nostatic inline int v4l_media_s_fd (struct v4l_fd *f,int fd,const char *devname) { -if (f->fd >=0 ) - f->close (f); - f->fd = fd; f->direct = false;if (fd <0 )
Signature · 1 line
2.18.0.rc2
PATCH 2/2 v4l-helpers: Fix EXPBUF queue type
v4l_queue_export_bufs uses the v4l_fd type when calling EXPBUF ioctl. However, this doesn't work on mem2mem where there are one capture queue and one output queue associated to the device.
The current code calls v4l_queue_export_bufs with the wrong type, failing as:
fail: v4l2-test-buffers.cpp(544): q_.export_bufs(node) test VIDIOC_EXPBUF: FAIL
Fix this by using the queue type instead.
--- utils/common/v4l-helpers.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
utils/common/v4l-helpers.h +1 -1
@@ -1633,7 +1633,7 @@ static inline int v4l_queue_export_bufs(struct v4l_fd *f, struct v4l_queue *q,unsigned b, p;int ret =0 ; - expbuf.type = exp_type ? : f->type; + expbuf.type = exp_type ? : q->type; expbuf.flags =O_RDWR ;memset (expbuf.reserved,0 ,sizeof (expbuf.reserved));for (b =0 ; b <v4l_queue_g_buffers (q); b++) {
Signature · 1 line
2.18.0.rc2
Re: [PATCH 2/2] v4l-helpers: Fix EXPBUF queue type
Quoted 5 lines · Ezequiel Garcia
v4l_queue_export_bufsuses thev4l_fdtype when calling EXPBUF ioctl. However, this doesn't work on mem2mem where there are one capture queue and one output queue associated to the device.
Quoted 7 lines
The current code calls
v4l_queue_export_bufswith the wrong type, failing as:fail: v4l2-test-buffers.cpp(544):
q_.export_bufs(node)testVIDIOC_EXPBUF: FAILFix this by using the queue type instead.
I changed this by requiring that the exp_type is provided by the caller.
Thanks for reporting this!
Regards,
Hans
Quoted 20 lines
Signed-off-by Ezequiel Garcia <[email protected]>--- utils/common/v4l-helpers.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)utils/common/v4l-helpers.h +1
-1@@ -1633,7 +1633,7 @@ static inline int v4l_queue_export_bufs(struct v4l_fd *f, struct v4l_queue *q,unsigned b, p;int ret =0 ; - expbuf.type = exp_type ? : f->type; + expbuf.type = exp_type ? : q->type; expbuf.flags =O_RDWR ;memset (expbuf.reserved,0 ,sizeof (expbuf.reserved));for (b =0 ; b <v4l_queue_g_buffers (q); b++) {
Quoted 9 lines · Ezequiel Garcia
When creating a second node via copy or assignment:
node2 = node
The node being assigned to, i.e. node2, obtains the fd. This causes a later call to node2.media_open to
close()the fd, thus unintendenly closing the original node fd, via the call path (e.g. for media devices):
Quoted 6 lines
node2.media_open
v4l_media_open v4l_media_s_fdSimilar call paths apply for other device types. Fix this by removing the close in
xxx_s_fd.
I fixed this in a different way by overloading the assignment operator and calling dup(fd). That solves this as well.
Regards,
Hans
Quoted 41 lines
Signed-off-by Ezequiel Garcia <[email protected]>--- utils/common/v4l-helpers.h | 9 --------- 1 file changed, 9 deletions(-)utils/common/v4l-helpers.h +0
-9@@ -444,9 +444,6 @@ static inline int v4l_s_fd(struct v4l_fd *f, int fd, const char *devname, bool dstruct v4l2_queryctrl qc;struct v4l2_selection sel; -if (f->fd >=0 ) - f->close (f); - f->fd = fd; f->direct = direct;if (fd <0 )@@ -492,9 +489,6 @@ static inline int v4l_open(struct v4l_fd *f, const char *devname, bool non_blockstatic inline int v4l_subdev_s_fd (struct v4l_fd *f,int fd,const char *devname) { -if (f->fd >=0 ) - f->close (f); - f->fd = fd; f->direct = false;if (fd <0 )@@ -525,9 +519,6 @@ static inline int v4l_subdev_open(struct v4l_fd *f, const char *devname, bool nostatic inline int v4l_media_s_fd (struct v4l_fd *f,int fd,const char *devname) { -if (f->fd >=0 ) - f->close (f); - f->fd = fd; f->direct = false;if (fd <0 )
Quoted 14 lines · Hans Verkuil
Quoted 13 lines · Ezequiel Garcia
When creating a second node via copy or assignment:
node2 = node
The node being assigned to, i.e. node2, obtains the fd. This causes a later call to node2.media_open to
close()the fd, thus unintendenly closing the original node fd, via the call path (e.g. for media devices):node2.media_open
v4l_media_open v4l_media_s_fd
Quoted 7 lines
Quoted 2 lines
Similar call paths apply for other device types. Fix this by removing the close in
xxx_s_fd.I fixed this in a different way by overloading the assignment operator
and calling dup(fd). That solves this as well.
Yes, but I am now seeing another EBADF error in the compliance run.
close(3) = 0openat(AT_FDCWD, "/dev/video2", O_RDWR) = 3
close(3) = 0ioctl(3, VIDIOC_QUERYCAP, 0x7ffe54788794) = -1 EBADF
close(3) = -1 EBADF
Let me see if I can dig it.
Hey Hans,
Quoted 14 lines · Hans Verkuil
Quoted 13 lines · Ezequiel Garcia
When creating a second node via copy or assignment:
node2 = node
The node being assigned to, i.e. node2, obtains the fd. This causes a later call to node2.media_open to
close()the fd, thus unintendenly closing the original node fd, via the call path (e.g. for media devices):node2.media_open
v4l_media_open v4l_media_s_fd
Quoted 7 lines
Quoted 2 lines
Similar call paths apply for other device types. Fix this by removing the close in
xxx_s_fd.I fixed this in a different way by overloading the assignment operator
and calling dup(fd). That solves this as well.
This patch is also needed to prevent the compliance tool from unintendenly closing a descriptor.
utils/common/v4l-helpers.h +0 -0
@@ -118,7 +118,11 @@ static inline int v4l_wrap_open(struct v4l_fd *f,
const char *file, int oflag, .
static inline int v4l_wrap_close(struct v4l_fd *f) {
- return close(f->fd);
+ int ret;
+
+ ret = close(f->fd); + f->fd = -1; + return ret;
}
static inline ssize_t v4l_wrap_read(struct v4l_fd *f, void *buffer,size_t n)
Regards,
Eze