| Message ID | 20230607021029.30466-1-deborah.brouwer@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | v4l2-compliance: call select before dequeuing event | expand |
On 07/06/2023 04:10, Deborah Brouwer wrote: > When the streaming option is called with a stateful decoder, > testUserPtr() and testDmaBuf() will hang indefinitely when attempting to > dequeue a source change event. To prevent this call select() with a > timeout. > > Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com> > --- > It looks like this was the same issue that was previously fixed for testMmap() > in commit f0a5b17d9 ("v4l2-compliance: add timeout when waiting for event"). > > utils/v4l2-compliance/v4l2-test-buffers.cpp | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp > index a097a0ff..0396e17e 100644 > --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp > +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp > @@ -1746,8 +1746,17 @@ int testUserPtr(struct node *node, struct node *node_m2m_cap, unsigned frame_cou > > if (node->is_m2m) { > if (node->codec_mask & STATEFUL_DECODER) { > + int fd_flags = fcntl(node->g_fd(), F_GETFL); > + struct timeval tv = { 1, 0 }; > + fd_set efds; > v4l2_event ev; > > + fcntl(node->g_fd(), F_SETFL, fd_flags | O_NONBLOCK); > + FD_ZERO(&efds); > + FD_SET(node->g_fd(), &efds); > + ret = select(node->g_fd() + 1, nullptr, nullptr, &efds, &tv); > + fail_on_test_val(ret < 0, ret); > + fail_on_test(ret == 0); > fail_on_test(node->dqevent(ev)); This code keeps the fd in non-blocking mode. You need to add a fcntl(node->g_fd(), F_SETFL, fd_flags) line here. See also testMmap(). > fail_on_test(ev.type != V4L2_EVENT_SOURCE_CHANGE); > fail_on_test(!(ev.u.src_change.changes & V4L2_EVENT_SRC_CH_RESOLUTION)); > @@ -1949,8 +1958,17 @@ int testDmaBuf(struct node *expbuf_node, struct node *node, struct node *node_m2 > > if (node->is_m2m) { > if (node->codec_mask & STATEFUL_DECODER) { > + int fd_flags = fcntl(node->g_fd(), F_GETFL); > + struct timeval tv = { 1, 0 }; > + fd_set efds; > v4l2_event ev; > > + fcntl(node->g_fd(), F_SETFL, fd_flags | O_NONBLOCK); > + FD_ZERO(&efds); > + FD_SET(node->g_fd(), &efds); > + ret = select(node->g_fd() + 1, nullptr, nullptr, &efds, &tv); > + fail_on_test_val(ret < 0, ret); > + fail_on_test(ret == 0); > fail_on_test(node->dqevent(ev)); Ditto. > fail_on_test(ev.type != V4L2_EVENT_SOURCE_CHANGE); > fail_on_test(!(ev.u.src_change.changes & V4L2_EVENT_SRC_CH_RESOLUTION)); Regards, Hans
On Wed, Jun 07, 2023 at 01:13:49PM +0200, Hans Verkuil wrote: > On 07/06/2023 04:10, Deborah Brouwer wrote: > > When the streaming option is called with a stateful decoder, > > testUserPtr() and testDmaBuf() will hang indefinitely when attempting to > > dequeue a source change event. To prevent this call select() with a > > timeout. > > > > Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com> > > --- > > It looks like this was the same issue that was previously fixed for testMmap() > > in commit f0a5b17d9 ("v4l2-compliance: add timeout when waiting for event"). > > > > utils/v4l2-compliance/v4l2-test-buffers.cpp | 18 ++++++++++++++++++ > > 1 file changed, 18 insertions(+) > > > > diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp > > index a097a0ff..0396e17e 100644 > > --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp > > +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp > > @@ -1746,8 +1746,17 @@ int testUserPtr(struct node *node, struct node *node_m2m_cap, unsigned frame_cou > > > > if (node->is_m2m) { > > if (node->codec_mask & STATEFUL_DECODER) { > > + int fd_flags = fcntl(node->g_fd(), F_GETFL); > > + struct timeval tv = { 1, 0 }; > > + fd_set efds; > > v4l2_event ev; > > > > + fcntl(node->g_fd(), F_SETFL, fd_flags | O_NONBLOCK); > > + FD_ZERO(&efds); > > + FD_SET(node->g_fd(), &efds); > > + ret = select(node->g_fd() + 1, nullptr, nullptr, &efds, &tv); > > + fail_on_test_val(ret < 0, ret); > > + fail_on_test(ret == 0); > > fail_on_test(node->dqevent(ev)); > > This code keeps the fd in non-blocking mode. You need to add a > fcntl(node->g_fd(), F_SETFL, fd_flags) line here. See also testMmap(). Oh sorry yes I missed that line. I'll send v2 with it fixed. Related question: would you ever expect this test to do anything other than timeout? Probably I am missing something, but it seems like there are no output buffers being queued, so there could never be a source change event? I was just testing it with vicodec and I always get the dmesg error message "No buffer was provided with the request" and then it times out. > > > fail_on_test(ev.type != V4L2_EVENT_SOURCE_CHANGE); > > fail_on_test(!(ev.u.src_change.changes & V4L2_EVENT_SRC_CH_RESOLUTION)); > > @@ -1949,8 +1958,17 @@ int testDmaBuf(struct node *expbuf_node, struct node *node, struct node *node_m2 > > > > if (node->is_m2m) { > > if (node->codec_mask & STATEFUL_DECODER) { > > + int fd_flags = fcntl(node->g_fd(), F_GETFL); > > + struct timeval tv = { 1, 0 }; > > + fd_set efds; > > v4l2_event ev; > > > > + fcntl(node->g_fd(), F_SETFL, fd_flags | O_NONBLOCK); > > + FD_ZERO(&efds); > > + FD_SET(node->g_fd(), &efds); > > + ret = select(node->g_fd() + 1, nullptr, nullptr, &efds, &tv); > > + fail_on_test_val(ret < 0, ret); > > + fail_on_test(ret == 0); > > fail_on_test(node->dqevent(ev)); > > Ditto. > > > fail_on_test(ev.type != V4L2_EVENT_SOURCE_CHANGE); > > fail_on_test(!(ev.u.src_change.changes & V4L2_EVENT_SRC_CH_RESOLUTION)); > > Regards, > > Hans
diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp index a097a0ff..0396e17e 100644 --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp @@ -1746,8 +1746,17 @@ int testUserPtr(struct node *node, struct node *node_m2m_cap, unsigned frame_cou if (node->is_m2m) { if (node->codec_mask & STATEFUL_DECODER) { + int fd_flags = fcntl(node->g_fd(), F_GETFL); + struct timeval tv = { 1, 0 }; + fd_set efds; v4l2_event ev; + fcntl(node->g_fd(), F_SETFL, fd_flags | O_NONBLOCK); + FD_ZERO(&efds); + FD_SET(node->g_fd(), &efds); + ret = select(node->g_fd() + 1, nullptr, nullptr, &efds, &tv); + fail_on_test_val(ret < 0, ret); + fail_on_test(ret == 0); fail_on_test(node->dqevent(ev)); fail_on_test(ev.type != V4L2_EVENT_SOURCE_CHANGE); fail_on_test(!(ev.u.src_change.changes & V4L2_EVENT_SRC_CH_RESOLUTION)); @@ -1949,8 +1958,17 @@ int testDmaBuf(struct node *expbuf_node, struct node *node, struct node *node_m2 if (node->is_m2m) { if (node->codec_mask & STATEFUL_DECODER) { + int fd_flags = fcntl(node->g_fd(), F_GETFL); + struct timeval tv = { 1, 0 }; + fd_set efds; v4l2_event ev; + fcntl(node->g_fd(), F_SETFL, fd_flags | O_NONBLOCK); + FD_ZERO(&efds); + FD_SET(node->g_fd(), &efds); + ret = select(node->g_fd() + 1, nullptr, nullptr, &efds, &tv); + fail_on_test_val(ret < 0, ret); + fail_on_test(ret == 0); fail_on_test(node->dqevent(ev)); fail_on_test(ev.type != V4L2_EVENT_SOURCE_CHANGE); fail_on_test(!(ev.u.src_change.changes & V4L2_EVENT_SRC_CH_RESOLUTION));
When the streaming option is called with a stateful decoder, testUserPtr() and testDmaBuf() will hang indefinitely when attempting to dequeue a source change event. To prevent this call select() with a timeout. Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com> --- It looks like this was the same issue that was previously fixed for testMmap() in commit f0a5b17d9 ("v4l2-compliance: add timeout when waiting for event"). utils/v4l2-compliance/v4l2-test-buffers.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)