-
Notifications
You must be signed in to change notification settings - Fork 246
fix: limit video device open attempts to prevent infinite loops #998
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
91e58d7
fix: limit video device open attempts to prevent infinite loops in st…
adamshiervani 444f765
fix: improve logging and diagnostics in video stream handling for bet…
adamshiervani 2d6c431
fix: enhance video stream cleanup and frame capture diagnostics for i…
adamshiervani File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -387,6 +387,10 @@ void *run_video_stream(void *arg) | |
|
|
||
| streaming_stopped = false; | ||
|
|
||
| // Prevent infinite loop: count how many times we've tried to open device in this thread | ||
| int open_attempts = 0; | ||
| const int MAX_OPEN_ATTEMPTS = 20; | ||
|
|
||
| while (streaming_flag) | ||
| { | ||
| if (detected_signal == false) | ||
|
|
@@ -395,14 +399,27 @@ void *run_video_stream(void *arg) | |
| continue; | ||
| } | ||
|
|
||
| open_attempts++; | ||
| log_info("==== OUTER LOOP ITERATION %d/%d (streaming_flag=%d, detected_signal=%d) ====", | ||
| open_attempts, MAX_OPEN_ATTEMPTS, streaming_flag, detected_signal); | ||
|
|
||
| if (open_attempts > MAX_OPEN_ATTEMPTS) { | ||
| log_error("VIDEO RESTART LOOP DETECTED! Opened device %d times with no success. " | ||
| "Exiting native process to trigger supervisor restart and eventual failsafe.", | ||
| MAX_OPEN_ATTEMPTS); | ||
| exit(1); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe better as |
||
| } | ||
|
|
||
| int video_dev_fd = open(VIDEO_DEV, O_RDWR); | ||
| if (video_dev_fd < 0) | ||
| { | ||
| log_error("failed to open video capture device %s: %s", VIDEO_DEV, strerror(errno)); | ||
| log_error("failed to open video capture device %s: %s (attempt %d)", | ||
| VIDEO_DEV, strerror(errno), open_attempts); | ||
| usleep(1000000); | ||
| continue; | ||
| } | ||
| log_info("opened video capture device %s", VIDEO_DEV); | ||
| log_info("opened video capture device %s (fd=%d, attempt=%d)", | ||
| VIDEO_DEV, video_dev_fd, open_attempts); | ||
|
|
||
| uint32_t width = detected_width; | ||
| uint32_t height = detected_height; | ||
|
|
@@ -510,24 +527,38 @@ void *run_video_stream(void *arg) | |
| close(video_dev_fd); | ||
| return (void *)errno; | ||
| } | ||
| log_info("VIDIOC_STREAMON successful (attempt=%d)", open_attempts); | ||
|
|
||
| // Check hardware status after starting stream | ||
| struct v4l2_control ctrl; | ||
| memset(&ctrl, 0, sizeof(ctrl)); | ||
| ctrl.id = V4L2_CID_DV_RX_POWER_PRESENT; | ||
| if (ioctl(video_dev_fd, VIDIOC_G_CTRL, &ctrl) == 0) { | ||
| log_info("Hardware status: DV_RX_POWER_PRESENT=%d", ctrl.value); | ||
| } | ||
|
|
||
| struct v4l2_plane tmp_plane; | ||
|
|
||
| // Set VENC parameters | ||
| log_info("Calling venc_start (attempt=%d)...", open_attempts); | ||
| int32_t bitrate = calculate_bitrate(quality_factor, width, height); | ||
| RK_S32 ret = venc_start(bitrate, bitrate * 2, width, height); | ||
| if (ret != RK_SUCCESS) | ||
| { | ||
| log_error("Set VENC parameters failed with %#x", ret); | ||
| log_error("venc_start FAILED with %#x (attempt=%d)", ret, open_attempts); | ||
| goto cleanup; | ||
| } | ||
| log_info("venc_start successful (attempt=%d)", open_attempts); | ||
|
|
||
| fd_set fds; | ||
| struct timeval tv; | ||
| int r; | ||
| uint32_t num = 0; | ||
| VIDEO_FRAME_INFO_S stFrame; | ||
|
|
||
| log_info("Entering frame capture loop (attempt=%d, fd=%d, detected_signal=%d, detected_width=%d, detected_height=%d)", | ||
| open_attempts, video_dev_fd, detected_signal, detected_width, detected_height); | ||
|
|
||
| while (streaming_flag) | ||
| { | ||
| FD_ZERO(&fds); | ||
|
|
@@ -538,7 +569,32 @@ void *run_video_stream(void *arg) | |
| r = select(video_dev_fd + 1, &fds, NULL, NULL, &tv); | ||
| if (r == 0) | ||
| { | ||
| log_info("select timeout"); | ||
| log_info("SELECT TIMEOUT! (num_frames=%u, open_attempts=%d/%d, detected_signal=%d)", | ||
| num, open_attempts, MAX_OPEN_ATTEMPTS, detected_signal); | ||
|
|
||
| // Diagnose WHY we timed out - check hardware state | ||
| struct v4l2_control diag_ctrl; | ||
| memset(&diag_ctrl, 0, sizeof(diag_ctrl)); | ||
| diag_ctrl.id = V4L2_CID_DV_RX_POWER_PRESENT; | ||
| if (ioctl(video_dev_fd, VIDIOC_G_CTRL, &diag_ctrl) == 0) { | ||
| log_info("At timeout: DV_RX_POWER_PRESENT=%d (signal present?)", diag_ctrl.value); | ||
| } else { | ||
| log_info("At timeout: Cannot query DV_RX_POWER_PRESENT: %s", strerror(errno)); | ||
| } | ||
|
|
||
| // Check if buffers are still queued | ||
| struct v4l2_buffer check_buf; | ||
| memset(&check_buf, 0, sizeof(check_buf)); | ||
| check_buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; | ||
| check_buf.memory = V4L2_MEMORY_DMABUF; | ||
| check_buf.index = 0; | ||
| struct v4l2_plane check_plane; | ||
| check_buf.m.planes = &check_plane; | ||
| check_buf.length = 1; | ||
| if (ioctl(video_dev_fd, VIDIOC_QUERYBUF, &check_buf) == 0) { | ||
| log_info("At timeout: buffer 0 flags=0x%x (queued?)", check_buf.flags); | ||
| } | ||
|
|
||
| break; | ||
| } | ||
| if (r == -1) | ||
|
|
@@ -547,7 +603,8 @@ void *run_video_stream(void *arg) | |
| { | ||
| continue; | ||
| } | ||
| log_error("select in video streaming"); | ||
| log_error("select error in video streaming: %s (open_attempts=%d/%d)", | ||
| strerror(errno), open_attempts, MAX_OPEN_ATTEMPTS); | ||
| break; | ||
| } | ||
| memset(&buf, 0, sizeof(buf)); | ||
|
|
@@ -557,9 +614,14 @@ void *run_video_stream(void *arg) | |
| buf.length = 1; | ||
| if (ioctl(video_dev_fd, VIDIOC_DQBUF, &buf) < 0) | ||
| { | ||
| log_error("VIDIOC_DQBUF failed: %s", strerror(errno)); | ||
| log_error("VIDIOC_DQBUF failed: %s (attempt=%d, num_frames=%u)", | ||
| strerror(errno), open_attempts, num); | ||
| break; | ||
| } | ||
|
|
||
| if (num == 0) { | ||
| log_info("FIRST FRAME SUCCESS! Video capture working (attempt=%d)", open_attempts); | ||
| } | ||
| log_trace("got frame, bytesused = %d", tmp_plane.bytesused); | ||
| memset(&stFrame, 0, sizeof(VIDEO_FRAME_INFO_S)); | ||
| MB_BLK blk = RK_NULL; | ||
|
|
@@ -600,7 +662,8 @@ void *run_video_stream(void *arg) | |
| log_error("failure VIDIOC_QBUF: %s", strerror(errno)); | ||
| } | ||
| cleanup: | ||
| log_info("cleaning up video capture device %s", VIDEO_DEV); | ||
| log_info("cleaning up video capture device %s (attempt=%d, num_frames=%u)", | ||
| VIDEO_DEV, open_attempts, num); | ||
| if (ioctl(video_dev_fd, VIDIOC_STREAMOFF, &type) < 0) | ||
| { | ||
| log_error("VIDIOC_STREAMOFF failed: %s", strerror(errno)); | ||
|
|
@@ -628,11 +691,25 @@ void *run_video_stream(void *arg) | |
| } | ||
| } | ||
|
|
||
| log_info("closing video capture device %s", VIDEO_DEV); | ||
| log_info("closing video capture device %s (attempt=%d completed)", VIDEO_DEV, open_attempts); | ||
| close(video_dev_fd); | ||
|
|
||
| // If we successfully captured frames, reset the attempt counter | ||
| if (num > 30) { | ||
| log_info("Successfully captured %u frames, resetting attempt counter", num); | ||
| open_attempts = 0; | ||
| } | ||
| // If we got 0 frames 3 times in a row, clear the stale signal flag | ||
| else if (num == 0 && open_attempts >= 3) { | ||
| log_warn("3 consecutive zero-frame timeouts - clearing stale detected_signal"); | ||
| detected_signal = false; | ||
| } | ||
|
|
||
| log_info("==== OUTER LOOP END: going back to start (streaming_flag=%d) ====", streaming_flag); | ||
| } | ||
|
|
||
| log_info("video stream thread exiting"); | ||
| log_info("video stream thread exiting (total_attempts=%d, streaming_flag=%d)", | ||
| open_attempts, streaming_flag); | ||
|
|
||
| streaming_stopped = true; | ||
|
|
||
|
|
@@ -671,11 +748,15 @@ void video_start_streaming() | |
| if (streaming_thread != NULL) | ||
| { | ||
| if (streaming_stopped == true) { | ||
| log_error("video streaming already stopped but streaming_thread is not NULL"); | ||
| assert(streaming_stopped == true); | ||
| log_info("cleaning up stopped thread before starting new one"); | ||
| pthread_join(*streaming_thread, NULL); | ||
| free(streaming_thread); | ||
| streaming_thread = NULL; | ||
| // Fall through to start new thread | ||
| } else { | ||
| log_warn("video streaming already started"); | ||
| return; | ||
| } | ||
| log_warn("video streaming already started"); | ||
| return; | ||
| } | ||
|
|
||
| pthread_t *new_thread = malloc(sizeof(pthread_t)); | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bool stream_on == false;