Skip to content
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.

Sign up for GitHub

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

Avoid extra ioctl by allowing control query by description #62

Merged
merged 6 commits into from
Nov 10, 2023
Merged

Avoid extra ioctl by allowing control query by description #62

Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f3c04f3
refactor to avoid extra ioctl
donkeyteethUX Sep 15, 2022
f575bb5
fix doc link and clippy lint
donkeyteethUX Feb 7, 2023
8ba7d19
inline format args to please our pedantic overlords
donkeyteethUX Feb 7, 2023
13cbcb1
Add missing backticks around intradoc link
MarijnS95 Jan 22, 2023
85f1e89
Revert "inline format args to please our pedantic overlords"
MarijnS95 Nov 1, 2023
08db864
Drop unneeded `control_from_id()`
MarijnS95 Nov 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 34 additions & 23 deletions src/device.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ use std::{io, mem};

use libc;

use crate::control;
use crate::capability::Capabilities;
use crate::control::{self, Control, Description};
use crate::v4l2;
use crate::v4l2::videodev::v4l2_ext_controls;
use crate::v4l_sys::*;
use crate::{capability::Capabilities, control::Control};

/// Linux capture device abstraction
pub struct Device {
Expand Down Expand Up @@ -92,7 +92,7 @@ impl Device {
}

/// Returns the supported controls for a device such as gain, focus, white balance, etc.
pub fn query_controls(&self) -> io::Result<Vec<control::Description>> {
donkeyteethUX marked this conversation as resolved.
Show resolved Hide resolved
pub fn query_controls(&self) -> io::Result<Vec<Description>> {
let mut controls = Vec::new();
unsafe {
let mut v4l2_ctrl: v4l2_query_ext_ctrl = mem::zeroed();
Expand All @@ -107,7 +107,7 @@ impl Device {
) {
Ok(_) => {
// get the basic control information
let mut control = control::Description::from(v4l2_ctrl);
let mut control = Description::from(v4l2_ctrl);

// if this is a menu control, enumerate its items
if control.typ == control::Type::Menu
Expand Down Expand Up @@ -167,29 +167,16 @@ impl Device {
Ok(controls)
}

/// Returns the control value for an ID
/// Returns the current control value from its [`Description`]
///
/// # Arguments
///
/// * `id` - Control identifier
pub fn control(&self, id: u32) -> io::Result<Control> {
/// * `desc` - Control description
pub fn control(&self, desc: &Description) -> io::Result<Control> {
unsafe {
let mut queryctrl = v4l2_query_ext_ctrl {
id,
..mem::zeroed()
};
v4l2::ioctl(
self.handle().fd(),
v4l2::vidioc::VIDIOC_QUERY_EXT_CTRL,
&mut queryctrl as *mut _ as *mut std::os::raw::c_void,
)?;

// determine the control type
let description = control::Description::from(queryctrl);

// query the actual control value
let mut v4l2_ctrl = v4l2_ext_control {
id,
id: desc.id,
..mem::zeroed()
};
let mut v4l2_ctrls = v4l2_ext_controls {
Expand All @@ -203,7 +190,7 @@ impl Device {
&mut v4l2_ctrls as *mut _ as *mut std::os::raw::c_void,
)?;

let value = match description.typ {
let value = match desc.typ {
control::Type::Integer64 => {
control::Value::Integer(v4l2_ctrl.__bindgen_anon_1.value64)
}
Expand All @@ -221,10 +208,34 @@ impl Device {
}
};

Ok(Control { id, value })
Ok(Control { id: desc.id, value })
}
}

/// Convenience method to get the current control by ID. Requires an extra syscall compared to
/// [`Device::control()`] when a [`Description`] is already available.
///
/// # Arguments
///
/// * `id` - Control identifier
pub fn control_from_id(&self, id: u32) -> io::Result<Control> {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, this can be removed. We don't need backwards compatibility here, no need to inflate the API surface when the new function does the same thing, but better.

Copy link
Collaborator

@MarijnS95 MarijnS95 Nov 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed if we're pushing to land this in a breaking release.

EDIT: Which is needed anyway because the signature of the existing fn control() changed rather than leaving it untouched and introducing a new fn control_from_description() instead. We probably discussed this a while ago but I forgot.

let queryctrl = unsafe {
let mut queryctrl = v4l2_query_ext_ctrl {
id,
..mem::zeroed()
};
v4l2::ioctl(
self.handle().fd(),
v4l2::vidioc::VIDIOC_QUERY_EXT_CTRL,
&mut queryctrl as *mut _ as *mut std::os::raw::c_void,
)?;

queryctrl
};

self.control(&Description::from(queryctrl))
}

/// Modifies the control value
///
/// # Arguments
Expand Down
Loading