v4l2-compliance: call select before dequeuing event
Commit Message
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(+)
Comments
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
@@ -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));