Don't block when unplugging camera #48
Conversation
|
Thanks! This is a rather invasive change so I'll probably need some more days to review everything. Side note: I wonder if it would be better to rely on the |
|
Take your time :). This is using the pselect of the libc crate: 324458d#diff-3580938c9bca860af321337225afa27f299ca34ebfb674d4d4a0a7c3f516331dR45 |
|
Have you had time to look at it yet? Is there any way that I can support you here? |
| @@ -354,13 +361,19 @@ impl io::Write for Device { | |||
| /// Acquiring a handle facilitates (possibly mutating) interactions with the device. | |||
| pub struct Handle { | |||
| fd: std::os::raw::c_int, | |||
| fd_set: FdSet, | |||
There was a problem hiding this comment.
I think I would prefer a lazy initialization approach: create the FdSet inside fd_set as needed and store it in an Option<FdSet> in Handle. An fd set is a pure function of the fd in our case and I feel like our code should reflect that.
There was a problem hiding this comment.
The problem if I understand your suggestion correctly is, that initializing the FdSet later requires mutable access to the Handl, but it is stored in an Arc.
| } | ||
| } | ||
|
|
||
| pub fn make_timespec(duration: time::Duration) -> libc::timespec { |
There was a problem hiding this comment.
I added a commit for timeouts inside streams.
src/io/mmap/stream.rs
Outdated
| @@ -214,6 +243,15 @@ impl<'a, 'b> OutputStream<'b> for Stream<'a> { | |||
|
|
|||
| fn dequeue(&mut self) -> io::Result<usize> { | |||
| let mut v4l2_buf: v4l2_buffer; | |||
|
|
|||
| pselect( | |||
There was a problem hiding this comment.
What is the reason for doing pselect here?
IIRC, a device unplug event can affect the dequeue event of capture streams and the queue event of output streams, no?
There was a problem hiding this comment.
TBH, I put the select before each dequeue and queue because I thought waiting for readiness is always correct.
This resolves #16.
Additionally, are you interested in supporting a
timeoutfor polling the frames? Usingpselectthis is doable now.