diff mbox series

v4l2-ctl: process events before queues in stateful_m2m()

Message ID 20250124115606.734788-1-gnurou@gmail.com
State New
Headers show
Series v4l2-ctl: process events before queues in stateful_m2m() | expand

Commit Message

Alexandre Courbot Jan. 24, 2025, 11:56 a.m. UTC
Events can change the streaming state and affect our ability (or
willingness) to further dequeue/requeue buffers.

For instance, a V4L2_EVENT_EOS means that we will stop streaming, so we
shouldn't process the OUTPUT queue any further once we receive it.
However, the current code processes the queues before the events, and
this can lead to issues such as an OUTPUT buffer dequeue attempt being
performed even though there is no buffer currently queued, resulting in
an indefinitely blocking DQBUF.

The issue was difficult to spot because (I assume) this usecase has been
mostly tested with software drivers like vicodec, which publish their
side-effects (notably in terms of events) before the ioctl that caused
them returns. But as far as I can tell this is not a requirement, and
events can also be sent asynchronously. In this case, a race condition
can cause the decoding loop to iterate one extra time and perform the
unfortunate DQBUF.

The fix is simple: process events before queues, so that the latter are
immediately affected by the result of published events instead of on the
next iteration of the loop. Events are supposed to be higher priority
anyway, and this is already done in that order in streaming_set_cap(),
so this sounds reasonable to do in any case.

Signed-off-by: Alexandre Courbot <gnurou@gmail.com>
---
 utils/v4l2-ctl/v4l2-ctl-streaming.cpp | 54 +++++++++++++--------------
 1 file changed, 27 insertions(+), 27 deletions(-)
diff mbox series

Patch

diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
index 1068ef2e6..a565628d3 100644
--- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
@@ -2460,6 +2460,33 @@  static void stateful_m2m(cv4l_fd &fd, cv4l_queue &in, cv4l_queue &out,
 			return;
 		}
 
+		if (ex_fds && FD_ISSET(fd.g_fd(), ex_fds)) {
+			struct v4l2_event ev;
+
+			while (!fd.dqevent(ev)) {
+				if (ev.type == V4L2_EVENT_EOS) {
+					wr_fds = nullptr;
+					if (!verbose)
+						stderr_info("\n");
+					stderr_info("EOS EVENT\n");
+					fflush(stderr);
+				} else if (ev.type == V4L2_EVENT_SOURCE_CHANGE) {
+					if (!verbose)
+						stderr_info("\n");
+					stderr_info("SOURCE CHANGE EVENT\n");
+					in_source_change_event = true;
+
+					/*
+					 * if capture is not streaming, the
+					 * driver will not send a last buffer so
+					 * we set it here
+					 */
+					if (!cap_streaming)
+						last_buffer = true;
+				}
+			}
+		}
+
 		if (rd_fds && FD_ISSET(fd.g_fd(), rd_fds)) {
 			r = do_handle_cap(fd, in, fin, nullptr,
 					  count[CAP], fps_ts[CAP], fmt_in,
@@ -2495,33 +2522,6 @@  static void stateful_m2m(cv4l_fd &fd, cv4l_queue &in, cv4l_queue &out,
 			}
 		}
 
-		if (ex_fds && FD_ISSET(fd.g_fd(), ex_fds)) {
-			struct v4l2_event ev;
-
-			while (!fd.dqevent(ev)) {
-				if (ev.type == V4L2_EVENT_EOS) {
-					wr_fds = nullptr;
-					if (!verbose)
-						stderr_info("\n");
-					stderr_info("EOS EVENT\n");
-					fflush(stderr);
-				} else if (ev.type == V4L2_EVENT_SOURCE_CHANGE) {
-					if (!verbose)
-						stderr_info("\n");
-					stderr_info("SOURCE CHANGE EVENT\n");
-					in_source_change_event = true;
-
-					/*
-					 * if capture is not streaming, the
-					 * driver will not send a last buffer so
-					 * we set it here
-					 */
-					if (!cap_streaming)
-						last_buffer = true;
-				}
-			}
-		}
-
 		if (last_buffer) {
 			if (in_source_change_event) {
 				struct v4l2_control ctrl = {