From 0cf0eb72c11a5248b135874ee0036ac737197e30 Mon Sep 17 00:00:00 2001 From: Nico Kreipke Date: Mon, 7 Nov 2022 16:29:36 +0100 Subject: [PATCH 1/4] Add dependency on thiserror --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index 70df0cc..bdd531b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,7 @@ keywords = ["linux", "video", "webcam"] log = "0.4.14" nix = "0.25.0" bitflags = "1.2.1" +thiserror = "1.0" [dev-dependencies] env_logger = { version = "0.9.0", default-features = false } From 95f5716ccd044f1a8ae9845951cddf93b7745fc8 Mon Sep 17 00:00:00 2001 From: Nico Kreipke Date: Mon, 7 Nov 2022 16:29:42 +0100 Subject: [PATCH 2/4] Implement dedicated error type --- src/error.rs | 27 +++++++++++++++++++++++++++ src/lib.rs | 1 + 2 files changed, 28 insertions(+) create mode 100644 src/error.rs diff --git a/src/error.rs b/src/error.rs new file mode 100644 index 0000000..2d5b8e2 --- /dev/null +++ b/src/error.rs @@ -0,0 +1,27 @@ +use thiserror::Error; + +use crate::BufType; + +/// The error type for interactions with this library. +#[derive(Error, Debug)] +#[non_exhaustive] +pub enum Error { + /// The number of allocated buffers was less than the amount requested. + #[error("failed to allocate {required_count} buffers (driver only allocated {actual_count})")] + BufferAllocationFailed { + required_count: u32, + actual_count: u32, + }, + /// The requested buffer type is not supported. + #[error("unsupported buffer type {0:?}")] + UnsupportedBufferType(BufType), + /// An underlying I/O error has occurred. + #[error(transparent)] + Io(#[from] std::io::Error), +} + +impl From for Error { + fn from(error: nix::Error) -> Self { + Error::Io(std::io::Error::from_raw_os_error(error as i32)) + } +} diff --git a/src/lib.rs b/src/lib.rs index aba95c2..191860b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,6 +9,7 @@ mod macros; mod buf_type; pub mod controls; +mod error; pub mod format; mod pixelformat; mod raw; From 86f415dfe863eebd887fdb3a39fb8990a52e192a Mon Sep 17 00:00:00 2001 From: Nico Kreipke Date: Mon, 7 Nov 2022 16:31:56 +0100 Subject: [PATCH 3/4] Replace Box with dedicated error type --- src/lib.rs | 8 ++++---- src/stream.rs | 11 +++++------ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 191860b..f99dbf8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -35,12 +35,12 @@ use shared::{CaptureParamFlags, Memory, StreamParamCaps}; use stream::{ReadStream, WriteStream}; pub use buf_type::*; +pub use error::Error; pub use shared::{ AnalogStd, CapabilityFlags, Fract, InputCapabilities, InputStatus, InputType, OutputCapabilities, OutputType, }; -pub type Error = Box; pub type Result = std::result::Result; /// Returns an iterator over all connected V4L2 devices. @@ -198,7 +198,7 @@ impl Device { /// /// The returned `Format` variant will match `buf_type`. /// - /// If no format is set, this returns `EINVAL`. + /// If no format is set, this returns [`Error::UnsupportedBufferType`]. pub fn format(&self, buf_type: BufType) -> Result { unsafe { let mut format = raw::Format { @@ -206,8 +206,8 @@ impl Device { ..mem::zeroed() }; raw::g_fmt(self.fd(), &mut format)?; - let fmt = Format::from_raw(format) - .ok_or_else(|| format!("unsupported buffer type {:?}", buf_type))?; + let fmt = + Format::from_raw(format).ok_or_else(|| Error::UnsupportedBufferType(buf_type))?; Ok(fmt) } } diff --git a/src/stream.rs b/src/stream.rs index d4e004a..dac871b 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -12,7 +12,7 @@ use nix::sys::mman::{mmap, munmap, MapFlags, ProtFlags}; use crate::buf_type::BufType; use crate::shared::{BufFlag, Memory}; -use crate::{raw, Result}; +use crate::{raw, Error, Result}; enum AllocType { /// The buffer was `mmap`ped into our address space, use `munmap` to free it. @@ -55,11 +55,10 @@ impl Buffers { log::debug!("{:?}", req_bufs); if req_bufs.count < buffer_count { - return Err(format!( - "failed to allocate {} buffers (driver only allocated {})", - buffer_count, req_bufs.count - ) - .into()); + return Err(Error::BufferAllocationFailed { + required_count: buffer_count, + actual_count: req_bufs.count, + }); } // Query the buffer locations and map them into our process. From 2c6ab108f3bbe88d64375f39c4434cd6afff699d Mon Sep 17 00:00:00 2001 From: Nico Kreipke Date: Mon, 7 Nov 2022 16:58:57 +0100 Subject: [PATCH 4/4] Update error handling in examples --- examples/control.rs | 24 ++++++++++++------------ examples/drain-read.rs | 10 +++++++--- examples/drain-stream.rs | 10 +++++++--- examples/info.rs | 8 +++++++- examples/output-stream.rs | 17 +++++++++-------- examples/output-write.rs | 17 +++++++++-------- examples/save-jpeg.rs | 13 +++++++------ examples/uvc-xu.rs | 18 ++++++++++-------- examples/uvc.rs | 10 ++++++++-- 9 files changed, 76 insertions(+), 51 deletions(-) diff --git a/examples/control.rs b/examples/control.rs index 3b59f12..05fac7f 100644 --- a/examples/control.rs +++ b/examples/control.rs @@ -2,22 +2,22 @@ use std::{env, path::Path}; use linuxvideo::Device; -fn usage() -> String { - format!("usage: control []") -} - fn main() -> linuxvideo::Result<()> { env_logger::init(); let mut args = env::args_os().skip(1); - let path = args.next().ok_or_else(usage)?; + let path = match args.next() { + Some(path) => path, + None => { + println!("usage: control []"); + std::process::exit(1); + } + }; + let control_name = args.next(); let control_name = match control_name.as_ref() { - Some(name) => Some( - name.to_str() - .ok_or_else(|| format!("control name must be UTF-8"))?, - ), + Some(name) => Some(name.to_str().expect("control name must be UTF-8")), None => None, }; let value = args.next(); @@ -25,8 +25,8 @@ fn main() -> linuxvideo::Result<()> { Some(value) => Some( value .to_str() - .ok_or_else(|| format!("control name must be UTF-8"))? - .parse()?, + .and_then(|id| id.parse().ok()) + .expect("control name must be UTF-8"), ), None => None, }; @@ -72,7 +72,7 @@ fn main() -> linuxvideo::Result<()> { println!("{:?} control value: {}", cid, value); } }, - None => return Err(format!("device does not have control named {}", control_name).into()), + None => panic!("device does not have control named {}", control_name), } Ok(()) diff --git a/examples/drain-read.rs b/examples/drain-read.rs index 0471f0b..b6ea3a7 100644 --- a/examples/drain-read.rs +++ b/examples/drain-read.rs @@ -21,9 +21,13 @@ fn main() -> linuxvideo::Result<()> { let mut args = env::args_os().skip(1); - let path = args - .next() - .ok_or_else(|| format!("usage: drain-read "))?; + let path = match args.next() { + Some(path) => path, + None => { + println!("usage: drain-read "); + std::process::exit(1); + } + }; let device = Device::open(Path::new(&path))?; diff --git a/examples/drain-stream.rs b/examples/drain-stream.rs index 1acda66..70349d7 100644 --- a/examples/drain-stream.rs +++ b/examples/drain-stream.rs @@ -21,9 +21,13 @@ fn main() -> linuxvideo::Result<()> { let mut args = env::args_os().skip(1); - let path = args - .next() - .ok_or_else(|| format!("usage: drain-stream "))?; + let path = match args.next() { + Some(path) => path, + None => { + println!("usage: drain-stream "); + std::process::exit(1); + } + }; let device = Device::open(Path::new(&path))?; diff --git a/examples/info.rs b/examples/info.rs index b46a6ba..f94bf47 100644 --- a/examples/info.rs +++ b/examples/info.rs @@ -9,7 +9,13 @@ fn main() -> linuxvideo::Result<()> { let mut args = env::args_os().skip(1); - let path = args.next().ok_or_else(|| format!("usage: info "))?; + let path = match args.next() { + Some(path) => path, + None => { + println!("usage: info "); + std::process::exit(1); + } + }; let device = Device::open(Path::new(&path))?; diff --git a/examples/output-stream.rs b/examples/output-stream.rs index f3a49ad..769bb36 100644 --- a/examples/output-stream.rs +++ b/examples/output-stream.rs @@ -30,9 +30,13 @@ const TRANSPARENT: [u8; 4] = [0, 0, 0, 0]; fn main() -> linuxvideo::Result<()> { let mut args = env::args_os().skip(1); - let path = args - .next() - .ok_or_else(|| format!("usage: write "))?; + let path = match args.next() { + Some(path) => path, + None => { + println!("usage: write "); + std::process::exit(1); + } + }; let device = Device::open(Path::new(&path))?; if !device @@ -40,10 +44,7 @@ fn main() -> linuxvideo::Result<()> { .device_capabilities() .contains(CapabilityFlags::VIDEO_OUTPUT) { - return Err(format!( - "cannot write data: selected device does not support `VIDEO_OUTPUT` capability" - ) - .into()); + panic!("cannot write data: selected device does not support `VIDEO_OUTPUT` capability"); } let output = device.video_output(PixFormat::new(WIDTH, HEIGHT, PIXFMT))?; @@ -51,7 +52,7 @@ fn main() -> linuxvideo::Result<()> { println!("set format: {:?}", fmt); if fmt.pixelformat() != PIXFMT { - return Err(format!("driver does not support the requested parameters").into()); + panic!("driver does not support the requested parameters"); } let mut image = (0..fmt.height()) diff --git a/examples/output-write.rs b/examples/output-write.rs index 73a59ce..d349822 100644 --- a/examples/output-write.rs +++ b/examples/output-write.rs @@ -28,9 +28,13 @@ const TRANSPARENT: [u8; 4] = [0, 0xff, 0xff, 120]; fn main() -> linuxvideo::Result<()> { let mut args = env::args_os().skip(1); - let path = args - .next() - .ok_or_else(|| format!("usage: write "))?; + let path = match args.next() { + Some(path) => path, + None => { + println!("usage: write "); + std::process::exit(1); + } + }; let device = Device::open(Path::new(&path))?; if !device @@ -38,10 +42,7 @@ fn main() -> linuxvideo::Result<()> { .device_capabilities() .contains(CapabilityFlags::VIDEO_OUTPUT) { - return Err(format!( - "cannot write data: selected device does not support `VIDEO_OUTPUT` capability" - ) - .into()); + panic!("cannot write data: selected device does not support `VIDEO_OUTPUT` capability"); } let mut output = device.video_output(PixFormat::new(WIDTH, HEIGHT, PIXFMT))?; @@ -49,7 +50,7 @@ fn main() -> linuxvideo::Result<()> { println!("set format: {:?}", fmt); if fmt.pixelformat() != PIXFMT { - return Err(format!("driver does not support the requested parameters").into()); + panic!("driver does not support the requested parameters"); } let mut image = (0..fmt.height()) diff --git a/examples/save-jpeg.rs b/examples/save-jpeg.rs index 0f6f3ce..08c7aee 100644 --- a/examples/save-jpeg.rs +++ b/examples/save-jpeg.rs @@ -15,13 +15,14 @@ fn main() -> linuxvideo::Result<()> { let mut args = env::args_os().skip(1); - let device = args - .next() - .ok_or_else(|| format!("usage: save-stream "))?; + let (device, file_path) = match (args.next(), args.next()) { + (Some(device), Some(file_path)) => (device, file_path), + _ => { + println!("usage: save-stream "); + std::process::exit(1); + } + }; - let file_path = args - .next() - .ok_or_else(|| format!("usage: save-stream "))?; let mut file = File::create(file_path)?; let device = Device::open(Path::new(&device))?; diff --git a/examples/uvc-xu.rs b/examples/uvc-xu.rs index 02d1d27..1e7c921 100644 --- a/examples/uvc-xu.rs +++ b/examples/uvc-xu.rs @@ -4,21 +4,23 @@ use std::{env, path::Path}; use linuxvideo::{uvc::UvcExt, Device}; -fn usage() -> String { - format!("usage: uvc-xu ") -} - fn main() -> linuxvideo::Result<()> { env_logger::init(); let mut args = env::args_os().skip(1); - let path = args.next().ok_or_else(usage)?; - let unit_id = args.next().ok_or_else(usage)?; + let (path, unit_id) = match (args.next(), args.next()) { + (Some(path), Some(unit_id)) => (path, unit_id), + _ => { + println!("usage: uvc-xu "); + std::process::exit(1); + } + }; + let unit_id: u8 = unit_id .to_str() - .ok_or_else(|| format!("unit ID must be an integer"))? - .parse()?; + .and_then(|id| id.parse().ok()) + .expect("unit ID must be an integer"); let device = Device::open(Path::new(&path))?; diff --git a/examples/uvc.rs b/examples/uvc.rs index 2b2156e..5873ec6 100644 --- a/examples/uvc.rs +++ b/examples/uvc.rs @@ -13,7 +13,13 @@ fn main() -> linuxvideo::Result<()> { let mut args = env::args_os().skip(1); - let path = args.next().ok_or_else(|| format!("usage: uvc "))?; + let path = match args.next() { + Some(path) => path, + None => { + println!("usage: uvc "); + std::process::exit(1); + } + }; let device = Device::open(Path::new(&path))?; @@ -22,7 +28,7 @@ fn main() -> linuxvideo::Result<()> { .device_capabilities() .contains(CapabilityFlags::META_CAPTURE) { - return Err("device does not support `META_CAPTURE` capability".into()); + panic!("device does not support `META_CAPTURE` capability"); } let meta = device.meta_capture(MetaFormat::new(Pixelformat::UVC))?;