ioctl/querybuf: make conversion from v4l2_buffer a fallible operation

Make the conversion from v4l2_buffer capable of failure. This allows us to validate the field when converting to a V4l2Buffer, which in turns allows us to return safe types in its methods later, at the cost of an added generic in the error types of operations that do the conversion.
lu-zero · Sep 6, 2023 · f5f8c34 · f5f8c34
1 parent 7cddcdc
commit f5f8c34
Show file tree

Hide file tree

Showing 9 changed files with 107 additions and 62 deletions.
diff --git a/lib/src/decoder/stateful.rs b/lib/src/decoder/stateful.rs
@@ -17,7 +17,10 @@ use crate::{
         },
         AllocatedQueue, Device, DeviceConfig, DeviceOpenError, Stream, TryDequeue,
     },
-    ioctl::{self, subscribe_event, BufferCapabilities, DqBufError, FormatFlags, StreamOnError},
+    ioctl::{
+        self, subscribe_event, BufferCapabilities, DqBufError, FormatFlags, StreamOnError,
+        V4l2Buffer,
+    },
     memory::{BufferHandles, PrimitiveBufferHandles},
 };
 
@@ -489,7 +492,7 @@ where
     }
 
     /// Attempts to dequeue and release output buffers that the driver is done with.
-    fn dequeue_output_buffers(&self) -> Result<(), DqBufError> {
+    fn dequeue_output_buffers(&self) -> Result<(), DqBufError<V4l2Buffer>> {
         let output_queue = &self.state.output_queue;
 
         while output_queue.num_queued_buffers() > 0 {
@@ -540,7 +543,7 @@ where
 #[derive(Debug, Error)]
 pub enum GetBufferError {
     #[error("error while dequeueing buffer")]
-    DequeueError(#[from] DqBufError),
+    DequeueError(#[from] DqBufError<V4l2Buffer>),
     #[error("error during poll")]
     PollError(#[from] PollError),
     #[error("error while obtaining buffer")]
@@ -625,7 +628,7 @@ where
     /// that owns the decoder every time a decoded frame is produced.
     /// That way the client can recycle its input buffers
     /// and the decoding process does not get stuck.
-    pub fn kick(&self) -> Result<(), DqBufError> {
+    pub fn kick(&self) -> Result<(), DqBufError<V4l2Buffer>> {
         info!("Kick!");
         self.dequeue_output_buffers()
     }

diff --git a/lib/src/device/queue.rs b/lib/src/device/queue.rs
@@ -8,6 +8,7 @@ pub mod qbuf;
 use self::qbuf::{get_free::GetFreeOutputBuffer, get_indexed::GetOutputBufferByIndex};
 
 use super::{AllocatedQueue, Device, FreeBuffersResult, Stream, TryDequeue};
+use crate::ioctl::V4l2Buffer;
 use crate::{bindings, memory::*};
 use crate::{
     ioctl::{
@@ -206,7 +207,7 @@ pub enum RequestBuffersError {
     #[error("error while requesting buffers")]
     ReqbufsError(#[from] ioctl::ReqbufsError),
     #[error("error while querying buffer")]
-    QueryBufferError(#[from] ioctl::QueryBufError),
+    QueryBufferError(#[from] ioctl::QueryBufError<QueryBuffer>),
 }
 
 impl<D: Direction> Queue<D, QueueInit> {
@@ -484,7 +485,7 @@ impl<D: Direction, P: BufferHandles> Stream for Queue<D, BuffersAllocated<P>> {
 impl<D: Direction, P: BufferHandles> TryDequeue for Queue<D, BuffersAllocated<P>> {
     type Dequeued = DqBuffer<D, P>;
 
-    fn try_dequeue(&self) -> DqBufResult<Self::Dequeued> {
+    fn try_dequeue(&self) -> DqBufResult<Self::Dequeued, V4l2Buffer> {
         let dqbuf: ioctl::V4l2Buffer = ioctl::dqbuf(&self.inner, self.inner.type_)?;
 
         let id = dqbuf.index() as usize;

diff --git a/lib/src/device/queue/qbuf.rs b/lib/src/device/queue/qbuf.rs
@@ -19,7 +19,7 @@ pub mod get_indexed;
 #[derive(Error)]
 #[error("{}", self.error)]
 pub struct QueueError<P: BufferHandles> {
-    pub error: ioctl::QBufError,
+    pub error: ioctl::QBufError<()>,
     pub plane_handles: P,
 }
 
@@ -272,7 +272,7 @@ impl<P: PrimitiveBufferHandles + Default, Q: BufferHandles + From<P>> QBuffer<'_
 where
     <P::HandleType as PlaneHandle>::Memory: SelfBacked,
 {
-    pub fn queue(self) -> Result<(), ioctl::QBufError> {
+    pub fn queue(self) -> Result<(), ioctl::QBufError<()>> {
         let planes: Vec<_> = (0..self.num_expected_planes())
             .map(|_| ioctl::QBufPlane::new(0))
             .collect();
@@ -290,7 +290,7 @@ impl<P: PrimitiveBufferHandles + Default, Q: BufferHandles + From<P>> QBuffer<'_
 where
     <P::HandleType as PlaneHandle>::Memory: SelfBacked,
 {
-    pub fn queue(self, bytes_used: &[usize]) -> Result<(), ioctl::QBufError> {
+    pub fn queue(self, bytes_used: &[usize]) -> Result<(), ioctl::QBufError<()>> {
         // TODO make specific error for bytes_used?
         if bytes_used.len() != self.num_expected_planes() {
             return Err(ioctl::QBufError::NumPlanesMismatch(

diff --git a/lib/src/device/traits.rs b/lib/src/device/traits.rs
@@ -1,7 +1,7 @@
 use ioctl::DqBufResult;
 
 use super::queue::{direction::Direction, Queue, QueueInit};
-use crate::ioctl;
+use crate::ioctl::{self, V4l2Buffer};
 use std::fmt::Debug;
 
 /// Trait for trying to dequeue a readable buffer from a queue.
@@ -14,7 +14,7 @@ pub trait TryDequeue {
     /// It can be moved into a `Rc` or `Arc` and passed across threads.
     ///
     /// The data in the `DQBuffer` is read-only.
-    fn try_dequeue(&self) -> DqBufResult<Self::Dequeued>;
+    fn try_dequeue(&self) -> DqBufResult<Self::Dequeued, V4l2Buffer>;
 }
 
 /// Trait for streaming a queue on and off.

diff --git a/lib/src/encoder.rs b/lib/src/encoder.rs
@@ -18,7 +18,7 @@ use crate::{
         },
         AllocatedQueue, Device, DeviceConfig, DeviceOpenError, Stream, TryDequeue,
     },
-    ioctl::{self, DqBufError, EncoderCommand, FormatFlags, GFmtError},
+    ioctl::{self, DqBufError, EncoderCommand, FormatFlags, GFmtError, V4l2Buffer},
     memory::{BufferHandles, PrimitiveBufferHandles},
     Format,
 };
@@ -313,7 +313,7 @@ pub enum CompletedOutputBuffer<OP: BufferHandles> {
 #[derive(Debug, Error)]
 pub enum GetBufferError {
     #[error("error while dequeueing buffer")]
-    DequeueError(#[from] DqBufError),
+    DequeueError(#[from] DqBufError<V4l2Buffer>),
     #[error("error during poll")]
     PollError(#[from] PollError),
     #[error("error while obtaining buffer")]
@@ -376,7 +376,7 @@ where
     }
 
     /// Attempts to dequeue and release output buffers that the driver is done with.
-    fn dequeue_output_buffers(&self) -> Result<(), DqBufError> {
+    fn dequeue_output_buffers(&self) -> Result<(), DqBufError<V4l2Buffer>> {
         let output_queue = &self.state.output_queue;
 
         while output_queue.num_queued_buffers() > 0 {

diff --git a/lib/src/ioctl.rs b/lib/src/ioctl.rs
@@ -249,12 +249,12 @@ impl V4l2Buffer {
         self.buffer.index
     }
 
-    pub fn queue_type(&self) -> u32 {
-        self.buffer.type_
+    pub fn queue_type(&self) -> QueueType {
+        QueueType::n(self.buffer.type_).unwrap()
     }
 
-    pub fn memory(&self) -> u32 {
-        self.buffer.memory
+    pub fn memory(&self) -> MemoryType {
+        MemoryType::n(self.buffer.memory).unwrap()
     }
 
     pub fn flags(&self) -> BufferFlags {

diff --git a/lib/src/ioctl/dqbuf.rs b/lib/src/ioctl/dqbuf.rs
@@ -18,7 +18,9 @@ mod ioctl {
 }
 
 #[derive(Debug, Error)]
-pub enum DqBufError {
+pub enum DqBufError<Q: QueryBuf> {
+    #[error("error while converting from v4l2_buffer: {0}")]
+    ConvertionError(Q::Error),
     #[error("end-of-stream reached")]
     Eos,
     #[error("no buffer ready for dequeue")]
@@ -27,7 +29,7 @@ pub enum DqBufError {
     IoctlError(Errno),
 }
 
-impl From<Errno> for DqBufError {
+impl<Q: QueryBuf> From<Errno> for DqBufError<Q> {
     fn from(error: Errno) -> Self {
         match error {
             Errno::EAGAIN => Self::NotReady,
@@ -37,20 +39,21 @@ impl From<Errno> for DqBufError {
     }
 }
 
-impl From<DqBufError> for Errno {
-    fn from(err: DqBufError) -> Self {
+impl<Q: QueryBuf> From<DqBufError<Q>> for Errno {
+    fn from(err: DqBufError<Q>) -> Self {
         match err {
+            DqBufError::ConvertionError(_) => Errno::EINVAL,
             DqBufError::Eos => Errno::EPIPE,
             DqBufError::NotReady => Errno::EAGAIN,
             DqBufError::IoctlError(e) => e,
         }
     }
 }
 
-pub type DqBufResult<T> = Result<T, DqBufError>;
+pub type DqBufResult<T, Q> = Result<T, DqBufError<Q>>;
 
 /// Safe wrapper around the `VIDIOC_DQBUF` ioctl.
-pub fn dqbuf<T: QueryBuf>(fd: &impl AsRawFd, queue: QueueType) -> DqBufResult<T> {
+pub fn dqbuf<T: QueryBuf>(fd: &impl AsRawFd, queue: QueueType) -> DqBufResult<T, T> {
     let mut v4l2_buf = bindings::v4l2_buffer {
         type_: queue as u32,
         ..unsafe { mem::zeroed() }
@@ -62,10 +65,10 @@ pub fn dqbuf<T: QueryBuf>(fd: &impl AsRawFd, queue: QueueType) -> DqBufResult<T>
         v4l2_buf.length = plane_data.len() as u32;
 
         unsafe { ioctl::vidioc_dqbuf(fd.as_raw_fd(), &mut v4l2_buf) }?;
-        T::from_v4l2_buffer(v4l2_buf, Some(plane_data))
+        T::try_from_v4l2_buffer(v4l2_buf, Some(plane_data)).map_err(DqBufError::ConvertionError)?
     } else {
         unsafe { ioctl::vidioc_dqbuf(fd.as_raw_fd(), &mut v4l2_buf) }?;
-        T::from_v4l2_buffer(v4l2_buf, None)
+        T::try_from_v4l2_buffer(v4l2_buf, None).map_err(DqBufError::ConvertionError)?
     };
 
     Ok(dequeued_buffer)

diff --git a/lib/src/ioctl/qbuf.rs b/lib/src/ioctl/qbuf.rs
@@ -30,7 +30,9 @@ bitflags! {
 }
 
 #[derive(Debug, Error)]
-pub enum QBufError {
+pub enum QBufError<Q: QueryBuf> {
+    #[error("error while converting from v4l2_buffer: {0}")]
+    ConvertionError(Q::Error),
     #[error("invalid number of planes specified for the buffer: got {0}, expected {1}")]
     NumPlanesMismatch(usize, usize),
     #[error("data offset specified while using the single-planar API")]
@@ -39,15 +41,16 @@ pub enum QBufError {
     IoctlError(Errno),
 }
 
-impl From<Errno> for QBufError {
+impl<Q: QueryBuf> From<Errno> for QBufError<Q> {
     fn from(errno: Errno) -> Self {
         Self::IoctlError(errno)
     }
 }
 
-impl From<QBufError> for Errno {
-    fn from(err: QBufError) -> Self {
+impl<Q: QueryBuf> From<QBufError<Q>> for Errno {
+    fn from(err: QBufError<Q>) -> Self {
         match err {
+            QBufError::ConvertionError(_) => Errno::EINVAL,
             QBufError::NumPlanesMismatch(_, _) => Errno::EINVAL,
             QBufError::DataOffsetNotSupported => Errno::EINVAL,
             QBufError::IoctlError(e) => e,
@@ -56,26 +59,28 @@ impl From<QBufError> for Errno {
 }
 
 /// Implementors can pass buffer data to the `qbuf` ioctl.
-pub trait QBuf {
+pub trait QBuf<Q: QueryBuf> {
     /// Fill the buffer information into the single-planar `v4l2_buf`. Fail if
     /// the number of planes is different from 1.
-    fn fill_splane_v4l2_buffer(self, v4l2_buf: &mut bindings::v4l2_buffer)
-        -> Result<(), QBufError>;
+    fn fill_splane_v4l2_buffer(
+        self,
+        v4l2_buf: &mut bindings::v4l2_buffer,
+    ) -> Result<(), QBufError<Q>>;
     /// Fill the buffer information into the multi-planar `v4l2_buf`, using
     /// `v4l2_planes` to store the plane data. Fail if the number of planes is
     /// not between 1 and `VIDEO_MAX_PLANES` included.
     fn fill_mplane_v4l2_buffer(
         self,
         v4l2_buf: &mut bindings::v4l2_buffer,
         v4l2_planes: &mut V4l2BufferPlanes,
-    ) -> Result<(), QBufError>;
+    ) -> Result<(), QBufError<Q>>;
 }
 
-impl QBuf for V4l2Buffer {
+impl<Q: QueryBuf> QBuf<Q> for V4l2Buffer {
     fn fill_splane_v4l2_buffer(
         self,
         v4l2_buf: &mut bindings::v4l2_buffer,
-    ) -> Result<(), QBufError> {
+    ) -> Result<(), QBufError<Q>> {
         *v4l2_buf = self.buffer;
 
         Ok(())
@@ -85,7 +90,7 @@ impl QBuf for V4l2Buffer {
         self,
         v4l2_buf: &mut bindings::v4l2_buffer,
         v4l2_planes: &mut V4l2BufferPlanes,
-    ) -> Result<(), QBufError> {
+    ) -> Result<(), QBufError<Q>> {
         *v4l2_buf = self.buffer;
         for (dest, src) in v4l2_planes
             .iter_mut()
@@ -172,11 +177,11 @@ impl<H: PlaneHandle> QBuffer<H> {
     }
 }
 
-impl<H: PlaneHandle> QBuf for QBuffer<H> {
+impl<H: PlaneHandle, Q: QueryBuf> QBuf<Q> for QBuffer<H> {
     fn fill_splane_v4l2_buffer(
         self,
         v4l2_buf: &mut bindings::v4l2_buffer,
-    ) -> Result<(), QBufError> {
+    ) -> Result<(), QBufError<Q>> {
         if self.planes.len() != 1 {
             return Err(QBufError::NumPlanesMismatch(self.planes.len(), 1));
         }
@@ -197,7 +202,7 @@ impl<H: PlaneHandle> QBuf for QBuffer<H> {
         self,
         v4l2_buf: &mut bindings::v4l2_buffer,
         v4l2_planes: &mut V4l2BufferPlanes,
-    ) -> Result<(), QBufError> {
+    ) -> Result<(), QBufError<Q>> {
         if self.planes.is_empty() || self.planes.len() > v4l2_planes.len() {
             return Err(QBufError::NumPlanesMismatch(
                 self.planes.len(),
@@ -241,12 +246,12 @@ mod ioctl {
 /// accessed by anyone else, the caller also needs to guarantee that the backing
 /// memory won't be freed until the corresponding buffer is returned by either
 /// `dqbuf` or `streamoff`.
-pub fn qbuf<I: QBuf, O: QueryBuf>(
+pub fn qbuf<I: QBuf<O>, O: QueryBuf>(
     fd: &impl AsRawFd,
     queue: QueueType,
     index: usize,
     buf_data: I,
-) -> Result<O, QBufError> {
+) -> Result<O, QBufError<O>> {
     let mut v4l2_buf = bindings::v4l2_buffer {
         index: index as u32,
         type_: queue as u32,
@@ -259,10 +264,11 @@ pub fn qbuf<I: QBuf, O: QueryBuf>(
         v4l2_buf.m.planes = plane_data.as_mut_ptr();
 
         unsafe { ioctl::vidioc_qbuf(fd.as_raw_fd(), &mut v4l2_buf) }?;
-        Ok(O::from_v4l2_buffer(v4l2_buf, Some(plane_data)))
+        Ok(O::try_from_v4l2_buffer(v4l2_buf, Some(plane_data))
+            .map_err(QBufError::ConvertionError)?)
     } else {
         buf_data.fill_splane_v4l2_buffer(&mut v4l2_buf)?;
         unsafe { ioctl::vidioc_qbuf(fd.as_raw_fd(), &mut v4l2_buf) }?;
-        Ok(O::from_v4l2_buffer(v4l2_buf, None))
+        Ok(O::try_from_v4l2_buffer(v4l2_buf, None).map_err(QBufError::ConvertionError)?)
     }
 }