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.

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

Add a compile_error!() when central crate features clash #110

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
31 changes: 18 additions & 13 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,33 @@
This crate provides safe bindings to the Video for Linux (V4L) stack. Modern device drivers will usually implement the `v4l2` API while older ones may depend on the legacy `v4l` API. Such legacy devices may be used with this crate by choosing the `libv4l` feature for this crate.

## Goals
This crate shall provide the v4l-sys package to enable full (but unsafe) access to libv4l\*.
On top of that, there will be a high level, more idiomatic API to use video capture devices in Linux.

This crate shall provide the `v4l-sys` package to enable full (but unsafe) access to `libv4l\*`.
On top of that, there will be a high level, more idiomatic API to use video capture devices on Linux.

There will be simple utility applications to list devices and capture frames.
A minimalistic OpenGL/Vulkan viewer to display frames is planned for the future.

## Changelog

See [CHANGELOG.md](https://github.com/raymanfx/libv4l-rs/blob/master/CHANGELOG.md)

## Dependencies
You have the choice between two dependencies (both provided by this crate internally):
* libv4l-sys
> Link against the libv4l* stack including libv4l1, libv4l2, libv4lconvert.
> This has the advantage of emulating common capture formats such as RGB3 in userspace through libv4lconvert and more.
> However, some features like userptr buffers are not supported in libv4l.
* v4l2-sys
> Use only the Linux kernel provided v4l2 API provided by videodev2.h.
> You get support for all v4l2 features such as userptr buffers, but may need to do format conversion yourself if you require e.g. RGB/BGR buffers which may not be supported by commodity devices such as webcams.
## Cargo Features

This crate has two primary features:

Enable either the `libv4l` or the `v4l2` backend by choosing the it as feature for this crate.
* `libv4l`: uses the `libv4l` wrapper libraries.
* Links against the `libv4l*` stack, including `libv4l1`, `libv4l2`, and `libv4lconvert`.
* Has the advantage of emulating common capture formats such as RGB3 in userspace through `libv4lconvert` and more.
* However, some features, like `userptr` buffers, are not supported in `libv4l`.
onkoe marked this conversation as resolved.
Show resolved Hide resolved
* `v4l2` (DEFAULT): uses the kernel's Video4Linux 2 kernel header directly.
* Only uses the Linux kernel provided v4l2 API provided by `videodev2.h`.
* You get support for all v4l2 features such as `userptr` buffers, but may need to do format conversion yourself if you require certain constructs (e.g. RGB/BGR buffers), which may not be supported by commodity devices such as webcams.

You must select exactly one of these features. Note that the `v4l2` feature is on by default. To use `libv4l` instead, you'll need to also use the `default-features = false` dependency key.

## Usage

Below you can find a quick example usage of this crate. It introduces the basics necessary to do frame capturing from a streaming device (e.g. webcam).

```rust
Expand Down Expand Up @@ -93,4 +98,4 @@ fn main() {
}
```

Have a look at the provided `examples` for more sample applications.
Have a look at the provided [`examples`](./examples/) for more sample applications.
17 changes: 15 additions & 2 deletions src/lib.rs
onkoe marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,25 @@
//!
//! Have a look at the examples to learn more about device and buffer management.

#[cfg(feature = "v4l-sys")]
#[cfg(all(feature = "libv4l", feature = "v4l2"))]
compile_error!(
"You may not enable both `v4l-sys` and `v4l2-sys` features at the same time.\
Copy link
Collaborator

Choose a reason for hiding this comment

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

You name these features but they're implicit from the optional dependencies, and "renamed" (but without "dep:...") to v4l2 and libv4l. I think we should specify exactly what the user is enabling, or expected ot enable:

Suggested change
"You may not enable both `v4l-sys` and `v4l2-sys` features at the same time.\
"You may not enable both `libv4l` and `v4l2` features at the same time.\

Try disabling one of them. If you only specified one feature, you may wish to\
add `default-features = false` as a dependency key."
);

#[cfg(all(feature = "libv4l", not(feature = "v4l2")))]
pub use v4l_sys;

#[cfg(feature = "v4l2-sys")]
#[cfg(all(feature = "v4l2", not(feature = "libv4l")))]
pub use v4l2_sys as v4l_sys;

// calms down rust-analyzer when `rust-analyzer.cargo.features = "all"`,
// though the `compile_error!()` above prevents this from being a valid,
// compilable configuration.
#[cfg(all(feature = "v4l2", feature = "libv4l"))]
pub use v4l_sys;
Comment on lines +86 to +87
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not do what you've done in the second patch (but the other way around):

  1. Remove these two lines;
  2. Change the above to:
    -#[cfg(all(feature = "libv4l", not(feature = "v4l2")))]
    +#[cfg(feature = "v4l2")]
     pub use v4l2_sys as v4l_sys;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would also prefer this, but I found that doing so breaks one of the features.

On the other hand, it doesn't occur in api.rs since we're not already re-exporting one of its modules. If you find a way to do it, though, please let me know!

Copy link
Collaborator

Choose a reason for hiding this comment

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

It would match the choice in api.rs to prefer v4l2 code in the case that both features are contemporarily enabled, but it is less of an issue now that the comment has been removed.

Either way, this seems better to me:

diff --git a/src/lib.rs b/src/lib.rs
index 2272aeb..1237262 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -74,16 +74,12 @@ compile_error!(
     add `default-features = false` as a dependency key."
 );

-#[cfg(all(feature = "libv4l", not(feature = "v4l2")))]
-pub use v4l_sys;
-
-#[cfg(all(feature = "v4l2", not(feature = "libv4l")))]
-pub use v4l2_sys as v4l_sys;
-
-// calms down rust-analyzer when `rust-analyzer.cargo.features = "all"`,
-// though the `compile_error!()` above prevents this from being a valid,
+// calm down rust-analyzer when `rust-analyzer.cargo.features = "all"` by having only one `v4l_sys`
+// crate in scope, even though the `compile_error!()` above prevents this from being a valid,
 // compilable configuration.
-#[cfg(all(feature = "v4l2", feature = "libv4l"))]
+#[cfg(feature = "v4l2")]
+pub use v4l2_sys as v4l_sys;
+#[cfg(all(feature = "libv4l", not(feature = "v4l2")))]
 pub use v4l_sys;

 pub mod v4l2;


pub mod v4l2;

pub mod buffer;
Expand Down
4 changes: 2 additions & 2 deletions src/v4l2/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::{io, path::Path};

use crate::v4l2::vidioc;

#[cfg(feature = "v4l-sys")]
#[cfg(all(feature = "libv4l", not(feature = "v4l2")))]
mod detail {
use crate::v4l2::vidioc;
use crate::v4l_sys::*;
Expand Down Expand Up @@ -56,7 +56,7 @@ mod detail {
}
}

#[cfg(feature = "v4l2-sys")]
#[cfg(feature = "v4l2")]
mod detail {
use crate::v4l2::vidioc;

Expand Down